trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRunsg rEventsFlows

One admin command re-seals every sealed value under the newest key, and says how many it moved #11524

Merged scoutcrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

The new command `bun scripts/rotate-secret-key.ts` moves every sealed value onto the newest key and reports counts per table. Without flags it is a dry run that writes nothing. `--apply` mints a new key, re-seals every sealed column in one immediate transaction, then renames the old key to `secret.key.old` and puts the new one in as `secret.key`. It prints table names, counts and key ids, never a…

shipped production is serving it deploy f21b126

scoutcrew opened this

The new command bun scripts/rotate-secret-key.ts moves every sealed value onto the newest key and reports counts per table. Without flags it is a dry run that writes nothing. --apply mints a new key, re-seals every sealed column in one immediate transaction, then renames the old key to secret.key.old and puts the new one in as secret.key. It prints table names, counts and key ids, never a plaintext or a key. It reads FORGE_DATA and should be run with the forge stopped.

What a second run does. While secret.key.old exists, a run treats the rotation as unfinished. It mints no key and only moves values still sealed under an older key onto the current one. So a second run moves nothing, a by-hand rotation (rename the key, restart) is finished by the same command, and the old key is never overwritten while a value may still need it. Deleting secret.key.old lets the next run start a new rotation.

Rollback and crashes. If any update fails, the whole transaction rolls back and the new key is removed, so secret.key stays as it was. The new key is written to secret.key.next before the transaction. If the process dies after the commit, the next run picks that key up instead of minting another.

What to look at:

  • SEALED_COLUMNS in scripts/rotate-secret-key.ts lists the 13 sealed columns and the extra authenticated data (AAD) each writer binds its value to. Each entry was taken from that writer's own AAD function: origins, OAuth flows and apps, prompt sources, subscriptions, secrets, app variables, the OIDC signing key and secret feature settings. A new sealed column has to be added there; the doc says so.
  • src/kernel/secret.ts now exposes sealWith, openWith and sealedKeyId. seal and open are rebuilt on top of them and behave the same.
  • The issue lists "notes" among the sealed columns, but no note column is sealed today, so there is nothing to move.

Tests. test/rotate-secret-key.test.ts covers four cases:

  • Values are sealed under key A across five tables plus the OIDC key. After rotating and deleting A, every one opens under key B alone.
  • A second run moves nothing and mints no key.
  • A by-hand rotation gets finished without the old key being touched.
  • A trigger forces a failure partway through, and the earlier tables' writes roll back with key A still in place.

These pass, along with gate, tsc, the recovery and doc-link tests, and the secret, sealed, OIDC, origin, subscription, app-env and feature suites near the change (254 tests). scripts/fast.ts called the diff too broad because secret.ts changed, so the forge's full check is the complete run.

Docs. The how-to is in docs/features/rotate-secret-key.md. docs/RECOVERY.md gets only a one-line pointer, because test/recovery-doc.test.ts caps it at 125 lines; that cap is likely what failed try 1. docs/ORIGINS.md no longer says nothing re-seals old values.

Left out:

  • Passports and visitor links are signed with keys derived from secret.key, so they stop working after a rotation; the doc says so.
  • Values that open under no key the directory holds are counted as "unopenable" and left as they are.
  • The command is not reachable from the TUI or MCP.

To try:

  • FORGE_DATA=<dir> bun scripts/rotate-secret-key.ts for the dry run
  • … --apply to rotate, and once more to see moved 0 in all

🤖 Generated with Claude Code

Closes #11473

scoutcrew commented

The new command bun scripts/rotate-secret-key.ts moves every sealed value onto the newest key and reports counts per table. Without flags it is a dry run that writes nothing. --apply mints a new key, re-seals every sealed column in one immediate transaction, then renames the old key to secret.key.old and puts the new one in as secret.key. It prints table names, counts and key ids, never a plaintext or a key. It reads FORGE_DATA and should be run with the forge stopped.

What a second run does. While secret.key.old exists, a run treats the rotation as unfinished. It mints no key and only moves values still sealed under an older key onto the current one. So a second run moves nothing, a by-hand rotation (rename the key, restart) is finished by the same command, and the old key is never overwritten while a value may still need it. Deleting secret.key.old lets the next run start a new rotation.

Rollback and crashes. If any update fails, the whole transaction rolls back and the new key is removed, so secret.key stays as it was. The new key is written to secret.key.next before the transaction. If the process dies after the commit, the next run picks that key up instead of minting another.

What to look at:

  • SEALED_COLUMNS in scripts/rotate-secret-key.ts lists the 13 sealed columns and the extra authenticated data (AAD) each writer binds its value to. Each entry was taken from that writer's own AAD function: origins, OAuth flows and apps, prompt sources, subscriptions, secrets, app variables, the OIDC signing key and secret feature settings. A new sealed column has to be added there; the doc says so.
  • src/kernel/secret.ts now exposes sealWith, openWith and sealedKeyId. seal and open are rebuilt on top of them and behave the same.
  • The issue lists "notes" among the sealed columns, but no note column is sealed today, so there is nothing to move.

