ADR-0029: Audit Row Durability Contract
Accepted Cross-Project UniversalDate: 2026-05-28 Compliance: ISO 27001 A.5.33 (records protection — tamper-detectable audit trail) + A.8.15 (logging — all security events recorded)
Context
ADR-0001 codifies that every audited action writes a hash-chained audit row from inside the Action's transaction. The chain shape is well-defined: SHA-256 over canonical payload + previous hash, append-only, point-in-time actor snapshot. What ADR-0001 does not specify is the relationship between the audit row and everything else the Action does — the order of operations within the transaction closure and the disposition of non-transactional side effects.
That gap surfaces as two distinct failure modes, observed twice in 2026-05 across two compliance territories:
Failure mode 1 — throw inside the closure rolls back the failure-audit row
Pattern: an Action validates input, finds a violation, audits the failure, then throws. The natural shape is:
DB::transaction(function () {
// validate...
$auditLogger->logFailure(...);
throw new InvalidCredentialsException;
});The throw propagates out of the closure. The DB::transaction() wrapper sees the exception and rolls back the transaction — including the audit row. The failure happened. The audit row that recorded the failure didn't. A.8.15 is violated: a security-relevant event is not logged.
Surfaced via: ISMS-0003 PR #7 review pre-Copilot ([[feedback_sentinel_return_audit_transaction]]); the writer's runtime assertion assertWithinTransaction proves the closure is the right scope, but throwing inside it nullifies the write.
Resolution (already in memory): sentinel return. The closure returns the exception as a value rather than throwing; post-transaction code re-throws after commit. The audit row survives.
$result = DB::transaction(fn() => /* validate, audit, return sentinel */);
if ($result instanceof InvalidCredentialsException) throw $result;Failure mode 2 — non-transactional state mutated inside the closure persists past rollback
Pattern: an Action succeeds, mutates non-transactional state (StatefulGuard::login, Session::regenerate, Cache::put, queue dispatch, external API call), then writes the audit row, all inside the same transaction closure.
DB::transaction(function () use ($user) {
$this->guard->login($user);
$this->session->regenerate();
$this->session->put('awaiting_two_factor', true);
$auditLogger->logLoggedIn($user, ...);
});If the audit write throws (DB connection drops, CHECK constraint violation, advisory-lock timeout, hash-chain integrity violation), the transaction rolls back the audit row but the session/guard mutation persists — session storage is file/cookie/Redis-backed, not the application's primary DB. The user is logged in. There is no audit row recording that they logged in. A.8.15 is violated symmetrically to failure mode 1.
Surfaced via: ISMS-0003 PR #7 Copilot review 2026-05-28 across AuthenticateWorkerAction, VerifyTwoFactorChallengeAction, and (missed by Copilot but same defect class) LogoutWorkerAction. Fixed in commit f1d357b.
Resolution: post-commit mutation. Non-transactional state changes move outside the closure, after the transaction commits.
$user = DB::transaction(fn() => /* validate, audit, return user */);
$this->guard->login($user);
$this->session->regenerate();
$this->session->put('awaiting_two_factor', true);The shared invariant
Both failure modes share a root cause: the transaction closure contains code whose effects are observable independently of the audit row's durability. A throw is observable (callers see the exception). A session mutation is observable (the client's next request carries the session state). When the audit row commits, those effects are correct. When the audit row rolls back, those effects shouldn't have happened — but they did.
The contract that closes both modes is symmetric:
The audit transaction's closure contains only operations whose effects are confined to the database transaction it wraps. Throws happen via sentinel-return outside the closure. Non-transactional state mutations happen post-commit outside the closure. Nothing observable to a caller, a session, an external system, a queue, or a cache happens inside the closure other than the audit row write itself (and any sibling DB writes that must commit or roll back atomically with it).
This ADR codifies that contract.
Decision
Every audited Action obeys the Audit Row Durability Contract:
Closure scope is bounded by transactional atomicity. The body of
DB::transaction(...)MAY contain only:- The audit row write.
- Sibling DB writes that MUST commit or roll back atomically with the audit row (e.g., an entity mutation that the audit row records).
- Read queries against the DB that inform the writes above.
- Pure computation that doesn't touch external state.
Throws happen outside the closure via sentinel return. Failure paths inside the closure return an exception (or other sentinel value) rather than throwing. Post-transaction code branches on the return value and either throws the exception or proceeds. The audit row of the failure event survives the throw.
Non-transactional state mutations happen outside the closure post-commit. Session storage, authentication guards, cache, queue dispatch, external API calls, file I/O, broadcast events, and anything else whose effect outlives a DB transaction MUST live after the closure returns. If the audit row write fails, the closure throws (caught by the wrapping caller or the framework's exception handler); none of the non-transactional state has been touched.
Reference shape — success-only Action
public function execute(Input $input): Result
{
$result = $this->db->transaction(function() use ($input): Result {
// Read + audit + commit-coupled DB writes.
$entity = $this->repo->find($input->id);
$entity->status = $input->status;
$entity->save();
$this->auditLogger->logUpdated($entity, $input);
return new Result($entity);
});
// Post-commit: audit row is durable. Mutate non-transactional state.
$this->cache->forget($result->entity->cacheKey());
$this->broadcaster->dispatch(new EntityUpdated($result->entity));
return $result;
}Reference shape — failure-via-sentinel-return
public function execute(Input $input): User
{
$result = $this->db->transaction(function() use ($input): User|InvalidCredentialsException {
$user = $this->repo->findByEmail($input->email);
if (!$user || !$this->hasher->check($input->password, $user->password)) {
$this->auditLogger->logLoginFailed(null, $input->email, ['reason' => 'invalid-credentials']);
return new InvalidCredentialsException;
}
$this->auditLogger->logLoggedIn($user);
return $user;
});
if ($result instanceof InvalidCredentialsException) {
throw $result;
}
// Post-commit: audit row is durable. Mutate non-transactional state.
$this->guard->login($result);
$this->session->regenerate();
return $result;
}Reference shape — audit-write-only Action (e.g. logout)
When the only DB write the Action performs IS the audit row and everything else (guard teardown, session invalidation) is non-transactional:
public function execute(User $user): void
{
$this->db->transaction(function() use ($user): void {
$this->auditLogger->logLoggedOut($user);
});
// Post-commit: audit row is durable. Tear down non-transactional state.
$this->guard->logout();
$this->session->invalidate();
$this->session->regenerateToken();
}The transaction wrapper is still required because the audit writer's assertWithinTransaction precondition is load-bearing for hash-chain locking semantics — even single-write audit actions need the transaction for serialization, not just atomicity.
Closure-scope reasoning anchors
Three test questions an Engineer applies when reviewing whether a line belongs inside the closure:
- "Can this throw?" If yes and the throw represents a domain-meaningful failure (validation, business rule), use sentinel-return. If yes and the throw represents an infrastructure failure (connection drop, integrity violation), accept that the closure will roll back — that's correct behavior, the failure happens before any observable effect.
- "Is this effect rolled back if the audit write fails?" If no (session, cache, queue, external API), move it outside the closure.
- "Does this effect need to commit atomically with the audit row?" If yes (the audited entity's own state change), keep it inside.
Options Considered
| Option | Verdict | Reason |
|---|---|---|
| Audit Row Durability Contract (closure-scope discipline) | Accepted | Closes both failure modes with a single invariant. The contract is mechanical enough to enforce structurally and small enough that it doesn't accumulate edge cases. |
| Eventual-consistency audit (write audit row from a queued job after the action commits) | Rejected | Defers durability; a queue drain failure means the audit row never lands. A.8.15 requires the audit be tied to the event, not to a later async process. |
| Defensive try/catch around session/guard mutations inside the closure | Rejected | Adds noise to every Action. The pattern asks the Engineer to remember to catch in a specific way; the Audit Row Durability Contract asks the Engineer to remember a single structural rule (closure boundary). The latter survives rotating maintainers. |
| Outer transaction wraps the inner audit transaction; outer transaction's commit includes the session save | Rejected | Session save in Laravel is a middleware concern, not an Action concern — pushing it into the Action's transaction is a layering violation. Also doesn't work for non-session non-transactional effects (cache, queue, external API). |
| Two-phase commit between DB and session store | Rejected | Way more machinery than the failure mode justifies. The single-DB-transaction + post-commit ordering achieves the same forensic guarantee without distributed-transaction complexity. |
Consequences
Positive
- Forensic guarantee restored. Every observable effect of an audited Action is preceded by a durable audit row. The audit log is the source of truth; if it didn't record the event, the event didn't have user-visible effects.
- A.5.33 + A.8.15 invariant strengthened. ISO 27001 audit reviewers can read the contract back to the implementation in a single hop. The hash chain proves tamper-detection; the durability contract proves nothing-without-audit.
- Closure body becomes smaller and easier to review. Read-and-audit-and-write becomes the entire closure for most Actions. Reviewers scanning a new Action can read the closure in a glance and verify it doesn't violate the contract.
- Sentinel-return pattern generalizes. The shape that solved the failure-audit-row problem in ISO-compliance territories also solves the success-side problem; the reasoning is dual.
Negative
- More verbose than throw-inside-closure. Sentinel-return adds a branch outside the closure (
if ($result instanceof X) throw $result;). For Actions with multiple failure modes, the branch can grow — though unioning the sentinel types (User|InvalidCredentialsException|RoleMismatchException) keeps the post-transaction code tractable. - Reader's first-time tax. A reader new to the codebase will reach for the "why is this exception returned not thrown?" question. The closure comment block (
// @audit-snapshot-retry-safety: failure paths RETURN the exception as a sentinel rather than throwing — see ADR-0029) is load-bearing documentation, not optional. - Closure-scope discipline is not yet enforced by tooling. Static analysis can't directly read "this line touches non-transactional state" — see §Enforcement. Until a custom rule lands, code review is the gate.
Risks
- Risk: An Engineer mid-task forgets the contract and puts a
Cache::forget()or queue dispatch inside the closure. Audit row rolls back; cache or queue effect persists. Mitigation: PHPStan rule (audit-warroom-rules) candidate — detect specific facade/method calls insideDB::transaction(...)closures in Action classes. Pattern: walk the AST undertransaction()'s callback, flag calls to a blocklist ofCache::*,Queue::*,Bus::dispatch,Mail::*,Notification::*,Session::*,Auth::*, andStorage::*. Closure-scope analysis is non-trivial but tractable. Until the rule lands, enforcement is the Engineer SOP §3 reading-checklist + ADR-0029 doctrine in territory CLAUDE.md. - Risk: A future Action genuinely needs non-transactional state to mutate atomically with the audit row (e.g., a Stripe charge — the external API call can't be rolled back, so it MUST happen after the audit row commits, but also MUST happen at all once the audit row commits). Mitigation: the contract handles this — Stripe call happens post-commit. The only loss is "Stripe charge succeeds but audit row already wrote 'charge initiated' without 'charge completed'" — that's a different problem (idempotency, reconciliation), solved by a follow-up audit row "charge succeeded" rather than by widening the contract.
- Risk: Sentinel-return obscures the call-graph in IDEs that don't follow union-type returns gracefully. Mitigation: PHPStan + Larastan resolve the union; IDE drift is an editor-tooling problem, not an architectural one.
- Risk: The "single audit-row write per Action" assumption breaks for Actions that write multiple sequential audit rows in one transaction (e.g., bulk-end-of-day batch close). Mitigation: the contract still holds — multiple audit writes in sequence inside the closure, all roll back together if any throws. The closure's atomicity guarantee is about the set of audit rows, not a single one. Post-commit non-transactional effects still wait for the whole set.
Enforcement
| What | Mechanism | Scope |
|---|---|---|
| Audit writer requires enclosing transaction | Runtime assertWithinTransaction() in the writer | Already shipped in [[project_isms_per_entity_audit_writers]]; kendo's generic AuditLogWriter carries the same assertion. Closes "audit row written without transaction." Does NOT close the closure-scope discipline. |
| Sentinel-return for failure-audit rows | Code review + ADR-0029 doctrine reading | Until a static rule lands. Memory binding: [[feedback_sentinel_return_audit_transaction]]. |
| Post-commit mutation for non-transactional side effects | Code review + ADR-0029 doctrine reading + ->ordered() Mockery contracts in unit tests | Unit test ordering pins the contract per Action; a Mockery ->ordered() violation surfaces at CI time. Cheap belt-and-braces until a static rule lands. |
Custom PHPStan rule — detect non-transactional facade calls inside DB::transaction(...) closures in App\Actions\* | Candidate for script-development/phpstan-warroom-rules | Enforcement queue candidate. Closes both failure modes mechanically. Scope: walk the AST under ConnectionInterface::transaction() (and DB::transaction() facade form) callback; flag method calls to the blocklist namespaces. False-positive risk on legitimate Auth-internal reads (Auth::user() for read) — start with a narrow blocklist of mutation methods (login, logout, attempt, regenerate, invalidate, put, forget, dispatch, send, notify) and expand. |
| Custom Pest arch test — Actions following the success-then-post-commit shape | Possible | Lower-confidence than the PHPStan rule (arch tests can detect file shape but struggle with intra-method control flow). Use as a secondary signal if the PHPStan rule turns out to be too noisy. |
The PHPStan rule is the long-term enforcement target. Until it lands, doctrine + Mockery ->ordered() contracts + ADR-0029 awareness in territory CLAUDE.md projections form the enforcement layer.
Resolved Questions
Why not put audit writes in a per-Action observer instead?
Resolved 2026-05-28. Observers fire on model events (saved, deleted, etc.) and live outside the Action's transaction control. Observers see the entity state, not the user intent (an observer can't distinguish "admin updated this" from "scheduler updated this" without ambient state lookup). ADR-0001 already rejects observers; ADR-0029 inherits that rejection. The Action is the authority for what gets audited and how.
Should the contract require a single audit row per Action?
Resolved 2026-05-28. No. The contract is about closure scope, not row count. Bulk-close Actions, retry-loop Actions, and cascade-delete Actions all write multiple audit rows in one transaction. They all benefit from the contract identically — multiple writes inside the closure, all observable post-commit effects outside. The contract scales by transaction boundary, not by row count.
What about Actions that don't write any DB rows except the audit row?
Resolved 2026-05-28. They still wrap the audit write in a transaction, because the writer's assertWithinTransaction() precondition encodes hash-chain serialization semantics (Postgres advisory lock + lockForUpdate() chain-tail), not atomicity-with-other-writes semantics. Single-write audit Actions look like the §Reference shape — audit-only Action (e.g., logout). The transaction is the lock scope, not the consistency scope.
What about cross-DB or cross-aggregate audit writes?
Resolved 2026-05-28 — open with caveat. ISMS, kendo, emmie, and other current war-room territories use a single primary DB; the closure protects the audit chain on that DB. For territories that grow to multiple DBs (read replicas, multi-tenant DB-per-customer per ADR-0008) the contract still holds: each chain has its own transaction, each transaction obeys the closure-scope discipline. If a single Action audits across two DBs, that Action has two transactions (one per DB) and either two sentinel-return branches or one if the first DB's write determines the second. The fanout case is rare enough that case-by-case engineering judgment outweighs a generalized rule.
Why isn't the deliberation captured as a campaign report?
Resolved 2026-05-28. Both patterns surfaced incident-first across a 48-hour window: sentinel-return from a 2026-05-27 ISMS Medic halt (6/11 feature tests RED on the original throw-inside-closure shape), post-commit mutation from a 2026-05-28 Copilot review on ISMS PR #7. Neither has a standalone campaign report; the synthesis happened in this ADR. Allies reading ADR-0029 should know the depth is incident-driven, not multi-day deliberated — the fix is well-evidenced (failed tests, production-bound code, fault-injection smoke), but the cross-territory propagation claim ("this pattern applies to all audited Actions in every Laravel territory") rests on a Sapper-sweep verification queued under [[deferred]] rather than already-completed reconnaissance.
Does the contract apply to non-audited Actions?
Resolved 2026-05-28. No. ADR-0011 already requires every mutating Action to wrap in a transaction; ADR-0029 layers an additional discipline on top for audited Actions. Non-audited Actions follow ADR-0011 alone — their closure can contain non-transactional state mutations because there's no audit row whose durability needs protecting. Reviewers should consider whether a non-audited Action ought to be audited (most are), but that's a different decision.
Implementation
Status legend:
- In Progress — Contract adopted; reference shapes shipped; Mockery
->ordered()contracts pin the ordering in production code. - Currently Safe / Doctrine Pending — Code survey shows no defect-class violations today, but the contract is not documented in territory CLAUDE.md, no
->ordered()contracts exist, and the next developer adding audit-Action work has no reference shape. Prospective risk = high; immediate risk = low. - Pre-Adoption Blocking — Audit infrastructure is about to land; the audit-landing dispatch MUST include ADR-0029 as a prerequisite. Skipping the projection now would mean adopting the contract retroactively, harder than landing it as part of the first audit-Action.
- Not Started — No audit infrastructure exists; contract becomes scope when audit infrastructure lands.
| Territory | State | Notes |
|---|---|---|
| isms | In Progress | First territory to land the contract. PR #7 commit f1d357b (2026-05-28) implements both reference shapes across AuthenticateWorkerAction, VerifyTwoFactorChallengeAction, LogoutWorkerAction. ->ordered() Mockery contracts pin the ordering in three unit tests. ADR projection landed in territory CLAUDE.md. |
| kendo | Currently Safe / Doctrine Pending | Generic AuditLogWriter carries the assertWithinTransaction precondition (same shape as ISMS). Cross-territory recon (2026-05-28 sample of 10 audited Actions in App\Actions\Auth\*) observed zero defect-class violations — closure scope holds by accident of Kendo's call-site patterns (no queue dispatch / session mutation / external HTTP inside db->transaction(...) closures). BUT: no projection in territory CLAUDE.md, no ->ordered() contracts, no reference shape documentation. Future developer unfamiliar with the contract can ship a violation past code review. Sapper sweep across full audit-Action surface queued via deferred.md. |
| emmie | Currently Safe / Doctrine Pending | Larger audit surface than Kendo (6 logger files per multi-tenant per-entity writer pattern; ADR-0008). Sampled audit-Action survey 2026-05-28 found no defect-class violations but did not exhaust the surface. Sapper sweep queued via deferred.md covers full audit-Action surface verification. Multi-tenant DB-per-customer adds nuance — see §Resolved Question "What about cross-DB or cross-aggregate audit writes?"; each tenant DB carries its own chain, each chain's transaction obeys the closure-scope discipline independently. |
| codebook | Pre-Adoption Blocking | NEN 7510 + AVG compliance gravity + audit infrastructure landing approaching. The next audit-landing dispatch order in codebook MUST include ADR-0029 projection in the territory CLAUDE.md AND adopt the reference shapes for the inaugural audit-writing Actions. Adopting retroactively post-landing is materially harder than landing it together; this is the cheapest moment to wire the discipline. |
| entreezuil | Currently Safe / Doctrine Pending | Audit surface smaller than kendo/emmie; same shape (ISO 27001, script-development alliance). Sapper sweep coverage extends here on the same dispatch as kendo + emmie if cycle permits. |
| ublgenie | Currently Safe / Doctrine Pending | Audit surface smaller; same compliance gravity (ISO 27001 + AVG). Same Sapper sweep coverage. |
| brick-inventory-orchestrator | Currently Safe / Doctrine Pending | Commander's personal domain; no compliance pressure but the contract still applies wherever audit infrastructure exists. Sweep is lowest priority; verify on next Engineer dispatch that touches an audited Action. |
| daymate | Not Started | AVG-minimum; no current audit infrastructure. Apply contract when audit lands. |