{{ message }}
Repository navigation
fix(lib): fail-closed upload containment — default UPLOAD_BASE_DIR to CWD - #367
Open
SavioBS629 wants to merge 3 commits into
Open
SavioBS629 wants to merge 3 commits into
SavioBS629 wants to merge 3 commits into
Conversation
… CWD Upload path containment was skipped entirely when MCP_UPLOAD_BASE_DIR was unset (the default), leaving arbitrary-path uploads open for allowlisted extensions. Containment now defaults to the process working directory at both layers: config.ts falls back to process.cwd(), and validateUploadPath() itself defaults a missing allowedBaseDir to process.cwd() so no caller can re-open the gap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ner, docs - Reject a defaulted base dir that resolves to the filesystem root: with cwd "/" (how some MCP clients launch stdio servers) every absolute path passes the startsWith containment check, silently allowing the whole filesystem. An explicitly configured root is honored as a deliberate opt-out. - Make the validator the single owner of the CWD default; config.ts passes MCP_UPLOAD_BASE_DIR through as undefined when unset so the validator can tell a defaulted base dir from a configured one. - Branch the realpath-failure error message: blame the working directory when nothing was configured, MCP_UPLOAD_BASE_DIR when it was. - Harden the default-containment test to mock process.cwd() instead of relying on tmpdir being outside the vitest cwd; add root-cwd rejection and explicit-root opt-out tests. - Document upload containment and MCP_UPLOAD_BASE_DIR in the README. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SavioBS629
marked this pull request as ready for review
August 10, 2026 09:29
SavioBS629
commented
Aug 10, 2026
SavioBS629
left a comment
SavioBS629
left a comment
Collaborator
Author
There was a problem hiding this comment.
Claude Code Review (automated) — 1 inline finding(s). Full report in the PR comment below. Verdict: Passed.
Collaborator
Author
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.
Summary
Upload path containment was skipped entirely when
MCP_UPLOAD_BASE_DIRwas unset (the default), leaving arbitrary-path uploads open for allowlisted extensions.Containment now defaults to the process working directory at both layers:
src/config.ts—UPLOAD_BASE_DIRfalls back toprocess.cwd()when the env var is unsetsrc/lib/upload-validator.ts—validateUploadPath()defaults a missingallowedBaseDirtoprocess.cwd(), so no caller can re-open the gapBehavior change
Uploads from outside the working directory now fail unless
MCP_UPLOAD_BASE_DIRis set. The rejection message tells the user how to widen the boundary.Testing
npm run buildgreen — lint, format, 275/275 tests (2 new: outside-CWD rejected by default, inside-CWD accepted), tsc.🤖 Generated with Claude Code