Skip to main content

ADR-0187: An unchained audit row is a named condition, not tampering evidence


id: 0187-an-unchained-audit-row-is-not-tampering sidebar_position: 188 title: "ADR-0187: An unchained audit row is a named condition, not tampering evidence" ---# ADR-0187: An unchained audit row is a named condition, not tampering evidence

Date: 2026-08-17 Status: Accepted Deciders: Adrian (project owner), Claude Opus 5 Refines: ADR-0109 (append-only hash chain over audit_events), ADR-0181 (audit-hash timestamp normalisation and the two-epoch chain)

Context

GET /monitoring/audit-chain/verify is the surface a relying party consults to ask whether this platform's audit trail has been tampered with. ADR-0109 built it, and ADR-0181 corrected the timestamp rule that had put 1,709 rows outside the chain.

The chain columns are nullable and nothing in the database computes them. Migration 088 adds one trigger, the ADR-0064 immutability guard; chain_seq, prev_hash and entry_hash are filled by AuditService.log_event alone, which takes a per-tenant advisory lock, reads the chain head and computes the digest before inserting. A writer that constructs AuditEvent(...) and hands it to the session persists a row with all three NULL.

Measured by AST (tests/test_audit_chain_writer_bypass_1034.py::_bypass_sites): 34 such sites across 20 modules. On the live pilot, 292 of 2,804 rows carry neither chain_seq nor entry_hash (#1053). Issue #1034 already landed the one-way ratchet that stops number 35, and recorded why a detector had to precede the migration.

That figure was corrected while writing this ADR, and the correction is part of the record. #1034 and the ratchet both said 35 sites across 21 modules; resolving each site's import rather than its name gives 34/20. The 35th was compliance_case.py::_log_audit, which constructs app.models.workflow_state.AuditEvent — a TypedDict in Temporal workflow state, not the ORM row. It writes nothing to audit_events, and a Temporal workflow cannot call log_event at all, so it was a declared bypass that no migration could ever repair, holding the ratchet permanently above zero. The ratchet's own docstring records that a grep confused a construction with a reference; this is the same confusion one layer deeper, between two classes sharing a name, which the AST does not resolve by itself. The tell was a timestamp= kwarg the ORM row does not have.

What neither issue addressed is what the verifier says about those rows, and it says the wrong thing.

verify_tenant_chain reads ORDER BY chain_seq ASC. Postgres sorts NULLs last, so the replay reaches a chain-less row after every real one, with prev_hash = None. That is never the expected predecessor digest, so the row falls through the relink arm and the verdict reads:

> prev_hash does not link to the previous entry (re-link or deletion)

Reproduced on the real function, not inferred: a clean two-row chain returns ok=True; add one chain-less row and it returns ok=False, break_kind="relink", that reason.

That sentence is the strongest claim this system makes. It says a historical audit row was deleted — cryptographic evidence of tampering, the thing ADR-0109 exists to produce. The actual cause is a code defect in a writer. Nothing in the verdict let an operator tell the two apart, and there is no benign reading available to them: an unattributable deletion in an AML audit trail is an incident.

This is the claim-vs-check class (ADR-0157/0163/0167) in its most expensive form — not a control reporting its shape instead of its state, but a control reporting the wrong finding with maximum confidence. And per ADR-0121, a control that raises a false integrity incident on genuine inputs is one that gets switched off.

Decision

break_kind gains a third break value, unchained, returned when a row carries no chain link (entry_hash or prev_hash absent). Its reason names the cause and, explicitly, what it is not:

> row carries no chain link (prev_hash/entry_hash absent): it was written without joining > the chain, by a writer that bypassed AuditService.log_event. This is NOT evidence of > deletion or tampering — the row is present and unaltered — but it is outside > tamper-evidence, so the chain cannot be verified past it (#1034).

Four properties make this sound rather than merely kinder.

1. The verdict does not move. ok stays False, the replay still stops, and the row is still named. An unchained row genuinely is outside tamper-evidence, so the chain is not verified past it. Only the diagnosis changed, which means an attacker gains nothing by reaching the new arm instead of the old one.

2. The two conditions cannot be confused, and the reason is structural. A deletion or a re-link leaves every surviving row fully populated: ADR-0064's trigger forbids UPDATE on audit_events, so nothing can blank a digest that was written, and a DELETE removes the row rather than emptying it. Absent chain columns therefore mean exactly one thing — the row was never chained at write time. This classification rests on that trigger. If the immutability control were ever dropped, an attacker could blank a digest to downgrade the label, and this ADR would need revisiting rather than the code quietly becoming unsound.

3. Tampering still outranks it. The check runs inside the replay, in order, so a content break at an earlier position is still reported as content. Reclassification cannot hide an altered row behind a code defect. (Pinned by test: an altered row at position 2 followed by an unchained row at 3 reports content at 2.)

4. The count is disclosed, and reaches a consumer. ChainVerdict.unchained_rows counts chain-less rows across the whole chain — not only up to the first break, and present on every verdict including one that breaks earlier for another reason. It is projected by the endpoint and typed on the frontend. ADR-0181 records the precedent directly: legacy_verified existed on the verdict and reached no response body, and "the disclosure was internal-only, which is the same as absent." first_break_seq names one row; "a stray call site" and "292 rows outside the chain" are different findings with different responses, and a verdict that reports only the first cannot distinguish them.

The per-case verdict is deliberately NOT relaxed. Unlike a deletion, an unchained row is visible and attributable, so a case whose rows all precede it could in principle still be attested — the same logic that lets a content break leave a preceding case intact. Granting that here would relax a control as a side effect of improving a diagnosis, which is how false negatives get introduced. verify_case_segment grants intact via an allowlist on break_kind == "content", so the new kind fails closed by construction rather than by an added branch. A test asserts the allowlist form in both directions, so a later change to a denylist (!= "relink") — which would silently grant every future kind the permissive path — fails.

On the operator surface unchained maps to broken, not to ADR-0181's amber not_attested. Those two states say different things: legacy_offset_unattested means "consistent with intact, the attestation rests on configuration"; unchained means "these rows are not covered at all". Collapsing the second into the first would understate a real integrity gap. The improvement is to the reason shown alongside it.

Consequences

Positive

- The verifier stops alleging deletion for a condition that is a code defect. An operator reading ok: false can now tell which of four findings it is.

Corrected in review. This first read "the primary integrity SURFACE stops alleging deletion", and that was measurably false when written. The verdict and its reason were fixed; the two surfaces that render them were not. The admin panel's BrokenBody carried a hard-coded headline — "Chain BROKEN — tampering evidence detected", and "evidence that a historical audit row was altered, deleted, or re-linked" — and printed the correct reason four lines below it, in the same alert box. The AMLR regulator dossier did the same, and its shared note alleged tampering from a second element even where a headline was right. Both were reproduced by rendering, not by reading. Fixed in the same PR; the claim above is now about the verifier, which is what was actually true. - The scale of #1034's debt becomes visible from the running system rather than only from a test that parses the source: unchained_rows says how many rows are outside the chain right now, per tenant. - Genuine tampering evidence gets sharper, not weaker. Before this, relink meant either a deletion or any of 35 bypassed call sites; now it means a deletion. - Nothing about the fix depends on the writer migration landing, so the two proceed independently and #1034's ratchet keeps its own job.

Negative

- Four break kinds is more contract than three. Every consumer that branches on break_kind must know the new value or fail closed.

Corrected in review: there are THREE such consumers, not two, and the third is the one this ADR failed to enumerate. The admin panel and the per-case badge do fail closed on the VERDICT — one by an allowlist on "content", the other because first_break_seq is set. What neither of them, nor the AMLR dossier's build_chain_section, did was fail closed on the COPY: each carried hard-coded text asserting alteration or deletion, and a new break kind inherited it. Failing closed on a verdict and failing closed on what the document SAYS are different properties, and enumerating the consumers for one is not enumerating them for the other. That is the lesson worth more than the fix: a new break_kind obliges an audit of every surface that renders a verdict, not only of every branch that reads one. - The classification is only as sound as the ADR-0064 trigger. Stated in the decision and repeated here because it is the assumption a future reader is most likely to miss. - A case affected by an unchained row still reports broken, a word that reads as tampering, even though the accurate reason travels beside it. The per-case surface has three states and gaining a fourth is a separate change; the honest half-measure is that the reason is now correct while the label is coarse. - The verdict now walks the chain twice — once to count, once to replay. Both are O(n) over rows already in memory; nothing is re-read from the database.

Neutral

- No migration, no schema change, no feature flag. The new field defaults to 0, so a consumer that never reads it behaves exactly as before. - 292 rows on the pilot remain outside the chain. This ADR does not chain them — it names them. Back-filling a digest for a row whose write was never locked or ordered would fabricate an attestation, which is worse than an honest gap.

Alternatives Considered

Alternative 1: Skip chain-less rows and keep verifying

- Filter WHERE entry_hash IS NOT NULL, so the chain verifies over the rows it covers. - Why rejected: it is the false clear ADR-0067 forbids, and it is the more dangerous failure than the one being fixed. A tenant with 292 uncovered rows would report ok: true — "the audit trail is intact" — while a tenth of its trail carried no tamper-evidence at all. Excluding the inconvenient rows makes the control agree with itself rather than with the data.

- Keep the code, describe the ambiguity in the architecture page and the endpoint docstring. - Why rejected: it puts the burden on a reader who has just been told a row was deleted, at the moment they are least likely to go reading. It is also the shape this repository has repeatedly measured as ineffective — a documented caveat beside a confident machine verdict loses to the verdict.

Alternative 3: Reclassify AND grant the per-case relaxation in the same change

- Let a case whose rows all precede an unchained row report intact, since the row is visible and attributable. - Why rejected for this change, not on the merits: the argument is probably right, and it is a control relaxation. Bundling it here would mean a diagnostic improvement quietly turned some red badges green, reviewed as though it were a labelling fix. Relaxing a control requires re-proving detection on both sides in the same run, and that deserves its own change with its own evidence.

Alternative 4: Migrate the 35 writers first, so the condition stops arising

- Fix the cause and skip the classification. - Why rejected as a substitute: it does not help the 292 rows already written, which cannot be chained after the fact without fabricating an attestation. So the verifier would keep alleging deletion for historical rows indefinitely. The migration is #1034's own work and is not replaced by this — it is unblocked by it, because the count now measures progress.

Decision context: - Latency: one extra O(n) pass over rows already in memory, plus two None checks per row inside the existing loop. Not measured because the work added is a generator expression over a list the caller has already materialised, alongside a SHA-256 per row. - Dependency surface: none added. - Debuggability: improved, and that is the change's purpose. Every arm returns a reason naming what it checked and what it concluded; the new one additionally names what it is not, and unchained_rows gives the scale. - Reversibility: one arm in one pure function plus one projected field. No schema, no stored verdict to migrate. Reverting restores the previous (wrong) label exactly. - Blast radius: verify_chain and verify_case_segment (both pure), the tenant endpoint's response body, one TypeScript interface, one deriveState clause. Additive: every previously-reachable verdict is still reachable, and the two break_kind consumers fail closed on an unknown value. - Alternative considered: filter chain-less rows out of the verification — rejected because it converts a misdiagnosis into a false clear (see Alternative 1).