feat: M11 Locksmith — envelope encryption with multi-recipient DEK/KEK#12
Merged
flyingrobots merged 19 commits intomainfrom Feb 28, 2026
Merged
feat: M11 Locksmith — envelope encryption with multi-recipient DEK/KEK#12flyingrobots merged 19 commits intomainfrom
flyingrobots merged 19 commits intomainfrom
Conversation
…chema M11 Locksmith Task 11.3 — extend manifest schema for multi-recipient envelope encryption. RecipientSchema validates label, wrappedDek, nonce, tag, and optional kekType. Manifest constructor deep-copies recipients for immutability.
M11 Locksmith Task 11.1 — store() accepts recipients array for envelope encryption. A random DEK encrypts content; each recipient's KEK wraps the DEK. restoreStream() scans recipient entries to unwrap the DEK. Backward compatible with legacy single-key manifests.
M11 Locksmith Task 11.2 — addRecipient unwraps the DEK with an existing key and re-wraps for a new recipient. removeRecipient drops an entry (guards against removing the last one). listRecipients returns labels. Facade delegates added.
M11 Locksmith Task 11.4 — add --recipient flag to store command, recipient add/remove/list subcommands, and error hints for all envelope encryption error codes. Refactor store() and _resolveDecryptionKey() to satisfy lint complexity limits.
Address 8 issues from self-code-review of M11 Locksmith: - C1 (critical): _wrapDek/_unwrapDek now async with await — prevents silent garbage on Bun/Deno where encryptBuffer/decryptBuffer are async - H1: delete dead _resolveEncryptionKey (superseded by _resolveDecryptionKey) - M1: guard --recipient + --vault-passphrase mutual exclusivity in CLI - M2: add @param recipients JSDoc to store() and storeFile() - M3: re-export RecipientEntry from index.d.ts - L1: collapse redundant RECIPIENT_NOT_FOUND guards in removeRecipient - L2: reuse _resolveKeyForRecipients in addRecipient - L3: update store() JSDoc description for both encryption modes
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds envelope (DEK/KEK) multi-recipient encryption and recipient management: RecipientSchema/RecipientEntry, manifest recipients field, CasService envelope logic (_wrapDek/_unwrapDek and key resolution), store() recipients option, add/remove/list recipient APIs, CLI recipient subcommands, hints, and extensive tests. Changes
Sequence DiagramsequenceDiagram
participant Client
participant CasService
participant Crypto as CryptoService
participant Persistence
Client->>CasService: store(source, recipients=[{label,key},...])
activate CasService
CasService->>Crypto: generate DEK
activate Crypto
Crypto-->>CasService: DEK
deactivate Crypto
loop for each recipient
CasService->>Crypto: wrap DEK with recipient KEK
activate Crypto
Crypto-->>CasService: wrappedDek, nonce, tag
deactivate Crypto
end
CasService->>Crypto: encrypt content with DEK
activate Crypto
Crypto-->>CasService: encrypted chunks
deactivate Crypto
CasService->>Persistence: store chunks and manifest (includes encryption.recipients)
activate Persistence
Persistence-->>CasService: stored refs / manifest oid
deactivate Persistence
CasService-->>Client: manifest (includes encryption.recipients)
deactivate CasService
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
bin/git-cas.js (1)
117-124: Add an explicit empty-keyfile guard inparseRecipient.Line 123 can be empty for malformed input like
--recipient alice:; explicit validation gives clearer CLI errors before file I/O throws.Proposed tweak
function parseRecipient(value, previous) { const sep = value.indexOf(':'); if (sep < 1) { throw new Error(`Invalid --recipient format "${value}": expected label:keyfile`); } const label = value.slice(0, sep); const keyfile = value.slice(sep + 1); + if (!keyfile) { + throw new Error(`Invalid --recipient format "${value}": missing keyfile path`); + } const key = readKeyFile(keyfile); const list = previous || []; list.push({ label, key }); return list; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/git-cas.js` around lines 117 - 124, The parseRecipient function should explicitly validate that the extracted keyfile is non-empty before attempting file I/O: after computing const keyfile = value.slice(sep + 1) in parseRecipient, add a guard that throws a clear Error (e.g., Invalid --recipient format "<value>": expected non-empty keyfile) when keyfile === '' (or .length === 0), then continue to call readKeyFile(keyfile); this prevents confusing file-read errors for inputs like --recipient alice:.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/domain/services/CasService.js`:
- Around line 396-404: In _resolveRecipientsForStore reject invalid input before
generating the DEK: validate that the recipients array is non-empty and contains
no duplicate keys/labels, throw a clear error if empty or duplicates found, and
only then proceed to call this.crypto._validateKey for each recipient, generate
dek via this.crypto.randomBytes(32), and build entries with await this._wrapDek.
Use the existing symbols _resolveRecipientsForStore, recipients,
this.crypto._validateKey, this.crypto.randomBytes, dek, entries, and _wrapDek to
locate and update the logic.
- Around line 925-947: The current removeRecipient implementation uses
recipients.filter(...) which removes all entries with the matching label and can
yield zero recipients if duplicates exist; change the logic in removeRecipient
to remove only a single recipient instance (the first match) instead of
filtering all matches: use recipients.findIndex(r => r.label === label) to
locate the index, throw the same CasError if index === -1, create a shallow copy
of recipients, splice out that single index, then construct
updatedEncryption/from manifest.toJSON() and return new Manifest(...) as before,
ensuring recipients length remains > 0.
In `@test/unit/domain/schemas/RecipientSchema.test.js`:
- Around line 78-81: The test currently treats an empty recipients array as
valid, but the EncryptionSchema should reject empty arrays to avoid
undecryptable manifests; update the EncryptionSchema definition so the
recipients field (the same one validated by EncryptionSchema and constructed by
baseEncryption()) enforces at least one entry (e.g. use an array validator with
min(1) or equivalent) when the field is present, then update this test
(RecipientSchema.test.js) to expect failure for recipients: [] or remove the
test that accepts empty arrays. Ensure you reference the existing
EncryptionSchema and baseEncryption() symbols so the change lands on the correct
validator.
---
Nitpick comments:
In `@bin/git-cas.js`:
- Around line 117-124: The parseRecipient function should explicitly validate
that the extracted keyfile is non-empty before attempting file I/O: after
computing const keyfile = value.slice(sep + 1) in parseRecipient, add a guard
that throws a clear Error (e.g., Invalid --recipient format "<value>": expected
non-empty keyfile) when keyfile === '' (or .length === 0), then continue to call
readKeyFile(keyfile); this prevents confusing file-read errors for inputs like
--recipient alice:.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
CHANGELOG.mdbin/actions.jsbin/git-cas.jsindex.d.tsindex.jssrc/domain/schemas/ManifestSchema.d.tssrc/domain/schemas/ManifestSchema.jssrc/domain/services/CasService.d.tssrc/domain/services/CasService.jssrc/domain/value-objects/Manifest.d.tssrc/domain/value-objects/Manifest.jstest/unit/cli/actions.test.jstest/unit/domain/schemas/RecipientSchema.test.jstest/unit/domain/services/CasService.envelope.test.jstest/unit/domain/services/CasService.recipients.test.jstest/unit/domain/value-objects/Manifest.test.js
- M1: reject empty recipients array in store() (prevents unrecoverable data) - M2: reject duplicate recipient labels in store() - L1: add missing await on _resolveKeyForRecipients return - L2: add instanceof CasError guard in addRecipient error catch - L3: merge dual recipient loops into single pass - L4: add trailing newline to bin/git-cas.js - 2 new unit tests (753 total)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/domain/services/CasService.js (1)
932-954: Consider post-filter validation for defense in depth.The past review comment noted that if a manifest somehow contains duplicate labels,
filterat Line 949 removes all matching entries, potentially leaving zero recipients despite the pre-filter length check at Line 942. While_resolveRecipientsForStorerejects duplicates at creation time, manifests could be corrupted or manually crafted.Adding a post-filter guard would provide defense in depth:
🛡️ Proposed fix
const filtered = recipients.filter((r) => r.label !== label).map((r) => ({ ...r })); + if (filtered.length === 0) { + throw new CasError( + 'Cannot remove the last recipient', + 'CANNOT_REMOVE_LAST_RECIPIENT', + ); + } const json = manifest.toJSON();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/domain/services/CasService.js` around lines 932 - 954, In removeRecipient, after creating filtered (in the removeRecipient function), add a post-filter validation to ensure filtered.length >= 1 and that at least one recipient remains; if not, throw a CasError (same 'CANNOT_REMOVE_LAST_RECIPIENT' code or a new 'RECIPIENT_REMOVAL_FAILED') including the label and any context to indicate duplicates caused removal of all recipients, so corrupted/manually crafted manifests can't end up with zero recipients before returning the new Manifest.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/domain/services/CasService.js`:
- Around line 932-954: In removeRecipient, after creating filtered (in the
removeRecipient function), add a post-filter validation to ensure
filtered.length >= 1 and that at least one recipient remains; if not, throw a
CasError (same 'CANNOT_REMOVE_LAST_RECIPIENT' code or a new
'RECIPIENT_REMOVAL_FAILED') including the label and any context to indicate
duplicates caused removal of all recipients, so corrupted/manually crafted
manifests can't end up with zero recipients before returning the new Manifest.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
bin/git-cas.jssrc/domain/services/CasService.jstest/unit/domain/services/CasService.envelope.test.js
Round 2 review fixes: - M1: _resolveKeyForRecipients now only catches DEK_UNWRAP_FAILED, re-throws unexpected errors instead of swallowing them - M2: removeRecipient throws INVALID_OPTIONS (not RECIPIENT_NOT_FOUND) for non-envelope manifests, matching addRecipient behavior - M3: add @param recipients to store() JSDoc in index.js facade - L1: include originalError in addRecipient DEK_UNWRAP_FAILED re-throw - L2: update CLI version from 4.0.1 to 5.0.0 - L3: add tests for removeRecipient on non-envelope/unencrypted manifests - L4: add envelope + compression round-trip test - N1: fix CHANGELOG test count (756 total) - N3: remove unreachable !recipients guard in _resolveRecipientsForStore
…tion - Add post-filter guard in removeRecipient to prevent zero recipients when duplicate labels exist in corrupted/crafted manifests - Enforce min(1) on EncryptionSchema recipients array to reject empty arrays that would create undecryptable envelope manifests - Add empty keyfile validation in parseRecipient CLI parser - Add test for duplicate-label post-filter defense-in-depth
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/domain/services/CasService.js (1)
397-410:⚠️ Potential issue | 🟡 MinorHarden recipient input validation before array/object access.
This block can throw raw
TypeErrorfor malformed runtime input (e.g., non-arrayrecipientsor null entries), which bypasses your normalCasErrorcontract.💡 Proposed fix
async _resolveRecipientsForStore(recipients) { - if (recipients.length === 0) { + if (!Array.isArray(recipients) || recipients.length === 0) { throw new CasError('At least one recipient is required', 'INVALID_OPTIONS'); } - const labels = recipients.map((r) => r.label); + const labels = recipients.map((r) => { + if (!r || typeof r.label !== 'string' || r.label.length === 0) { + throw new CasError('Recipient label is required', 'INVALID_OPTIONS'); + } + this.crypto._validateKey(r.key); + return r.label; + }); if (new Set(labels).size !== labels.length) { throw new CasError('Duplicate recipient labels are not allowed', 'INVALID_OPTIONS'); } const dek = this.crypto.randomBytes(32); const entries = []; for (const r of recipients) { - this.crypto._validateKey(r.key); entries.push({ label: r.label, ...(await this._wrapDek(dek, r.key)) }); } return { key: dek, encExtra: { recipients: entries } }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/domain/services/CasService.js` around lines 397 - 410, The _resolveRecipientsForStore function currently assumes recipients is a non-empty array of objects and will throw raw TypeError on bad inputs; validate inputs first and throw CasError for invalid types: ensure recipients is an Array and not null, check recipients.length>0, iterate and verify each entry is a non-null object with a present string label and a present key (type as expected by this.crypto._validateKey) before accessing properties, then dedupe labels and proceed to call this.crypto._validateKey and this._wrapDek; preserve use of CasError for all validation failures to maintain the contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/domain/services/CasService.js`:
- Around line 397-410: The _resolveRecipientsForStore function currently assumes
recipients is a non-empty array of objects and will throw raw TypeError on bad
inputs; validate inputs first and throw CasError for invalid types: ensure
recipients is an Array and not null, check recipients.length>0, iterate and
verify each entry is a non-null object with a present string label and a present
key (type as expected by this.crypto._validateKey) before accessing properties,
then dedupe labels and proceed to call this.crypto._validateKey and
this._wrapDek; preserve use of CasError for all validation failures to maintain
the contract.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CHANGELOG.mdbin/git-cas.jsindex.jssrc/domain/schemas/ManifestSchema.jssrc/domain/services/CasService.jstest/unit/domain/schemas/RecipientSchema.test.jstest/unit/domain/services/CasService.envelope.test.jstest/unit/domain/services/CasService.recipients.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
- test/unit/domain/schemas/RecipientSchema.test.js
- CHANGELOG.md
…layer caching - Pin denoland/deno:2.7.1 instead of :latest for reproducible builds - Remove apt nodejs/npm — Deno 2.x resolves npm: specifiers natively - Two-stage COPY (package.json first) for Docker layer caching on deps
Integration tests shell out to `node bin/git-cas.js`, so nodejs is needed. npm is still removed (not needed — Deno 2.x resolves npm: specifiers natively).
Node ≤20 with piped stdio in Docker keeps the libuv event loop alive at the C++ level even with no JS handles or requests. This caused each CLI integration test to take 30s (hitting execSync timeout) instead of <1s. Integration suite drops from 365s → 6s on Deno container.
Copy the node binary from node:22-slim via multi-stage build instead of installing Debian's ancient nodejs package. Also removes the npm apt package (Deno 2.x resolves npm: specifiers natively) and adds Docker layer caching for deno install. CLI process.exit() is standard practice — Commander doesn't exit on its own and spawned git child processes leave libuv handles that prevent natural exit in containerized environments. Deno CI integration tests: 365s → 2.3s.
- Add test/helpers/crypto-adapter.js that auto-selects the right crypto adapter per runtime (Node/Bun/Deno), mirroring index.js - Replace hardcoded NodeCryptoAdapter in all 22 test files with the runtime-adaptive helper — unit tests now exercise BunCryptoAdapter on Bun and WebCryptoAdapter on Deno - Fix missing await on crypto.sha256() in 6 test mock writeBlob callbacks (was silent on sync NodeCryptoAdapter, broke on async adapters) - vault-cli.test.js now detects runtime and invokes CLI with deno run / bun run / node accordingly — no longer skipped on Bun - Remove Node from Deno Docker image entirely (no apt nodejs, no multi-stage copy) — Deno image is now just Deno + git - CLI process.exit() with stdout/stderr flush after parseAsync() to prevent 30s hang from orphaned libuv handles
Member
Author
|
@coderabbitai review please 🥕 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
Dockerfile (2)
24-25: Static analysis: address root user and missing--no-install-recommends.Two issues flagged by Trivy:
- The stage runs as
rootand never switches back to a non-root user.- The apt-get command is missing
--no-install-recommends, which increases image size.For a CI/test container this is lower risk, but worth aligning with best practices. Note that the Node and Bun stages have the same apt-get pattern—consider updating all three for consistency.
🐳 Suggested fix for the Deno stage
FROM denoland/deno:2.7.1 AS deno USER root -RUN apt-get update && apt-get install -y git && rm -rf /var/lib/apt/lists/* +RUN apt-get update && apt-get install -y --no-install-recommends git && rm -rf /var/lib/apt/lists/* WORKDIR /app COPY package.json deno.lock* ./ RUN deno install --allow-scripts || true COPY . . RUN deno install --allow-scripts ENV GIT_STUNTS_DOCKER=1 CMD ["deno", "run", "-A", "npm:vitest", "run", "test/unit"] +USER deno🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 24 - 25, The Dockerfile currently sets USER root and runs RUN apt-get update && apt-get install -y git ... without --no-install-recommends; change the Dockerfile so you create and switch to a non-root user (e.g., add a user/group, chown necessary files, and use USER <nonroot>) instead of leaving USER root, and update the apt-get invocation in the RUN apt-get command (and any similar Node/Bun stage commands) to include --no-install-recommends and keep apt-get clean steps; ensure the git install step runs before switching to the non-root user or adjust ownership so the non-root user has required permissions.
28-28: The|| truefallback may silently mask install failures.This pre-install step ignores errors, which could hide legitimate issues (e.g., network failures, corrupt lockfile). Per the commit messages, this was added for CI reliability when
deno.lockmay not exist yet.Consider logging a warning or limiting the fallback scope so real errors surface:
💡 Alternative: log when fallback triggers
-RUN deno install --allow-scripts || true +RUN deno install --allow-scripts || echo "WARN: initial deno install failed, will retry after full copy"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` at line 28, Replace the unconditional "RUN deno install --allow-scripts || true" fallback with a conditional that only skips the install when the expected lockfile is absent and otherwise surfaces real install failures; specifically, check for the presence of deno.lock before skipping, run the deno install command normally so errors cause build failure, and when skipping emit a clear warning message so CI logs show the fallback was used (refer to the existing RUN deno install --allow-scripts || true line to locate the change).test/benchmark/cas.bench.js (1)
9-11: Consider simplifying the redundant assignment.The intermediate
testCryptovariable is unused elsewhere in this file. You could simplify to:-const testCrypto = await getTestCryptoAdapter(); - -const crypto = testCrypto; +const crypto = await getTestCryptoAdapter();This is a minor style nit; the current code works correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/benchmark/cas.bench.js` around lines 9 - 11, Remove the redundant temporary variable by assigning the result of getTestCryptoAdapter() directly to crypto: replace the two-step creation of testCrypto and subsequent assignment to crypto with a single declaration that sets crypto = await getTestCryptoAdapter(); target the lines where getTestCryptoAdapter(), testCrypto, and crypto are defined (in the cas.bench.js snippet) to make this change.test/unit/domain/services/CasService.merkle.test.js (1)
32-36: Minor redundancy:Promise.resolve()unnecessary in async function.Since
writeBlobis already anasyncfunction, the explicitPromise.resolve(oid)wrapping is redundant—return oidwould suffice.writeBlob: vi.fn().mockImplementation(async (content) => { const oid = await crypto.sha256(Buffer.isBuffer(content) ? content : Buffer.from(content)); blobs.set(oid, Buffer.isBuffer(content) ? content : Buffer.from(content)); - return Promise.resolve(oid); + return oid; }),Not blocking; the current code works correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/domain/services/CasService.merkle.test.js` around lines 32 - 36, The mock async function writeBlob contains an unnecessary Promise.resolve(oid) return; simply return the computed oid instead. Locate the writeBlob mock implementation (the async function that calls crypto.sha256 and sets blobs.set(oid,...)) and replace the final "return Promise.resolve(oid)" with "return oid" so the async function returns the value directly.test/unit/domain/services/CasService.kdf.test.js (1)
4-30: LGTM with minor nitpick.The crypto adapter migration is correct. The
writeBlobmock properly awaitscrypto.sha256().One minor redundancy: at line 30,
return Promise.resolve(oid)is unnecessary since the function is alreadyasync— returningoiddirectly would suffice. This is purely cosmetic.🔧 Optional: Simplify return statement
writeBlob: vi.fn().mockImplementation(async (content) => { const oid = await crypto.sha256(Buffer.isBuffer(content) ? content : Buffer.from(content)); blobs.set(oid, Buffer.isBuffer(content) ? content : Buffer.from(content)); - return Promise.resolve(oid); + return oid; }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/domain/services/CasService.kdf.test.js` around lines 4 - 30, In the mockPersistence.writeBlob async implementation inside setup(), remove the unnecessary Promise.resolve wrapper and simply return the computed oid directly (i.e., replace "return Promise.resolve(oid)" with "return oid"); this keeps the function async and preserves behavior while eliminating the redundant Promise creation—locate the writeBlob mock in the setup() factory that uses testCrypto.sha256 to compute oid.test/unit/domain/services/CasService.envelope.test.js (2)
148-156: Same pattern: redundant double-call for error verification.Apply the same refactor as above to avoid calling
service.restore()twice.♻️ Suggested refactor
- await expect( - service.restore({ manifest: tamperedManifest, encryptionKey: kek }), - ).rejects.toThrow(CasError); - - try { - await service.restore({ manifest: tamperedManifest, encryptionKey: kek }); - } catch (err) { - expect(err.code).toBe('NO_MATCHING_RECIPIENT'); - } + await expect( + service.restore({ manifest: tamperedManifest, encryptionKey: kek }), + ).rejects.toMatchObject({ + code: 'NO_MATCHING_RECIPIENT', + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/domain/services/CasService.envelope.test.js` around lines 148 - 156, The test currently calls service.restore({ manifest: tamperedManifest, encryptionKey: kek }) twice to assert it throws a CasError and that the thrown error has code 'NO_MATCHING_RECIPIENT'; instead, replace the double-call with a single assertion using Jest's promise-rejection matchers: keep await expect(service.restore({ manifest: tamperedManifest, encryptionKey: kek })).rejects.toThrow(CasError) (or combine into one assertion) and then assert the error shape with a single rejects matcher like await expect(service.restore({ manifest: tamperedManifest, encryptionKey: kek })).rejects.toEqual(expect.objectContaining({ code: 'NO_MATCHING_RECIPIENT' })), removing the try/catch and the second invocation.
117-126: Avoid calling the failing operation twice to verify error details.The test calls
service.restore()with a wrong key twice—once forrejects.toThrowand again in a try/catch for the error code. This is redundant and slightly less efficient.♻️ Suggested refactor using single catch
- await expect( - service.restore({ manifest, encryptionKey: wrongKey }), - ).rejects.toThrow(CasError); - - try { - await service.restore({ manifest, encryptionKey: wrongKey }); - } catch (err) { - expect(err.code).toBe('NO_MATCHING_RECIPIENT'); - } + await expect( + service.restore({ manifest, encryptionKey: wrongKey }), + ).rejects.toMatchObject({ + code: 'NO_MATCHING_RECIPIENT', + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/domain/services/CasService.envelope.test.js` around lines 117 - 126, Remove the duplicated call to service.restore(...) and assert both the error type and code from a single catch: call await service.restore({ manifest, encryptionKey: wrongKey }) inside a try/catch, and in the catch assert the error is an instance of CasError (or use expect(err).toBeInstanceOf(CasError)) and that err.code === 'NO_MATCHING_RECIPIENT'; keep references to service.restore, CasError, manifest, and wrongKey to locate the code.test/integration/vault-cli.test.js (1)
32-43: Prefer argv-based process execution over shell-concatenated commands.Building
execSynccommand strings with interpolatedargs/cwdis fragile (spaces/shell metacharacters) and makes cross-runtime behavior harder to reason about. UseexecFileSyncwith explicit argv arrays.Proposed refactor
-import { execSync } from 'node:child_process'; +import { execSync, execFileSync } from 'node:child_process'; -const RUNTIME_CMD = globalThis.Bun - ? `bun run ${BIN}` - : globalThis.Deno - ? `deno run -A ${BIN}` - : `node ${BIN}`; +const RUNTIME = globalThis.Bun + ? { cmd: 'bun', baseArgs: ['run', BIN] } + : globalThis.Deno + ? { cmd: 'deno', baseArgs: ['run', '-A', BIN] } + : { cmd: 'node', baseArgs: [BIN] }; -function cli(args, cwd) { - return execSync(`${RUNTIME_CMD} ${args} --cwd ${cwd}`, { +function cli(args, cwd) { + return execFileSync(RUNTIME.cmd, [...RUNTIME.baseArgs, ...args, '--cwd', cwd], { encoding: 'utf8', timeout: 30_000, }).trim(); }Then migrate call sites from string args to arrays, e.g.:
-const out = cli('vault init', repoDir); +const out = cli(['vault', 'init'], repoDir);Also applies to: 48-51
🤖 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 32 - 43, The test builds runtime command strings (RUNTIME_CMD) and later calls using execSync with interpolated args/cwd — replace string-based execSync invocation with execFileSync using explicit argv arrays to avoid shell concatenation and quoting issues; update RUNTIME_CMD usage in vault-cli.test.js (and the similar call sites around the 48-51 range) to construct an argv array ([runtimeBinary, ...runtimeArgs, BIN, ...args]) and pass cwd via options instead of embedding into the command string, ensuring all call sites that currently pass a single command string now pass an array of argv to execFileSync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/integration/vault-cli.test.js`:
- Line 8: Update the runtime-specific hint in the vault-cli integration test to
match the new runtime-agnostic header: replace the hard-coded "npm run
test:integration:node" guidance with a runtime-neutral instruction (e.g., "npm
run test:integration:<runtime>" or "npm run test:integration" with a note to set
the desired runtime) in the test description/comment that contains the phrase
"runtime-agnostic invocation" so the guidance aligns with the multi-runtime
execution change.
- Around line 71-80: The afterAll teardown should guard against partial
beforeAll failures by checking that repoDir and inputDir are defined before
calling rmSync; update the cleanup in the afterAll block that references repoDir
and inputDir (variables set in beforeAll) to only call rmSync when those
variables are non-null/undefined (e.g., guard with if (repoDir) and if
(inputDir)) so rmSync isn't invoked with undefined and hides the original setup
error.
---
Nitpick comments:
In `@Dockerfile`:
- Around line 24-25: The Dockerfile currently sets USER root and runs RUN
apt-get update && apt-get install -y git ... without --no-install-recommends;
change the Dockerfile so you create and switch to a non-root user (e.g., add a
user/group, chown necessary files, and use USER <nonroot>) instead of leaving
USER root, and update the apt-get invocation in the RUN apt-get command (and any
similar Node/Bun stage commands) to include --no-install-recommends and keep
apt-get clean steps; ensure the git install step runs before switching to the
non-root user or adjust ownership so the non-root user has required permissions.
- Line 28: Replace the unconditional "RUN deno install --allow-scripts || true"
fallback with a conditional that only skips the install when the expected
lockfile is absent and otherwise surfaces real install failures; specifically,
check for the presence of deno.lock before skipping, run the deno install
command normally so errors cause build failure, and when skipping emit a clear
warning message so CI logs show the fallback was used (refer to the existing RUN
deno install --allow-scripts || true line to locate the change).
In `@test/benchmark/cas.bench.js`:
- Around line 9-11: Remove the redundant temporary variable by assigning the
result of getTestCryptoAdapter() directly to crypto: replace the two-step
creation of testCrypto and subsequent assignment to crypto with a single
declaration that sets crypto = await getTestCryptoAdapter(); target the lines
where getTestCryptoAdapter(), testCrypto, and crypto are defined (in the
cas.bench.js snippet) to make this change.
In `@test/integration/vault-cli.test.js`:
- Around line 32-43: The test builds runtime command strings (RUNTIME_CMD) and
later calls using execSync with interpolated args/cwd — replace string-based
execSync invocation with execFileSync using explicit argv arrays to avoid shell
concatenation and quoting issues; update RUNTIME_CMD usage in vault-cli.test.js
(and the similar call sites around the 48-51 range) to construct an argv array
([runtimeBinary, ...runtimeArgs, BIN, ...args]) and pass cwd via options instead
of embedding into the command string, ensuring all call sites that currently
pass a single command string now pass an array of argv to execFileSync.
In `@test/unit/domain/services/CasService.envelope.test.js`:
- Around line 148-156: The test currently calls service.restore({ manifest:
tamperedManifest, encryptionKey: kek }) twice to assert it throws a CasError and
that the thrown error has code 'NO_MATCHING_RECIPIENT'; instead, replace the
double-call with a single assertion using Jest's promise-rejection matchers:
keep await expect(service.restore({ manifest: tamperedManifest, encryptionKey:
kek })).rejects.toThrow(CasError) (or combine into one assertion) and then
assert the error shape with a single rejects matcher like await
expect(service.restore({ manifest: tamperedManifest, encryptionKey: kek
})).rejects.toEqual(expect.objectContaining({ code: 'NO_MATCHING_RECIPIENT' })),
removing the try/catch and the second invocation.
- Around line 117-126: Remove the duplicated call to service.restore(...) and
assert both the error type and code from a single catch: call await
service.restore({ manifest, encryptionKey: wrongKey }) inside a try/catch, and
in the catch assert the error is an instance of CasError (or use
expect(err).toBeInstanceOf(CasError)) and that err.code ===
'NO_MATCHING_RECIPIENT'; keep references to service.restore, CasError, manifest,
and wrongKey to locate the code.
In `@test/unit/domain/services/CasService.kdf.test.js`:
- Around line 4-30: In the mockPersistence.writeBlob async implementation inside
setup(), remove the unnecessary Promise.resolve wrapper and simply return the
computed oid directly (i.e., replace "return Promise.resolve(oid)" with "return
oid"); this keeps the function async and preserves behavior while eliminating
the redundant Promise creation—locate the writeBlob mock in the setup() factory
that uses testCrypto.sha256 to compute oid.
In `@test/unit/domain/services/CasService.merkle.test.js`:
- Around line 32-36: The mock async function writeBlob contains an unnecessary
Promise.resolve(oid) return; simply return the computed oid instead. Locate the
writeBlob mock implementation (the async function that calls crypto.sha256 and
sets blobs.set(oid,...)) and replace the final "return Promise.resolve(oid)"
with "return oid" so the async function returns the value directly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
Dockerfilebin/git-cas.jssrc/domain/services/CasService.jstest/benchmark/cas.bench.jstest/helpers/crypto-adapter.jstest/integration/vault-cli.test.jstest/unit/domain/schemas/ChunkingSchema.test.jstest/unit/domain/services/CasService.chunking.test.jstest/unit/domain/services/CasService.codec.test.jstest/unit/domain/services/CasService.compression.test.jstest/unit/domain/services/CasService.crypto.test.jstest/unit/domain/services/CasService.deleteAsset.test.jstest/unit/domain/services/CasService.empty-file.test.jstest/unit/domain/services/CasService.envelope.test.jstest/unit/domain/services/CasService.errors.test.jstest/unit/domain/services/CasService.events.test.jstest/unit/domain/services/CasService.findOrphanedChunks.test.jstest/unit/domain/services/CasService.kdf.test.jstest/unit/domain/services/CasService.key-validation.test.jstest/unit/domain/services/CasService.merkle.test.jstest/unit/domain/services/CasService.parallel.test.jstest/unit/domain/services/CasService.readManifest.test.jstest/unit/domain/services/CasService.recipients.test.jstest/unit/domain/services/CasService.restore.test.jstest/unit/domain/services/CasService.restoreStream.test.jstest/unit/domain/services/CasService.stream-error.test.jstest/unit/domain/services/CasService.test.js
|
✅ Actions performedReview triggered.
|
…ease Add Docker/CLI fixes to CHANGELOG, set release date, bump version in package.json and jsr.json.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
Dockerfile (2)
27-28: The|| truefallback silently swallows all errors.The intent appears to be tolerating a missing
deno.lockor undefined scripts, but this pattern also masks legitimate failures (network issues, permission errors, etc.). Since line 30 runs the same command without the fallback, real errors should surface there—but debugging may be confusing if the first attempt fails silently.Consider being more explicit about what you're tolerating:
💡 Alternative: check for lock file existence
COPY package.json deno.lock* ./ -RUN deno install --allow-scripts || true +RUN if [ -f deno.lock ]; then deno install --allow-scripts; fi COPY . . RUN deno install --allow-scriptsThis makes the intent clearer: only attempt pre-install if the lock file exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 27 - 28, The RUN line that executes "deno install --allow-scripts || true" silently swallows all errors; replace this with an explicit conditional that only tolerates a missing lock file (e.g., test for deno.lock existence before attempting the pre-install) so real failures (network/permission) are not hidden; adjust the COPY/CMD area around the "COPY package.json deno.lock* ./" and the first "RUN deno install --allow-scripts" invocation to run only when deno.lock is present or remove the "|| true" and rely on the subsequent explicit install to surface errors.
24-25: Address static analysis findings: USER root and missing--no-install-recommends.Two Trivy findings for the Deno stage:
- DS-0002: Container runs as
root— consider dropping privileges after package installation.- DS-0029: Missing
--no-install-recommendsincreases image size and attack surface.While these are lower risk for a CI/test image, fixing them aligns with container best practices.
🐳 Proposed fix
# --- Deno --- FROM denoland/deno:2.7.1 AS deno USER root -RUN apt-get update && apt-get install -y git && rm -rf /var/lib/apt/lists/* +RUN apt-get update && apt-get install -y --no-install-recommends git && rm -rf /var/lib/apt/lists/* WORKDIR /app COPY package.json deno.lock* ./ RUN deno install --allow-scripts || true COPY . . RUN deno install --allow-scripts ENV GIT_STUNTS_DOCKER=1 +USER deno CMD ["deno", "run", "-A", "npm:vitest", "run", "test/unit"]Note: Apply the same
--no-install-recommendsfix to the Node (line 3) and Bun (line 14) stages for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 24 - 25, The Dockerfile currently uses USER root and RUN apt-get install -y git without --no-install-recommends; update each stage (the Deno stage where USER root is set, plus the Node and Bun stages that run apt-get installs) to add --no-install-recommends to apt-get install commands and avoid leaving the container as root by performing package installation as root then creating a non-root user (e.g., adduser or groupadd/useradd), chown any needed files, and switch to that non-root user with USER <username> after installation; ensure the RUN line uses apt-get update && apt-get install -y --no-install-recommends git && rm -rf /var/lib/apt/lists/* and reference the USER root and the apt-get RUN commands when making these edits.test/benchmark/cas.bench.js (1)
9-11: Minor: Redundant intermediate variable.The
testCryptovariable is immediately assigned tocryptoon the next line. Consider simplifying:-const testCrypto = await getTestCryptoAdapter(); - -const crypto = testCrypto; +const crypto = await getTestCryptoAdapter();This is a minor style nit; the current code works correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/benchmark/cas.bench.js` around lines 9 - 11, Remove the redundant intermediate variable by assigning the awaited adapter directly to crypto: replace the two lines that declare testCrypto and then set crypto (using getTestCryptoAdapter()) with a single declaration that initializes crypto directly from await getTestCryptoAdapter(); update any references expecting testCrypto to use crypto and remove the now-unused testCrypto identifier.test/unit/domain/services/CasService.recipients.test.js (1)
285-288: Deep-clone before tampering to avoid aliasing totoJSON()internals.At Line 285, mutating
manifest.toJSON()directly can make this test sensitive toManifest.toJSON()implementation details. A defensive deep clone keeps the tamper intent explicit and isolated.Minimal change
- const json = manifest.toJSON(); + const json = JSON.parse(JSON.stringify(manifest.toJSON()));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/domain/services/CasService.recipients.test.js` around lines 285 - 288, The test mutates the object returned by manifest.toJSON(), which can alias internal structures; instead deep-clone the toJSON result before modifying: call manifest.toJSON(), deep-clone that result (e.g., via JSON.parse(JSON.stringify(...)) or structuredClone) into a new object, then change the cloned object's encryption.recipients[1].label to 'alice' and construct tampered with new Manifest(cloned); keep references to manifest.toJSON(), Manifest, and tampered to locate the change.test/unit/domain/services/CasService.envelope.test.js (1)
325-338: Make fuzz cases reproducible with a deterministic seed.Using
Math.random()at Line 326 and Line 337 makes failures hard to replay in CI. Consider a seeded PRNG (optionally from an env var) so failing iterations can be reproduced exactly.Seeded fuzz example
+ const seed = Number(process.env.VITEST_SEED ?? 1337); + let state = seed >>> 0; + const next = () => { + state = (1664525 * state + 1013904223) >>> 0; + return state / 0x100000000; + }; + for (let i = 0; i < 50; i++) { - const size = Math.floor(Math.random() * 4096); + const size = Math.floor(next() * 4096); const original = randomBytes(size); const keys = [randomBytes(32), randomBytes(32), randomBytes(32)]; @@ - const idx = Math.floor(Math.random() * 3); + const idx = Math.floor(next() * 3); const { buffer } = await service.restore({ manifest, encryptionKey: keys[idx] });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/domain/services/CasService.envelope.test.js` around lines 325 - 338, Replace non-deterministic Math.random() and crypto randomBytes usage in the fuzz loop with a seeded PRNG so failing iterations can be reproduced; initialize a PRNG at the top of the test (e.g., const seed = process.env.FUZZ_SEED || 'fixed-seed'; const rng = seededPrng(seed)) and use rng() instead of Math.random() for computing size and idx, and add a deterministicBytes(size, rng) helper to generate buffers/keys (use rng to fill bytes) instead of randomBytes. Update the loop to call deterministicBytes for original and each key and keep the rest of the logic (service.store, service.restore, manifest) unchanged so a single seed reproduces the exact failing iteration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 18: The release note line "48 new unit tests (757 total)." is a volatile
hardcoded test-count; either verify and update the total to match CI/test-run
output before final release or remove the absolute total and leave a stable
phrasing like "added 48 unit tests" (or simply "added new unit tests"); update
the CHANGELOG.md entry so it no longer risks drifting from CI results.
In `@test/unit/domain/services/CasService.envelope.test.js`:
- Around line 121-126: The test currently calls service.restore(...) twice (once
in try/catch and again to assert err.code) which can silently skip assertions if
the second call resolves; instead, replace the try/catch + second invocation
with a single Jest rejection expectation that asserts both the error type and
code in one go, e.g. use await expect(service.restore({ manifest, encryptionKey:
wrongKey })).rejects.toMatchObject({ code: 'NO_MATCHING_RECIPIENT' }) and/or
.rejects.toBeInstanceOf(Error) for both occurrences around restore in this file
(the tests referencing service.restore, manifest and wrongKey at the two failing
blocks).
---
Nitpick comments:
In `@Dockerfile`:
- Around line 27-28: The RUN line that executes "deno install --allow-scripts ||
true" silently swallows all errors; replace this with an explicit conditional
that only tolerates a missing lock file (e.g., test for deno.lock existence
before attempting the pre-install) so real failures (network/permission) are not
hidden; adjust the COPY/CMD area around the "COPY package.json deno.lock* ./"
and the first "RUN deno install --allow-scripts" invocation to run only when
deno.lock is present or remove the "|| true" and rely on the subsequent explicit
install to surface errors.
- Around line 24-25: The Dockerfile currently uses USER root and RUN apt-get
install -y git without --no-install-recommends; update each stage (the Deno
stage where USER root is set, plus the Node and Bun stages that run apt-get
installs) to add --no-install-recommends to apt-get install commands and avoid
leaving the container as root by performing package installation as root then
creating a non-root user (e.g., adduser or groupadd/useradd), chown any needed
files, and switch to that non-root user with USER <username> after installation;
ensure the RUN line uses apt-get update && apt-get install -y
--no-install-recommends git && rm -rf /var/lib/apt/lists/* and reference the
USER root and the apt-get RUN commands when making these edits.
In `@test/benchmark/cas.bench.js`:
- Around line 9-11: Remove the redundant intermediate variable by assigning the
awaited adapter directly to crypto: replace the two lines that declare
testCrypto and then set crypto (using getTestCryptoAdapter()) with a single
declaration that initializes crypto directly from await getTestCryptoAdapter();
update any references expecting testCrypto to use crypto and remove the
now-unused testCrypto identifier.
In `@test/unit/domain/services/CasService.envelope.test.js`:
- Around line 325-338: Replace non-deterministic Math.random() and crypto
randomBytes usage in the fuzz loop with a seeded PRNG so failing iterations can
be reproduced; initialize a PRNG at the top of the test (e.g., const seed =
process.env.FUZZ_SEED || 'fixed-seed'; const rng = seededPrng(seed)) and use
rng() instead of Math.random() for computing size and idx, and add a
deterministicBytes(size, rng) helper to generate buffers/keys (use rng to fill
bytes) instead of randomBytes. Update the loop to call deterministicBytes for
original and each key and keep the rest of the logic (service.store,
service.restore, manifest) unchanged so a single seed reproduces the exact
failing iteration.
In `@test/unit/domain/services/CasService.recipients.test.js`:
- Around line 285-288: The test mutates the object returned by
manifest.toJSON(), which can alias internal structures; instead deep-clone the
toJSON result before modifying: call manifest.toJSON(), deep-clone that result
(e.g., via JSON.parse(JSON.stringify(...)) or structuredClone) into a new
object, then change the cloned object's encryption.recipients[1].label to
'alice' and construct tampered with new Manifest(cloned); keep references to
manifest.toJSON(), Manifest, and tampered to locate the change.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
CHANGELOG.mdDockerfilebin/git-cas.jsjsr.jsonpackage.jsonsrc/domain/services/CasService.jstest/benchmark/cas.bench.jstest/helpers/crypto-adapter.jstest/integration/vault-cli.test.jstest/unit/domain/schemas/ChunkingSchema.test.jstest/unit/domain/services/CasService.chunking.test.jstest/unit/domain/services/CasService.codec.test.jstest/unit/domain/services/CasService.compression.test.jstest/unit/domain/services/CasService.crypto.test.jstest/unit/domain/services/CasService.deleteAsset.test.jstest/unit/domain/services/CasService.empty-file.test.jstest/unit/domain/services/CasService.envelope.test.jstest/unit/domain/services/CasService.errors.test.jstest/unit/domain/services/CasService.events.test.jstest/unit/domain/services/CasService.findOrphanedChunks.test.jstest/unit/domain/services/CasService.kdf.test.jstest/unit/domain/services/CasService.key-validation.test.jstest/unit/domain/services/CasService.merkle.test.jstest/unit/domain/services/CasService.parallel.test.jstest/unit/domain/services/CasService.readManifest.test.jstest/unit/domain/services/CasService.recipients.test.jstest/unit/domain/services/CasService.restore.test.jstest/unit/domain/services/CasService.restoreStream.test.jstest/unit/domain/services/CasService.stream-error.test.jstest/unit/domain/services/CasService.test.js
✅ Files skipped from review due to trivial changes (1)
- package.json
Eliminates redundant second restore() invocation that could silently skip error-code assertions if the call unexpectedly resolved.
Member
Author
|
@coderabbitai review please 🥕 |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/unit/domain/services/CasService.envelope.test.js (1)
350-350: Consider extracting the Manifest import.The
Manifestclass is dynamically imported here and also at line 139. For cleaner test organization, consider importing it once at the module level or creating a small helper.♻️ Optional: Extract Manifest import to module level
import CasError from '../../../../src/domain/errors/CasError.js'; +import Manifest from '../../../../src/domain/value-objects/Manifest.js'; const testCrypto = await getTestCryptoAdapter();Then remove the dynamic imports at lines 139 and 350.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/domain/services/CasService.envelope.test.js` at line 350, The test repeatedly performs dynamic imports of the Manifest value-object (Manifest) in multiple places; extract a single import of Manifest to the module scope (or create a small test helper that returns Manifest) and replace the in-test dynamic imports used in the CasService.envelope tests with that single shared reference; update references in the tests that currently call (await import(...)).default to use the module-level Manifest so the duplicate dynamic imports at the two test sites are removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/unit/domain/services/CasService.envelope.test.js`:
- Line 350: The test repeatedly performs dynamic imports of the Manifest
value-object (Manifest) in multiple places; extract a single import of Manifest
to the module scope (or create a small test helper that returns Manifest) and
replace the in-test dynamic imports used in the CasService.envelope tests with
that single shared reference; update references in the tests that currently call
(await import(...)).default to use the module-level Manifest so the duplicate
dynamic imports at the two test sites are removed.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.mdtest/unit/domain/services/CasService.envelope.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
Member
Author
|
@coderabbitai review please 🥕 |
|
✅ Actions performedReview triggered.
|
This was referenced Feb 28, 2026
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
RecipientSchema— Zod validation for recipient entries in manifests.addRecipient/removeRecipient/listRecipientsAPI onCasServiceand facade.--recipient <label:keyfile>CLI flag +git cas recipient add/remove/listsubcommands.awaiton async crypto in_wrapDek/_unwrapDekthat would silently produce garbage on Bun/Deno.Test plan
npx eslint .— 0 errorsnpm test— 751 tests pass (Node)docker compose run --build --rm test-bun bunx vitest run test/unit— 751 passdocker compose run --build --rm test-deno deno run -A npm:vitest run test/unit— 751 passSummary by CodeRabbit
New Features
Bug Fixes
Tests
Chores