diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ca5b9ff1..7e9003eb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixes + +- Indexing no longer checks whether files outside your project exist. A relative import that points above the project directory (`../../something`) made CodeGraph probe that location on disk while resolving it. Nothing outside the project was ever read, and no such file was ever added to the index or linked to, but the check itself should not have happened — such an import now simply resolves to nothing. Symlinks inside your project that point at code kept elsewhere are unaffected and still index as before. Thanks @ErQrYfkrju. (#1631) + ## [1.6.0] - 2026-08-26 diff --git a/__tests__/resolution-fileexists-containment.test.ts b/__tests__/resolution-fileexists-containment.test.ts new file mode 100644 index 000000000..311ef17c4 --- /dev/null +++ b/__tests__/resolution-fileexists-containment.test.ts @@ -0,0 +1,64 @@ +/** + * `fileExists` must not probe outside the project root (#1631). + * + * `resolveRelativeImport` hands this callback paths built with + * `path.relative(projectRoot, basePath)`, which can carry `../` segments, and + * `path.join` does not clamp — so a crafted relative import in an indexed file + * made the resolver stat arbitrary absolute paths. Nothing outside is read (the + * content sinks are guarded separately, #527) and no edge is produced, but the + * probe itself is an existence oracle driven by repository content. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { ReferenceResolver } from '../src/resolution'; +import type { QueryBuilder } from '../src/db/queries'; + +describe('fileExists containment (#1631)', () => { + let sandbox: string; + let projectRoot: string; + + /** The resolver only needs a project root here — `fileExists` never queries. */ + const contextFor = (root: string) => + new ReferenceResolver(root, {} as unknown as QueryBuilder).getResolutionContext(); + + beforeEach(() => { + sandbox = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-test-')); + projectRoot = path.join(sandbox, 'proj'); + fs.mkdirSync(path.join(projectRoot, 'src'), { recursive: true }); + fs.writeFileSync(path.join(projectRoot, 'src', 'a.js'), 'export const a = 1;'); + // A real file two levels above the root, as the reproduction in #1631 has. + fs.mkdirSync(path.join(sandbox, 'outside'), { recursive: true }); + fs.writeFileSync(path.join(sandbox, 'outside', 'secret.js'), 'export const secret = 42;'); + }); + + afterEach(() => { + fs.rmSync(sandbox, { recursive: true, force: true }); + }); + + it('still reports files inside the root', () => { + expect(contextFor(projectRoot).fileExists('src/a.js')).toBe(true); + expect(contextFor(projectRoot).fileExists('src/missing.js')).toBe(false); + }); + + it('refuses to probe a path that escapes the root, even though it exists', () => { + const escaping = path.join('..', 'outside', 'secret.js'); + // Baseline: the target really is there — so `false` can only come from the guard. + expect(fs.existsSync(path.join(projectRoot, escaping))).toBe(true); + + expect(contextFor(projectRoot).fileExists(escaping)).toBe(false); + }); + + it('keeps following an in-root symlink whose target is outside the root (#935)', () => { + const link = path.join(projectRoot, 'vendor'); + try { + fs.symlinkSync(path.join(sandbox, 'outside'), link, 'dir'); + } catch { + return; // symlink creation not permitted (e.g. Windows without privilege) + } + // Lexically inside the root, physically outside — the indexing tier allows this. + expect(contextFor(projectRoot).fileExists(path.join('vendor', 'secret.js'))).toBe(true); + }); +}); diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 7b4bccc18..21556db51 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -26,6 +26,7 @@ import { loadProjectAliases, type AliasMap } from './path-aliases'; import { loadGoModule, type GoModule } from './go-module'; import { loadWorkspacePackages, type WorkspacePackages } from './workspace-packages'; import { logDebug } from '../errors'; +import { lexicalPathWithinRoot } from '../utils'; import type { ReExport } from './types'; import { LRUCache } from './lru-cache'; @@ -538,8 +539,18 @@ export class ReferenceResolver { return true; } } - // Fall back to filesystem for files not yet indexed - const fullPath = path.join(this.projectRoot, filePath); + // Fall back to filesystem for files not yet indexed. `path.join` does + // not clamp, and relative-import resolution hands us paths carrying + // `../` segments, so the probe has to be contained (#1631): a path + // outside the root can never be an indexed project file, and the + // `knownFiles` check above already answered for everything that is. + // Lexical containment only: this is a per-candidate hot path, and the + // symlink half of `validatePathWithinRoot` costs two `realpathSync` + // calls per probe (~70x slower here). It would also be wrong to apply + // — indexing deliberately follows in-root symlinks whose targets live + // outside the root (#935), so only the `../` escape is refused. + const fullPath = lexicalPathWithinRoot(this.projectRoot, filePath); + if (fullPath === null) return false; try { return fs.existsSync(fullPath); } catch (error) { diff --git a/src/utils.ts b/src/utils.ts index ba5c13261..892617274 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -107,19 +107,36 @@ function isWithinDir(child: string, parent: string): boolean { * @returns The resolved absolute path (realpath when it exists), or null if it * escapes the root */ +/** + * The lexical half of {@link validatePathWithinRoot}, on its own. + * + * Returns the resolved absolute path when `filePath` stays inside + * `projectRoot` after `../` segments are applied, or null when it escapes. + * No filesystem access — for callers on a hot path that only need to refuse a + * lexical escape, and for which the realpath half would be both unnecessary + * and far too expensive (the existence probe in resolution's `fileExists`, + * #1631: two `realpathSync` calls per probe made it ~70x slower). + * + * This is NOT a substitute for `validatePathWithinRoot` on any path whose + * contents get served — those must keep the symlink-aware check (#527). + */ +export function lexicalPathWithinRoot(projectRoot: string, filePath: string): string | null { + const resolved = path.resolve(projectRoot, filePath); + return isWithinDir(resolved, path.resolve(projectRoot)) ? resolved : null; +} + export function validatePathWithinRoot( projectRoot: string, filePath: string, options?: { allowSymlinkEscape?: boolean } ): string | null { - const resolved = path.resolve(projectRoot, filePath); - const normalizedRoot = path.resolve(projectRoot); - // 1. Lexical containment — cheap, catches `../` traversal. Applies even on // the indexing read path: a crafted `../` escape is still rejected. - if (!isWithinDir(resolved, normalizedRoot)) { + const resolved = lexicalPathWithinRoot(projectRoot, filePath); + if (resolved === null) { return null; } + const normalizedRoot = path.resolve(projectRoot); // 2. Symlink-aware containment — resolve symlinks on both sides and re-check, // so an in-repo symlink whose real target escapes the root is rejected.