[glasstiger]

Summary

• Add an ACL audit log guide covering recorded changes, principal attribution, redaction, read permissions, delivery guarantees, and SQL examples.
• Document audit retention configuration, replication, upgrades, rollback limitations, and restoration from older backups.
• Update the service-account creation reference for unchanged existing accounts under IF NOT EXISTS, and link the audit log from RBAC, governance, and the sidebar.

Dependencies

• https://github.com/questdb/questdb-enterprise/pull/1326

[github-actions]

🚀 Build success!

Latest successful preview: https://preview-618--questdb-documentation.netlify.app/docs/

Commit SHA: 5f0efa3

📦 Build generates a preview & updates the link on each commit.

[glasstiger]

Verdict: approve-with-nits

Reviewed 567d1abbd70aee9fc1f6238251650bf1c6d25f54 at level 2, including all six review passes. No confirmed Critical or Major issues introduced by this PR.

Blocking issues

None.

Non-blocking findings

1. Minor — Qualify the introduction’s attribution promise
documentation/security/acl-audit-log.md:11–12

“Each row identifies the authenticated principal, the principal it acted as, and the statement that caused the change.”

This overstates the guarantee: owner-grant entries omit statement text, and drop/rename cleanup entries omit both principals and text, as documented at lines 101–105 and confirmed in the implementation.

Fix: Add “when available” and link to Recorded changes, so readers retrieving only the introduction do not receive an unconditional attribution promise.

2. Minor — Document the assumed-account query’s name-collision limitation
documentation/security/acl-audit-log.md:76–82

The predicate session_principal != effective_principal misses an external/OIDC user who assumes a local service account with the same name. The implementation distinguishes those identities, but their recorded principal strings are equal.

Fix: Explain that this example detects assumptions where the principal names differ. Same-name cases cannot be distinguished conclusively from these columns alone: is_external describes the authenticated identity, not whether an assumption occurred. Simply adding OR is_external would also include ordinary external-user activity.

Validation

• yarn build: passed, with no broken links or rendering errors. Only dependency-update advisories appeared.
• Checked technical claims against Enterprise dependency commit 2bfb0e497dd8dcd1275e15441a433a680e9b7a1a, its pinned QuestDB source, and relevant tests.
• Checked Enterprise 4.1.0 source for the documented historical behavior.
• Checked the current SQL-parser default branch; no additional tokens are needed.
• SQL examples were statically checked, not runtime-executed.
• Review-only; no tracked files changed.

What works

• Direct Security → ACL audit log placement, with useful inbound links from governance, RBAC, configuration, upgrades, and backup guidance.
• Consistent separation of upcoming functionality from Enterprise 4.1.0 behavior.
• Particularly good explanations of audit commitment versus statement success, recovery-related ID reuse, and replicated ACL logs versus node-local view audits.
• The partition-start retention corrections are consistent across the affected pages.

Downgraded and excluded findings

2 new findings confirmed; 3 false positives removed; 11 pre-existing findings excluded.

The one-shot reviewer’s missing PGWire grant, authentication-setting values, and admin-disable procedure are covered by the page’s explicit RBAC/IAM links; they are not blocking documentation gaps.

The adversarial pass also flagged the following, but each passage is unchanged from the merge base:

Candidate finding	Disposition
Inline comments in backup hash settings	Pre-existing; separate correction
Delegated view-audit retention permissions	Separate behavior change already tracked by PR #609
TLS enablement without restart	Pre-existing
Reused built-in administrator name	Pre-existing
DROP REMOTE described as uploading	Pre-existing
Claim that only DROP REMOTE deletes the last copy	Pre-existing
Password-only TCP ingestion account	Pre-existing
View-audit timestamp described as completion time	Pre-existing
Missing cold-storage qualification in recovery summary	Pre-existing
Missing credential clauses in service-account grammar	Pre-existing
Kafka’s granular-permission example targets another account	Pre-existing

These are not all harmless, but they should not be presented as regressions introduced by #618.