mcp: sandbox get_agent_file to allowed editor roots#1449
Open
Evanfeenstra wants to merge 1 commit into
Open
Conversation
get_agent_file served any absolute path via res.sendFile with no containment check, allowing arbitrary file reads. Route the path through resolveInCwd against editorRoots (/tmp cloned repos, OS scratch dir, and AGENT_ARTIFACTS_DIR), returning 403 for paths that escape those roots.
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
1 participant
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.
Problem
get_agent_file(GET /repo/agent/file) served any absolute path viares.sendFilewith no containment check — an arbitrary file read (e.g.?path=/etc/passwd, secrets, etc.). The route also has no auth middleware.Fix
Route the requested path through the existing sandbox primitive
resolveInCwdagainsteditorRoots— the same roots the editor tool is already confined to:/tmp(cloned repos live at/tmp/owner/repo)os.tmpdir()(scratch)AGENT_ARTIFACTS_DIR(durable artifacts, when set)Paths that escape those roots (absolute escapes /
../traversal) now return403.editorRootswas made exported to reuse it.Notes
resolveInCwd(already covered bytextEdit.test.ts).npx tsc --noEmitpasses.