From dbc191b19d23c1e482782459a54d2e6996bb723e Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Tue, 27 Oct 2026 11:32:20 -0500 Subject: [PATCH] fix(paths): a project at a Windows drive root indexes its files A project at the root of a drive, such as a mapped network drive Z:\, a subst drive or a UNC share root, indexed nothing on Windows: every file was refused as "Path traversal blocked", and code reads under the root were refused too. The containment check appended a separator to the root before comparing, and a root such as C:\ already ends with one, so the prefix became C:\\ and no path under the root matched. The dead code report's default file reader had its own copy of the same check, so it treated every file of such a project as unreadable. The check now adds a separator only when the root doesn't already end with one. The dead code reader uses the shared lexical containment check instead of its own copy. Validation on Windows 11: - subst R: onto a two-file fixture, then codegraph init -y --force R:\: main indexes 0 files ("all 2 files had errors", 2 "Path traversal blocked"), and validatePathWithinRoot(R:\, a.ts) is null. The fix indexes both files (6 nodes, 6 edges) and getCode returns both functions' source. The mapping was removed after each run. - Dead code report against one R:\ index: main's reader drops the unused function as unreadable; the fix reports it. - Tests: 3 new tests fail on main's source and pass with the fix: a file under the filesystem root (this drive's root here, / on POSIX), path math under Q:\ and \\server\share\, and read-only realpaths under C:\. Reported in PR #415 by @Kamilslaoui93; this reimplements the fix independently. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/security.test.ts | 36 +++++++++++++++++++++++++++++++++++- src/graph/dead-code.ts | 10 ++++++---- src/utils.ts | 26 ++++++++++++++------------ 4 files changed, 56 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c555c2ada8..941ec16003 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - The bundled Node runtime moves to 24.22.1, Node's October security release, and every release now checks the Node it bundles against the checksums nodejs.org publishes. The new runtime also fixes a crash on Windows as a command like `codegraph sync` exits, after its work is done, printing `Assertion failed: !(handle->flags & UV_HANDLE_CLOSING)`, which made a script or CI run that had succeeded fail. Thanks @Ancienttwo for the report and the first fix, and @IdealBoy709 for the report on the security release. (#2617, #2780) - Building CodeGraph from source with bun, as in `bun run build`, no longer runs until it is stopped: bun ran one step of the build as the whole build again, so it kept starting itself over. Building with npm is unchanged. Thanks @bompus. - When CodeGraph can't open a project's database, it now says what went wrong: SQLite's error, the file's path, and what to check, such as another program holding the file open, a folder you can't write to, or a network share. Before, every such failure read "CodeGraph requires node:sqlite (Node.js 22.5+)", and an agent was told no project was loaded. That hint now appears only when Node really can't load its built-in SQLite, a case that used to end in an unhandled error instead. +- On Windows, a project at the root of a drive, such as a mapped network drive like `Z:\`, a `subst` drive or the root of a network share, now indexes. Before, every file was refused as "Path traversal blocked", so the index came out empty, and the code of such a project couldn't be shown either. Thanks @Kamilslaoui93. - `codegraph upgrade` no longer wipes your agent instructions file, such as a global `AGENTS.md`, `CLAUDE.md` or `GEMINI.md`, when it refreshes CodeGraph's section in it: a stray `` line above that section, like one left behind when part of an older section was deleted by hand, made the refresh replace everything in between, often the whole file. CodeGraph now rewrites only its own section, in `codegraph install` and `codegraph uninstall` too, and keeps everything around it exactly as it was, Windows line endings included. Thanks @pzaj2 for the report. (#2506) - `codegraph install`, `upgrade` and `uninstall` now write through an instructions or config file that is a symbolic link, such as a `~/.codex/AGENTS.md` or `~/.claude.json` linked from your dotfiles, instead of replacing the link with a copy: the shared file gets CodeGraph's settings, uninstall takes them out again, and the link stays in place. A link CodeGraph can't write through, like one to a read-only file or into a folder that doesn't exist, is now reported by name and left alone, and an upgrade still refreshes your other agents. Thanks @jguerra6 for the report and @danusha2345 for the first fix. (#2721) - `codegraph install` and `upgrade` no longer break Codex's `config.toml` when CodeGraph's `[mcp_servers.codegraph]` entry is written differently from how CodeGraph writes it, such as indented, with spaces or quotes inside the brackets, or with Windows line endings: the entry is now updated in place, where before a second copy was added and Codex could load none of the file. A file already broken that way is repaired on the next install or upgrade, an example of the entry inside a multiline setting such as `developer_instructions` is no longer mistaken for it, uninstall removes the entry however it is written, and an entry you wrote with dotted keys or as an inline table is left as you wrote it. When your `mcp_servers` settings are one inline table, CodeGraph now says it can't add itself instead of breaking the file. Thanks @dencoseca for the report, and @danusha2345 and @rudycelekli for the first fixes. (#2562) diff --git a/__tests__/security.test.ts b/__tests__/security.test.ts index 4051fbda87..6c128b4c39 100644 --- a/__tests__/security.test.ts +++ b/__tests__/security.test.ts @@ -12,7 +12,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; -import { FileLock, validateProjectPath, validatePathWithinRoot } from '../src/utils'; +import { FileLock, validateProjectPath, validatePathWithinRoot, lexicalPathWithinRoot } from '../src/utils'; import CodeGraph from '../src/index'; import { ToolHandler, tools, __setLoadCodeGraphForTests } from '../src/mcp/tools'; import { scanDirectory, isSourceFile } from '../src/extraction'; @@ -193,6 +193,40 @@ describe('Path Traversal Prevention', () => { }); }); +// #415: a root that already ends with its separator (a Windows drive root such +// as a `Z:\` network drive or a `subst` drive, a UNC share root, POSIX `/`) got +// a second one appended, so no path under it counted as inside it, and a +// project there indexed no files. Path math, plus read-only realpaths of `C:\`. +describe('Path containment at a filesystem root (#415)', () => { + it('counts a file under the filesystem root as inside it', () => { + const root = path.parse(process.cwd()).root; // `/`, or this drive's root on Windows + expect(lexicalPathWithinRoot(root, 'a.ts')).toBe(path.join(root, 'a.ts')); + expect(lexicalPathWithinRoot(root, path.join('src', 'b.ts'))).toBe(path.join(root, 'src', 'b.ts')); + }); + + it.runIf(process.platform === 'win32')('counts a file under a drive root or a share root as inside it', () => { + // Neither the drive nor the share has to exist: this is path math only. + expect(lexicalPathWithinRoot('Q:\\', 'a.ts')).toBe('Q:\\a.ts'); + expect(lexicalPathWithinRoot('q:\\', 'src\\b.ts')).toBe('q:\\src\\b.ts'); + expect(lexicalPathWithinRoot('\\\\server\\share\\', 'a.ts')).toBe('\\\\server\\share\\a.ts'); + // Another drive or another share is still outside. + expect(lexicalPathWithinRoot('Q:\\', 'C:\\Windows\\win.ini')).toBeNull(); + expect(lexicalPathWithinRoot('\\\\server\\share\\', '\\\\server\\other\\a.ts')).toBeNull(); + }); + + it.runIf(process.platform === 'win32' && fs.existsSync('C:\\Windows'))( + 'lets the realpath check through for a folder under a drive root', + () => { + expect(validatePathWithinRoot('C:\\', 'Windows')?.toLowerCase()).toBe('c:\\windows'); + // A path that doesn't exist yet is judged by its nearest existing + // ancestor, which here is the drive root itself. + expect(validatePathWithinRoot('C:\\', 'codegraph-no-such-dir-415\\a.ts')).toBe( + 'C:\\codegraph-no-such-dir-415\\a.ts' + ); + } + ); +}); + describe('Symlink escape prevention (#527)', () => { // An in-repo symlink whose logical path is inside the project root but whose // REAL target escapes the root must never be served. validatePathWithinRoot diff --git a/src/graph/dead-code.ts b/src/graph/dead-code.ts index d712a00009..0c6a5f1afc 100644 --- a/src/graph/dead-code.ts +++ b/src/graph/dead-code.ts @@ -100,10 +100,10 @@ */ import fs from 'fs'; -import path from 'path'; import type CodeGraph from '../index'; import type { EdgeKind, Node, NodeKind } from '../types'; import { isTestFile } from '../search/query-utils'; +import { lexicalPathWithinRoot } from '../utils'; // ============================================================================= // Caps and defaults @@ -706,11 +706,13 @@ export function buildDeadCodeReport(cg: CodeGraph, query: DeadCodeQuery = {}): D * on disk and this module should not be the thing that trusts it. */ function defaultSourceReader(cg: CodeGraph): (filePath: string) => string | null { - const root = path.resolve(cg.getProjectRoot()); + const root = cg.getProjectRoot(); return (filePath: string): string | null => { try { - const absolute = path.resolve(root, filePath); - if (absolute !== root && !absolute.startsWith(root + path.sep)) return null; + // The shared containment check, which also holds for a project at a + // drive root: appending a separator to `C:\` refused every file (#415). + const absolute = lexicalPathWithinRoot(root, filePath); + if (absolute === null) return null; const stat = fs.statSync(absolute); if (!stat.isFile() || stat.size > MAX_CORROBORATION_BYTES) return null; return fs.readFileSync(absolute, 'utf8'); diff --git a/src/utils.ts b/src/utils.ts index a8f09ae4d6..d817003c8a 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -82,19 +82,28 @@ export function isConfigLeafNode(node: { kind: string; language?: string }): boo return node.kind === 'constant' && !!node.language && CONFIG_LEAF_LANGUAGES.has(node.language); } +/** + * Whether `child` is `parent` or lies under it, compared exactly as given. + * + * A root (`C:\`, `\\server\share\`, `/`) already ends with its separator, so + * none is added to it. Adding one made `C:\\` the prefix, which no path under + * the root starts with, so a project at a drive root indexed nothing (#415). + */ +function isSameOrUnder(child: string, parent: string): boolean { + if (child === parent) return true; + return child.startsWith(parent.endsWith(path.sep) ? parent : parent + path.sep); +} + /** * Whether `child` is `parent` itself or sits underneath it. Case-insensitive on * Windows — NTFS is case-insensitive, and realpathSync can hand back a different * case than the lexical root, which would otherwise false-reject a valid file. */ function isWithinDir(child: string, parent: string): boolean { - let c = child; - let p = parent; if (process.platform === 'win32') { - c = c.toLowerCase(); - p = p.toLowerCase(); + return isSameOrUnder(child.toLowerCase(), parent.toLowerCase()); } - return c === p || c.startsWith(p + path.sep); + return isSameOrUnder(child, parent); } /** @@ -208,13 +217,6 @@ function foldPathCase(p: string): string { return process.platform === 'win32' || process.platform === 'darwin' ? p.toLowerCase() : p; } -/** Whether `child` is `parent` or lies under it. Both are already folded. */ -function isSameOrUnder(child: string, parent: string): boolean { - if (child === parent) return true; - // A root (`C:\`, `/`) already ends with its separator. - return child.startsWith(parent.endsWith(path.sep) ? parent : parent + path.sep); -} - /** * The plain spelling of a Win32 namespace path: `\\?\C:\x` and `\\.\C:\x` are * `C:\x`, and `\\?\UNC\host\share` is `\\host\share`. `path.resolve` keeps the