Home Dashboard GitHub

2026-06-11-inc1.5-audit-chain-crash-safety

ADR / Ship Log: Audit-chain Crash-Safety — DB-Transactional Chain Head (Increment 1.5)

  • Status: ACCEPTED (locked, go-with-changes) — hardened by a 5-lens distributed-systems design panel (G1).
  • Date: 2026-06-11
  • Stacks on: Increment 1 (feat/inc1-scoped-grants).

Context

The tamper-evident completion_audit hash chain kept its per-tenant head in a Redis-backed in-memory structure while rows live in Postgres, with no shared transaction. Increment 1's reviews proved this split causes: dangling-head on crash/insert-failure (permanent chain break), fire-and-forget allow-path writes, out-of-id-order concurrent denials, and a tenant_seq column added but unassigned (so suppression outside the verify window is undetectable). Increment 1 applied only a bounded single-writer deny-path rollback. This increment makes the chain genuinely crash-safe, fork-free, correctly ordered, and suppression-detecting — for both allow and deny paths.

Decision

Promote the chain head to a DB-transactional source of truth. Serialize per tenant with a Postgres advisory transaction lock taken inside the existing withTenant() transaction; read predecessor, compute link, insert row, assign tenant_seq — all atomic. Redis demoted to a non-authoritative cache.

  • Locking: SELECT pg_advisory_xact_lock(hashtextextended('completion_audit_chain:' || $tenantId, 0)) as the FIRST statement after SET LOCAL set_config (so the predecessor read is RLS-scoped). Auto-releases on COMMIT/ROLLBACK — PgBouncer-transaction-pooling-safe, the proven idiom in transcript-store.ts / session-write-lock.ts. Rejected: session-scoped advisory locks (leak under pooling); FOR UPDATE on the tail row (phantom/gap race — a not-yet-inserted tail can't be locked); SERIALIZABLE+retry (no fire-and-forget caller can drive the retry loop). Advisory key derivation documented next to MIGRATION_LOCK_ID to keep the keyspace collision-audited.
  • Hot path: new logChained() runs ONE withTenant() transaction: lock → SELECT event_hash, tenant_seq … ORDER BY id DESC LIMIT 1 → prevHash = tail.event_hash ?? genesisHash(tenant) → eventHash = compute(prevHash, buildAuditChainPayload(payload)) → INSERT … tenant_seq = COALESCE(tail.tenant_seq,0)+1 → return {eventHash, tenantSeq}. Awaited, not background-queued (a queue would re-introduce the head/row split). The per-tenant lock makes id-order == seq-order by construction. Allow-path stays fail-open on the user's completion but the chain-write failure becomes observable (critical audit.deny_record_failed-style event, not a silent catch). Deny-path stays fail-closed. Cap audit-write DB concurrency with a small dedicated budget so it can't starve the 20-conn completion pool.

Must implement

  1. CompletionAuditStore.logChained(entry, payload) — the transactional method above (completion-audit-store.ts).
  2. Rewrite recordCompletionAudit (helpers.ts:259): stop calling computeHash() (no Redis side effects), AWAIT logChained(), source eventHash from the committed row, emit a critical event on failure instead of the silent empty catch (helpers.ts:319).
  3. Collapse recordAuthorizationDenial (helpers.ts:351-509) to a single AWAITED logChained(); delete the CAS-then-insert retry loop and the rollbackChainHeadCas mitigation (the lock supersedes them); keep fail-closed + emitDenyRecordFailure on durable failure.
  4. Streaming (streaming.ts:~1290/1333): remove computeHash() at [DONE]; do the awaited audit write in the post-stream finalize block; emit the committed eventHash; remove precomputedChain plumbing.
  5. Non-streaming (non-streaming.ts:977): await the audit write; set X-BR-Audit-Hash from the committed hash.
  6. Demote Redis (audit-chain.ts): delete CHAIN_CAS_LUA, advanceChainHead, commitChainHeadCas, commitHead, rollbackChainHeadCas, computeHash side effects. Keep genesisHash, pure compute, buildAuditChainPayload, verifyChain. Optionally retain a post-commit cache.
  7. Migration v57 (migrate.ts): backfill tenant_seq for existing rows in id-order per tenant (row_number() OVER (PARTITION BY tenant_id ORDER BY id)), batched under the existing migration advisory lock.
  8. Boot seed (boot-db.ts:91): ORDER BY tenant_id, id DESC (cache warm only; authoritative tail read in-transaction).
  9. verifyChain (audit-chain.ts:438): add tenantSeq to AuditChainEntry; Check 3 — for i>0, assert entries[i].tenantSeq === entries[i-1].tenantSeq + 1n; never fail the first row on seq (window boundary); skip null-seq legacy rows; distinct brokenReason 'hash'|'continuity'|'seq-gap'.
  10. Governance verify (governance.ts:812): thread tenantSeq; add a window lower-edge check — fetch MAX(tenant_seq) WHERE id < windowStart and assert entries[0].tenantSeq === priorMaxSeq + 1 to detect suppression at the window edge.

Honest residual

Tail truncation (deleting the newest row, or the newest K rows, with no surviving row after them) leaves no seq gap and no continuity break inside any fetched window, so it remains undetectable from the chain alone.

G2 decision — DOCUMENTED, not shipped (anchor deferred). We did NOT ship the per-tenant signed anchor this increment. What this increment DID close:

  • Interior suppression — a deleted row strictly between two surviving rows — is now caught two independent ways: adjacent-row continuity (Check 2) and per-tenant tenant_seq gaplessness (Check 3, new this increment).
  • Window lower-edge suppression — rows deleted just below the first row of a verify window — is caught by the governance verifier's new lower-edge probe: it reads MAX(tenant_seq) WHERE id < windowStart and asserts entries[0].tenantSeq === priorMaxSeq + 1.

What remains undetectable WITHOUT an external anchor:

  • Tail truncation of the most-recent row(s). After deleting the tail, the new tail row is still internally self-consistent, its prevHash still points at a present predecessor, and its tenant_seq is still the max — there is no gap to find. Detecting this requires an out-of-band, monotonic record of the latest (tenant_seq, eventHash) that the deleter cannot also roll back in the same privileged action.

Why deferred (cost/risk, not laziness): a correct anchor is its own increment, not a cheap add-on. It needs (1) a new durable, append-only or monotonic-guarded store for the signed (seq, head) per tenant; (2) a post-commit signed write (HMAC infra exists in audit-signer.ts, but the freshness/monotonic-guard + cross-process write-after-commit ordering do not); (3) a verify-side freshness comparison that distinguishes "legitimately no new rows since last anchor" from "tail was truncated"; and (4) a story for the anchor store's OWN tamper-resistance (an anchor an attacker with the same DB privileges can also rewrite buys nothing — it must live on a different trust boundary, e.g. a WORM/object-lock sink or an external transparency log). Shipping a same-DB anchor would create a false sense of tail-truncation detection while adding write-amplification to the hot path. We therefore document it explicitly and carry it as a dedicated later increment behind its own design gate. The chain head transactionalization (the crash-safety headline of this increment) stands on its own and is unaffected by this residual.

Test obligations

(1) N concurrent logChained() for one tenant → gapless tenant_seq 1..N, id-order==seq-order, verifyChain valid. (2) Insert failure under the lock → no head advance, no dangling hash (no Redis split). (3) PgBouncer transaction-pool e2e → advisory xact lock auto-releases (extend pgbouncer-compat.e2e.test.ts). (4) Suppression: DELETE an interior row → verifyChain reports seq-gap; DELETE at window lower edge → governance boundary check catches it. (5) Migration v57 backfill produces gapless seqs for existing rows. (6) Allow-path chain-write failure is observable (critical event), completion still succeeds.

Verification status (executed)

The DB-transactional concurrency/crash/backfill behavior is executed-verified: both src/db/stores/completion-audit-logchained.e2e.test.ts and src/db/pgbouncer-compat.e2e.test.ts run green (10/10) from a fresh v1→v57 migration under a non-superuser, non-BYPASSRLS prod-posture role (is_superuser=off, rolbypassrls=false) with FORCE RLS enforced — i.e. the advisory-lock serialization (gapless tenant_seq 1..N, id-order==seq-order), insert-failure rollback, PgBouncer xact-lock auto-release, and the v57 mixed/resume renumber are proven against a real database, not idiom alone.

Three merge-blockers raised by the G5 jury were cleared: (1) the pre-existing v1 RLS bootstrap bug (v1's RLS loop applied RLS to 23 not-yet-created tables; replaced the exclude-filter with an explicit allowlist of the 15 tables v1 creates — fresh DB now migrates clean); (2) the v57 renumber RLS/tenant-context defect (now runs each per-tenant renumber under SET LOCAL app.current_tenant, activating the tenant's existing UPDATE policy — verified under the non-BYPASSRLS role, no security weakening, system_admin stays SELECT-only); (3) the executed e2e above. The prod-posture run also surfaced and fixed a pre-existing v33 membership-backfill RLS brick (unset GUC on a FORCE-RLS table — a real fresh-deploy failure superuser testing hid).

Remaining documented follow-ups (not blocking)

  • Audit-write concurrency budget (ADR design item 16): NOT implemented. Every completion now awaits a per-tenant-advisory-locked write on the shared 20-conn pool, so a single hot tenant can self-serialize its audit writes. This is a throughput hardening (a small dedicated semaphore / statement_timeout for audit writes), not a correctness issue — carried as an explicit follow-up.
  • Tail-truncation anchor: see Honest residual above — external signed anchor on a different trust boundary, its own later increment.
  • Lower-edge probe is now observable (emits audit_chain_lower_edge_skipped_total when inconclusive instead of silently passing) and unit-tested.