mirror of
https://github.com/modelcontextprotocol/servers.git
synced 2026-04-27 16:25:14 +02:00
fix(git): add missing argument injection guards to git_show, git_create_branch, git_log, and git_branch (#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.
This commit is contained in:
@@ -142,6 +142,11 @@ def git_reset(repo: git.Repo) -> str:
|
|||||||
|
|
||||||
def git_log(repo: git.Repo, max_count: int = 10, start_timestamp: Optional[str] = None, end_timestamp: Optional[str] = None) -> list[str]:
|
def git_log(repo: git.Repo, max_count: int = 10, start_timestamp: Optional[str] = None, end_timestamp: Optional[str] = None) -> list[str]:
|
||||||
if start_timestamp or end_timestamp:
|
if start_timestamp or end_timestamp:
|
||||||
|
# Defense in depth: reject timestamps starting with '-' to prevent flag injection
|
||||||
|
if start_timestamp and start_timestamp.startswith("-"):
|
||||||
|
raise ValueError(f"Invalid start_timestamp: '{start_timestamp}' - cannot start with '-'")
|
||||||
|
if end_timestamp and end_timestamp.startswith("-"):
|
||||||
|
raise ValueError(f"Invalid end_timestamp: '{end_timestamp}' - cannot start with '-'")
|
||||||
# Use git log command with date filtering
|
# Use git log command with date filtering
|
||||||
args = []
|
args = []
|
||||||
if start_timestamp:
|
if start_timestamp:
|
||||||
@@ -177,6 +182,11 @@ def git_log(repo: git.Repo, max_count: int = 10, start_timestamp: Optional[str]
|
|||||||
return log
|
return log
|
||||||
|
|
||||||
def git_create_branch(repo: git.Repo, branch_name: str, base_branch: str | None = None) -> str:
|
def git_create_branch(repo: git.Repo, branch_name: str, base_branch: str | None = None) -> str:
|
||||||
|
# Defense in depth: reject names starting with '-' to prevent flag injection
|
||||||
|
if branch_name.startswith("-"):
|
||||||
|
raise BadName(f"Invalid branch name: '{branch_name}' - cannot start with '-'")
|
||||||
|
if base_branch and base_branch.startswith("-"):
|
||||||
|
raise BadName(f"Invalid base branch: '{base_branch}' - cannot start with '-'")
|
||||||
if base_branch:
|
if base_branch:
|
||||||
base = repo.references[base_branch]
|
base = repo.references[base_branch]
|
||||||
else:
|
else:
|
||||||
@@ -197,6 +207,10 @@ def git_checkout(repo: git.Repo, branch_name: str) -> str:
|
|||||||
|
|
||||||
|
|
||||||
def git_show(repo: git.Repo, revision: str) -> str:
|
def git_show(repo: git.Repo, revision: str) -> str:
|
||||||
|
# Defense in depth: reject revisions starting with '-' to prevent flag injection,
|
||||||
|
# even if a malicious ref with that name exists (e.g. via filesystem manipulation)
|
||||||
|
if revision.startswith("-"):
|
||||||
|
raise BadName(f"Invalid revision: '{revision}' - cannot start with '-'")
|
||||||
commit = repo.commit(revision)
|
commit = repo.commit(revision)
|
||||||
output = [
|
output = [
|
||||||
f"Commit: {commit.hexsha!r}\n"
|
f"Commit: {commit.hexsha!r}\n"
|
||||||
@@ -241,6 +255,12 @@ def validate_repo_path(repo_path: Path, allowed_repository: Path | None) -> None
|
|||||||
|
|
||||||
|
|
||||||
def git_branch(repo: git.Repo, branch_type: str, contains: str | None = None, not_contains: str | None = None) -> str:
|
def git_branch(repo: git.Repo, branch_type: str, contains: str | None = None, not_contains: str | None = None) -> str:
|
||||||
|
# Defense in depth: reject values starting with '-' to prevent flag injection
|
||||||
|
if contains and contains.startswith("-"):
|
||||||
|
raise BadName(f"Invalid contains value: '{contains}' - cannot start with '-'")
|
||||||
|
if not_contains and not_contains.startswith("-"):
|
||||||
|
raise BadName(f"Invalid not_contains value: '{not_contains}' - cannot start with '-'")
|
||||||
|
|
||||||
match contains:
|
match contains:
|
||||||
case None:
|
case None:
|
||||||
contains_sha = (None,)
|
contains_sha = (None,)
|
||||||
|
|||||||
@@ -423,3 +423,62 @@ def test_git_checkout_rejects_malicious_refs(test_repository):
|
|||||||
|
|
||||||
# Cleanup
|
# Cleanup
|
||||||
malicious_ref_path.unlink()
|
malicious_ref_path.unlink()
|
||||||
|
|
||||||
|
|
||||||
|
# Tests for argument injection protection in git_show, git_create_branch,
|
||||||
|
# git_log, and git_branch — matching the existing guards on git_diff and
|
||||||
|
# git_checkout.
|
||||||
|
|
||||||
|
def test_git_show_rejects_flag_injection(test_repository):
|
||||||
|
"""git_show should reject revisions starting with '-'."""
|
||||||
|
with pytest.raises(BadName):
|
||||||
|
git_show(test_repository, "--output=/tmp/evil")
|
||||||
|
|
||||||
|
with pytest.raises(BadName):
|
||||||
|
git_show(test_repository, "-p")
|
||||||
|
|
||||||
|
|
||||||
|
def test_git_show_rejects_malicious_refs(test_repository):
|
||||||
|
"""git_show should reject refs starting with '-' even if they exist."""
|
||||||
|
sha = test_repository.head.commit.hexsha
|
||||||
|
refs_dir = Path(test_repository.git_dir) / "refs" / "heads"
|
||||||
|
malicious_ref_path = refs_dir / "--format=evil"
|
||||||
|
malicious_ref_path.write_text(sha)
|
||||||
|
|
||||||
|
with pytest.raises(BadName):
|
||||||
|
git_show(test_repository, "--format=evil")
|
||||||
|
|
||||||
|
malicious_ref_path.unlink()
|
||||||
|
|
||||||
|
|
||||||
|
def test_git_create_branch_rejects_flag_injection(test_repository):
|
||||||
|
"""git_create_branch should reject branch names starting with '-'."""
|
||||||
|
with pytest.raises(BadName):
|
||||||
|
git_create_branch(test_repository, "--track=evil")
|
||||||
|
|
||||||
|
with pytest.raises(BadName):
|
||||||
|
git_create_branch(test_repository, "-f")
|
||||||
|
|
||||||
|
|
||||||
|
def test_git_create_branch_rejects_base_branch_flag_injection(test_repository):
|
||||||
|
"""git_create_branch should reject base branch names starting with '-'."""
|
||||||
|
with pytest.raises(BadName):
|
||||||
|
git_create_branch(test_repository, "new-branch", "--track=evil")
|
||||||
|
|
||||||
|
|
||||||
|
def test_git_log_rejects_timestamp_flag_injection(test_repository):
|
||||||
|
"""git_log should reject timestamps starting with '-'."""
|
||||||
|
with pytest.raises(ValueError):
|
||||||
|
git_log(test_repository, start_timestamp="--exec=evil")
|
||||||
|
|
||||||
|
with pytest.raises(ValueError):
|
||||||
|
git_log(test_repository, end_timestamp="--exec=evil")
|
||||||
|
|
||||||
|
|
||||||
|
def test_git_branch_rejects_contains_flag_injection(test_repository):
|
||||||
|
"""git_branch should reject contains/not_contains values starting with '-'."""
|
||||||
|
with pytest.raises(BadName):
|
||||||
|
git_branch(test_repository, "local", contains="--exec=evil")
|
||||||
|
|
||||||
|
with pytest.raises(BadName):
|
||||||
|
git_branch(test_repository, "local", not_contains="--exec=evil")
|
||||||
|
|||||||
Reference in New Issue
Block a user