diff --git a/__tests__/security.test.ts b/__tests__/security.test.ts index 75ac8432..5fc5df1a 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 } from '../src/utils'; +import { FileLock, validateProjectPath, validatePathWithinRoot, isPathWithinRoot } from '../src/utils'; import CodeGraph from '../src/index'; import { ToolHandler, tools } from '../src/mcp/tools'; import { scanDirectory, isSourceFile } from '../src/extraction'; @@ -206,6 +206,38 @@ describe('validateProjectPath — sensitive directory blocking', () => { ); }); +describe('validatePathWithinRoot / isPathWithinRoot — containment check', () => { + // A drive root ("W:\") or POSIX root ("/") already ends with a separator. + // The check used to append another, comparing against "W:\\" / "//", which + // no real path starts with — so every file under a drive-root project was + // wrongly rejected and the whole index failed with "files could not be read". + it.runIf(process.platform === 'win32')('accepts files when the project root is a drive root', () => { + expect(validatePathWithinRoot('C:\\', 'project\\src\\app.ts')).toBe('C:\\project\\src\\app.ts'); + expect(validatePathWithinRoot('W:\\', 'CyberMAX\\Source\\foo.pas')).toBe('W:\\CyberMAX\\Source\\foo.pas'); + expect(isPathWithinRoot('W:\\CyberMAX\\Source\\foo.pas', 'W:\\')).toBe(true); + }); + + it.runIf(process.platform !== 'win32')('accepts files when the project root is the filesystem root', () => { + expect(validatePathWithinRoot('/', 'project/src/app.ts')).toBe('/project/src/app.ts'); + expect(isPathWithinRoot('/project/src/app.ts', '/')).toBe(true); + }); + + it('still works for a normal nested project root', () => { + const root = path.join(os.tmpdir(), 'cg-root'); + expect(validatePathWithinRoot(root, 'src/app.ts')).toBe(path.join(root, 'src', 'app.ts')); + expect(isPathWithinRoot(path.join(root, 'src', 'app.ts'), root)).toBe(true); + }); + + it('still rejects traversal escapes and sibling-prefix paths', () => { + const root = path.join(os.tmpdir(), 'cg-root'); + // Classic traversal escape. + expect(validatePathWithinRoot(root, path.join('..', 'escape', 'x.ts'))).toBeNull(); + // Sibling directory that shares a name prefix (cg-root vs cg-root-evil) + // must not be treated as inside — the trailing separator guards this. + expect(isPathWithinRoot(`${root}-evil${path.sep}x.ts`, root)).toBe(false); + }); +}); + describe('MCP Input Validation', () => { let testDir: string; let cg: CodeGraph; diff --git a/src/utils.ts b/src/utils.ts index 1ee1c937..b4d57181 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -46,6 +46,18 @@ const SENSITIVE_PATHS = new Set([ 'c:\\', 'c:\\windows', 'c:\\windows\\system32', ]); +/** + * Build the prefix that a path must start with to be considered *inside* `root`. + * + * A drive or filesystem root ("W:\\", "C:\\", "/") already ends with a + * separator. Blindly appending `path.sep` would produce "W:\\\\", which no real + * path starts with — so every file under a drive-root project would be wrongly + * rejected as a traversal escape. Only append a separator when one is missing. + */ +function rootPrefix(root: string): string { + return root.endsWith(path.sep) ? root : root + path.sep; +} + /** * Validate that a resolved file path stays within the project root. * Prevents path traversal attacks (e.g. node.filePath = "../../etc/passwd"). @@ -58,7 +70,7 @@ export function validatePathWithinRoot(projectRoot: string, filePath: string): s const resolved = path.resolve(projectRoot, filePath); const normalizedRoot = path.resolve(projectRoot); - if (!resolved.startsWith(normalizedRoot + path.sep) && resolved !== normalizedRoot) { + if (!resolved.startsWith(rootPrefix(normalizedRoot)) && resolved !== normalizedRoot) { return null; } return resolved; @@ -119,7 +131,7 @@ export function validateProjectPath(dirPath: string): string | null { export function isPathWithinRoot(filePath: string, rootDir: string): boolean { const resolvedPath = path.resolve(rootDir, filePath); const resolvedRoot = path.resolve(rootDir); - return resolvedPath.startsWith(resolvedRoot + path.sep) || resolvedPath === resolvedRoot; + return resolvedPath.startsWith(rootPrefix(resolvedRoot)) || resolvedPath === resolvedRoot; } /** @@ -139,7 +151,7 @@ export function isPathWithinRootReal(filePath: string, rootDir: string): boolean try { const realPath = fs.realpathSync(path.resolve(rootDir, filePath)); const realRoot = fs.realpathSync(rootDir); - return realPath.startsWith(realRoot + path.sep) || realPath === realRoot; + return realPath.startsWith(rootPrefix(realRoot)) || realPath === realRoot; } catch { // If realpath fails (broken symlink, permissions), fall back to logical check return true;