fix(git): add missing argument injection guards#3545
fix(git): add missing argument injection guards#3545olaservo merged 1 commit intomodelcontextprotocol:mainfrom
Conversation
…te_branch, git_log, and git_branch
git_diff and git_checkout already reject user-supplied values starting
with '-' to prevent flag injection (even when a malicious ref exists via
filesystem manipulation). The same defense-in-depth pattern was missing
from four other functions:
- git_show: revision parameter passed directly to repo.commit()
- git_create_branch: branch_name and base_branch unchecked
- git_log: start_timestamp and end_timestamp passed to --since/--until
- git_branch: contains and not_contains passed as raw args to repo.git.branch()
Adds the same startswith("-") guards with matching tests for each function.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
olaservo
left a comment
olaservo
left a comment
There was a problem hiding this comment.
Solid security fix that closes real gaps in argument injection protection. The threat model is well-understood — GitPython shells out to git, so user-supplied values starting with - can be interpreted as CLI flags. The existing guards on git_diff and git_checkout already acknowledge this; this PR extends the same pattern to the four remaining vulnerable functions.
All four guard placements are correct:
git_show: guardsrevisionbeforerepo.commit()git_create_branch: guardsbranch_nameandbase_branchbeforerepo.references[]git_log: guardsstart_timestampandend_timestampbeforerepo.git.log()git_branch: guardscontainsandnot_containsbefore they reachrepo.git.branch()
Using ValueError for timestamps (instead of BadName) is a reasonable choice since timestamps aren't git refs. Test coverage is thorough — 6 new tests including a malicious-ref-on-disk scenario for git_show.
LGTM — thanks @ElliotJLT!
Reviewed with the assistance of Claude Code (claude-opus-4-6). Claims verified against the existing codebase and CI results.
…te_branch, git_log, and git_branch (modelcontextprotocol#3545) fix(git): add missing argument injection guards Extends existing startswith("-") input validation to git_show, git_create_branch, git_log, and git_branch, preventing user-supplied values from being interpreted as CLI flags by GitPython's subprocess calls to git.
2 participants
Summary
git_diffandgit_checkoutalready reject user inputs starting with-to block flag injection attacks — but four other functions lacked the same protection:git_show,git_create_branch,git_log(timestamps), andgit_branch(contains/not_contains)Context
An attacker with filesystem write access (e.g. via
mcp-filesystem) could create malicious git refs like--format=evilunder.git/refs/heads/. Without the leading-dash check, these values would be passed as flags to git commands via GitPython. The existing guards ongit_diffandgit_checkout(+ tests for malicious refs) demonstrate this threat model is already recognised — this PR closes the remaining gaps.Test plan
git_show,git_create_branch,git_log, andgit_branchgit_showrejects malicious refs even when they exist on disk🤖 Generated with Claude Code