## Problem Four things about the secret lifecycle, all one knot. ### 1. Two words for one secret `SecretRepositoryInterface::getCurrentSecretId()` returns "the id of the secret to use for new writes", and `AuditTrailSecretInterface` documents the `active` status as "signs new rows". Those are the same secret. In any healthy install the *current* secret and the *active* secret are one thing, described by two competing status adjectives, and a reader has to discover that for themselves. The second word exists only because of #4 below: two entities can hold `active` at once, so `getCurrentSecretId()` picks the highest-id one. That is a recovery heuristic for a fault, not a second concept, and it should not be paying for a whole extra noun in the vocabulary. ### 2. `rotate()` names something it does not do `rotate()` creates nothing. It finds the highest-id `pending` entity, calls `activate()` on it, and throws when there is none. Key material is provisioned by the operator: a `drupal/key` Key entity in whatever provider their policy requires, plus an `audit_trail_secret` entity referencing it. Nothing in the module ever handles the bytes, and the only key generation anywhere in it is `crypto.getRandomValues()` in `js/audit-trail-secret-key-generator.js`, in the browser, so the material never reaches the server. "Rotate" is the right word for in-with-the-new-out-with-the-old, so the verb was never the problem. What made it misleading is that the operator-facing text around it claimed it minted a key, which #3620328 corrects. What is left is that the operation is better named after what it sets. ### 3. A key cannot be made the signing secret from the command line `audit_trail:rotate-secret` takes no argument, so it can only mean "whichever one happens to be pending". And there is no `audit_trail:create-secret`: the `audit_trail_secret` record can only be created through the admin form. So the first half of putting a new key into use is UI-only, and a key cannot be put into service from a shell at all. For a module whose interface for automation is its commands, that is the gap worth closing. ### 4. Two secrets can hold `active`, and nothing says so `activate()` saves the new entity active FIRST and retires the previous one second, deliberately: the reverse order would leave a zero-active window on a crash and halt every chained write. The cost is that a crash between the two saves leaves two entities `active`. That is documented on `getCurrentSecretId()` as "a transient state", and `AuditTrailSecretRetireForm` allows for it too. Nothing prevents it, and nothing reports it. The secrets list renders both rows as active, the status report does not mention it, and an operator would need to know the highest-id-wins rule to work out which secret is signing. So a half-finished rotation looks like a finished one, and the outgoing secret is never retired. ## Proposed resolution Name the **role**, not the status. A role has one holder by definition, so there is nothing left to disambiguate, and the module already reaches for the phrase when it needs to be unambiguous: "signing secret" appears 14 times, and the interface explains `active` as "signs new rows". - `getCurrentSecretId()` becomes `getSigningSecretId()`. 17 call sites across the verifier, writer, archiver, forms, status report and commands. - `activate(int $secret_id)` becomes `setSigningSecret(int $secret_id)`, pairing with the reader. Its guards do not change: the entity must exist, must be `pending`, and its Key must resolve to at least `MIN_SECRET_BYTES`. - `rotate()` is removed. One production caller (the Drush command) plus a test helper and some test docblocks. - The `pending` / `active` / `retired` statuses stay exactly as they are. They are lifecycle facts about one entity, not answers to "which one signs", so no word is doing double duty any more. - The admin form's **Activate** operation becomes **Use for signing**. Then give the command line the whole operation: ``` drush audit_trail:set-signing-secret --key=<key_id> ``` `--key` mandatory. It creates the `audit_trail_secret` record for that Key if one does not exist, sets it as the signing secret, and retires the outgoing one. The module still never sees the bytes: the Key holds them and the record only points at it. Two policy decisions this needs: - A Key already referenced by the signing secret is **refused**. Re-pointing at the same bytes would read as a rotation in the trail while nothing changed. - A Key that already has a `pending` record **reuses that record** rather than allocating a second id. Two ids pointing at one set of bytes is the thing `SecretUsage` exists to prevent. And treat two-active as the fault it is: report it on the status report, so a half-finished rotation is visible rather than silent. Preventing it outright would mean making the activate-and-retire pair atomic, which is worth doing if it can be done without reintroducing the zero-active window the current order exists to avoid. ## Tests - Setting the signing secret from a Key id, end to end through the command, which is not possible today at all. - A Key already signing is refused, and the trail records no transition. - A Key with an existing `pending` record reuses it, so no second id is allocated for the same bytes. - Two `active` entities reach the status report. - `getSigningSecretId()` still answers correctly while two are active, so the recovery heuristic survives the rename. ## Not in scope `retire-secret` is a genuinely separate operation and is correctly named. `AuditTrailSecret::activate()` also clears the retirement timestamp (`$this->retired = NULL`), which the `isPending()` guard above it makes unreachable; worth removing while the file is open, but it changes nothing. --- AI-Generated: Yes (Claude Code was used to read the secret lifecycle end to end and draft this issue summary. Every claim above was checked against the code: the `rotate()` and `activate()` bodies, the absence of a create-secret command, the 17 `getCurrentSecretId()` call sites, and the 14 existing uses of "signing secret". No code has been written for it yet.)