Skip to content

feat(storage): persist re-encrypted token on keychain key rotation#2847

Open
afonsojramos wants to merge 6 commits intomainfrom
safe-storage-key-rotation
Open

feat(storage): persist re-encrypted token on keychain key rotation#2847
afonsojramos wants to merge 6 commits intomainfrom
safe-storage-key-rotation

Conversation

@afonsojramos
Copy link
Copy Markdown
Member

Summary

Closes the second half of the #2839 review feedback: when safeStorage.decryptStringAsync reports shouldReEncrypt: true (the OS keychain rotated keys), we now re-encrypt the token with the current key and persist the new ciphertext, so future sessions stay aligned with the active keychain.

Without this, key rotation works for the current session (the plaintext is correctly decrypted under the old key) but the stored ciphertext silently drifts behind the keychain. Eventually it would fail.

Changes

  • IPC contract: SAFE_STORAGE_DECRYPT now returns ISafeStorageDecryptResult = { token: string; reEncryptedToken?: string } instead of string. The contract is enforced via the EventContracts map added in refactor(events): add typed IPC contracts for compile-time safety #2843.
  • Main handler (src/main/handlers/storage.ts): when shouldReEncrypt fires, calls encryptStringAsync on the decrypted plaintext to produce a new ciphertext; returns it under reEncryptedToken. Otherwise returns just {token}.
  • Renderer wrapper (src/renderer/utils/system/comms.ts): decryptValue returns the result object.
  • octokit.ts: destructures .token for auth. Per-request decrypts don't persist (rotation is rare; next app start picks it up via migrateAuthTokens). Keeps octokit decoupled from the auth store.
  • App.tsx migrateAuthTokens: now the canonical persistence path. On startup, if any account's token comes back with reEncryptedToken, it swaps the stored ciphertext and persistAuth's the change.

Tests

  • 7 storage handler tests (including new "re-encrypts when shouldReEncrypt is true" assertion verifying both encryptStringAsync is called and reEncryptedToken is in the response).
  • comms.test.ts updated to expect the new return shape.
  • octokit.test.ts updated to mock the new shape.
  • Renderer test setup mock (vitest.setup.ts) updated.

pnpm tsc --noEmit clean. pnpm test 938/938.

@github-actions github-actions Bot added the enhancement New feature or enhancement to existing functionality label May 7, 2026
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 7, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or enhancement to existing functionality

Development

Successfully merging this pull request may close these issues.

1 participant