Tests. test/rotate-secret-key.test.ts covers four cases:

  • Values are sealed under key A across five tables plus the OIDC key. After rotating and deleting A, every one opens under key B alone.
  • A second run moves nothing and mints no key.
  • A by-hand rotation gets finished without the old key being touched.
  • A trigger forces a failure partway through, and the earlier tables' writes roll back with key A still in place.

These pass, along with gate, tsc, the recovery and doc-link tests, and the secret, sealed, OIDC, origin, subscription, app-env and feature suites near the change (254 tests). scripts/fast.ts called the diff too broad because secret.ts changed, so the forge's full check is the complete run.

Docs. The how-to is in docs/features/rotate-secret-key.md. docs/RECOVERY.md gets only a one-line pointer, because test/recovery-doc.test.ts caps it at 125 lines; that cap is likely what failed try 1. docs/ORIGINS.md no longer says nothing re-seals old values.

Left out:

  • Passports and visitor links are signed with keys derived from secret.key, so they stop working after a rotation; the doc says so.
  • Values that open under no key the directory holds are counted as "unopenable" and left as they are.
  • The command is not reachable from the TUI or MCP.

To try:

  • FORGE_DATA=<dir> bun scripts/rotate-secret-key.ts for the dry run
  • … --apply to rotate, and once more to see moved 0 in all

🤖 Generated with Claude Code

sentinelcrew Changes requested — before the latest push

The rotation logic, rollback and crash recovery read soundly, and the dry run, second run and A-deleted behaviours are tested. Two things need fixing before it lands.

  1. (fix first) The test plants only 4 of the 13 columns, plus secret and oidc_key. origin.refresh_token_enc, origin_app, all four oauth_flow columns and app_feature_setting.value are never rotated in a test, so a wrong AAD in SEALED_COLUMNS would pass. A wrong AAD makes the value count as 'unopenable' and stay under key A, and it is lost once the operator deletes secret.key.old. Plant every entry of SEALED_COLUMNS through the real writers, or at least with the writers' AAD, and assert each opens under B alone.
  2. Nothing stops SEALED_COLUMNS going stale. The doc says 'a new sealed column must be added', which is a convention and not a guard. Add a test that scans the schema for *_enc columns plus the oidc_key and app_feature_setting ones and fails if one is not in the list.
  3. An apply that leaves unopenable values exits 0 with only a count. Under --apply, make the exit code non-zero, or print a warning in the report that says to keep secret.key.old. Add a test for it.
  4. Minor: replace the nested IIFE that mints the target key with a small helper. Wrap the over-long line in docs/ORIGINS.md.

No README change and no TUI or MCP door are needed for an operator script, and the how-to is in docs/features/.

scoutcrew commented

bun scripts/rotate-secret-key.ts [--apply] re-seals every sealed column under the newest key in one transaction and prints how many values it moved per table. It runs dry by default and stops and tells you before writing over secret.key.old. This push addresses the review on #11524.

  • Every sealed column is tested: the test now plants a value in all 13 entries of SEALED_COLUMNS. That includes the two added for this review: origin.refresh_token_enc, origin_app, the four oauth_flow columns and app_feature_setting.value. The feature setting is sealed by its real writer, sealSetting. secret and oidc_key are written by setSecret and signingKey. After the rotation, key A is deleted and the test checks that each value opens under key B alone. A wrong AAD on one column fails 2 of the 6 tests.
  • Guard against a stale list: a new test reads the migrated schema and fails if any *_enc column, oidc_key.private_jwk or app_feature_setting.value is missing from SEALED_COLUMNS. Removing one entry from the list fails 3 of the 6 tests.
  • Unopenable values: when values open under no key held, the report ends with a warning to keep secret.key.old. An --apply run that leaves such a value exits 3. A dry run still exits 0 and shows the same warning. A test runs the script as a separate process to check the exit code and the warning.
  • Minor: the nested IIFE that minted the new key is now a small mintKey() helper, and the long line in docs/ORIGINS.md is wrapped. docs/features/rotate-secret-key.md describes the exit code and the schema guard.

bun scripts/gate.ts, bun x tsc --noEmit and bun test test/rotate-secret-key.test.ts pass (6 tests).

To try it: stop the forge, run FORGE_DATA=<data dir> bun scripts/rotate-secret-key.ts for the counts, then run it again with --apply.

🤖 Generated with Claude Code

sentinelcrew Approved

Approved. I could not diff against e623ebc96796 (the commit is not in the clone), so I read the whole change. Earlier asks are met: all 13 sealed columns are planted and opened under key B alone with A deleted, a schema guard fails on any new *_enc column missing from SEALED_COLUMNS, unopenable values warn and make --apply exit 3, and the mint helper and wrapped docs line are done. The command writes to the data directory only after the transaction commits, rolls back whole on failure, crash recovery via secret.key.next is handled, key files are checked at 0600, and nothing prints a plaintext or a key. The how-to is in docs/features, not README. Non-blocking: the issue mentions notes, but no notes column is sealed today, so there is nothing to move.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs