From b60732eb3dab2cc3482acd7881c880b66864d31b Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Renard Date: Thu, 17 Sep 2026 14:33:58 +0200 Subject: [PATCH 1/3] fix(memory): the project-root files go through the same gate as every other listing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `get-memories` looked up `CLAUDE.md`, `GEMINI.md` and `agents.md` at each known project root with `existsSync` + a bare `readFileSync`, and pushed the content into the FTS index. The sibling listings pass `isSensitivePath` and `isAllowedMemoryPath`; this one passed nothing. A project whose `CLAUDE.md` is a symlink to a key — a cloned repository added as a project brings its own — put that key in the searchable index: ANCIEN existsSync + readFileSync -> "PRIVATE-KEY-FAKE-FOR-TEST" NOUVEAU acceptMdFile -> null The duplication is what let the two drift, so the fix removes it rather than adding a second copy of the checks. `acceptMdFile` is the single rule for accepting a listable Markdown file — resolve it, refuse a non-regular file, apply the denylist, apply the caller's allowlist, skip it when empty — and `scanMdFiles` becomes its directory-wide form. The root files are looked up by name, as before, and then go through it. Two things follow from sharing the rule. `existsSync` disappears: `statSync` already fails the same way for an absent path, and looking the file up twice was a race with nothing to gain. And the root files inherit the `isFile()` check, so a project holding a FIFO named `CLAUDE.md` no longer freezes the Electron main process, which runs this synchronously inside an `ipcMain.handle`. Closes #295. --- .ai/contexts/ipc-bridge.md | 2 +- main.js | 17 +++----- scan-md-files.js | 45 ++++++++++++++-------- test/scan-md-files.test.js | 79 +++++++++++++++++++++++++++++++++++++- 4 files changed, 113 insertions(+), 30 deletions(-) diff --git a/.ai/contexts/ipc-bridge.md b/.ai/contexts/ipc-bridge.md index 0e0a69cf..fc25f504 100644 --- a/.ai/contexts/ipc-bridge.md +++ b/.ai/contexts/ipc-bridge.md @@ -147,7 +147,7 @@ Every handler that takes a renderer-supplied path or derives a spawn location fr | IPC | Guard | Kind | |---|---|---| | `read-memory` / `save-memory` | `resolveAllowedMemoryPath` (read/write the returned path, not the caller's own re-resolved one) | disk-resolved allowlist | -| `get-memories` (the list, and the FTS bodies it indexes) | `scanMdFiles` (`scan-md-files.js`) applies `isSensitivePath` itself and the `isAllowedMemoryPath` predicate `get-memories` passes in | disk-resolved denylist + allowlist — a listed file is read twice more downstream, once by `read-memory` behind `resolveAllowedMemoryPath` and once by the FTS indexer behind nothing at all, so the guard belongs on the list. The denylist is not the caller's to choose: containment in an allowed project root says nothing about a cloned repo's own `.ssh`/`.env` | +| `get-memories` (the list, and the FTS bodies it indexes) | `acceptMdFile` (`scan-md-files.js`) applies `isSensitivePath` itself and the `isAllowedMemoryPath` predicate `get-memories` passes in; `scanMdFiles` is the directory-wide form of it, and the project-root `CLAUDE.md`/`GEMINI.md`/`agents.md` go through it by name | disk-resolved denylist + allowlist — a listed file is read twice more downstream, once by `read-memory` behind `resolveAllowedMemoryPath` and once by the FTS indexer behind nothing at all, so the guard belongs on the list. The denylist is not the caller's to choose: containment in an allowed project root says nothing about a cloned repo's own `.ssh`/`.env` | | `open-path` / `read-file-for-panel` / `save-file-for-panel` / `watch-file` | `isSensitivePath` | disk-resolved denylist | | `delete-worktree` / `worktree-status` | `WORKTREE_PATH_RE` (shape) + `isKnownProjectRoot` (disk-resolved exact match) | shape + disk-resolved allowlist | | `delete-session-preview` / `delete-session` | `resolveDeletionTargets` (`delete-session-target.js`) | disk-resolved containment | diff --git a/main.js b/main.js index d327f909..b7ce6ebd 100644 --- a/main.js +++ b/main.js @@ -73,7 +73,7 @@ function spawnPty(file, args, opts) { const { discoverShellProfiles, getShellProfiles, resolveShell, isWindows, isWslShell, windowsToWslPath, shellArgs, quoteArgvForShell } = require('./shell-profiles'); const { startScheduler } = require('./schedule-runner'); const { encodeProjectPath } = require('./encode-project-path'); -const { scanMdFiles } = require('./scan-md-files'); +const { scanMdFiles, acceptMdFile } = require('./scan-md-files'); const { isSensitivePath, isAllowedMemoryPath: _isAllowedMemoryPath, resolveAllowedMemoryPath: _resolveAllowedMemoryPath, isKnownProjectRoot: _isKnownProjectRoot } = require('./ipc-path-validator'); const { validatePreLaunchCmd } = require('./pre-launch-cmd-guard'); const { normalizePtySize } = require('./pty-size'); @@ -1273,16 +1273,11 @@ ipcMain.handle('get-memories', () => { if (projectPath) { for (const name of ['CLAUDE.md', 'GEMINI.md', 'agents.md']) { const fp = path.join(projectPath, name); - try { - if (fs.existsSync(fp)) { - const content = fs.readFileSync(fp, 'utf8').trim(); - if (content && !seenPaths.has(fp)) { - const stat = fs.statSync(fp); - files.push({ filename: name, filePath: fp, modified: stat.mtime.toISOString(), displayPath: shortName + '/', source: 'project' }); - seenPaths.add(fp); - } - } - } catch {} + const accepted = acceptMdFile(fp, isAllowedMemoryPath); + if (accepted && !seenPaths.has(fp)) { + files.push({ ...accepted, displayPath: shortName + '/', source: 'project' }); + seenPaths.add(fp); + } } // 3. {projectPath}/.claude/ — commands/*.md and other .md files diff --git a/scan-md-files.js b/scan-md-files.js index 7f78c524..629c3fd7 100644 --- a/scan-md-files.js +++ b/scan-md-files.js @@ -1,5 +1,6 @@ -// scan-md-files.js — non-recursive scan of a directory for the Markdown files -// the Memory/brain tab lists. Electron-free so it can be tested directly. +// scan-md-files.js — the Markdown files the Memory/brain tab lists: one +// acceptance rule, and a non-recursive scan built on it. Electron-free so it +// can be tested directly. // See .ai/contexts/ipc-bridge.md, "IPC path-guard inventory". 'use strict'; @@ -8,7 +9,27 @@ const path = require('path'); const { isSensitivePath } = require('./ipc-path-validator'); /** - * Scan `dir` for non-empty `.md` files (non-recursive). + * Accept `filePath` as a listable Markdown file, or return null. + * + * @param {string} filePath + * @param {(filePath: string) => boolean} [isAllowed] - caller's allowlist, given the literal path + * @returns {{filename: string, filePath: string, modified: string}|null} + */ +function acceptMdFile(filePath, isAllowed) { + try { + const stat = fs.statSync(filePath); + if (!stat.isFile()) return null; + if (isSensitivePath(filePath)) return null; + if (isAllowed && !isAllowed(filePath)) return null; + if (!fs.readFileSync(filePath, 'utf8').trim()) return null; + return { filename: path.basename(filePath), filePath, modified: stat.mtime.toISOString() }; + } catch { + return null; // unreadable, dangling or absent + } +} + +/** + * Scan `dir` for listable `.md` files (non-recursive). * * @param {string} dir * @param {(filePath: string) => boolean} [isAllowed] - caller's allowlist, given the literal path @@ -18,24 +39,14 @@ function scanMdFiles(dir, isAllowed) { const results = []; try { if (!fs.existsSync(dir)) return results; - const entries = fs.readdirSync(dir, { withFileTypes: true }); - for (const e of entries) { + for (const e of fs.readdirSync(dir, { withFileTypes: true })) { if (!e.name.endsWith('.md')) continue; if (!e.isFile() && !e.isSymbolicLink()) continue; - const fp = path.join(dir, e.name); - try { - const stat = fs.statSync(fp); - if (!stat.isFile()) continue; - if (isSensitivePath(fp)) continue; - if (isAllowed && !isAllowed(fp)) continue; - const content = fs.readFileSync(fp, 'utf8').trim(); - if (content) { - results.push({ filename: e.name, filePath: fp, modified: stat.mtime.toISOString() }); - } - } catch { /* unreadable or dangling — skip this entry, keep the rest */ } + const accepted = acceptMdFile(path.join(dir, e.name), isAllowed); + if (accepted) results.push(accepted); } } catch { /* unreadable directory */ } return results; } -module.exports = { scanMdFiles }; +module.exports = { scanMdFiles, acceptMdFile }; diff --git a/test/scan-md-files.test.js b/test/scan-md-files.test.js index 75f939b3..19baa6d1 100644 --- a/test/scan-md-files.test.js +++ b/test/scan-md-files.test.js @@ -16,7 +16,7 @@ const os = require('os'); const path = require('path'); const { spawnSync } = require('child_process'); -const { scanMdFiles } = require('../scan-md-files'); +const { scanMdFiles, acceptMdFile } = require('../scan-md-files'); function rig() { const root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'sb-scan-md-'))); @@ -174,3 +174,80 @@ test('scanMdFiles: a directory that does not exist scans to an empty list, not a assert.deepEqual(scanMdFiles(path.join(r.root, 'nope')), []); } finally { r.cleanup(); } }); + +// ── acceptMdFile ───────────────────────────────────────────────────────────── +// The project-root CLAUDE.md / GEMINI.md / agents.md are looked up by name +// rather than scanned, so they reach the list through this function directly. +// It is the single acceptance rule behind both paths — the memory listing had +// two of them, and only one carried the guards. + +test('acceptMdFile: accepts an ordinary file and reports its own name and mtime', () => { + const r = rig(); + try { + const fp = path.join(r.dir, 'CLAUDE.md'); + fs.writeFileSync(fp, 'project instructions'); + const out = acceptMdFile(fp); + assert.equal(out.filename, 'CLAUDE.md'); + assert.equal(out.filePath, fp); + assert.equal(out.modified, fs.statSync(fp).mtime.toISOString()); + } finally { r.cleanup(); } +}); + +test('acceptMdFile: refuses a root CLAUDE.md that is a link to a credential location', (t) => { + const r = rig(); + try { + // A cloned repository added as a project brings its own CLAUDE.md. Reading + // one that points at a key puts the key in the searchable FTS index. + const sshDir = path.join(r.elsewhere, '.ssh'); + fs.mkdirSync(sshDir, { recursive: true }); + const key = path.join(sshDir, 'id_rsa'); + fs.writeFileSync(key, '-----BEGIN OPENSSH PRIVATE KEY-----'); + const fp = path.join(r.dir, 'CLAUDE.md'); + if (!symlink(key, fp, t)) return; + + assert.equal(acceptMdFile(fp, () => true), null); + } finally { r.cleanup(); } +}); + +test('acceptMdFile: refuses a file the allowlist does not cover', (t) => { + const r = rig(); + try { + const outside = path.join(r.elsewhere, 'notes.md'); + fs.writeFileSync(outside, 'content kept elsewhere'); + const fp = path.join(r.dir, 'CLAUDE.md'); + if (!symlink(outside, fp, t)) return; + + const allowed = (p) => { + const real = fs.realpathSync(p); + return real === r.dir || real.startsWith(r.dir + path.sep); + }; + assert.equal(acceptMdFile(fp, allowed), null); + assert.ok(acceptMdFile(fp), 'without an allowlist the link itself is fine'); + } finally { r.cleanup(); } +}); + +test('acceptMdFile: refuses a FIFO instead of blocking forever on it', (t) => { + const r = rig(); + try { + // Same hazard as the scan, reached by name instead: a project root holding + // a FIFO called CLAUDE.md would freeze the Electron main process, which + // runs this synchronously. See the scan's FIFO test for why a regression + // here hangs the suite rather than reddening it. + const fifo = path.join(r.dir, 'CLAUDE.md'); + const mk = spawnSync('mkfifo', [fifo]); + if (mk.error || mk.status !== 0) return t.skip('mkfifo unavailable on this machine'); + + assert.equal(acceptMdFile(fifo), null); + } finally { r.cleanup(); } +}); + +test('acceptMdFile: refuses an absent path, a directory and an empty file', () => { + const r = rig(); + try { + assert.equal(acceptMdFile(path.join(r.dir, 'nothing-here.md')), null); + assert.equal(acceptMdFile(r.dir), null); + const empty = path.join(r.dir, 'empty.md'); + fs.writeFileSync(empty, ' \n'); + assert.equal(acceptMdFile(empty), null); + } finally { r.cleanup(); } +}); From bde91698ab4e7bf9f9a3c7f6aeeb397eabdde63c Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Renard Date: Thu, 17 Sep 2026 15:01:00 +0200 Subject: [PATCH 2/3] fix(memory): index the body that passed the checks, not the path they ran on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `acceptMdFile` validated a path and returned it; the FTS block then read that path again. Two independent resolutions of the same string, with the rest of the handler between them — every other project enumerated, sorted, signature computed. `resolve-path-on-disk.js` states the contract the other readers honour: perform the operation on what the guard returned, never on a path re-derived afterwards. What fits in the window, both reproduced: acceptMdFile accepts /CLAUDE.md (a regular file) rename a symlink -> /.ssh/id_rsa over it the FTS index stores "-----BEGIN OPENSSH PRIVATE KEY-----" and the same swap with a FIFO instead of a key never returns at all — the read is synchronous inside an ipcMain.handle, so the Electron main process is gone until it is killed. The checks this PR adds were only ever as good as the instant they ran in. `acceptMdFile` already read the body and threw it away, so it now hands it back. `get-memories` keeps those bodies in a Map and the index takes them from there. The file is read once instead of twice, and the bodies stay out of the entries returned to the renderer, which has no use for them. Also: the allowlist was rebuilt from disk on every call — a readdir of every project folder plus a 256 KiB read off a JSONL each — and it is called once per file. It is now bound once per handler call. Tests: `test/get-memories-wiring.test.js` reads the handler source, the way the FTS dirty-flag tests already do, because nothing in `scan-md-files.test.js` can see how the handler wires it. Re-inlining the root-file lookup, re-reading the file for the index, dropping `listed`, or aliasing the per-call wrapper back in each fail it. --- main.js | 38 ++++++++++----- scan-md-files.js | 12 +++-- test/get-memories-wiring.test.js | 82 ++++++++++++++++++++++++++++++++ test/scan-md-files.test.js | 29 ++++++++++- 4 files changed, 144 insertions(+), 17 deletions(-) create mode 100644 test/get-memories-wiring.test.js diff --git a/main.js b/main.js index b7ce6ebd..987e361a 100644 --- a/main.js +++ b/main.js @@ -1230,8 +1230,22 @@ ipcMain.handle('get-memories', () => { const global = getSetting('global') || {}; const hiddenProjects = new Set(global.hiddenProjects || []); + // The known-project set is rebuilt from disk on every isAllowedMemoryPath + // call, so binding it once per handler call keeps this off a per-file path. + const knownRoots = [...getKnownProjectPaths()]; + const isAllowed = (filePath) => _isAllowedMemoryPath(filePath, knownRoots); + + // Bodies of the files accepted below, kept for the FTS index at the end of + // this handler. They travel here rather than on the entries themselves: the + // entries go to the renderer, the bodies have no business going with them. + const bodies = new Map(); + const listed = ({ content, ...entry }) => { + bodies.set(entry.filePath, content); + return entry; + }; + // --- Global files --- - const globalFiles = scanMdFiles(CLAUDE_DIR, isAllowedMemoryPath).map(f => ({ ...f, displayPath: '~/.claude' })); + const globalFiles = scanMdFiles(CLAUDE_DIR, isAllowed).map(f => ({ ...listed(f), displayPath: '~/.claude' })); // --- Per-project files --- const projects = []; @@ -1256,16 +1270,16 @@ ipcMain.handle('get-memories', () => { const seenPaths = new Set(); // 1. ~/.claude/projects/{folder}/ — claude-home .md files - const claudeHomeFiles = scanMdFiles(folderPath, isAllowedMemoryPath); + const claudeHomeFiles = scanMdFiles(folderPath, isAllowed); for (const f of claudeHomeFiles) { - files.push({ ...f, displayPath: '~/.claude', source: 'claude-home' }); + files.push({ ...listed(f), displayPath: '~/.claude', source: 'claude-home' }); seenPaths.add(f.filePath); } // memory/MEMORY.md const memoryDir = path.join(folderPath, 'memory'); - const memoryFiles = scanMdFiles(memoryDir, isAllowedMemoryPath); + const memoryFiles = scanMdFiles(memoryDir, isAllowed); for (const f of memoryFiles) { - files.push({ ...f, displayPath: '~/.claude', source: 'claude-home' }); + files.push({ ...listed(f), displayPath: '~/.claude', source: 'claude-home' }); seenPaths.add(f.filePath); } @@ -1273,28 +1287,28 @@ ipcMain.handle('get-memories', () => { if (projectPath) { for (const name of ['CLAUDE.md', 'GEMINI.md', 'agents.md']) { const fp = path.join(projectPath, name); - const accepted = acceptMdFile(fp, isAllowedMemoryPath); + const accepted = acceptMdFile(fp, isAllowed); if (accepted && !seenPaths.has(fp)) { - files.push({ ...accepted, displayPath: shortName + '/', source: 'project' }); + files.push({ ...listed(accepted), displayPath: shortName + '/', source: 'project' }); seenPaths.add(fp); } } // 3. {projectPath}/.claude/ — commands/*.md and other .md files const dotClaudeDir = path.join(projectPath, '.claude'); - const dotClaudeFiles = scanMdFiles(dotClaudeDir, isAllowedMemoryPath); + const dotClaudeFiles = scanMdFiles(dotClaudeDir, isAllowed); for (const f of dotClaudeFiles) { if (!seenPaths.has(f.filePath)) { - files.push({ ...f, displayPath: shortName + '/.claude/', source: 'project' }); + files.push({ ...listed(f), displayPath: shortName + '/.claude/', source: 'project' }); seenPaths.add(f.filePath); } } // commands/*.md const commandsDir = path.join(dotClaudeDir, 'commands'); - const commandFiles = scanMdFiles(commandsDir, isAllowedMemoryPath); + const commandFiles = scanMdFiles(commandsDir, isAllowed); for (const f of commandFiles) { if (!seenPaths.has(f.filePath)) { - files.push({ ...f, displayPath: shortName + '/.claude/commands/', source: 'project' }); + files.push({ ...listed(f), displayPath: shortName + '/.claude/commands/', source: 'project' }); seenPaths.add(f.filePath); } } @@ -1334,7 +1348,7 @@ ipcMain.handle('get-memories', () => { upsertSearchEntries(allFiles.map(f => ({ id: f.filePath, type: 'memory', folder: null, title: f.label + ' ' + f.filename, - body: fs.readFileSync(f.filePath, 'utf8'), + body: bodies.get(f.filePath) ?? '', }))); } } catch {} diff --git a/scan-md-files.js b/scan-md-files.js index 629c3fd7..41d91760 100644 --- a/scan-md-files.js +++ b/scan-md-files.js @@ -11,9 +11,14 @@ const { isSensitivePath } = require('./ipc-path-validator'); /** * Accept `filePath` as a listable Markdown file, or return null. * + * The accepted content comes back with the entry. A caller that needs the body + * must use it rather than read the path again: a second read is a second + * resolution of the same string, free to land somewhere the checks above never + * saw. See resolve-path-on-disk.js. + * * @param {string} filePath * @param {(filePath: string) => boolean} [isAllowed] - caller's allowlist, given the literal path - * @returns {{filename: string, filePath: string, modified: string}|null} + * @returns {{filename: string, filePath: string, modified: string, content: string}|null} */ function acceptMdFile(filePath, isAllowed) { try { @@ -21,8 +26,9 @@ function acceptMdFile(filePath, isAllowed) { if (!stat.isFile()) return null; if (isSensitivePath(filePath)) return null; if (isAllowed && !isAllowed(filePath)) return null; - if (!fs.readFileSync(filePath, 'utf8').trim()) return null; - return { filename: path.basename(filePath), filePath, modified: stat.mtime.toISOString() }; + const content = fs.readFileSync(filePath, 'utf8'); + if (!content.trim()) return null; + return { filename: path.basename(filePath), filePath, modified: stat.mtime.toISOString(), content }; } catch { return null; // unreadable, dangling or absent } diff --git a/test/get-memories-wiring.test.js b/test/get-memories-wiring.test.js new file mode 100644 index 00000000..53d1266a --- /dev/null +++ b/test/get-memories-wiring.test.js @@ -0,0 +1,82 @@ +// test/get-memories-wiring.test.js — source-text assertions on the get-memories +// handler in main.js. +// +// The acceptance rule and its tests live in scan-md-files.js, which can be +// required directly; the handler cannot (Electron + better-sqlite3 are compiled +// against the Electron ABI), so it is checked the same way the FTS dirty-flag +// helpers are — by extracting the source and reading it. These are the +// properties that make the unit tests next door mean anything: a handler that +// stops calling the rule, or reads a file a second time behind its back, passes +// every test in scan-md-files.test.js. +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +const mainSrc = fs.readFileSync(path.join(__dirname, '..', 'main.js'), 'utf8'); + +const start = mainSrc.indexOf("ipcMain.handle('get-memories'"); +const end = mainSrc.indexOf('// --- IPC: read-memory ---'); +assert.ok(start !== -1, "get-memories handler not found in main.js"); +assert.ok(end > start, '"// --- IPC: read-memory ---" marker not found after the handler'); + +const handler = mainSrc.slice(start, end); + +test('get-memories: every listing goes through the shared acceptance rule', () => { + // The project-root files are looked up by name, the rest by directory scan. + // Both must end up in acceptMdFile — a second hand-rolled lookup beside it is + // how the gate came to be missing from the root files in the first place. + assert.match(handler, /acceptMdFile\(/, 'the project-root files must go through acceptMdFile'); + assert.doesNotMatch(handler, /existsSync\([^)]*\bfp\b/, + 'a by-name lookup must not probe the file itself — acceptMdFile owns that'); +}); + +test('get-memories: no scan is made without an allowlist', () => { + const calls = handler.match(/scanMdFiles\([^)]*\)/g) || []; + assert.ok(calls.length >= 4, `expected the handler to scan several directories, found ${calls.length}`); + for (const call of calls) { + assert.match(call, /,\s*isAllowed\s*\)/, `${call} must pass the allowlist`); + } + assert.match(handler, /acceptMdFile\([^)]*,\s*isAllowed\s*\)/, 'the by-name lookup must pass it too'); +}); + +test('get-memories: the FTS index uses the body already read, never a second read', () => { + // A second fs read of the same path is a second resolution of it, free to + // land somewhere the allowlist and the denylist never saw — and, if what + // lands there is a FIFO, to freeze the main process for good. + const ftsStart = handler.indexOf('upsertSearchEntries'); + assert.ok(ftsStart !== -1, 'the FTS block was not found in the handler'); + + assert.doesNotMatch(handler, /body:\s*fs\.readFileSync/, 'the index must not re-read the file'); + assert.match(handler, /body:\s*bodies\.get\(/, 'the index must use the body acceptMdFile returned'); + assert.doesNotMatch(handler.slice(ftsStart), /fs\.readFileSync/, + 'nothing in the FTS block may touch the filesystem again'); +}); + +test('get-memories: the bodies it collects do not travel to the renderer', () => { + // `listed` is what strips them; every push of a listing entry must go + // through it, or a file's full content rides the IPC payload to the renderer. + assert.match(handler, /const listed = \(\{ content, \.\.\.entry \}\)/, '`listed` must destructure content off the entry'); + const pushes = handler.match(/files\.push\(\{ \.\.\.[a-zA-Z]+/g) || []; + assert.ok(pushes.length >= 4, `expected several listing pushes, found ${pushes.length}`); + for (const push of pushes) { + assert.match(push, /\.\.\.listed$/, `${push}...) must spread listed(...), not the raw entry`); + } +}); + +test('get-memories: the known-project set is resolved once, not per file', () => { + // isAllowedMemoryPath rebuilds it from disk on every call — readdir of every + // project folder, plus a 256 KiB read off a JSONL each — so calling the + // module-level wrapper per file makes this handler quadratic in projects, + // synchronously, on the Electron main thread. + assert.match(handler, /const knownRoots = \[\.\.\.getKnownProjectPaths\(\)\]/, + 'the handler must bind the known roots once'); + assert.match(handler, /_isAllowedMemoryPath\([^)]*knownRoots\)/, + 'the predicate the handler passes around must close over that binding'); + assert.doesNotMatch(handler, /isAllowed\s*=\s*isAllowedMemoryPath\b/, + 'aliasing the per-call wrapper puts the rebuild back on the per-file path'); + assert.doesNotMatch(handler, /,\s*isAllowedMemoryPath\s*\)/, + 'the per-call wrapper must not be passed into a per-file path'); +}); diff --git a/test/scan-md-files.test.js b/test/scan-md-files.test.js index 19baa6d1..c2c3daab 100644 --- a/test/scan-md-files.test.js +++ b/test/scan-md-files.test.js @@ -181,15 +181,40 @@ test('scanMdFiles: a directory that does not exist scans to an empty list, not a // It is the single acceptance rule behind both paths — the memory listing had // two of them, and only one carried the guards. -test('acceptMdFile: accepts an ordinary file and reports its own name and mtime', () => { +test('acceptMdFile: accepts an ordinary file and reports its own name, mtime and content', () => { const r = rig(); try { const fp = path.join(r.dir, 'CLAUDE.md'); - fs.writeFileSync(fp, 'project instructions'); + fs.writeFileSync(fp, 'project instructions\n'); const out = acceptMdFile(fp); assert.equal(out.filename, 'CLAUDE.md'); assert.equal(out.filePath, fp); assert.equal(out.modified, fs.statSync(fp).mtime.toISOString()); + // The body comes back so the caller never has to read the path again. + assert.equal(out.content, 'project instructions\n'); + } finally { r.cleanup(); } +}); + +test('acceptMdFile: the content returned is the one that passed the checks, not whatever the path holds later', (t) => { + const r = rig(); + try { + // The checks run against a path, the read has to happen while that path + // still means what was checked. A caller that re-reads it afterwards gets + // whatever has been moved into place since — which is the whole point of + // handing the body back instead of the path alone. + const fp = path.join(r.dir, 'CLAUDE.md'); + fs.writeFileSync(fp, 'the legitimate note'); + const out = acceptMdFile(fp, () => true); + + const swapped = path.join(r.elsewhere, 'swapped.md'); + fs.writeFileSync(swapped, 'content moved in afterwards'); + const tmp = path.join(r.dir, 'tmp-link'); + try { fs.symlinkSync(swapped, tmp); } + catch { return t.skip('cannot create a symlink on this machine'); } + fs.renameSync(tmp, fp); + + assert.equal(out.content, 'the legitimate note'); + assert.notEqual(fs.readFileSync(out.filePath, 'utf8'), out.content); } finally { r.cleanup(); } }); From e20693b5c230c64f995b5d0d1f3cffff660c4a40 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Renard Date: Thu, 17 Sep 2026 15:06:36 +0200 Subject: [PATCH 3/3] test(memory): pin the root-file lookup as the whole assignment, not a call somewhere `null && acceptMdFile(fp, isAllowed)` stops the project-root files from being listed at all and still satisfies a test that only looks for the call. The assertion now covers the assignment. --- test/get-memories-wiring.test.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/get-memories-wiring.test.js b/test/get-memories-wiring.test.js index 53d1266a..2ce09161 100644 --- a/test/get-memories-wiring.test.js +++ b/test/get-memories-wiring.test.js @@ -28,7 +28,11 @@ test('get-memories: every listing goes through the shared acceptance rule', () = // The project-root files are looked up by name, the rest by directory scan. // Both must end up in acceptMdFile — a second hand-rolled lookup beside it is // how the gate came to be missing from the root files in the first place. - assert.match(handler, /acceptMdFile\(/, 'the project-root files must go through acceptMdFile'); + // Asserted as the whole assignment, not just a call somewhere in the handler: + // `null && acceptMdFile(fp, isAllowed)` stops listing the root files entirely + // and still contains the call. + assert.match(handler, /const accepted = acceptMdFile\(fp, isAllowed\);/, + 'the project-root files must go through acceptMdFile, unconditionally'); assert.doesNotMatch(handler, /existsSync\([^)]*\bfp\b/, 'a by-name lookup must not probe the file itself — acceptMdFile owns that'); });