Resolve #3591791 -- split chain writer + flip event() to throw on write failures

Frank Mablyrequested to merge
issue/audit_trail-3591791:3591791-split-audittraillogger-into into 1.x

Closes #3591791.

Summary

Two related changes on one MR:

1. Break the cycle (commit a65c2a5)

Production crash on the segments admin page caused by a circular service reference detected at cold-resolve time:

audit_trail.chain_archiver
  -> audit_trail.logger (tagged `logger`)
  -> logger.factory (collector)

The logger-tagged service injected @logger.factory for diagnostic warnings, and the container's RegisterLoggersPass needs logger.factory to wire tagged loggers via addLogger() -- cycle.

Architectural fix:

  • New AuditTrailChainWriter (NOT tagged) does the heavy lifting: lock acquisition, secret resolution, previous-hash read, canonical + HMAC + INSERT. 4 deps, ~250 lines.
  • AuditTrailLogger becomes a thin PSR-3 entry point (the only logger-tagged service in audit_trail). Dependency graph never reaches logger.factory.
  • AuditTrail::event() calls chainedWrite() directly with pre-built buckets -- no PSR-3 round-trip for the chain row. dblog / syslog fanout via a separate plain PSR-3 call carrying chain: FALSE.
  • AuditTrailInternalWriterInterface renamed to AuditTrailChainWriterInterface.
  • ChainTimestamper + the tugboat seeder migrated off the chain: TRUE PSR-3 pattern.

2. Flip the write-failure contract (commit 047d278)

Previous design: AuditTrail::event() wrapped dispatchEvent() in a catch-all that converted every write failure into a chain: FALSE warning. This silently dropped audit rows on lock contention / secret-repo failure / DB outage. An attacker who could provoke a transient failure would choose which incriminating events disappear, while operators saw a chain that looked complete -- defeating the tamper-evidence guarantee.

Changes:

  • AuditTrailChainWriter::chainedWrite() returns array (was ?array) and throws on every failure path. Lock contention: throws instead of returning NULL. Secret-repo / DB errors: already threw.
  • AuditTrail::event() no longer swallows. Exceptions propagate to the bridge / host op -- the highest-level decider and the right place to retry, abort the host op so the underlying side-effect rolls back, or alert.
  • Plugin-isolation catches (filter + contributor try/catch) STAY at the framework level: a buggy third-party plugin degrades the row's payload, it doesn't break event ingestion. Different swallow level for a different concern.
  • AuditTrailLogger::log() (the PSR-3 path) still catches per PSR-3 §1.3 and is documented "OK to lose events" in AuditTrailInterface. The error_log() breadcrumb is enriched to a full structured JSON payload (chain / channel / action / resource / severity / message / transient) so operators can grep + reconstruct dropped PSR-3 events from PHP logs.

Why

Audit completeness is a tamper-evidence contract. Two doors:

  • \Drupal::logger($chan)->log() -> casual best-effort. PSR-3 §1.3 forbids throwing from log() anyway. OK to lose events. Breadcrumb in PHP error log for manual recovery.
  • AuditTrail::event() / AuditTrailChainWriterInterface::chainedWrite() -> tamper-evidence contract. Caller wants the row to land or be told it didn't. Throws on failure.

Follow-up

Issue draft prepared for a deferred-retry sink on the PSR-3 path (audit_trail_deferred table + cron retry hook) so even casual PSR-3 callers get no event loss. Out of scope here, follow-up issue to be filed shortly.

Test plan

  • phpcs clean on changed code
  • phpstan clean on changed code (5 pre-existing new.static warnings in untouched files unchanged)
  • 423 audit_trail unit + kernel tests pass (Tests: 423, Assertions: 1890+, 0 errors, 0 failures)
  • Cold-resolve cycle no longer reproducible (tested manually against the segments admin page)
  • Drupal.org CI green

Files changed (29 files, 2 commits)

20 + 9 files across the writer / logger / orchestrator / submodule / tests

Merge request reports