fix(security): block path traversal via symlinks in /api/file and read_file (RAN-8)#65
Merged
Merged
Conversation
…d_file (RAN-8) Resolve symlinks with `Path.toRealPath()` and re-check the resolved path against the codebase root on both the REST `/api/file` endpoint and the MCP `read_file` tool. `Path.normalize()` is purely lexical and left symlinks inside the indexed repo usable for exfiltrating off-tree files (e.g. `link -> /etc/passwd`). - GraphController: canonicalize root, lexical guard, then toRealPath() and re-check; 404 on NoSuchFileException, 403 on out-of-root. - McpTools: same two-stage guard, returns "Path traversal detected". - Tests: positive (escape symlink rejected) + negative (in-repo symlink read succeeds) for both REST and MCP. Skip gracefully on filesystems without symlink support. Co-Authored-By: Paperclip <noreply@paperclip.ing> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+265
to
+267
| return ResponseEntity.status(500) | ||
| .contentType(MediaType.TEXT_PLAIN) | ||
| .body("Failed to resolve codebase root: " + e.getMessage()); |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/api/file(REST) andread_file(MCP) against path-traversal via symbolic links: the previous lexicalnormalize() + startsWith(root)guard did not resolve symlinks, so an in-repo link pointing at e.g./etc/passwdwould pass the check and then leak the target's contents.../traversal, thenPath.toRealPath()and re-check against a canonicalized codebase root.NoSuchFileExceptionnow maps to 404 on REST (was previously "isRegularFile false → 404"), preserving existing behaviour.Changes
api/GraphController.java— canonicalize root withtoRealPath(), lexical guard, thencandidate.toRealPath()and re-check. 403 on escape, 404 on missing.mcp/McpTools.java— same guard; returns{"error":"Path traversal detected"}on escape.GraphControllerTest:readFileShouldRejectSymlinkEscapingRoot(positive reject) +readFileShouldAllowInRepoSymlink(legit symlink still works).McpToolsTest: same pair for the MCP side. Both skip cleanly on filesystems that don't support symlinks.Test plan
mvn -Dskip.npm=true test→ 3399 run, 0 failures, 0 errorsreadFile*runs pass (7 GraphController + 9 McpTools)../../../etc/passwdlexical traversal tests still passFixes RAN-8.
🤖 Generated with Claude Code