fix(security): resolve path traversal bypass in evaluateFilePath#288
Open
pbbadenhorst wants to merge 1 commit intomksglu:nextfrom
Open
fix(security): resolve path traversal bypass in evaluateFilePath#288pbbadenhorst wants to merge 1 commit intomksglu:nextfrom
pbbadenhorst wants to merge 1 commit intomksglu:nextfrom
Conversation
Read deny patterns were matched only against the raw input string, so absolute-path globs like Read(/Users/you/.ssh/**) could be bypassed by passing a relative path with `..` segments that resolved into the same location (e.g. ../../.ssh/id_rsa from a project nested under $HOME). evaluateFilePath now accepts an optional projectRoot and matches deny globs against both the raw input and the fully-resolved absolute path. checkFilePathDenyPolicy in server.ts passes CLAUDE_PROJECT_DIR (falling back to process.cwd()) so the resolved form is checked for ctx_execute_file and any other tool routed through the policy. Backwards compatible: callers that don't pass projectRoot see identical behavior — covered by a regression-guard test alongside the new bypass test.
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
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.
What / Why / How
What:
evaluateFilePath(and thereforecheckFilePathDenyPolicyforctx_execute_file/ Read-routed tools) now matches deny globs against both the raw input string and the fully-resolved absolute path.Why: Read deny patterns were matched only against the raw input. An absolute-path deny rule like
Read(/Users/you/.ssh/**)could be bypassed by passing a relative path with..segments that resolves into the same location — e.g.../../.ssh/id_rsafrom a project nested under$HOME. A prompt-injectedctx_execute_filecall could then read SSH keys, AWS creds,.envfiles, etc. that the user explicitly tried to deny.How:
evaluateFilePath(src/security.ts) gains an optional 4th parameterprojectRoot. When supplied, the path is also resolved viapath.resolve(projectRoot, filePath)and the resolved form is matched against every deny glob in addition to the raw input.checkFilePathDenyPolicy(src/server.ts) passesprocess.env.CLAUDE_PROJECT_DIR ?? process.cwd()so the resolved form is checked at runtime.projectRootsee identical behavior. A regression-guard test documents that pre-fix behavior so any future change is intentional.Affected platforms
(The fix is in shared security/server code that all adapters route through.)
Test plan
Added two tests to
tests/security.test.tsin the existingevaluateFilePathdescribe block:traversal does not bypass absolute deny glob when projectRoot is supplied— the bypass case. WithprojectRoot = ~/some/projectand deny glob~/.ssh/**, asserts that../../.ssh/id_rsais denied. Fails before the fix, passes after.without projectRoot, absolute deny glob is still bypassable (regression guard)— pins the legacy 3-arg behavior so future changes to it are deliberate.Also verified:
npm test→ 1540 passed / 17 skipped / 0 failed (46 files)npm run typecheck→ cleanChecklist
npm testpassesnpm run typecheckpassespath.resolveis platform-aware)nextbranch