Resolution: fileExists probes paths outside the project root (uncontained path.join fallback)

[ErQrYfkrju]

fileExists in src/resolution/index.ts falls back to the filesystem without containing the path:

fileExists: (filePath: string) => {
  if (this.knownFiles) { /* … index hit, O(1) … */ }
  // Fall back to filesystem for files not yet indexed
  const fullPath = path.join(this.projectRoot, filePath);
  try { return fs.existsSync(fullPath); } catch { … }
}

resolveRelativeImport computes path.relative(projectRoot, basePath), which can carry ../ segments, and path.join does not clamp — path.join('/root', '../../etc/passwd') is /etc/passwd. So a relative import in an indexed file probes existence outside the project root.

Reproduction

Nine lines, no OpenSCAD involved — this affects every language with relative imports:

mkdir -p esc/proj/src esc/outside
echo 'export function outsideSecret() { return 42; }' > esc/outside/secret.js
printf "import { outsideSecret } from '../../../outside/secret.js';\nexport function useIt() { return outsideSecret(); }\n" > esc/proj/src/a.js
cd esc/proj && git init -q . && git add -A && git -c user.email=t@t -c user.name=t commit -qm init
strace -f -e trace=newfstatat,stat,lstat,access -o /tmp/tr.txt codegraph init -i .
grep -c "outside/secret.js" /tmp/tr.txt

Result: 9 access() calls against paths two levels above the project root.

access(".../esc/outside/secret.js.js",   F_OK) = -1 ENOENT
access(".../esc/outside/secret.js.jsx",  F_OK) = -1 ENOENT
access(".../esc/outside/secret.js.mjs",  F_OK) = -1 ENOENT
…

Only access — no openat for that path, so nothing outside is read.

Severity, stated plainly

Low, and I would rather understate it than dress it up:

• No read. The content-serving sinks are guarded separately by the validatePathWithinRoot work in Bug: validatePathWithinRoot allows reading files outside project root via symlinks #527, so this is not the file-disclosure path that fixed.
• No wrong edge. The target is not in the index, so resolution declines and no edge is produced.

What remains is an existence oracle driven by repository content: a .scad/.js/.py file in an indexed repo can make the indexer probe whether an arbitrary absolute path exists, and timing or subsequent behaviour could in principle expose that. It is also just surprising — every other layer contains paths, and this one quietly does not.

Suggested fix

The knownFiles set is consulted first, so any indexed file resolves regardless of where it lives. Only the filesystem fallback needs bounding, and a path outside the root can never be an indexed project file — so returning false there is the correct answer rather than a new restriction:

const fullPath = path.join(this.projectRoot, filePath);
if (!validatePathWithinRoot(this.projectRoot, fullPath, { allowSymlinkEscape: true })) {
  return false;
}

allowSymlinkEscape matches the indexing tier established in #935, so an in-root symlink to a vendored library outside the repo keeps working; only the lexical ../ escape is refused.

I have deliberately not included this in #1630 (OpenSCAD support). Fixing it there would give one language a guarantee the other forty lack, and the change belongs to whoever owns this code path rather than to a language PR.

Found while validating OpenSCAD support; confirmed on 6a056ec (v1.6.0).

[maxmilian]

Confirmed on 6a056ec and taking this one — #1632 is up (draft). Thanks for keeping it out of #1630; that was the right call and it made this easy to pick up.

One finding worth flagging, because it changes the suggested fix. validatePathWithinRoot does two realpathSync calls per invocation, and fileExists runs per import candidate. Over 20,000 calls:

	existing file	missing file
before (unguarded)	1.54 µs/call	1.38 µs/call
validatePathWithinRoot with allowSymlinkEscape	105.77 µs/call	153.24 µs/call
lexical containment only	1.86 µs/call	1.91 µs/call

The missing-file column is worse because the realpath pair goes through the ENOENT throw path and building the exception dominates.

The symlink half is also the wrong check on this path: indexing deliberately follows in-root symlinks whose targets escape the root (#935) — that is why allowSymlinkEscape exists — so all that is actually wanted here is the lexical ../ refusal. The PR splits that half out as lexicalPathWithinRoot and has validatePathWithinRoot call it, so the containment rule stays in one place.

Your reproduction held up exactly as written, and your scoping of the severity did too — no read, no edge, just the probe. The regression test asserts the escaping target really exists before asserting fileExists returns false, so the false can only come from the guard; a third case pins the #935 symlink behaviour so this change can't quietly close it off.