Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .ai/contexts/ipc-bridge.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
51 changes: 30 additions & 21 deletions main.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@
}

// Shell profiles → shell-profiles.js
const { discoverShellProfiles, getShellProfiles, resolveShell, isWindows, isWslShell, windowsToWslPath, shellArgs, quoteArgvForShell } = require('./shell-profiles');

Check warning on line 73 in main.js

View workflow job for this annotation

GitHub Actions / lint

'isWindows' is assigned a value but never used. Allowed unused vars must match /^_/u

Check warning on line 73 in main.js

View workflow job for this annotation

GitHub Actions / lint

'discoverShellProfiles' is assigned a value but never used. Allowed unused vars must match /^_/u
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');
Expand Down Expand Up @@ -457,8 +457,8 @@
isInitialScanComplete, setInitialScanComplete,
},
});
const { readSessionFile, readFolderFromFilesystem, refreshFolder, reconcileCacheFromFilesystem,

Check warning on line 460 in main.js

View workflow job for this annotation

GitHub Actions / lint

'readFolderFromFilesystem' is assigned a value but never used. Allowed unused vars must match /^_/u

Check warning on line 460 in main.js

View workflow job for this annotation

GitHub Actions / lint

'readSessionFile' is assigned a value but never used. Allowed unused vars must match /^_/u
buildProjectsFromCache, notifyRendererProjectsChanged, sendStatus, populateCacheViaWorker,

Check warning on line 461 in main.js

View workflow job for this annotation

GitHub Actions / lint

'sendStatus' is assigned a value but never used. Allowed unused vars must match /^_/u
scanFoldersViaWorker, setRemoteRoots, resolveFolderDir } = sessionCache;
const { resolveJsonlPath, enumerateSessionFiles } = require('./read-session-file');

Expand Down Expand Up @@ -1230,8 +1230,22 @@
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 = [];
Expand All @@ -1256,50 +1270,45 @@
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);
}

// 2. {projectPath}/ — project root CLAUDE.md, agents.md
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, isAllowed);
if (accepted && !seenPaths.has(fp)) {
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);
}
}
Expand Down Expand Up @@ -1339,7 +1348,7 @@
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 {}
Expand Down Expand Up @@ -2274,7 +2283,7 @@
// WSL profiles only work for plain terminals — Claude CLI sessions need the
// Windows shell because session data lives on the Windows filesystem.
const requestedProfile = resolveShell(effectiveProfileId);
const useWslProfile = isWslShell(requestedProfile.path) && isPlainTerminal;

Check warning on line 2286 in main.js

View workflow job for this annotation

GitHub Actions / lint

'useWslProfile' is assigned a value but never used. Allowed unused vars must match /^_/u
const shellProfile = (isWslShell(requestedProfile.path) && !isPlainTerminal)
? resolveShell('auto')
: requestedProfile;
Expand Down
51 changes: 34 additions & 17 deletions scan-md-files.js
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -8,7 +9,33 @@ 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.
*
* 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, content: 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;
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
}
}

/**
* Scan `dir` for listable `.md` files (non-recursive).
*
* @param {string} dir
* @param {(filePath: string) => boolean} [isAllowed] - caller's allowlist, given the literal path
Expand All @@ -18,24 +45,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 };
86 changes: 86 additions & 0 deletions test/get-memories-wiring.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// 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.
// 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');
});

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