diff --git a/__tests__/security.test.ts b/__tests__/security.test.ts index 75ac84320..4b71daea6 100644 --- a/__tests__/security.test.ts +++ b/__tests__/security.test.ts @@ -12,7 +12,12 @@ 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'; @@ -176,6 +181,51 @@ describe('Path Traversal Prevention', () => { }); }); +describe('Path-within-root helpers — Windows drive-root regression', () => { + // path.resolve('Z:\\') returns 'Z:\\' (already trailing-sep) on Windows, + // so the old `resolvedRoot + path.sep` check produced 'Z:\\\\' which no + // file path under the drive could start with. This regressed indexing for + // any project at a drive root or mapped network drive root. + it.runIf(process.platform === 'win32')( + 'validatePathWithinRoot accepts files under a drive root (Z:\\)', + () => { + // Skip when Z: is not a real drive on this runner. + if (!fs.existsSync('Z:\\')) return; + const resolved = validatePathWithinRoot('Z:\\', 'configuration.yaml'); + expect(resolved).toBe('Z:\\configuration.yaml'); + } + ); + + it.runIf(process.platform === 'win32')( + 'isPathWithinRoot returns true for files under a drive root (Z:\\)', + () => { + if (!fs.existsSync('Z:\\')) return; + expect(isPathWithinRoot('configuration.yaml', 'Z:\\')).toBe(true); + expect(isPathWithinRoot('sub/dir/file.ts', 'Z:\\')).toBe(true); + } + ); + + it('validatePathWithinRoot still rejects traversal in a normal subdirectory', () => { + const tmp = createTempDir(); + try { + expect(validatePathWithinRoot(tmp, '../escape.txt')).toBeNull(); + expect(validatePathWithinRoot(tmp, 'inside.txt')).toBe(path.join(tmp, 'inside.txt')); + } finally { + cleanupTempDir(tmp); + } + }); + + it('isPathWithinRoot still rejects traversal in a normal subdirectory', () => { + const tmp = createTempDir(); + try { + expect(isPathWithinRoot('../escape.txt', tmp)).toBe(false); + expect(isPathWithinRoot('inside.txt', tmp)).toBe(true); + } finally { + cleanupTempDir(tmp); + } + }); +}); + describe('validateProjectPath — sensitive directory blocking', () => { // POSIX-only: on Windows '/etc' resolves to C:\etc (non-existent), not a // sensitive dir — the Windows case is covered by the win32-gated test below. diff --git a/src/utils.ts b/src/utils.ts index 1ee1c9372..439b6db62 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -54,11 +54,22 @@ const SENSITIVE_PATHS = new Set([ * @param filePath - The relative file path to validate * @returns The resolved absolute path, or null if it escapes the root */ +/** + * Append `path.sep` to a directory unless it already ends with one. + * + * Drive roots on Windows (e.g. `Z:\`) come back from `path.resolve` with a + * trailing separator already; naively appending another would produce `Z:\\`, + * which nothing under that root starts with — so every file gets rejected. + */ +function ensureTrailingSep(dir: string): string { + return dir.endsWith(path.sep) ? dir : dir + path.sep; +} + export function validatePathWithinRoot(projectRoot: string, filePath: string): string | null { const resolved = path.resolve(projectRoot, filePath); const normalizedRoot = path.resolve(projectRoot); - if (!resolved.startsWith(normalizedRoot + path.sep) && resolved !== normalizedRoot) { + if (!resolved.startsWith(ensureTrailingSep(normalizedRoot)) && resolved !== normalizedRoot) { return null; } return resolved; @@ -119,7 +130,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(ensureTrailingSep(resolvedRoot)) || resolvedPath === resolvedRoot; } /**