pkg/age: RotateKeys does not actually rotate the on-disk key #151

Summary

pkg/age.RotateKeys is documented as "rotate encryption keys", but in practice it does not rotate the on-disk identity. The function decrypts secrets with the old key, calls GenerateKey() expecting a fresh identity, then re-encrypts. Because GenerateKey() is a load-or-create helper, it returns the existing identity whenever talm.key already exists (the normal pre-rotation state), so the "new" key is the same key. The encrypted secrets file is rewritten with the same ciphertext-equivalent content under the same key.

Reproduction

oldPub, _ := age.GetPublicKeyFromFile(rootDir)
_ = age.RotateKeys(rootDir)
newPub, _ := age.GetPublicKeyFromFile(rootDir)
// oldPub == newPub  (rotation is a no-op)

A test that asserts oldPub != newPub after RotateKeys fails on current main.

Root cause

pkg/age/age.go:485-515 — the comment on line 511 says Generate new key (this overwrites talm.key), but GenerateKey at pkg/age/age.go:43 short-circuits when talm.key already exists:

if _, err := os.Stat(keyFile); err == nil {
    // Key exists, load it
    identity, err := LoadKey(rootDir)
    // ...
    return identity, false, nil   // <- returns OLD identity
}

Impact

Operators expecting key rotation (post-incident, scheduled rotation, departing-team-member offboarding) believe a compromised key has been retired, when in reality the same key still encrypts every secret. Any actor holding the old key continues to read every secret in secrets.encrypted.yaml after the alleged rotation.

Suggested fix

Either:

1. Remove talm.key before calling GenerateKey in RotateKeys:

   if err := os.Remove(filepath.Join(rootDir, "talm.key")); err != nil { ... }
   newIdentity, _, err := GenerateKey(rootDir)

2. Or introduce a separate GenerateNewKey(rootDir) helper that errors when talm.key already exists, and call it from RotateKeys.

Either approach should also write a test that asserts oldPub != newPub to lock in the contract.

Notes

Discovered while writing contract tests for pkg/age (see branch test/chart-contract, file pkg/age/contract_test.go). The contract test there explicitly does NOT pin the broken behaviour — it asserts only round-trip integrity and documents the bug in a comment so the test can be tightened once the fix lands.