feat: M12 Carousel — key rotation without re-encrypting data (v5.2.0)#13
Merged
flyingrobots merged 6 commits intomainfrom Feb 28, 2026
Merged
feat: M12 Carousel — key rotation without re-encrypting data (v5.2.0)#13flyingrobots merged 6 commits intomainfrom
flyingrobots merged 6 commits intomainfrom
Conversation
Add rotateKey() on CasService for re-wrapping the DEK with a new KEK, leaving data blobs untouched. Add rotateVaultPassphrase() on the facade for atomic vault-wide passphrase rotation. keyVersion tracking at both manifest-level and per-recipient level enables audit compliance. New CLI commands: `git cas rotate` and `git cas vault rotate`. New error code: ROTATION_NOT_SUPPORTED. 27 new unit tests (784 total). All three runtimes pass.
- Convert rotation helpers to #private methods in CasService - Add CLI rotation command reference to docs/API.md - Fix error code table to include rotateVaultPassphrase() contexts - Reduce test-only KDF iterations to prevent CI flapping
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughIntroduces key rotation capabilities for envelope-encrypted content. Adds rotateKey() and rotateVaultPassphrase() public APIs to ContentAddressableStore/CasService, with corresponding CLI commands (rotate, vault rotate). Extends Manifest schema with keyVersion tracking on recipients and encryption entries. Includes comprehensive test coverage, documentation updates, and version bump to 5.2.1. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CAS as CAS Service
participant Crypto
participant Git as Git Storage
participant KDF
Client->>CAS: rotateKey(manifest, oldKey, newKey, label)
CAS->>CAS: Validate envelope encryption present
CAS->>CAS: Find recipient by label or oldKey
CAS->>Crypto: Unwrap DEK with oldKey
Crypto-->>CAS: DEK
CAS->>Crypto: Wrap DEK with newKey
Crypto-->>CAS: Wrapped DEK (new)
CAS->>CAS: Build rotated manifest<br/>(keyVersion++)
CAS-->>Client: Updated Manifest
sequenceDiagram
participant Client
participant CAS as CAS Service
participant Vault
participant KDF
participant Crypto
participant Git as Git Storage
Client->>CAS: rotateVaultPassphrase(oldPass, newPass)
CAS->>Vault: Read vault metadata
Vault-->>CAS: Metadata + entries
CAS->>KDF: Derive old KEK from oldPass
KDF-->>CAS: Old KEK
CAS->>KDF: Derive new KEK from newPass
KDF-->>CAS: New KEK
loop For each envelope entry
CAS->>Crypto: Unwrap DEK with old KEK
Crypto-->>CAS: DEK
CAS->>Crypto: Wrap DEK with new KEK
Crypto-->>CAS: Wrapped DEK (new)
end
CAS->>Vault: Update metadata (new salt)
CAS->>Git: Commit rotated state<br/>(with retry on conflict)
Git-->>CAS: commitOid
CAS-->>Client: {commitOid, rotatedSlugs, skippedSlugs}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Patch version for private method refactor, doc fixes, and test stability.
…ap polish
CR-1: algorithm flag was silently ignored — kdf.algorithm always overwrote
kdfOptions.algorithm. Now uses kdfOptions?.algorithm || kdf.algorithm.
CR-2: CHANGELOG 5.2.1 section order → Added/Changed/Fixed per spec.
CR-3: Add VAULT_CONFLICT to rotateVaultPassphrase Throws in API.md.
CR-4: Move Task 12.1–12.4 from ROADMAP.md to COMPLETED_TASKS.md.
CR-5: Add @throws NO_MATCHING_RECIPIENT JSDoc to CasService.rotateKey().
CR-6: Split mixed CLI flag table into git-cas-rotate / vault-rotate tables.
createRepo() creates bare repos without user.name/user.email, causing "Author identity unknown" on CI runners that lack global git config.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
test/integration/vault-cli.test.js (1)
240-250: Consider adding a negative test to verify old key rejection after rotation.The current tests verify that the new key works after rotation, but don't verify that the old key is rejected. This would provide stronger confidence in the rotation behavior.
🧪 Suggested additional test
it('restore with old key fails after rotation', () => { const outDir = mkdtempSync(path.join(os.tmpdir(), 'cas-cli-rotate-fail-')); const outPath = path.join(outDir, 'restored.bin'); expect(() => { cli( `restore --slug rotate/asset --out ${outPath} --key-file ${oldKeyFile}`, rotateRepoDir, ); }).toThrow(); // or check for specific error code rmSync(outDir, { recursive: true, force: true }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/vault-cli.test.js` around lines 240 - 250, Add a new negative integration test that asserts the old key is rejected after rotation: create a temp directory (using mkdtempSync), build outPath, call cli with the same restore command but passing oldKeyFile and rotateRepoDir, and wrap that call in expect(() => cli(...)).toThrow() (or assert a specific error), then clean up the temp directory with rmSync; place this test alongside the existing "restore with new key succeeds after rotation" test and reference the same rotateOriginal/outPath setup so it validates failure with oldKeyFile.GUIDE.md (1)
10-28: Consider adding section 11b to the Table of Contents.The new "Multi-Recipient Encryption & Key Rotation" section (11b) is not listed in the Table of Contents. Users browsing the TOC won't discover this feature.
📝 Proposed addition
22. [Passphrase Encryption (KDF)](`#11-passphrase-encryption-kdf`) -23. [Merkle Manifests](`#12-merkle-manifests`) +23. [Multi-Recipient Encryption & Key Rotation](`#11b-multi-recipient-encryption--key-rotation`) +24. [Merkle Manifests](`#12-merkle-manifests`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GUIDE.md` around lines 10 - 28, Add the missing TOC entry for the new "Multi-Recipient Encryption & Key Rotation" section (labelled as 11b) by inserting a linked item between the existing "11. [Passphrase Encryption (KDF)](`#11-passphrase-encryption-kdf`)" and "12. [Merkle Manifests](`#12-merkle-manifests`)" entries; use the heading text "11b. [Multi-Recipient Encryption & Key Rotation](`#11b-multi-recipient-encryption--key-rotation`)" so the link matches the section anchor.test/unit/facade/ContentAddressableStore.rotation.test.js (1)
100-132: Consider using low KDF iterations consistently across all tests.The PR description mentions "Stabilizes a flapping test by reducing test-only KDF iterations." Line 66 uses
{ iterations: 1 }, but lines 103, 136, 145, 165, and 183 use default iterations which are much higher (100,000). This inconsistency could cause test slowness or flakiness.♻️ Proposed fix for consistent KDF iterations
it('mixed: 2 envelope + 1 non-envelope → 2 rotated, 1 skipped', async () => { const oldPass = 'old-pass'; const newPass = 'new-pass'; - await cas.initVault({ passphrase: oldPass }); + await cas.initVault({ passphrase: oldPass, kdfOptions: { iterations: 1 } });Apply similar changes to other test cases at lines 136, 145, 165, and 183.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/facade/ContentAddressableStore.rotation.test.js` around lines 100 - 132, The test uses default (high) KDF iterations for the vault while elsewhere tests set iterations:1; fix by initializing the test vault with low iterations so deriveKey uses iterations:1 consistently: call cas.initVault with an explicit KDF options object (e.g., cas.initVault({ passphrase: oldPass, kdf: { iterations: 1 } })) or otherwise ensure metadata.encryption.kdf.iterations is set to 1 before calling cas.deriveKey/getVaultMetadata; apply the same change to the other tests that call cas.initVault / cas.deriveKey so all tests use iterations:1.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@GUIDE.md`:
- Around line 10-28: Add the missing TOC entry for the new "Multi-Recipient
Encryption & Key Rotation" section (labelled as 11b) by inserting a linked item
between the existing "11. [Passphrase Encryption
(KDF)](`#11-passphrase-encryption-kdf`)" and "12. [Merkle
Manifests](`#12-merkle-manifests`)" entries; use the heading text "11b.
[Multi-Recipient Encryption & Key
Rotation](`#11b-multi-recipient-encryption--key-rotation`)" so the link matches
the section anchor.
In `@test/integration/vault-cli.test.js`:
- Around line 240-250: Add a new negative integration test that asserts the old
key is rejected after rotation: create a temp directory (using mkdtempSync),
build outPath, call cli with the same restore command but passing oldKeyFile and
rotateRepoDir, and wrap that call in expect(() => cli(...)).toThrow() (or assert
a specific error), then clean up the temp directory with rmSync; place this test
alongside the existing "restore with new key succeeds after rotation" test and
reference the same rotateOriginal/outPath setup so it validates failure with
oldKeyFile.
In `@test/unit/facade/ContentAddressableStore.rotation.test.js`:
- Around line 100-132: The test uses default (high) KDF iterations for the vault
while elsewhere tests set iterations:1; fix by initializing the test vault with
low iterations so deriveKey uses iterations:1 consistently: call cas.initVault
with an explicit KDF options object (e.g., cas.initVault({ passphrase: oldPass,
kdf: { iterations: 1 } })) or otherwise ensure
metadata.encryption.kdf.iterations is set to 1 before calling
cas.deriveKey/getVaultMetadata; apply the same change to the other tests that
call cas.initVault / cas.deriveKey so all tests use iterations:1.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
CHANGELOG.mdCOMPLETED_TASKS.mdGUIDE.mdREADME.mdROADMAP.mdbin/actions.jsbin/git-cas.jsdocs/API.mddocs/SECURITY.mdindex.d.tsindex.jsjsr.jsonpackage.jsonsrc/domain/schemas/ManifestSchema.d.tssrc/domain/schemas/ManifestSchema.jssrc/domain/services/CasService.d.tssrc/domain/services/CasService.jssrc/domain/value-objects/Manifest.d.tstest/integration/vault-cli.test.jstest/unit/domain/schemas/ManifestSchema.keyVersion.test.jstest/unit/domain/services/CasService.rotateKey.test.jstest/unit/facade/ContentAddressableStore.rotation.test.js
- Add missing 11b TOC entry in GUIDE.md for Multi-Recipient section - Add negative integration test: old key rejected after rotation - Use iterations:1 consistently in rotation unit tests for speed
Member
Author
|
All 3 nitpicks addressed in 5f8f7ef:
|
Member
Author
|
@coderabbitai review please |
|
✅ Actions performedReview triggered.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
rotateKey()re-wraps the DEK with a new KEK, leaving data blobs untouched. Enables key compromise response without re-storing assets.keyVersiontracking — manifest-level and per-recipient counters for audit compliance.rotateVaultPassphrase()— rotates the vault-level encryption passphrase across all envelope-encrypted entries in a single atomic commit with retry logic for concurrent updates.git cas rotate(single asset) andgit cas vault rotate(vault-wide passphrase).#privatemethods, CLI docs added to API.md, error code table updated, flapping test stabilized via reduced test-only KDF iterations.Test plan
Summary by CodeRabbit
Release Notes v5.2.1
New Features
rotateandvault rotatefor key and vault operationsDocumentation
Version Update