release: v2.0.0 — compression, KDF, Merkle manifests#6
release: v2.0.0 — compression, KDF, Merkle manifests#6flyingrobots wants to merge 9 commits intomainfrom
Conversation
Add gzip compression pipeline, passphrase-based encryption via PBKDF2/scrypt, and Merkle tree manifests for large files. Includes 52 new unit tests, updated API reference, guide, and README documentation.
📝 WalkthroughWalkthroughAdds v2.0.0: gzip compression for stored streams, passphrase-based key derivation (PBKDF2/scrypt) with stored KDF params and deriveKey APIs, and Merkle-tree manifests that split/merge large chunk lists; updates schemas, ports/adapters, public APIs, docs, and extensive tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CasService
participant CryptoPort
participant Compression
participant Persistence
Client->>CasService: store({ source, passphrase?, compression?, ... })
alt passphrase provided
CasService->>CryptoPort: deriveKey({ passphrase, ... })
CryptoPort-->>CasService: { key, salt, params }
end
alt compression enabled
CasService->>Compression: _compressStream(source)
Compression-->>CasService: compressed stream
else
CasService->>CasService: use source stream
end
CasService->>CryptoPort: encrypt(stream, key)
CryptoPort-->>CasService: encrypted chunks
alt chunks > merkleThreshold
CasService->>CasService: _createMerkleTree(chunks)
CasService->>Persistence: writeBlob(subManifest...), writeTree(rootManifest)
else
CasService->>Persistence: writeTree(manifest)
end
Persistence-->>CasService: manifest (with version/compression/subManifests/kdf)
CasService-->>Client: Manifest
sequenceDiagram
participant Client
participant CasService
participant Persistence
participant CryptoPort
participant Decompression
Client->>CasService: restore({ manifest, passphrase? })
alt manifest.subManifests present
CasService->>Persistence: readBlob(subManifest1)
Persistence-->>CasService: subManifest1
CasService->>Persistence: readBlob(subManifestN)
Persistence-->>CasService: subManifestN
CasService->>CasService: merge sub-manifests -> full chunk list
end
alt manifest.encryption.kdf present and passphrase provided
CasService->>CryptoPort: deriveKey({ passphrase, params from manifest })
CryptoPort-->>CasService: key
else
CasService->>CasService: use provided encryptionKey
end
CasService->>CryptoPort: decrypt(chunks, key)
CryptoPort-->>CasService: decrypted stream
alt manifest.compression present
CasService->>Decompression: gunzipAsync(decrypted)
Decompression-->>CasService: decompressed buffer
end
CasService-->>Client: { buffer, bytesWritten }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
index.js (1)
181-190:⚠️ Potential issue | 🔴 Critical
storeFiledoes not forwardpassphrase,kdfOptions, orcompressionto the service.This is the root cause of the README example issue. The destructured parameters on line 181 only include
{ filePath, slug, filename, encryptionKey }, so any v2.0.0 options passed by callers are silently dropped.🐛 Proposed fix
- async storeFile({ filePath, slug, filename, encryptionKey }) { + async storeFile({ filePath, slug, filename, encryptionKey, passphrase, kdfOptions, compression }) { const source = createReadStream(filePath); const service = await this.#getService(); return await service.store({ source, slug, filename: filename || path.basename(filePath), encryptionKey, + passphrase, + kdfOptions, + compression, }); }GUIDE.md (1)
1384-1384:⚠️ Potential issue | 🟡 MinorStale cross-reference: "see Section 10" should be "see Section 13".
The FAQ answer about resilience policy says "(see Section 10)" but after renumbering, the Architecture section (which covers resilience policy) is now Section 13, not Section 10.
📝 Proposed fix
-construction time (see Section 10). +construction time (see Section 13).src/domain/services/CasService.js (2)
350-399: 🛠️ Refactor suggestion | 🟠 MajorOrphaned JSDoc block for
restore()is separated from its method.The JSDoc comment at lines 350-361 originally documented
restore(), but the insertion of_resolveKeyFromPassphrase(lines 369-380) and_resolveEncryptionKey(lines 386-397) between the JSDoc and the actualrestoremethod (line 399) means the JSDoc now inadvertently documents_resolveKeyFromPassphraseinstead. Move therestoreJSDoc immediately above line 399, or remove the orphaned block since_resolveKeyFromPassphrasealready has its own JSDoc.♻️ Proposed fix: remove the orphaned JSDoc
- /** - * Restores a file from its manifest by reading and reassembling chunks. - * - * If the manifest has encryption metadata, decrypts the reassembled - * ciphertext using the provided key. - * - * `@param` {Object} options - * `@param` {import('../value-objects/Manifest.js').default} options.manifest - The file manifest. - * `@param` {Buffer} [options.encryptionKey] - 32-byte key, required if manifest is encrypted. - * `@returns` {Promise<{ buffer: Buffer, bytesWritten: number }>} - * `@throws` {CasError} MISSING_KEY if manifest is encrypted but no key is provided. - * `@throws` {CasError} INTEGRITY_ERROR if chunk verification or decryption fails. - */ /** * Resolves the encryption key from a passphrase using KDF params from the manifest.
550-573: 🛠️ Refactor suggestion | 🟠 MajorOrphaned JSDoc for
verifyIntegrityis separated from its method byderiveKey.Same issue: the
verifyIntegrityJSDoc at lines 550-555 is separated from the actual method at line 573 by thederiveKeymethod (lines 556-571). The JSDoc attaches toderiveKeyinstead.♻️ Proposed fix: move deriveKey above the verifyIntegrity JSDoc, or reorder
+ async deriveKey(options) { + return await this.crypto.deriveKey(options); + } + /** * Verifies the integrity of a stored file by re-hashing its chunks. * `@param` {import('../value-objects/Manifest.js').default} manifest * `@returns` {Promise<boolean>} */ - /** - * Derives an encryption key from a passphrase using PBKDF2 or scrypt. - * ... - */ - async deriveKey(options) { - return await this.crypto.deriveKey(options); - } - async verifyIntegrity(manifest) {index.d.ts (1)
120-125:⚠️ Potential issue | 🟠 Major
storeFile()does not acceptpassphrase,kdfOptions, orcompressionoptions, contradicting GUIDE.md examples.The
storeFile()method (lines 120-125 in index.d.ts) accepts onlyfilePath,slug,filename?, andencryptionKey?. However, GUIDE.md demonstratesstoreFile()withcompression(line 775) andpassphrase(line 836).The implementation (index.js lines 181-189) only forwards four parameters to
service.store()and does not forward these options. Users following the GUIDE examples will have these options silently ignored.Either extend
storeFile()to accept and forward these options, or update the GUIDE examples to usestore()instead.docs/API.md (2)
150-163:⚠️ Potential issue | 🟠 MajorUpdate method signature to include all parameters.
The method signature on line 150 shows only
{ filePath, slug, filename, encryptionKey }, but the parameters list documentspassphrase,kdfOptions, andcompression. This inconsistency will mislead API consumers.📝 Proposed fix to update the signature
#### storeFile ```javascript -await cas.storeFile({ filePath, slug, filename, encryptionKey }) +await cas.storeFile({ filePath, slug, filename, encryptionKey, passphrase, kdfOptions, compression })</details> --- `211-220`: _⚠️ Potential issue_ | _🟠 Major_ **Update method signature to include `passphrase` parameter.** The method signature on line 211 omits the `passphrase` parameter that is documented in the parameters list on line 220. <details> <summary>📝 Proposed fix to update the signature</summary> ```diff #### restoreFile ```javascript -await cas.restoreFile({ manifest, encryptionKey, outputPath }) +await cas.restoreFile({ manifest, encryptionKey, passphrase, outputPath })</details> </blockquote></details> </blockquote></details>🤖 Fix all issues with AI agents
In `@GUIDE.md`: - Around line 842-850: The example showing manifest.encryption.kdf includes a hash field but the types and schema (KdfParams in Manifest.d.ts and KdfSchema in ManifestSchema.js) do not define it; either remove the hash entry from the docs example or add a hash property to the type and schema. To fix, decide whether KDF implementations expose a hash name (e.g., 'sha-512'); if yes, add hash: string to KdfParams in Manifest.d.ts and add a corresponding { hash: { type: 'string' } } entry to KdfSchema in ManifestSchema.js (including any validation/defaults), otherwise update the example (manifest.encryption.kdf) to omit the hash field so it matches the current KdfParams/KdfSchema. - Around line 937-942: The public ContentAddressableStore constructor currently ignores the merkleThreshold option shown in GUIDE.md; update the constructor to accept merkleThreshold and forward it into the CasService initialization so callers can configure it. Specifically, modify ContentAddressableStore (constructor in index.js) to include merkleThreshold in its parameter parsing and pass merkleThreshold when creating new CasService(...), ensuring defaulting behavior matches CasService (1000) if not provided; alternatively, update GUIDE.md to remove the merkleThreshold example if you prefer not to expose it. In `@README.md`: - Around line 73-79: The README example is accurate but storeFile() currently ignores passphrase and compression; update the storeFile function (the exported storeFile in index.js) to accept passphrase and compression in its parameter destructuring (alongside filePath, slug, filename, encryptionKey) and forward those options through to service.store({ ... }) so service.store receives passphrase and compression; also update any related JSDoc/typing for storeFile and adjust unit tests or callers that rely on its signature. In `@src/infrastructure/adapters/NodeCryptoAdapter.js`: - Around line 80-86: The code uses saltBuf.toString('base64') which produces incorrect output for Uint8Array salts; update NodeCryptoAdapter so params.salt is created with Buffer.from(saltBuf).toString('base64') (matching BunCryptoAdapter) and ensure the function returns/wraps the salt value as the base64 string in the same shape the Bun adapter uses; locate saltBuf, params.salt and randomBytes in NodeCryptoAdapter and replace the toString call accordingly. In `@src/infrastructure/adapters/WebCryptoAdapter.js`: - Around line 155-165: The private method `#deriveScrypt` currently imports node:crypto (scrypt) which breaks non-Node environments; update `#deriveScrypt` to detect availability of node:crypto before importing and if unavailable throw a clear, descriptive error (e.g., "scrypt not supported in this runtime; requires Node.js crypto") or return a documented fallback; specifically modify the `#deriveScrypt` function to wrap the dynamic import in a try/catch or feature-check and throw that descriptive error (or call an alternative browser-friendly implementation) so callers of WebCryptoAdapter (per the class JSDoc) receive a clear failure in non-Node runtimes. - Around line 134-137: The code currently treats any algorithm value other than 'pbkdf2' as scrypt; update the logic around the opts/key assignment to explicitly validate the algorithm variable and throw an Error for unsupported KDFs (same behavior as NodeCryptoAdapter/BunCryptoAdapter) instead of silently falling back to `#deriveScrypt`; ensure you call this.#derivePbkdf2(opts) only when algorithm === 'pbkdf2', call this.#deriveScrypt(opts) only when algorithm === 'scrypt', and throw Error(`Unsupported KDF algorithm: ${algorithm}`) for any other value so consumers get a clear failure.🧹 Nitpick comments (9)
src/domain/value-objects/Manifest.js (2)
25-25: Prefer??over||for defaultingversion.
data.version || 1treats any falsy value (including0) as missing. While the Zod schema currently prevents0from reaching this line, using nullish coalescing (??) more precisely communicates the intent of "default when absent" and is resilient against future schema changes.♻️ Suggested change
- this.version = data.version || 1; + this.version = data.version ?? 1;
46-57:toJSON()correctly includes new fields, but JSDoc@returnsis now stale.The
@returnstype on line 44 still lists onlyslug, filename, size, chunks, encryption. Consider updating it to reflectversion,compression, andsubManifests.test/unit/domain/services/CasService.merkle.test.js (1)
44-51:readTreeregex will throw an unguarded NPE if the entry format changes.If
CasService.createTreeever changes its tree-entry format,matchwill benullandmatch[1]will throw aTypeErrorwith no useful message. A guard or descriptive assertion would make test failures easier to diagnose.♻️ Suggested improvement
return Promise.resolve(entries.map((e) => { const match = e.match(/^(\d+) (\w+) ([^\t]+)\t(.+)$/); + if (!match) throw new Error(`readTree mock: unexpected entry format: ${e}`); return { mode: match[1], type: match[2], oid: match[3], name: match[4] }; }));src/domain/schemas/ManifestSchema.js (1)
16-25:KdfSchemadoesn't enforce algorithm-specific required fields.For
pbkdf2,iterationsshould be required; forscrypt,cost/blockSize/parallelizationshould be required. Currently all are optional, so a malformed manifest (e.g., scrypt withoutcost) would pass validation but fail silently at re-derivation. A Zod discriminated union or.refine()would catch this at the schema boundary.Not blocking since the adapters always produce complete params, but worth hardening if manifests can be hand-crafted or come from untrusted sources.
test/unit/domain/services/CasService.compression.test.js (1)
11-56: Consider extracting shared test helpers (bufferSource, in-memory persistence factory) into a common module.
bufferSourceand the in-memory blob store setup are duplicated inCasService.merkle.test.jsand here. Extracting them to e.g.test/helpers/cas-test-utils.jswould reduce maintenance burden as the test suite grows. Not blocking for this PR.test/unit/domain/services/CasService.kdf.test.js (1)
258-267: Redundant secondrestorecall with wrong passphrase.Lines 258-260 already assert that the restore rejects with
CasError. The try/catch block at lines 262-266 re-executes the same failing call just to checkerr.code. This doubles the work (expensive for KDF). Consolidate into a single assertion.♻️ Suggested consolidation
- await expect( - service.restore({ manifest, passphrase: wrongPassphrase }), - ).rejects.toThrow(CasError); - - try { - await service.restore({ manifest, passphrase: wrongPassphrase }); - } catch (err) { - expect(err.code).toBe('INTEGRITY_ERROR'); - } + await expect( + service.restore({ manifest, passphrase: wrongPassphrase }), + ).rejects.toThrow( + expect.objectContaining({ code: 'INTEGRITY_ERROR' }), + );src/domain/services/CasService.js (2)
174-183: Inconsistent import style: dynamic imports in_compressStreamvs. static import ofgunzipat top.
gunzipis statically imported at line 7, but_compressStreamdynamically importscreateGzipandReadablefrom the same/sibling modules. Sincenode:zlibis already a static dependency, prefer static imports for consistency and to avoid the async overhead on each call.♻️ Suggested: move to static imports at the top of the file
import { gunzip } from 'node:zlib'; +import { createGzip } from 'node:zlib'; +import { Readable } from 'node:stream'; import { promisify } from 'node:util';Then simplify the method:
async *_compressStream(source) { - const { createGzip } = await import('node:zlib'); - const { Readable } = await import('node:stream'); const gz = createGzip(); const input = Readable.from(source);
386-397:_resolveEncryptionKeydoesn'tawaitthe passphrase path but returns a Promise — verify callers alwaysawait.Line 388 returns the result of
_resolveKeyFromPassphrase(a Promise) withoutawait, while theencryptionKeypath at line 396 returnsPromise.resolve(encryptionKey). Both paths return Promises, sorestoreat line 400 correctlyawaits. This is fine, but the method is notasync— it relies on returning raw Promises. Consider making itasyncfor consistency, since it mixes sync throws (line 394) with Promise returns.Note: The sync
throwat line 394 works correctly because the callerawaits the result, and a thrown error in a non-async function that returns a Promise will propagate as an unhandled exception rather than a rejected Promise. However, sincerestorewraps it inawait, the throw occurs before the Promise is created, so it propagates correctly as a synchronous exception fromrestore's perspective.Actually, this is a subtle correctness concern: if
manifest.encryption?.encryptedis true and no key/passphrase is provided, thethrowat line 394 is synchronous. Since_resolveEncryptionKeyis notasync, this throw propagates synchronously from withinrestore(). Becauserestore()isasync, the throw will be caught and converted to a rejected promise. This is safe. No action needed unless you prefer uniformity.docs/API.md (1)
360-375: Consider clarifying when to usederiveKey()directly vs.passphraseparameter.The example shows manually calling
deriveKey()and then passing the derived key tostore()via theencryptionKeyparameter. Sincestore()now accepts apassphraseparameter directly, it would help to explain when and why a developer would choose the manual approach (e.g., when you need explicit control over salt/params, want to derive once and reuse the key, or need to store KDF parameters separately for later verification).💡 Suggested enhancement
Add a clarifying sentence before the example:
**Example:** + +// Manual key derivation gives you explicit control over salt and KDF parameters. +// Alternatively, use the `passphrase` option directly in store() for automatic KDF. ```javascript const { key, salt, params } = await cas.deriveKey({
| const saltBuf = salt || randomBytes(32); | ||
| let key; | ||
| const params = { | ||
| algorithm, | ||
| salt: saltBuf.toString('base64'), | ||
| keyLength, | ||
| }; |
There was a problem hiding this comment.
saltBuf.toString('base64') silently produces wrong output for Uint8Array salts.
If a caller passes a Uint8Array as salt, Uint8Array.prototype.toString() ignores the encoding argument and returns a comma-separated byte list (e.g. "1,2,3") instead of base64. This would corrupt params.salt, making re-derivation impossible. The BunCryptoAdapter avoids this by using Buffer.from(saltBuf).toString('base64').
🐛 Proposed fix
const params = {
algorithm,
- salt: saltBuf.toString('base64'),
+ salt: Buffer.from(saltBuf).toString('base64'),
keyLength,
};Also wrap the return value for consistency with the Bun adapter:
- return { key, salt: saltBuf, params };
+ return { key, salt: Buffer.from(saltBuf), params };🤖 Prompt for AI Agents
In `@src/infrastructure/adapters/NodeCryptoAdapter.js` around lines 80 - 86, The
code uses saltBuf.toString('base64') which produces incorrect output for
Uint8Array salts; update NodeCryptoAdapter so params.salt is created with
Buffer.from(saltBuf).toString('base64') (matching BunCryptoAdapter) and ensure
the function returns/wraps the salt value as the base64 string in the same shape
the Bun adapter uses; locate saltBuf, params.salt and randomBytes in
NodeCryptoAdapter and replace the toString call accordingly.
The KDF output example showed `hash: 'sha-512'` but KdfParams schema does not include a `hash` field. Remove it to match the actual output.
The facade constructor now accepts and forwards merkleThreshold to CasService, matching the GUIDE documentation example.
storeFile() now accepts and forwards passphrase, kdfOptions, and compression options to service.store(), matching the store() API.
Uint8Array.prototype.toString() ignores encoding arguments, producing comma-separated byte lists instead of base64. Wrap with Buffer.from() to ensure correct base64 output matching BunCryptoAdapter behavior.
Previously any non-pbkdf2 algorithm silently fell through to scrypt. Now explicitly validates and throws for unsupported algorithms, matching NodeCryptoAdapter and BunCryptoAdapter behavior.
The scrypt KDF path imports node:crypto which isn't available in browsers. Wrap the import in try/catch so non-Node runtimes get a clear error instead of an opaque import failure.
Add Fixed section covering storeFile param forwarding, NodeCryptoAdapter salt encoding, and WebCryptoAdapter KDF validation.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
GUIDE.md (1)
1377-1384:⚠️ Potential issue | 🟡 MinorStale cross-reference: "see Section 10" should be "see Section 13".
The resilience policy is documented in the Architecture section, which was renumbered from 10 to 13 after the new sections were added.
📝 Proposed fix
-construction time (see Section 10). +construction time (see Section 13).
🧹 Nitpick comments (4)
index.js (1)
122-136: Factory methods don't forwardmerkleThreshold(orcrypto).
createJsonandcreateCbordon't accept or propagatemerkleThreshold, meaning callers using these convenience factories can't opt into Merkle manifests. Consider forwarding it (and optionallycrypto) for consistency with the constructor.♻️ Suggested change
- static createJson({ plumbing, chunkSize, policy }) { - return new ContentAddressableStore({ plumbing, chunkSize, codec: new JsonCodec(), policy }); + static createJson({ plumbing, chunkSize, policy, crypto, merkleThreshold }) { + return new ContentAddressableStore({ plumbing, chunkSize, codec: new JsonCodec(), policy, crypto, merkleThreshold }); }- static createCbor({ plumbing, chunkSize, policy }) { - return new ContentAddressableStore({ plumbing, chunkSize, codec: new CborCodec(), policy }); + static createCbor({ plumbing, chunkSize, policy, crypto, merkleThreshold }) { + return new ContentAddressableStore({ plumbing, chunkSize, codec: new CborCodec(), policy, crypto, merkleThreshold }); }index.d.ts (1)
98-108: Factory methodscreateJson/createCbordon't acceptmerkleThreshold.These convenience factories only expose
plumbing,chunkSize, andpolicy. If users needmerkleThreshold, they must use the main constructor. This is fine if intentional, but worth confirming it's a deliberate design choice — the GUIDE showsmerkleThresholdon the constructor, and these factories are meant for quick setup.src/infrastructure/adapters/NodeCryptoAdapter.js (1)
100-102: Inconsistent error type: plainErrorvsCasError.
deriveKeythrows a plainErrorfor unsupported algorithms while the rest of this adapter usesCasErrorwith structured codes. Consider usingCasErrorwith a descriptive code (e.g.,'UNSUPPORTED_KDF') for consistency, though this is low priority since the other adapters also use plainErrorhere.src/infrastructure/adapters/WebCryptoAdapter.js (1)
147-158: Side effect:#derivePbkdf2and#deriveScryptmutate the sharedparamsobject.Both private helpers modify
paramsby reference (e.g.,params.iterations = iterationson line 156,params.cost = coston line 172). This works becauseparamsis a freshly created local object inderiveKey, but the mutation-by-reference pattern couples these helpers to the caller's object lifecycle. A purely functional approach (returning the extra fields to be merged by the caller) would be cleaner, but this is a minor style nit given the current scope.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
- Move restore() JSDoc directly above its method (was orphaned by inserted helpers) - Move verifyIntegrity() JSDoc directly above its method (was orphaned by deriveKey) - Add missing passphrase param to restore() JSDoc - Update storeFile/restoreFile signatures in API.md to include v2 params - Fix stale "Section 10" cross-reference to "Section 13" in GUIDE.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/API.md (2)
486-498:⚠️ Potential issue | 🟡 MinorCasService constructor code block is missing
merkleThreshold.Line 488 shows
new CasService({ persistence, codec, crypto, chunkSize })butmerkleThresholdis documented as a parameter at line 497 and is part of the actual constructor signature.📝 Proposed fix
-new CasService({ persistence, codec, crypto, chunkSize }) +new CasService({ persistence, codec, crypto, chunkSize, merkleThreshold })
519-529:⚠️ Potential issue | 🟡 MinorCasService methods list is stale — missing new v2.0.0 parameters and
deriveKey.The method signatures listed here don't reflect the v2.0.0 additions:
store()is missingpassphrase,kdfOptions,compressionrestore()is missingpassphrasederiveKey(options)is not listed at all📝 Proposed fix
All methods from ContentAddressableStore delegate to CasService. See ContentAddressableStore documentation above for: -- `store({ source, slug, filename, encryptionKey })` -- `restore({ manifest, encryptionKey })` +- `store({ source, slug, filename, encryptionKey, passphrase, kdfOptions, compression })` +- `restore({ manifest, encryptionKey, passphrase })` - `createTree({ manifest })` - `verifyIntegrity(manifest)` - `readManifest({ treeOid })` - `deleteAsset({ treeOid })` - `findOrphanedChunks({ treeOids })` - `encrypt({ buffer, key })` - `decrypt({ buffer, key, meta })` +- `deriveKey(options)`src/domain/services/CasService.js (1)
280-318:⚠️ Potential issue | 🔴 CriticalSub-manifest blobs are not included as tree entries and will be garbage-collected.
The sub-manifest blobs are written to Git's ODB via
writeBlob()(line 289), with their OIDs stored in the root manifest'ssubManifestsarray. However, they are not added as entries in the Git tree (lines 312–315). Only the root manifest blob and chunk blobs are tree entries.Since Git's garbage collector walks only the object graph (refs → commits → trees → objects) and does not parse blob content for OID references, these sub-manifest blobs are unreachable. Running
git gc --prunewill delete them as loose objects. Subsequently,readManifest()will fail when attempting to read the sub-manifest blobs by OID.Add sub-manifest blobs as tree entries
const treeEntries = [ `100644 blob ${rootOid}\tmanifest.${this.codec.extension}`, + ...subManifestRefs.map((ref) => `100644 blob ${ref.oid}\tsub-manifest-${ref.startIndex}.${this.codec.extension}`), ...chunkBlobEntries, ];
🧹 Nitpick comments (2)
src/domain/services/CasService.js (2)
7-12: Inconsistent import strategy fornode:zlib.
gunzipis statically imported at line 7, but_compressStream(line 175) dynamically importscreateGzipfrom the same module. Consider using a static import forcreateGzipas well for consistency.♻️ Suggested refactor
-import { gunzip } from 'node:zlib'; -import { promisify } from 'node:util'; +import { gunzip, createGzip } from 'node:zlib'; +import { promisify } from 'node:util'; +import { Readable } from 'node:stream';Then in
_compressStream:async *_compressStream(source) { - const { createGzip } = await import('node:zlib'); - const { Readable } = await import('node:stream'); const gz = createGzip();
36-48: Consider validatingmerkleThreshold.
chunkSizeis validated (must be ≥ 1024), butmerkleThresholdhas no validation. A value of0or a negative number would cause every non-empty manifest to take the Merkle path, which is likely unintended.🛡️ Suggested validation
this.chunkSize = chunkSize; this.merkleThreshold = merkleThreshold; + if (!Number.isInteger(merkleThreshold) || merkleThreshold < 1) { + throw new Error('Merkle threshold must be a positive integer'); + } }
1 participant
Summary
compression: { algorithm: 'gzip' }onstore(). Compression is applied before encryption. Decompression onrestore()is automatic.deriveKey()andpassphraseoption onstore()/restore(). KDF parameters stored in manifest for deterministic re-derivation. All three crypto adapters implementderiveKey().merkleThreshold, default 1000) auto-split into sub-manifests. Root manifest usesversion: 2withsubManifestsreferences.readManifest()transparently reconstitutes v2 manifests. Full backward compat with v1.deriveKey, compression, passphrase options, Merkle fields).Test plan
npm test— 421 unit tests passnpx eslint .— lint cleannpx jsr publish --dry-run --allow-dirty— JSR validationnpm pack --dry-run— npm distribution checkSummary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests
Documentation