Repository navigation
Resolution: fileExists probes paths outside the project root (uncontained path.join fallback) #1631
Description
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.txtResult: 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
validatePathWithinRootwork 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).
Activity
maxmilian commentedon Aug 28, 2026
Confirmed on
6a056ecand 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.
validatePathWithinRootdoes tworealpathSynccalls per invocation, andfileExistsruns per import candidate. Over 20,000 calls:validatePathWithinRootwithallowSymlinkEscapeThe missing-file column is worse because the realpath pair goes through the
ENOENTthrow 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
allowSymlinkEscapeexists — so all that is actually wanted here is the lexical../refusal. The PR splits that half out aslexicalPathWithinRootand hasvalidatePathWithinRootcall 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
fileExistsreturns false, so thefalsecan only come from the guard; a third case pins the #935 symlink behaviour so this change can't quietly close it off.fix(resolution): keep the existence probe inside the project root (co…
fix(resolution): keep the existence probe inside the project root (co…
fix(resolution): keep the existence probe inside the project root (co…
fix(resolution): keep the existence probe inside the project root (#1631
colbymchenry commentedon Sep 8, 2026
Closed via #1632 (merged to
main@a6f52d7).Contributor fix from @maxmilian — rebased onto current
main(post-#1749), approved after Linux fail→pass on__tests__/resolution-fileexists-containment.test.ts, then squash-merged.Summary of the landed fix:
fileExistsfilesystem fallback now goes throughlexicalPathWithinRoot(cheap../refusal only — not fullvalidatePathWithinRoot/ realpath).perf(resolution): stop re-reading resolved targets and re-probing mis…
perf(resolution): remove the indexing regressions since v1.6.0 (vscod…