From 9a8b7a6cd41724aa96d03064862d0226f74f5275 Mon Sep 17 00:00:00 2001 From: David Snelling Date: Fri, 14 Nov 2025 15:31:06 -0800 Subject: [PATCH] fix: critical blob integrity regression with defense-in-depth architecture (v5.10.1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CRITICAL BUG FIX: v5.10.0 regressed the v5.7.2 blob integrity bug, causing 100% VFS file read failure. This fix restores functionality with production-grade defense-in-depth architecture and comprehensive testing. Problem: - v5.10.0 reintroduced bug where BlobStorage.read() hashed wrapped data - Symptom: "Blob integrity check failed" on every VFS file read - Impact: 100% failure rate in Workshop application - Root Cause: Missing defense-in-depth unwrap verification The Fix: 1. Defense-in-Depth Unwrapping - Added unwrap verification in BlobStorage.read() before hash check (line 342) - Added unwrap for metadata parsing (line 314) - Ensures data is always unwrapped regardless of adapter behavior 2. DRY Architecture - Created binaryDataCodec.ts as single source of truth - Refactored baseStorage to use shared utilities - All 8 storage adapters now use same implementation 3. Comprehensive Testing - Added TestWrappingAdapter that actually wraps like production - 3 new regression tests validate the fix - Tests exercise real wrapping scenario that caused the bug Architecture Improvements: - βœ… Defense-in-Depth: Unwrap at BOTH adapter and blob layers - βœ… DRY Principle: Single source of truth in binaryDataCodec.ts - βœ… Works Across ALL Storage Adapters (8 total) - βœ… Prevents Future Regressions: Real wrapping tests Files Changed: - NEW: src/storage/cow/binaryDataCodec.ts (single source of truth) - FIXED: src/storage/cow/BlobStorage.ts (defense-in-depth unwrap) - REFACTORED: src/storage/baseStorage.ts (uses shared codec) - NEW: tests/helpers/TestWrappingAdapter.ts (real wrapping adapter) - ADDED: 3 regression tests in tests/unit/storage/cow/BlobStorage.test.ts - UPDATED: CHANGELOG.md, package.json (v5.10.1) Related Issues: - v5.7.2: Original bug - hashed wrapper instead of content - v5.7.5: First fix - added unwrap to adapter (necessary but insufficient) - v5.10.0: Regression - missing defense-in-depth in BlobStorage - v5.10.1: Complete fix - defense-in-depth + DRY + tests πŸ€– Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- CHANGELOG.md | 37 +++++++ package-lock.json | 4 +- package.json | 2 +- src/storage/baseStorage.ts | 25 ++--- src/storage/cow/BlobStorage.ts | 20 +++- src/storage/cow/binaryDataCodec.ts | 122 +++++++++++++++++++++ tests/helpers/TestWrappingAdapter.ts | 92 ++++++++++++++++ tests/unit/storage/cow/BlobStorage.test.ts | 99 +++++++++++++++++ 8 files changed, 375 insertions(+), 26 deletions(-) create mode 100644 src/storage/cow/binaryDataCodec.ts create mode 100644 tests/helpers/TestWrappingAdapter.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a8d47e9..9a5a5329 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,43 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +### [5.10.1](https://github.com/soulcraftlabs/brainy/compare/v5.10.0...v5.10.1) (2025-11-14) + +### 🚨 CRITICAL BUG FIX - Blob Integrity Regression + +**v5.10.0 regressed the v5.7.2 blob integrity bug, causing 100% VFS file read failure. This hotfix restores functionality with defense-in-depth architecture.** + +### Bug Description +v5.10.0 reintroduced a critical bug where `BlobStorage.read()` was hashing wrapped binary data instead of unwrapped content, causing all blob integrity checks to fail: +- **Symptom**: `Blob integrity check failed: ` errors on every VFS file read +- **Root Cause**: Missing defense-in-depth unwrap verification in `BlobStorage.read()` +- **Impact**: 100% failure rate for VFS file operations in Workshop application + +### The Fix (v5.10.1) +1. **Defense-in-Depth Unwrapping**: Added unwrap verification in `BlobStorage.read()` before hash check +2. **DRY Architecture**: Created `binaryDataCodec.ts` as single source of truth for wrap/unwrap logic +3. **Metadata Unwrapping**: Fixed metadata parsing to handle wrapped format +4. **Comprehensive Tests**: Added 3 regression tests using `TestWrappingAdapter` + +### Changes +- **NEW**: `src/storage/cow/binaryDataCodec.ts` - Single source of truth for binary data encoding/decoding +- **FIXED**: `src/storage/cow/BlobStorage.ts` - Unwraps data and metadata before verification (lines 314, 342) +- **REFACTORED**: `src/storage/baseStorage.ts` - Uses shared binaryDataCodec utilities (lines 332, 340) +- **ADDED**: `tests/helpers/TestWrappingAdapter.ts` - Real wrapping adapter for testing +- **ADDED**: 3 regression tests in `tests/unit/storage/cow/BlobStorage.test.ts` + +### Architecture Improvements +- βœ… **Defense-in-Depth**: Unwrap at BOTH adapter layer (v5.7.5) and blob layer (v5.10.1) +- βœ… **DRY Principle**: All wrap/unwrap operations use shared `binaryDataCodec.ts` +- βœ… **Works Across ALL 8 Storage Adapters**: FileSystem, Memory, S3, GCS, Azure, R2, OPFS, Historical +- βœ… **Prevents Future Regressions**: Real wrapping tests catch this bug class + +### Related Issues +- v5.7.2: Original blob integrity bug - hashed wrapper instead of content +- v5.7.5: First fix - added unwrap to COW adapter (necessary but insufficient) +- v5.10.0: Regression - missing defense-in-depth in BlobStorage layer +- v5.10.1: Complete fix - defense-in-depth + DRY architecture + comprehensive tests + ### [5.9.0](https://github.com/soulcraftlabs/brainy/compare/v5.8.0...v5.9.0) (2025-11-14) - fix: resolve VFS tree corruption from blob errors (v5.8.0) (93d2d70) diff --git a/package-lock.json b/package-lock.json index 4d5be826..d1682c93 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@soulcraft/brainy", - "version": "5.10.0", + "version": "5.10.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@soulcraft/brainy", - "version": "5.10.0", + "version": "5.10.1", "license": "MIT", "dependencies": { "@aws-sdk/client-s3": "^3.540.0", diff --git a/package.json b/package.json index b3e1e614..03c9d81d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@soulcraft/brainy", - "version": "5.10.0", + "version": "5.10.1", "description": "Universal Knowledge Protocolβ„’ - World's first Triple Intelligence database unifying vector, graph, and document search in one API. Stage 3 CANONICAL: 42 nouns Γ— 127 verbs covering 96-97% of all human knowledge.", "main": "dist/index.js", "module": "dist/index.js", diff --git a/src/storage/baseStorage.ts b/src/storage/baseStorage.ts index 6bedbccc..19446eca 100644 --- a/src/storage/baseStorage.ts +++ b/src/storage/baseStorage.ts @@ -28,6 +28,7 @@ import { getShardIdFromUuid } from './sharding.js' import { RefManager } from './cow/RefManager.js' import { BlobStorage, type COWStorageAdapter } from './cow/BlobStorage.js' import { CommitLog } from './cow/CommitLog.js' +import { unwrapBinaryData, wrapBinaryData } from './cow/binaryDataCodec.js' import { prodLog } from '../utils/logger.js' /** @@ -325,31 +326,19 @@ export abstract class BaseStorage extends BaseStorageAdapter { if (data === null) { return undefined } - // Convert to Buffer - if (Buffer.isBuffer(data)) { - return data - } - // v5.7.5: Unwrap binary data stored as {_binary: true, data: "base64..."} + // v5.7.5/v5.10.1: Use shared binaryDataCodec utility (single source of truth) + // Unwraps binary data stored as {_binary: true, data: "base64..."} // Fixes "Blob integrity check failed" - hash must be calculated on original content - if (data._binary && typeof data.data === 'string') { - return Buffer.from(data.data, 'base64') - } - return Buffer.from(JSON.stringify(data)) + return unwrapBinaryData(data) } catch (error) { return undefined } }, put: async (key: string, data: Buffer): Promise => { - // Store as Buffer (for blob data) or parse JSON (for metadata) - let obj: any - try { - // Try to parse as JSON first (for metadata) - obj = JSON.parse(data.toString()) - } catch { - // Not JSON, store as binary (base64 encoded for JSON storage) - obj = { _binary: true, data: data.toString('base64') } - } + // v5.10.1: Use shared binaryDataCodec utility (single source of truth) + // Wraps binary data or parses JSON for storage + const obj = wrapBinaryData(data) await this.writeObjectToPath(`_cow/${key}`, obj) }, diff --git a/src/storage/cow/BlobStorage.ts b/src/storage/cow/BlobStorage.ts index 21998e99..d08a253b 100644 --- a/src/storage/cow/BlobStorage.ts +++ b/src/storage/cow/BlobStorage.ts @@ -16,6 +16,7 @@ import { createHash } from 'crypto' import { NULL_HASH, isNullHash } from './constants.js' +import { unwrapBinaryData } from './binaryDataCodec.js' /** * Simple key-value storage interface for COW primitives @@ -308,7 +309,10 @@ export class BlobStorage { metadataBuffer = await this.adapter.get(`${tryPrefix}-meta:${hash}`) if (metadataBuffer) { prefix = tryPrefix - metadata = JSON.parse(metadataBuffer.toString()) + // v5.10.1: Unwrap metadata before parsing (defense-in-depth) + // Metadata should be JSON, but adapter might return wrapped format + const unwrappedMetadata = unwrapBinaryData(metadataBuffer) + metadata = JSON.parse(unwrappedMetadata.toString()) break } } @@ -334,17 +338,23 @@ export class BlobStorage { finalData = await this.zstdDecompress(data) } - // Verify hash (optional, expensive) - if (!options.skipVerification && BlobStorage.hash(finalData) !== hash) { + // v5.10.1: Defense-in-depth unwrap (CRITICAL FIX for blob integrity regression) + // Even though COW adapter should unwrap (v5.7.5), verify it happened and re-unwrap if needed + // This prevents "Blob integrity check failed" errors if adapter returns wrapped data + // Uses shared binaryDataCodec utility (single source of truth for unwrap logic) + const unwrappedData = unwrapBinaryData(finalData) + + // Verify hash (on unwrapped data) + if (!options.skipVerification && BlobStorage.hash(unwrappedData) !== hash) { throw new Error(`Blob integrity check failed: ${hash}`) } // Add to cache (only if not skipped) if (!options.skipCache) { - this.addToCache(hash, finalData, metadata) + this.addToCache(hash, unwrappedData, metadata) } - return finalData + return unwrappedData } /** diff --git a/src/storage/cow/binaryDataCodec.ts b/src/storage/cow/binaryDataCodec.ts new file mode 100644 index 00000000..8a9dba4d --- /dev/null +++ b/src/storage/cow/binaryDataCodec.ts @@ -0,0 +1,122 @@ +/** + * Binary Data Codec: Single Source of Truth for Wrap/Unwrap Operations + * + * This module provides the ONLY implementation of binary data encoding/decoding + * used across all storage adapters and blob storage. + * + * Design Principles: + * - DRY: One implementation, used everywhere + * - Single Responsibility: Only handles binary ↔ JSON conversion + * - Type-Safe: Proper TypeScript types + * - Defensive: Handles all edge cases + * + * Used by: + * - BaseStorage COW adapter (write/read operations) + * - BlobStorage (defense-in-depth verification) + * - All storage adapters (via BaseStorage) + * + * @module storage/cow/binaryDataCodec + */ + +/** + * Wrapped binary data format + * Used when storing binary data in JSON-based storage + */ +export interface WrappedBinaryData { + _binary: true + data: string // base64-encoded +} + +/** + * Check if data is wrapped binary format + */ +export function isWrappedBinary(data: any): data is WrappedBinaryData { + return ( + typeof data === 'object' && + data !== null && + data._binary === true && + typeof data.data === 'string' + ) +} + +/** + * Unwrap binary data from JSON wrapper + * + * This is the SINGLE SOURCE OF TRUTH for unwrapping binary data. + * All storage operations MUST use this function. + * + * Handles: + * - Buffer β†’ Buffer (pass-through) + * - {_binary: true, data: "base64..."} β†’ Buffer (unwrap) + * - Plain object β†’ Buffer (JSON stringify) + * - Other types β†’ Error + * + * @param data - Data to unwrap (may be Buffer, wrapped object, or plain object) + * @returns Unwrapped Buffer + * @throws Error if data type is invalid + */ +export function unwrapBinaryData(data: any): Buffer { + // Case 1: Already a Buffer (no unwrapping needed) + if (Buffer.isBuffer(data)) { + return data + } + + // Case 2: Wrapped binary data {_binary: true, data: "base64..."} + if (isWrappedBinary(data)) { + return Buffer.from(data.data, 'base64') + } + + // Case 3: Plain object (shouldn't happen for binary blobs, but handle gracefully) + if (typeof data === 'object' && data !== null) { + return Buffer.from(JSON.stringify(data)) + } + + // Case 4: String (convert to Buffer) + if (typeof data === 'string') { + return Buffer.from(data) + } + + // Case 5: Invalid type + throw new Error( + `Invalid data type for unwrap: ${typeof data}. ` + + `Expected Buffer or {_binary: true, data: "base64..."}` + ) +} + +/** + * Wrap binary data for JSON storage + * + * This is the SINGLE SOURCE OF TRUTH for wrapping binary data. + * All storage operations MUST use this function. + * + * @param data - Buffer to wrap + * @returns Wrapped object or parsed JSON object + */ +export function wrapBinaryData(data: Buffer): any { + // Try to parse as JSON first (for metadata, trees, commits) + try { + return JSON.parse(data.toString()) + } catch { + // Not JSON - wrap as binary data + return { + _binary: true, + data: data.toString('base64') + } as WrappedBinaryData + } +} + +/** + * Ensure data is a Buffer + * + * Convenience function that combines type checking and unwrapping. + * Use this when you need to ensure you have a Buffer. + * + * @param data - Data that should be or can be converted to Buffer + * @returns Buffer + */ +export function ensureBuffer(data: any): Buffer { + if (Buffer.isBuffer(data)) { + return data + } + return unwrapBinaryData(data) +} diff --git a/tests/helpers/TestWrappingAdapter.ts b/tests/helpers/TestWrappingAdapter.ts new file mode 100644 index 00000000..4a2f8c1b --- /dev/null +++ b/tests/helpers/TestWrappingAdapter.ts @@ -0,0 +1,92 @@ +/** + * TestWrappingAdapter: Real COW storage adapter for testing + * + * This adapter ACTUALLY wraps binary data in JSON (like production storage) + * to properly test the unwrap logic in BlobStorage. + * + * Unlike InMemoryCOWAdapter which stores Buffers directly, + * this adapter simulates real storage behavior: + * 1. Compresses with gzip + * 2. Wraps binary as {_binary: true, data: "base64..."} + * 3. Parses JSON on read (returns JS object, not Buffer) + * + * This ensures tests exercise the ACTUAL code path that caused v5.10.0 regression. + */ + +import { COWStorageAdapter } from '../../src/storage/cow/BlobStorage.js' +import { gzip, gunzip } from 'zlib' +import { promisify } from 'util' + +const gzipAsync = promisify(gzip) +const gunzipAsync = promisify(gunzip) + +export class TestWrappingAdapter implements COWStorageAdapter { + private storage = new Map() + + async get(key: string): Promise { + const compressed = this.storage.get(key) + if (!compressed) { + return undefined + } + + // Decompress (like real storage) + const decompressed = await gunzipAsync(compressed) + + // Parse JSON (like real storage) + // This returns a JS object: {_binary: true, data: "base64..."} + // NOT a Buffer! + return JSON.parse(decompressed.toString()) + } + + async put(key: string, data: Buffer): Promise { + // Determine if data is JSON or binary + let obj: any + try { + // Try to parse as JSON (for metadata) + obj = JSON.parse(data.toString()) + } catch { + // Not JSON - wrap as binary (like production) + obj = { + _binary: true, + data: data.toString('base64') + } + } + + // Stringify to JSON + const jsonStr = JSON.stringify(obj) + + // Compress (like real storage) + const compressed = await gzipAsync(Buffer.from(jsonStr)) + + // Store compressed data + this.storage.set(key, compressed) + } + + async delete(key: string): Promise { + this.storage.delete(key) + } + + async list(prefix: string): Promise { + const keys: string[] = [] + for (const key of this.storage.keys()) { + if (key.startsWith(prefix)) { + keys.push(key) + } + } + return keys + } + + /** + * Clear all storage (for test cleanup) + */ + clear(): void { + this.storage.clear() + } + + /** + * Get storage size (for testing) + */ + size(): number { + return this.storage.size + } +} diff --git a/tests/unit/storage/cow/BlobStorage.test.ts b/tests/unit/storage/cow/BlobStorage.test.ts index cd9d6791..51476b3f 100644 --- a/tests/unit/storage/cow/BlobStorage.test.ts +++ b/tests/unit/storage/cow/BlobStorage.test.ts @@ -509,4 +509,103 @@ describe('BlobStorage', () => { expect(elapsed).toBeLessThan(1000) // Should be very fast from cache }) }) + + describe('v5.10.0 Regression - Blob Integrity with Wrapped Data', () => { + it('should handle wrapped binary data without hash mismatch (v5.10.0 fix)', async () => { + // Import TestWrappingAdapter that actually wraps data like production + const { TestWrappingAdapter } = await import('../../../helpers/TestWrappingAdapter.js') + const wrappingAdapter = new TestWrappingAdapter() + const testBlobStorage = new BlobStorage(wrappingAdapter) + + const originalData = Buffer.from('test content for v5.10.0 regression test') + + // Write blob (TestWrappingAdapter wraps as {_binary: true, data: "base64..."}) + const hash = await testBlobStorage.write(originalData) + + // Clear cache to force re-read from storage (bypasses in-memory cache) + testBlobStorage.clearCache() + + // v5.10.0 bug: This would fail with "Blob integrity check failed" + // because BlobStorage.read() was hashing the wrapped data instead of unwrapped + // v5.10.1 fix: unwrapBinaryData() is called before hash verification + const retrieved = await testBlobStorage.read(hash) + + expect(retrieved.equals(originalData)).toBe(true) + expect(retrieved.toString()).toBe('test content for v5.10.0 regression test') + }) + + it('should handle multiple wrapped blobs without integrity errors', async () => { + const { TestWrappingAdapter } = await import('../../../helpers/TestWrappingAdapter.js') + const wrappingAdapter = new TestWrappingAdapter() + const testBlobStorage = new BlobStorage(wrappingAdapter) + + const testData = [ + Buffer.from('first blob'), + Buffer.from('second blob'), + Buffer.from('third blob with more content'), + Buffer.from(JSON.stringify({ test: 'json data' })), + ] + + const hashes: string[] = [] + + // Write all blobs + for (const data of testData) { + const hash = await testBlobStorage.write(data) + hashes.push(hash) + } + + // Clear cache + testBlobStorage.clearCache() + + // Read all blobs (would fail in v5.10.0) + for (let i = 0; i < hashes.length; i++) { + const retrieved = await testBlobStorage.read(hashes[i]) + expect(retrieved.equals(testData[i])).toBe(true) + } + }) + + it('should work even if adapter fails to unwrap (defense-in-depth)', async () => { + // Create a buggy adapter that doesn't unwrap properly + class BuggyAdapter implements COWStorageAdapter { + private storage = new Map() + + async get(key: string): Promise { + // Simulate a bug where adapter returns wrapped data instead of Buffer + const data = this.storage.get(key) + if (!data) return undefined + + // Return wrapped object (bug) instead of unwrapped Buffer + return data // {_binary: true, data: "base64..."} + } + + async put(key: string, data: Buffer): Promise { + // Store as wrapped object (like real storage) + this.storage.set(key, { + _binary: true, + data: data.toString('base64') + }) + } + + async delete(key: string): Promise { + this.storage.delete(key) + } + + async list(prefix: string): Promise { + return Array.from(this.storage.keys()).filter(k => k.startsWith(prefix)) + } + } + + const buggyAdapter = new BuggyAdapter() + const testBlobStorage = new BlobStorage(buggyAdapter) + + const originalData = Buffer.from('test defense-in-depth') + const hash = await testBlobStorage.write(originalData) + + testBlobStorage.clearCache() + + // Even with buggy adapter, BlobStorage.read() should unwrap and verify correctly + const retrieved = await testBlobStorage.read(hash) + expect(retrieved.equals(originalData)).toBe(true) + }) + }) })