## Problem Operator-facing surfaces and contracts that describe something the module does not do, found in a full audit. ### 1. The status report warned about stale contributors and ignored stale filters `AuditTrailRequirementsHooks::checkContributors()` walked every active chain and warned when a configured contributor plugin id no longer resolves, because "silent skip preserves availability of the audit pipeline, but the operator loses signal". A chain's FILTERS are stored the same way and skipped the same way (`ChainFilters::loadFilter()` returns NULL on an unknown id), and nothing reported them. A vanished filter is the louder half of the pair: filters are the operator's statement about what a chain records, and `ChainFilters::loadFilter()` skips one it cannot resolve rather than reading it as a rejection, so the chain quietly starts recording everything that filter was there to decide on — more, never less, whichever way it was configured. Reachable exactly where the contributor case is: a plugin dropped by a module UPDATE, where `onDependencyRemoval()` never fires because the module is still installed. That case now has a section in `docs/configuration.md`; only the uninstall case did. The check is `checkChainPlugins()` and its requirement key `audit_trail_stale_chain_plugins`, because after adding filters the old name described half of what it reports. ### 2. The secret-key generator said "Copied!" when nothing was copied `copyValue()` in `js/audit-trail-secret-key-generator.js` returned TRUE whenever the field had a value, meaning "a copy was attempted", and the caller read that as success: it swapped the button to "Copied!" and announced "Key value copied to the clipboard." Both `document.execCommand('copy')` returning false and the `navigator.clipboard.writeText(...).catch(() => {})` fallback rejecting were swallowed. On a plain-HTTP admin host the operator — and a screen-reader user, who has only the announcement — is told the 256-bit key is on the clipboard when it is not. The value is generated in the browser and never sent anywhere, so navigating away loses it. The sibling file `js/audit-trail-copy-to-clipboard.js` gets this right: it awaits the write and announces a distinct assertive failure. ### 3. The secrets list said nothing about a secret whose key is an empty string `AuditTrailSecretListBuilder` offered "(missing: edit to set)" through `??`, which answers only for NULL. `key_id` saved as an empty string — the shape a partial config restore leaves, and the shape the repair tests in `AuditTrailSecretCreateTest` seed — showed a blank cell with nothing to say what was wrong with it, while `getSecret()` refuses both spellings identically. That test class's own docblock already described the message as covering an empty `key_id`. ### 4. Contracts and comments describing things that are no longer true - `SecretRepositoryInterface` documented the deleted state backend as the default. It now says what is true, including the part that is easy to get wrong: this module keeps no store of its own, but `drupal/key` ships a State provider, so a Key entity pointed at it puts the signing material back in the database this module exists to detect tampering in. Choosing the provider is the operator's, and it decides whether the HMAC layer is worth anything against an attacker who already has database write access. - Its `rotate()` said "Mints a fresh secret" (the only implementation promotes a pending entity and never touches key material), and `getCurrentSecretId()` documented a `\RuntimeException` telling the operator to "re-install the module to mint one" where the code throws `SecretNotAvailableException` and says to create and activate one. - `config/schema/audit_trail.schema.yml`: `key_id` was "or NULL for state-backed deployments"; `SecretRetireTest`'s docblock said "on the state backend". - `AuditTrailSecret`'s class docblock said a new secret's "status defaults to retired". It defaults to `pending`, and `activate()` refuses anything that is not pending, so a secret that really defaulted to retired could never be activated at all. - `audit_trail.install`: the paragraph describing verification checkpoints sat above the `audit_trail_outbox` table and named a `last_hmac` column that does not exist. - `SecretKeyChecker::buildRequirement()` documented its return as keyed `audit_trail_id_map`; every return is keyed `audit_trail_secret_keys`. The service was injected as `$keyMapDetector`, a leftover of a retired concept. - `docs/architecture.md` named `AuditTrailChainRegistryHooks`, a class that has never existed. `docs/configuration.md` and `docs/security.md` described the status report in terms of `REQUIREMENT_WARNING` / `REQUIREMENT_ERROR`, core's pre-11.x constants, where the module uses the `RequirementSeverity` enum. `docs/security.md` sent the reader to the roadmap for a future `audit_trail:file-purge --allow-missing-secret` flag: there is no `audit_trail:file-purge` command, and the roadmap says nothing about it. ### 6. Every operator-facing surface still said rotation mints a key The contract correction above left the surfaces an OPERATOR reads still asserting the retired behaviour, so the module said both things at once. The Drush command printed `Minted secret #7` on every rotation and its `--help` said "Mint a new HMAC secret and make it the current one"; `docs/commands.md` repeated it. All three claim the module did the one part of key management it deliberately does not do, and someone reading them could skip provisioning a Key and then meet `Cannot rotate: no pending audit_trail_secret entity available to promote`. `docs/security.md` went further, saying a reinstall "mints fresh ones". There is no install hook, no secret in shipped config, and nothing outside the forms and Drush creates one, so a reinstalled site has no active secret at all and cannot chain a row until an operator provisions one. That is a worse outcome than the doc described, and how it surfaces differs by ingress (`AuditTrailInterface::event()` raises; the PSR-3 path cannot throw and falls back to `error_log()`), so it now says which. ## Proposed resolution Report both kinds of stale plugin, announce the copy by its outcome, name a missing key however it was stored, and correct each contract to what the code does. ## Withdrawn from this issue Two formatting changes were proposed and both were wrong, for the same reason: a formatter invoked by hand is not the one the pipeline runs. - A wrap change in `css/audit-trail-diff.css`. **Dropped.** The stylelint job's prettier asks for the opposite wrap, so the shipped rule was already right and the change made the job red. The same reading also claimed that job cannot fail the module; it does lint the module's CSS and exits 2. - Reformatting both JavaScript files from single to double quotes. **Dropped.** The eslint job resolves prettier's configuration to core's, which sets `singleQuote: true`; invoking eslint by hand against core's `.eslintrc.passing.json` does not, and falls back to prettier's own double-quote default. The shipped style was already the one the job wants, and the job was never failing on these files. ## Tests - `CronVerifyTest::testStaleFilterPluginIsReported` — a chain configuring a filter nothing provides reaches the status report, named. Fails without the fix (the requirement key is absent entirely). - `AuditTrailSecretCreateTest::testSecretsListNamesTheMissingKey` — the list names a missing key when `key_id` is an empty string. Fails without the fix. - `DrushExitCodeTest::testRotateSecretReportsPromotingNotMinting` — the rotation reports the secret that is now current and does not claim to have created it. Asserted on the command's real output rather than on the source text, because what was wrong is what the operator is told. - The JavaScript change carries no automated test: reaching the failure needs a browser that refuses a clipboard write, and this module has no FunctionalJavascript coverage to build that on. It is stated here rather than left implied. --- AI-Generated: Yes (Claude Code was used to audit the module, draft this issue summary, and write the code and tests on the merge request. Each new test was verified to fail without its fix and to pass with it, and phpcs, PHPStan, cspell and the module's kernel suite were run before pushing.)