From 16b08cebae113f0b5e75ddc3d788757e5e068b58 Mon Sep 17 00:00:00 2001 From: James Ross Date: Sat, 28 Feb 2026 11:12:44 -0800 Subject: [PATCH 1/2] chore: deterministic fuzz, DRY chunk verify, DRY KDF metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three hygiene fixes from architecture audit: 1. Envelope fuzz test uses seeded xorshift32 PRNG instead of Math.random() 2. Extract _readAndVerifyChunk() — single chunk verification shared by buffered and streaming restore paths 3. Extract buildKdfMetadata() helper — eliminates duplicated KDF object construction in VaultService and ContentAddressableStore --- CHANGELOG.md | 7 +++ ROADMAP.md | 45 ++++++++++++++++ index.js | 11 +--- src/domain/helpers/buildKdfMetadata.js | 18 +++++++ src/domain/services/CasService.js | 49 +++++++++--------- src/domain/services/VaultService.js | 11 +--- .../domain/helpers/buildKdfMetadata.test.js | 51 +++++++++++++++++++ .../services/CasService.envelope.test.js | 18 ++++++- 8 files changed, 165 insertions(+), 45 deletions(-) create mode 100644 src/domain/helpers/buildKdfMetadata.js create mode 100644 test/unit/domain/helpers/buildKdfMetadata.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index d75000d..d2f2bb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed +- **Deterministic fuzz test** — envelope fuzz round-trip test now uses a seeded xorshift32 PRNG instead of `Math.random()`, making failures reproducible across runs. +- **DRY chunk verification** — extracted `_readAndVerifyChunk()` in `CasService`; both the buffered and streaming restore paths now delegate to the same single-chunk verification method. +- **DRY KDF metadata** — extracted `buildKdfMetadata()` helper (`src/domain/helpers/buildKdfMetadata.js`); `VaultService` and `ContentAddressableStore` both call it instead of duplicating the KDF object construction. + ## [5.2.2] — JSDoc total coverage (2026-02-28) ### Added diff --git a/ROADMAP.md b/ROADMAP.md index cdf9173..93de6c1 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -605,6 +605,51 @@ All tasks completed (13.1–13.6). See [COMPLETED_TASKS.md](./COMPLETED_TASKS.md --- +## M14 — Prism (code hygiene) + +Consistency and DRY fixes surfaced by architecture audit. No new features, no API changes. + +### 14.1 — Consistent async `sha256()` across CryptoPort adapters + +**Problem:** `NodeCryptoAdapter.sha256()` returns `string` (sync), while `BunCryptoAdapter` and `WebCryptoAdapter` return `Promise`. Callers must defensively `await` every call. This is a Liskov Substitution violation — adapters are not interchangeable without the caller knowing which one it got. + +**Fix:** Make `NodeCryptoAdapter.sha256()` return `Promise` (wrap the sync result). All three adapters then have the same async signature. CasService already awaits every call via `_sha256()`, so no call-site changes are needed outside the adapter itself. + +**Files:** +- `src/infrastructure/adapters/NodeCryptoAdapter.js` — wrap return in `Promise.resolve()` +- `src/ports/CryptoPort.js` — update JSDoc to document `Promise` as the contract + +**Risk:** None. All callers already `await`. Changing sync→async is backward compatible for awaiting code. + +**Tests:** Existing crypto adapter tests already assert on the resolved value. Add an explicit test: `expect(adapter.sha256(buf)).toBeInstanceOf(Promise)`. + +### 14.2 — Extract `KeyResolver` from CasService + +**Problem:** `CasService` is a ~1087-line god object. Key resolution logic (`_resolveDecryptionKey`, `_resolvePassphraseForDecryption`, `_resolveKeyForRecipients`, `_unwrapDek`, `_wrapDek`, `_validateKeySourceExclusive`) is ~70 lines of self-contained logic that has nothing to do with chunking, storage, or manifests. It's a distinct responsibility: "given a manifest and caller-provided credentials, produce the decryption key." + +**Fix:** Extract a `KeyResolver` class into `src/domain/services/KeyResolver.js`. It receives a `CryptoPort` via constructor injection. CasService delegates to it. + +**API:** +```js +class KeyResolver { + constructor(crypto) { this.crypto = crypto; } + async resolveForDecryption(manifest, encryptionKey, passphrase) { ... } + async resolveForStore(encryptionKey, passphrase, kdfOptions) { ... } + async resolveRecipients(recipients) { ... } + async wrapDek(dek, kek) { ... } + async unwrapDek(recipientEntry, kek) { ... } +} +``` + +**Files:** +- New: `src/domain/services/KeyResolver.js` +- Modified: `src/domain/services/CasService.js` — delegate key resolution +- New: `test/unit/domain/services/KeyResolver.test.js` + +**Risk:** Low — internal refactor, no public API change. CasService methods remain unchanged. + +--- + ## Backlog (unscheduled) Ideas for future milestones. Not committed, not prioritized — just captured. diff --git a/index.js b/index.js index c545867..5fd77e9 100644 --- a/index.js +++ b/index.js @@ -25,6 +25,7 @@ import EventEmitterObserver from './src/infrastructure/adapters/EventEmitterObse import StatsCollector from './src/infrastructure/adapters/StatsCollector.js'; import FixedChunker from './src/infrastructure/chunkers/FixedChunker.js'; import CdcChunker from './src/infrastructure/chunkers/CdcChunker.js'; +import buildKdfMetadata from './src/domain/helpers/buildKdfMetadata.js'; export { CasService, @@ -627,15 +628,7 @@ export default class ContentAddressableStore { ...metadata, encryption: { cipher: metadata.encryption.cipher, - kdf: { - algorithm: newParams.algorithm, - salt: newSalt.toString('base64'), - ...('iterations' in newParams && { iterations: newParams.iterations }), - ...('cost' in newParams && { cost: newParams.cost }), - ...('blockSize' in newParams && { blockSize: newParams.blockSize }), - ...('parallelization' in newParams && { parallelization: newParams.parallelization }), - keyLength: newParams.keyLength, - }, + kdf: buildKdfMetadata(newSalt, newParams), }, }; } diff --git a/src/domain/helpers/buildKdfMetadata.js b/src/domain/helpers/buildKdfMetadata.js new file mode 100644 index 0000000..f981b9d --- /dev/null +++ b/src/domain/helpers/buildKdfMetadata.js @@ -0,0 +1,18 @@ +/** + * Builds the KDF metadata object for vault/manifest encryption metadata. + * + * @param {Buffer} salt - KDF salt. + * @param {{ algorithm: string, keyLength: number, iterations?: number, cost?: number, blockSize?: number, parallelization?: number }} params - KDF parameters. + * @returns {{ algorithm: string, salt: string, keyLength: number, iterations?: number, cost?: number, blockSize?: number, parallelization?: number }} + */ +export default function buildKdfMetadata(salt, params) { + return { + algorithm: params.algorithm, + salt: salt.toString('base64'), + ...('iterations' in params && { iterations: params.iterations }), + ...('cost' in params && { cost: params.cost }), + ...('blockSize' in params && { blockSize: params.blockSize }), + ...('parallelization' in params && { parallelization: params.parallelization }), + keyLength: params.keyLength, + }; +} diff --git a/src/domain/services/CasService.js b/src/domain/services/CasService.js index 86d78a3..e04ddcb 100644 --- a/src/domain/services/CasService.js +++ b/src/domain/services/CasService.js @@ -519,6 +519,28 @@ export default class CasService { return await this.persistence.writeTree(treeEntries); } + /** + * Reads a single chunk blob from Git and verifies its SHA-256 digest. + * @private + * @param {{ index: number, size: number, digest: string, blob: string }} chunk - Chunk metadata. + * @returns {Promise} Verified chunk buffer. + * @throws {CasError} INTEGRITY_ERROR if the chunk digest does not match. + */ + async _readAndVerifyChunk(chunk) { + const blob = await this.persistence.readBlob(chunk.blob); + const digest = await this._sha256(blob); + if (digest !== chunk.digest) { + const err = new CasError( + `Chunk ${chunk.index} integrity check failed`, + 'INTEGRITY_ERROR', + { chunkIndex: chunk.index, expected: chunk.digest, actual: digest }, + ); + this.observability.metric('error', { code: err.code, message: err.message }); + throw err; + } + return blob; + } + /** * Reads chunk blobs from Git and verifies their SHA-256 digests. * @private @@ -529,17 +551,7 @@ export default class CasService { async _readAndVerifyChunks(chunks) { const buffers = []; for (const chunk of chunks) { - const blob = await this.persistence.readBlob(chunk.blob); - const digest = await this._sha256(blob); - if (digest !== chunk.digest) { - const err = new CasError( - `Chunk ${chunk.index} integrity check failed`, - 'INTEGRITY_ERROR', - { chunkIndex: chunk.index, expected: chunk.digest, actual: digest }, - ); - this.observability.metric('error', { code: err.code, message: err.message }); - throw err; - } + const blob = await this._readAndVerifyChunk(chunk); buffers.push(blob); this.observability.metric('chunk', { action: 'restored', index: chunk.index, size: blob.length, digest: chunk.digest }); } @@ -659,20 +671,7 @@ export default class CasService { const readAhead = this.concurrency; let totalSize = 0; - const readAndVerify = async (chunk) => { - const blob = await this.persistence.readBlob(chunk.blob); - const digest = await this._sha256(blob); - if (digest !== chunk.digest) { - const err = new CasError( - `Chunk ${chunk.index} integrity check failed`, - 'INTEGRITY_ERROR', - { chunkIndex: chunk.index, expected: chunk.digest, actual: digest }, - ); - this.observability.metric('error', { code: err.code, message: err.message }); - throw err; - } - return blob; - }; + const readAndVerify = (chunk) => this._readAndVerifyChunk(chunk); const ahead = []; for (let i = 0; i < Math.min(readAhead, chunks.length); i++) { diff --git a/src/domain/services/VaultService.js b/src/domain/services/VaultService.js index 2650ad3..d5a1ac2 100644 --- a/src/domain/services/VaultService.js +++ b/src/domain/services/VaultService.js @@ -2,6 +2,7 @@ * @fileoverview Domain service for vault (GC-safe ref-based asset index) operations. */ import CasError from '../errors/CasError.js'; +import buildKdfMetadata from '../helpers/buildKdfMetadata.js'; const VAULT_REF = 'refs/cas/vault'; const MAX_CAS_RETRIES = 3; @@ -327,15 +328,7 @@ export default class VaultService { static #buildEncryptionMeta(salt, params) { return { cipher: 'aes-256-gcm', - kdf: { - algorithm: params.algorithm, - salt: salt.toString('base64'), - ...('iterations' in params && { iterations: params.iterations }), - ...('cost' in params && { cost: params.cost }), - ...('blockSize' in params && { blockSize: params.blockSize }), - ...('parallelization' in params && { parallelization: params.parallelization }), - keyLength: params.keyLength, - }, + kdf: buildKdfMetadata(salt, params), }; } diff --git a/test/unit/domain/helpers/buildKdfMetadata.test.js b/test/unit/domain/helpers/buildKdfMetadata.test.js new file mode 100644 index 0000000..8283214 --- /dev/null +++ b/test/unit/domain/helpers/buildKdfMetadata.test.js @@ -0,0 +1,51 @@ +import { describe, it, expect } from 'vitest'; +import buildKdfMetadata from '../../../../src/domain/helpers/buildKdfMetadata.js'; + +describe('buildKdfMetadata', () => { + it('builds PBKDF2 metadata with iterations', () => { + const salt = Buffer.from('test-salt'); + const params = { algorithm: 'pbkdf2', iterations: 100000, keyLength: 32 }; + + const result = buildKdfMetadata(salt, params); + + expect(result).toEqual({ + algorithm: 'pbkdf2', + salt: salt.toString('base64'), + iterations: 100000, + keyLength: 32, + }); + }); + + it('builds scrypt metadata with cost/blockSize/parallelization', () => { + const salt = Buffer.from('scrypt-salt'); + const params = { algorithm: 'scrypt', cost: 16384, blockSize: 8, parallelization: 1, keyLength: 32 }; + + const result = buildKdfMetadata(salt, params); + + expect(result).toEqual({ + algorithm: 'scrypt', + salt: salt.toString('base64'), + cost: 16384, + blockSize: 8, + parallelization: 1, + keyLength: 32, + }); + }); + + it('omits optional fields when absent', () => { + const salt = Buffer.from('minimal'); + const params = { algorithm: 'pbkdf2', keyLength: 32 }; + + const result = buildKdfMetadata(salt, params); + + expect(result).toEqual({ + algorithm: 'pbkdf2', + salt: salt.toString('base64'), + keyLength: 32, + }); + expect(result).not.toHaveProperty('iterations'); + expect(result).not.toHaveProperty('cost'); + expect(result).not.toHaveProperty('blockSize'); + expect(result).not.toHaveProperty('parallelization'); + }); +}); diff --git a/test/unit/domain/services/CasService.envelope.test.js b/test/unit/domain/services/CasService.envelope.test.js index 9cf8946..a49b203 100644 --- a/test/unit/domain/services/CasService.envelope.test.js +++ b/test/unit/domain/services/CasService.envelope.test.js @@ -8,6 +8,19 @@ import CasError from '../../../../src/domain/errors/CasError.js'; const testCrypto = await getTestCryptoAdapter(); +// --------------------------------------------------------------------------- +// Deterministic PRNG (xorshift32) — keeps fuzz tests reproducible +// --------------------------------------------------------------------------- +function createSeededRng(seed = 42) { + let s = seed >>> 0 || 1; + return (max) => { + s ^= s << 13; + s ^= s >>> 17; + s ^= s << 5; + return (s >>> 0) % max; + }; +} + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- @@ -310,8 +323,9 @@ describe('CasService – envelope encryption (fuzz round-trips)', () => { beforeEach(() => { ({ service } = setup()); }); it('50 random plaintexts × 3 random KEKs all round-trip', async () => { + const rng = createSeededRng(12345); for (let i = 0; i < 50; i++) { - const size = Math.floor(Math.random() * 4096); + const size = rng(4096); const original = randomBytes(size); const keys = [randomBytes(32), randomBytes(32), randomBytes(32)]; @@ -322,7 +336,7 @@ describe('CasService – envelope encryption (fuzz round-trips)', () => { recipients: keys.map((key, j) => ({ label: `k${j}`, key })), }); - const idx = Math.floor(Math.random() * 3); + const idx = rng(3); const { buffer } = await service.restore({ manifest, encryptionKey: keys[idx] }); expect(buffer.equals(original)).toBe(true); } From 090f7a90d60e043053e5c7ab6e6e61aa106513a1 Mon Sep 17 00:00:00 2001 From: James Ross Date: Sat, 28 Feb 2026 11:20:23 -0800 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20rename=20Prism=20milestone=20M14=20?= =?UTF-8?q?=E2=86=92=20M15=20to=20avoid=20Conduit=20ID=20collision?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ROADMAP.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index 93de6c1..8ab1781 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -605,7 +605,7 @@ All tasks completed (13.1–13.6). See [COMPLETED_TASKS.md](./COMPLETED_TASKS.md --- -## M14 — Prism (code hygiene) +## M15 — Prism (code hygiene) Consistency and DRY fixes surfaced by architecture audit. No new features, no API changes.