Merge pull request #15 from git-stunts/chore/followups

Author: flyingrobots

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

ROADMAP.md

@@ -605,6 +605,51 @@ All tasks completed (13.1–13.6). See [COMPLETED_TASKS.md](./COMPLETED_TASKS.md
 
 ---
 
+## M15 — 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<string>`. 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<string>` (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<string>` 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.

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),
       },
     };
   }

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,
+  };
+}

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<Buffer>} 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++) {

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),
     };
   }
 

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');
+  });
+});

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);
     }