diff --git a/CHANGELOG.md b/CHANGELOG.md index cd1893c27..4d04ae1e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - Default MCP connector set reduced to a single connector (`chrome-devtools`) per the new connector policy (`docs/MCP-CONNECTOR-POLICY.md`). The six previous defaults (`github`, `context7`, `exa`, `memory`, `playwright`, `sequential-thinking`) were retired after the June 2026 audit: their jobs are covered by skills wrapping CLIs/REST APIs (`github-ops`, `documentation-lookup`, `exa-search`, e2e skills) or by harness-native features (memory, extended thinking, web search). All six remain opt-in via `mcp-configs/mcp-servers.json`. +### Fixed + +- `ecc memory` writes and `--body-file` reads failed on Windows under Node 22.12-22.16 and 24.0-24.1. libuv resolved path-based `stat()`/`lstat()` through `GetFileInformationByName` without setting the volume serial, while `fstat()` reported it, so the memory vault's TOCTOU guard rejected every operation. Fixed upstream in libuv 1.51.0; the guard no longer depends on the runtime's patch level. The guard's stat calls now request `BigInt` values, so Windows file IDs past `Number.MAX_SAFE_INTEGER` can no longer collapse two distinct files into one identity. + ## 2.0.0 - 2026-06-09 ### Added diff --git a/scripts/lib/memory-vault.js b/scripts/lib/memory-vault.js index 591737017..cdf0f4089 100644 --- a/scripts/lib/memory-vault.js +++ b/scripts/lib/memory-vault.js @@ -122,7 +122,21 @@ function assertMemoryDirectorySafe(directory, root) { } function sameFileIdentity(left, right) { - return left.dev === right.dev && left.ino === right.ino; + // The inode is the primary identity signal and must always match. + if (left.ino !== right.ino) { + return false; + } + // libuv 1.49.0 through 1.50.x resolve path-based stat() and lstat() on Windows + // through GetFileInformationByName, which leaves the volume serial unset, while + // fstat() on an open handle reports it. Comparing the two then never matches and + // every vault read and write is rejected. libuv 82cdfb75f fixed this in 1.51.0, + // so only Node 22.12-22.16 and 24.0-24.1 are affected, but the guard should not + // depend on the runtime's patch level. Compare dev only when both sides report + // one; POSIX always does, so the original strict behaviour is preserved there. + if (!left.dev || !right.dev) { + return true; + } + return left.dev === right.dev; } function readRegularTextFile(filePath, options = {}) { @@ -137,11 +151,11 @@ function readRegularTextFile(filePath, options = {}) { | (fs.constants.O_NONBLOCK || 0); const descriptor = fs.openSync(filePath, flags); try { - const opened = fs.fstatSync(descriptor); + const opened = fs.fstatSync(descriptor, { bigint: true }); if (!opened.isFile()) { throw new Error(`${label} must be a regular, non-symlink file.`); } - const after = fs.lstatSync(filePath); + const after = fs.lstatSync(filePath, { bigint: true }); if ( after.isSymbolicLink() || !after.isFile() @@ -152,7 +166,7 @@ function readRegularTextFile(filePath, options = {}) { if (options.trustedRoot) { assertWithinTrustedRoot(filePath, options.trustedRoot, `read ${label}`); } - if (opened.size > maxBytes) { + if (opened.size > BigInt(maxBytes)) { throw new Error(`${label} is too large (${opened.size} bytes).`); } @@ -189,8 +203,8 @@ function writeCreateOnlyTextFile(filePath, content, trustedRoot) { let cleanupError; try { descriptor = fs.openSync(temporaryPath, flags, 0o600); - const opened = fs.fstatSync(descriptor); - const after = fs.lstatSync(temporaryPath); + const opened = fs.fstatSync(descriptor, { bigint: true }); + const after = fs.lstatSync(temporaryPath, { bigint: true }); assertWithinTrustedRoot(temporaryPath, trustedRoot, 'write memory'); if ( !opened.isFile() @@ -770,6 +784,7 @@ module.exports = { readMemoryById, readMemoryFiles, resolveVaultRoots, + sameFileIdentity, saveMemory, scoreMemory, searchMemories, diff --git a/tests/lib/memory-vault.test.js b/tests/lib/memory-vault.test.js index 1cd08a6bc..f5343c0b6 100644 --- a/tests/lib/memory-vault.test.js +++ b/tests/lib/memory-vault.test.js @@ -19,6 +19,7 @@ const { readMemoryById, readRegularTextFile, resolveVaultRoots, + sameFileIdentity, saveMemory, searchMemories, serializeMemoryDocument, @@ -516,6 +517,47 @@ test('quarantines imported secrets and metadata that disagrees with its vault lo } }); +// Windows reports dev = 0 from path-based stat()/lstat() while fstat() on an open +// handle reports the real volume serial number, so a strict dev comparison can never +// match and every vault read/write is rejected. The stat pairs below are the values +// measured on Node v22.15.0 / Windows 11 10.0.26200 reported in issue #2626. +test('matches a Windows path-vs-handle stat pair where only dev differs', () => { + const openedByHandle = { dev: 1644385068, ino: 21110623254304612 }; + const openedByPath = { dev: 0, ino: 21110623254304612 }; + assert.strictEqual(sameFileIdentity(openedByPath, openedByHandle), true); +}); + +test('matches a Windows stat pair on a non-system volume', () => { + const openedByHandle = { dev: 3054669153, ino: 562949953451607 }; + const openedByPath = { dev: 0, ino: 562949953451607 }; + assert.strictEqual(sameFileIdentity(openedByPath, openedByHandle), true); +}); + +test('separates files that share an inode across two reported devices', () => { + const left = { dev: 16777232, ino: 42 }; + const right = { dev: 16777233, ino: 42 }; + assert.strictEqual(sameFileIdentity(left, right), false); +}); + +test('separates distinct inodes reported from the same device', () => { + const left = { dev: 16777232, ino: 42 }; + const right = { dev: 16777232, ino: 43 }; + assert.strictEqual(sameFileIdentity(left, right), false); +}); + +// Runs on every platform, but only the windows-latest CI leg exercises the +// path-vs-handle dev divergence that issue #2626 reports. +test('reads a regular file whose handle and path stats are compared', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-memory-identity-')); + const target = path.join(root, 'target.md'); + try { + fs.writeFileSync(target, 'durable'); + assert.strictEqual(readRegularTextFile(target, { maxBytes: 16 }), 'durable'); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + test('opens regular text files without following a stable symlink', () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-memory-file-')); const target = path.join(root, 'target.md'); @@ -577,6 +619,45 @@ test('opens a file descriptor before inspecting path metadata', () => { } }); +// Windows file IDs run past Number.MAX_SAFE_INTEGER, so two distinct files can +// collapse to the same value in a number-valued Stats. On the libuv versions that +// report dev = 0 the inode is the only identity signal left, so the stats have to +// be requested as BigInt for the guard to hold. The stubs below mimic fs: BigInt +// when { bigint: true } is requested, lossy numbers otherwise. +test('detects a swapped file whose inode differs beyond Number precision', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-memory-bigint-ino-')); + const target = path.join(root, 'target.md'); + const originalFstatSync = fs.fstatSync; + const originalLstatSync = fs.lstatSync; + + const stat = (base, fileId, options) => Object.assign( + Object.create(Object.getPrototypeOf(base)), + base, + { + dev: options && options.bigint ? 0n : 0, + ino: options && options.bigint ? fileId : Number(fileId), + size: options && options.bigint ? BigInt(base.size) : base.size, + } + ); + + try { + fs.writeFileSync(target, 'safe'); + fs.fstatSync = (descriptor, options) => + stat(originalFstatSync(descriptor), 21110623254304612n, options); + fs.lstatSync = (filePath, options) => + stat(originalLstatSync(filePath), 21110623254304613n, options); + + assert.throws( + () => readRegularTextFile(target, { maxBytes: 16 }), + /must remain a regular, non-symlink file/ + ); + } finally { + fs.fstatSync = originalFstatSync; + fs.lstatSync = originalLstatSync; + fs.rmSync(root, { recursive: true, force: true }); + } +}); + test('rejects a FIFO body path without blocking', () => { if (process.platform === 'win32') return;