pkg/age: RotateKeys is non-transactional — crash window can lose secrets #159

Summary

pkg/age.RotateKeys (post-#151 fix) performs a non-transactional sequence: decrypt with old key in memory, remove talm.key, generate new key, encrypt and write the secrets file. A crash, signal, disk-full, permission flip, or any non-clean exit between os.Remove(keyFile) and successful write of both the new key and the re-encrypted file leaves the project in a state from which only the in-memory plaintext can recover the secrets — and the operator likely won't know to dump it before the process exits.

Failure window

1. Read encrypted secrets, decrypt with old key   (in-memory plaintext)
2. os.Remove(talm.key)                            <-- old key gone from disk
3. GenerateKey -> writes new talm.key             <-- if step crashes: no key on disk; encrypted file still uses old key
4. Encrypt with new key                            (in-memory ciphertext)
5. yaml.Marshal                                    (still in-memory)
6. os.WriteFile(encryptedFile, ...)                <-- if step crashes: new key on disk, encrypted file still on old key

Steps 2-3 are the worst window: the encrypted file on disk pairs with a key that no longer exists. Steps 3-6 are recoverable by re-running the rotation against the still-on-disk old encrypted file (the new key is fresh and empty — re-rotate against backup), but only if the operator captured secrets.encrypted.yaml before the half-complete run.

Suggested fix

Make the rotation transactional. Two patterns work:

1. Temp + rename:
   • Generate new key under a temp name (e.g. .talm.key.new).
   • Re-encrypt secrets to a temp file (e.g. .secrets.encrypted.yaml.new).
   • Atomically rename both into place via os.Rename. If any step before rename fails, the originals are still on disk untouched.
2. Recovery snapshot: write the in-memory plaintext to secrets.yaml.recovery (mode 0600) before step 2, remove it after step 6 succeeds. Operator has a recovery path even if the temp+rename version is too invasive.

The temp+rename pattern is the more idiomatic Go / Unix solution and uses the same atomic-write pattern pkg/secureperm.WriteFile already implements. Reusing secureperm.WriteFile directly for the new key + encrypted file gets the rename atomicity for free.

Notes

Surfaced during the /branch-review of the #151 fix on PR (TBD). Not a regression introduced by that PR — before #151 the function was a silent no-op so the question never arose. Now that RotateKeys actually does work, the work needs to be transactional.

Recommend kind/bug, area/age, priority/important-soon, triage/accepted.