diff --git a/packages/travsr-lsif-py/src/security.ts b/packages/travsr-lsif-py/src/security.ts index a79b76f3..d5ce9cb2 100644 --- a/packages/travsr-lsif-py/src/security.ts +++ b/packages/travsr-lsif-py/src/security.ts @@ -46,11 +46,23 @@ export function assertPathsContained(filePaths: string[], repoRoot: string): voi * Pure in-memory containment check — no I/O, safe to call in hot loops. * Does NOT follow symlinks; use assertPathsContained for the authoritative check. * Used as a belt-and-suspenders filter before emitting each file's ranges. + * + * Both operands are resolved before comparing (#806). Resolving only filePath + * put the two sides in different namespaces on Windows, where resolve() + * prepends the current drive letter and normalize() does not, so a genuinely + * contained file compared "C:\repo\src\a.ts" against "\repo" and was dropped. + * + * pathImpl exists so the tests can pin this function's Windows and POSIX + * behaviour from any host; production callers never pass it. */ -export function isUnderRoot(filePath: string, repoRoot: string): boolean { - const normalized = path.normalize(path.resolve(filePath)); - const root = path.normalize(repoRoot); - return normalized.startsWith(root + path.sep) || normalized === root; +export function isUnderRoot( + filePath: string, + repoRoot: string, + pathImpl: typeof path = path +): boolean { + const normalized = pathImpl.normalize(pathImpl.resolve(filePath)); + const root = pathImpl.normalize(pathImpl.resolve(repoRoot)); + return normalized.startsWith(root + pathImpl.sep) || normalized === root; } // ── Internal helpers ─────────────────────────────────────────────────────────── diff --git a/packages/travsr-lsif-py/src/test/security.test.ts b/packages/travsr-lsif-py/src/test/security.test.ts index 631ea51c..ce2e8319 100644 --- a/packages/travsr-lsif-py/src/test/security.test.ts +++ b/packages/travsr-lsif-py/src/test/security.test.ts @@ -45,6 +45,52 @@ test('isUnderRoot: correctly rejects escaped paths', () => { assert.ok(!isUnderRoot('/etc/passwd', root)); assert.ok(!isUnderRoot('/', root)); }); +// #806: isUnderRoot resolved filePath but not repoRoot. On Windows resolve() +// prepends the current drive letter and normalize() does not, so the two sides +// compared "C:\...\foo.py" against "\home\user\project" and every +// contained file was rejected. The platform-native test above only catches that +// when run ON Windows; these two pin both platforms' semantics from any host by +// injecting the flavour explicitly. Inputs are fully qualified — a +// drive-relative path would pick up the host's cwd and stop being +// deterministic. +test('isUnderRoot: Windows semantics, verified from any host (#806)', () => { + const root = 'C:\\home\\user\\project'; + assert.ok(isUnderRoot('C:\\home\\user\\project\\src\\foo.py', root, path.win32)); + assert.ok(isUnderRoot('C:\\home\\user\\project', root, path.win32)); + assert.ok( + isUnderRoot('C:/home/user/project/src/foo.py', root, path.win32), + 'forward slashes normalize to backslashes' + ); + assert.ok(!isUnderRoot('C:\\home\\user\\other\\foo.py', root, path.win32)); + assert.ok( + !isUnderRoot('C:\\home\\user\\project-evil\\foo.py', root, path.win32), + 'prefix match is not enough' + ); + assert.ok( + !isUnderRoot('D:\\home\\user\\project\\src\\foo.py', root, path.win32), + 'same path on another drive is outside' + ); + assert.ok( + !isUnderRoot('C:\\home\\user\\project\\src\\..\\..\\..\\etc\\passwd', root, path.win32), + 'traversal collapses before comparing' + ); +}); + +test('isUnderRoot: POSIX semantics, verified from any host (#806)', () => { + const root = '/home/user/project'; + assert.ok(isUnderRoot('/home/user/project/src/foo.py', root, path.posix)); + assert.ok(isUnderRoot('/home/user/project', root, path.posix)); + assert.ok(!isUnderRoot('/home/user/other/foo.py', root, path.posix)); + assert.ok( + !isUnderRoot('/home/user/project-evil/foo.py', root, path.posix), + 'prefix match is not enough' + ); + assert.ok( + !isUnderRoot('/home/user/project/../project-evil/foo.py', root, path.posix), + 'traversal collapses before comparing' + ); +}); + test('assertPathsContained: accepts files inside root', () => { const root = resolveRoot('/tmp'); diff --git a/packages/travsr-lsif-ts/src/security.ts b/packages/travsr-lsif-ts/src/security.ts index 7eed7eac..e4769db3 100644 --- a/packages/travsr-lsif-ts/src/security.ts +++ b/packages/travsr-lsif-ts/src/security.ts @@ -134,11 +134,23 @@ export function assertFilesContained( * Pure in-memory containment check — no I/O, safe to call in hot filter loops. * Does NOT follow symlinks; use assertFilesContained for the authoritative check. * Used as a belt-and-suspenders filter on program.getSourceFiles(). + * + * Both operands are resolved before comparing (#806). Resolving only filePath + * put the two sides in different namespaces on Windows, where resolve() + * prepends the current drive letter and normalize() does not, so a genuinely + * contained file compared "C:\repo\src\a.ts" against "\repo" and was dropped. + * + * pathImpl exists so the tests can pin this function's Windows and POSIX + * behaviour from any host; production callers never pass it. */ -export function isUnderRoot(filePath: string, repoRoot: string): boolean { - const normalized = path.normalize(path.resolve(filePath)); - const root = path.normalize(repoRoot); - return normalized.startsWith(root + path.sep) || normalized === root; +export function isUnderRoot( + filePath: string, + repoRoot: string, + pathImpl: typeof path = path +): boolean { + const normalized = pathImpl.normalize(pathImpl.resolve(filePath)); + const root = pathImpl.normalize(pathImpl.resolve(repoRoot)); + return normalized.startsWith(root + pathImpl.sep) || normalized === root; } // ── Internal helpers ────────────────────────────────────────────────────────── diff --git a/packages/travsr-lsif-ts/src/test/security.test.ts b/packages/travsr-lsif-ts/src/test/security.test.ts index d2d890d7..0328872c 100644 --- a/packages/travsr-lsif-ts/src/test/security.test.ts +++ b/packages/travsr-lsif-ts/src/test/security.test.ts @@ -115,6 +115,52 @@ test('isUnderRoot: correctly identifies contained and escaped paths', () => { assert.ok(!isUnderRoot('/home/user/other/foo.ts', root)); assert.ok(!isUnderRoot('/home/user/project-evil/foo.ts', root), 'prefix match is not enough'); }); +// #806: isUnderRoot resolved filePath but not repoRoot. On Windows resolve() +// prepends the current drive letter and normalize() does not, so the two sides +// compared "C:\...\foo.ts" against "\home\user\project" and every +// contained file was rejected. The platform-native test above only catches that +// when run ON Windows; these two pin both platforms' semantics from any host by +// injecting the flavour explicitly. Inputs are fully qualified — a +// drive-relative path would pick up the host's cwd and stop being +// deterministic. +test('isUnderRoot: Windows semantics, verified from any host (#806)', () => { + const root = 'C:\\home\\user\\project'; + assert.ok(isUnderRoot('C:\\home\\user\\project\\src\\foo.ts', root, path.win32)); + assert.ok(isUnderRoot('C:\\home\\user\\project', root, path.win32)); + assert.ok( + isUnderRoot('C:/home/user/project/src/foo.ts', root, path.win32), + 'forward slashes normalize to backslashes' + ); + assert.ok(!isUnderRoot('C:\\home\\user\\other\\foo.ts', root, path.win32)); + assert.ok( + !isUnderRoot('C:\\home\\user\\project-evil\\foo.ts', root, path.win32), + 'prefix match is not enough' + ); + assert.ok( + !isUnderRoot('D:\\home\\user\\project\\src\\foo.ts', root, path.win32), + 'same path on another drive is outside' + ); + assert.ok( + !isUnderRoot('C:\\home\\user\\project\\src\\..\\..\\..\\etc\\passwd', root, path.win32), + 'traversal collapses before comparing' + ); +}); + +test('isUnderRoot: POSIX semantics, verified from any host (#806)', () => { + const root = '/home/user/project'; + assert.ok(isUnderRoot('/home/user/project/src/foo.ts', root, path.posix)); + assert.ok(isUnderRoot('/home/user/project', root, path.posix)); + assert.ok(!isUnderRoot('/home/user/other/foo.ts', root, path.posix)); + assert.ok( + !isUnderRoot('/home/user/project-evil/foo.ts', root, path.posix), + 'prefix match is not enough' + ); + assert.ok( + !isUnderRoot('/home/user/project/../project-evil/foo.ts', root, path.posix), + 'traversal collapses before comparing' + ); +}); + // ── Integration tests against malicious tsconfig fixtures ─────────────────────