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.

Author: ElliotJLT

src/git/src/mcp_server_git/server.py

@@ -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]:
     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
         args = []
         if start_timestamp:
@@ -177,6 +182,11 @@ def git_log(repo: git.Repo, max_count: int = 10, start_timestamp: Optional[str]
         return log
 
 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:
         base = repo.references[base_branch]
     else:
@@ -197,6 +207,10 @@ def git_checkout(repo: git.Repo, branch_name: 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)
     output = [
         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:
+    # 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:
         case None:
             contains_sha = (None,)

src/git/tests/test_server.py

@@ -423,3 +423,62 @@ def test_git_checkout_rejects_malicious_refs(test_repository):
 
     # Cleanup
     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")