colbymchenry/codegraph PR #384 — Path traversal security fix — PR #384
colbymchenry/codegraph · pull request #384 ·
Transcript
PlainEnglish
Let's walk through PR 384 for CodeGraph. This pull request closes a symlink escape vulnerability by unifying path validation into a two-stage security check.
PlainEnglish
Here's the problem we're solving. The existing path validation only checked logical paths using path dot resolve, which meant it missed symlink-based attacks entirely. There was a second function designed for symlink-aware checks, but it turned out that function was never actually called anywhere in the codebase. So an attacker with write access inside the project could plant a symlink pointing outside the project boundary — say to slash etc slash passwd — and the validation would let it through.
Architecture
Let's zoom out and see where the changes land. The core security fix lives in source utils dot T S, where we're rewriting the path validation logic. We're also adding a suite of symlink escape tests in the security test file. And there are three new documentation files in Japanese that record the implementation and describe a fork effort — those are fork-specific and not part of the security patch itself.
Architecture
First we add a helper called pathStartsWith that handles cross-platform path comparison correctly. On Windows, it lowercases both paths because C colon backslash Proj and lowercase c colon backslash proj are the same directory. And it checks for an exact match or a match followed by a path separator, which prevents the classic edge case where slash foo would incorrectly match slash foobar.
Architecture
Here's how the new validation works. Stage one does a cheap logical check using path dot resolve to reject relative dot-dot-slash escapes. Stage two calls F S dot realpathSync on both the candidate path and the project root, which resolves any symlinks to their actual filesystem locations. Then we compare the canonical paths with our helper, and that blocks both logical and symlink-based escapes.
Architecture
The error handling is careful here. If realpath sync throws E NO ENT, meaning the file doesn't exist yet, we fall back to the logical path so that indexing operations on not-yet-created files still work. But if we get any other error — like E LOOP for circular symlinks or E ACC E S for permission issues — we conservatively reject the path and return null.
CodeQuality
The PR adds seven new test cases to the security suite. We're testing rejection of relative dot-dot escapes, rejection of absolute paths outside the root, and — most importantly — rejection of symlinks pointing outside the project, which is the attack vector we're closing. We also verify that symlinks inside the project are still allowed, that nested paths work, that non-existent files are handled correctly, and that Windows case-insensitive roots behave as expected.
PlainEnglish
So here's what we've accomplished. Before this PR, we only had logical path dot resolve checks, and the symlink-aware function existed but was never used — meaning symlinks pointing to slash etc slash passwd would pass validation. After this PR, we have unified two-stage validation that catches both logical and symlink escapes, the dead code is removed, and every MCP tool entry point and file I O operation is now protected.
CodeQuality
One thing to keep an eye on. The error handling types the caught exception as unknown and extracts the code property with optional chaining. If something unusual gets thrown — like a non-Error object — the code becomes undefined and we reject the path. That's conservatively safe, but it means rare edge cases get treated as security violations rather than just being allowed. The comment on line six only documents E NO ENT; it doesn't mention that we're also conservatively rejecting E LOOP, E ACC E S, and any other error. The behavior is correct, it's just underdocumented.
CodeQuality
The other thing worth noting is a performance opportunity. Right now, validatePathWithinRoot calls F S dot realpathSync on the project root every single time it's invoked, even though the project root is almost always the same value. During indexing, this function gets called thousands of times, so we're making thousands of redundant filesystem syscalls to resolve the same root path over and over. Caching the real root keyed by project root would eliminate that overhead. It's not a correctness bug — the logic is safe — but it's a hot path that could be optimized.
How this was made
Lenzon read colbymchenry/codegraph at pull request #384 and generated this walkthrough automatically. The narration above is the transcript of what it says.
Explain a pull request from your own repo
Point Lenzon at a repo or a pull request and get a narrated walkthrough like this one.
Try it