mirror of
https://github.com/affaan-m/ECC.git
synced 2026-08-17 21:15:40 +02:00
fix(memory-vault): compare dev only when both stats report one (#2637)
ecc memory writes and --body-file reads fail on Windows. sameFileIdentity() compares the dev field of a path-based stat against a handle-based fstat, and 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() reports it. The comparison never matches, so the TOCTOU guard rejects every operation. Keep the inode strict and compare dev only when both sides report one. POSIX always reports a non-zero dev, so the original strict behaviour is preserved there. Request the guard's stats as BigInt. On the affected libuv versions dev is 0, which leaves the inode as the only identity signal, and Windows file IDs run past Number.MAX_SAFE_INTEGER where two distinct files can collapse to the same number-valued inode. Fixes #2626
This commit is contained in:
@@ -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`.
|
- 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
|
## 2.0.0 - 2026-06-09
|
||||||
|
|
||||||
### Added
|
### Added
|
||||||
|
|||||||
@@ -122,7 +122,21 @@ function assertMemoryDirectorySafe(directory, root) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function sameFileIdentity(left, right) {
|
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 = {}) {
|
function readRegularTextFile(filePath, options = {}) {
|
||||||
@@ -137,11 +151,11 @@ function readRegularTextFile(filePath, options = {}) {
|
|||||||
| (fs.constants.O_NONBLOCK || 0);
|
| (fs.constants.O_NONBLOCK || 0);
|
||||||
const descriptor = fs.openSync(filePath, flags);
|
const descriptor = fs.openSync(filePath, flags);
|
||||||
try {
|
try {
|
||||||
const opened = fs.fstatSync(descriptor);
|
const opened = fs.fstatSync(descriptor, { bigint: true });
|
||||||
if (!opened.isFile()) {
|
if (!opened.isFile()) {
|
||||||
throw new Error(`${label} must be a regular, non-symlink file.`);
|
throw new Error(`${label} must be a regular, non-symlink file.`);
|
||||||
}
|
}
|
||||||
const after = fs.lstatSync(filePath);
|
const after = fs.lstatSync(filePath, { bigint: true });
|
||||||
if (
|
if (
|
||||||
after.isSymbolicLink()
|
after.isSymbolicLink()
|
||||||
|| !after.isFile()
|
|| !after.isFile()
|
||||||
@@ -152,7 +166,7 @@ function readRegularTextFile(filePath, options = {}) {
|
|||||||
if (options.trustedRoot) {
|
if (options.trustedRoot) {
|
||||||
assertWithinTrustedRoot(filePath, options.trustedRoot, `read ${label}`);
|
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).`);
|
throw new Error(`${label} is too large (${opened.size} bytes).`);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -189,8 +203,8 @@ function writeCreateOnlyTextFile(filePath, content, trustedRoot) {
|
|||||||
let cleanupError;
|
let cleanupError;
|
||||||
try {
|
try {
|
||||||
descriptor = fs.openSync(temporaryPath, flags, 0o600);
|
descriptor = fs.openSync(temporaryPath, flags, 0o600);
|
||||||
const opened = fs.fstatSync(descriptor);
|
const opened = fs.fstatSync(descriptor, { bigint: true });
|
||||||
const after = fs.lstatSync(temporaryPath);
|
const after = fs.lstatSync(temporaryPath, { bigint: true });
|
||||||
assertWithinTrustedRoot(temporaryPath, trustedRoot, 'write memory');
|
assertWithinTrustedRoot(temporaryPath, trustedRoot, 'write memory');
|
||||||
if (
|
if (
|
||||||
!opened.isFile()
|
!opened.isFile()
|
||||||
@@ -770,6 +784,7 @@ module.exports = {
|
|||||||
readMemoryById,
|
readMemoryById,
|
||||||
readMemoryFiles,
|
readMemoryFiles,
|
||||||
resolveVaultRoots,
|
resolveVaultRoots,
|
||||||
|
sameFileIdentity,
|
||||||
saveMemory,
|
saveMemory,
|
||||||
scoreMemory,
|
scoreMemory,
|
||||||
searchMemories,
|
searchMemories,
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ const {
|
|||||||
readMemoryById,
|
readMemoryById,
|
||||||
readRegularTextFile,
|
readRegularTextFile,
|
||||||
resolveVaultRoots,
|
resolveVaultRoots,
|
||||||
|
sameFileIdentity,
|
||||||
saveMemory,
|
saveMemory,
|
||||||
searchMemories,
|
searchMemories,
|
||||||
serializeMemoryDocument,
|
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', () => {
|
test('opens regular text files without following a stable symlink', () => {
|
||||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-memory-file-'));
|
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-memory-file-'));
|
||||||
const target = path.join(root, 'target.md');
|
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', () => {
|
test('rejects a FIFO body path without blocking', () => {
|
||||||
if (process.platform === 'win32') return;
|
if (process.platform === 'win32') return;
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user