Merge commit from fork

Add validation to reject arguments starting with '-' and verify
arguments resolve to valid git refs via rev_parse before passing
to git CLI commands. This prevents flag-like values from being
interpreted as command-line options (e.g., --output=/path/to/file).

CWE-88: Improper Neutralization of Argument Delimiters in a Command

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <noreply@anthropic.com>

Author: jenn-newton

src/git/src/mcp_server_git/server.py

@@ -116,6 +116,11 @@ def git_diff_staged(repo: git.Repo, context_lines: int = DEFAULT_CONTEXT_LINES)
     return repo.git.diff(f"--unified={context_lines}", "--cached")
 
 def git_diff(repo: git.Repo, target: str, context_lines: int = DEFAULT_CONTEXT_LINES) -> str:
+    # Defense in depth: reject targets starting with '-' to prevent flag injection,
+    # even if a malicious ref with that name exists (e.g. via filesystem manipulation)
+    if target.startswith("-"):
+        raise git.exc.BadName(f"Invalid target: '{target}' - cannot start with '-'")
+    repo.rev_parse(target)  # Validates target is a real git ref, throws BadName if not
     return repo.git.diff(f"--unified={context_lines}", target)
 
 def git_commit(repo: git.Repo, message: str) -> str:
@@ -179,6 +184,11 @@ def git_create_branch(repo: git.Repo, branch_name: str, base_branch: str | None
     return f"Created branch '{branch_name}' from '{base.name}'"
 
 def git_checkout(repo: git.Repo, branch_name: str) -> str:
+    # Defense in depth: reject branch names starting with '-' to prevent flag injection,
+    # even if a malicious ref with that name exists (e.g. via filesystem manipulation)
+    if branch_name.startswith("-"):
+        raise git.exc.BadName(f"Invalid branch name: '{branch_name}' - cannot start with '-'")
+    repo.rev_parse(branch_name)  # Validates branch_name is a real git ref, throws BadName if not
     repo.git.checkout(branch_name)
     return f"Switched to branch '{branch_name}'"
 

src/git/tests/test_server.py

@@ -39,7 +39,7 @@ def test_git_checkout_existing_branch(test_repository):
 
 def test_git_checkout_nonexistent_branch(test_repository):
 
-    with pytest.raises(git.GitCommandError):
+    with pytest.raises(git.exc.BadName):
         git_checkout(test_repository, "nonexistent-branch")
 
 def test_git_branch_local(test_repository):
@@ -248,3 +248,115 @@ def test_git_show_initial_commit(test_repository):
     assert "Commit:" in result
     assert "initial commit" in result
     assert "test.txt" in result
+
+
+# Tests for argument injection protection
+
+def test_git_diff_rejects_flag_injection(test_repository):
+    """git_diff should reject flags that could be used for argument injection."""
+    with pytest.raises(git.exc.BadName):
+        git_diff(test_repository, "--output=/tmp/evil")
+
+    with pytest.raises(git.exc.BadName):
+        git_diff(test_repository, "--help")
+
+    with pytest.raises(git.exc.BadName):
+        git_diff(test_repository, "-p")
+
+
+def test_git_checkout_rejects_flag_injection(test_repository):
+    """git_checkout should reject flags that could be used for argument injection."""
+    with pytest.raises(git.exc.BadName):
+        git_checkout(test_repository, "--help")
+
+    with pytest.raises(git.exc.BadName):
+        git_checkout(test_repository, "--orphan=evil")
+
+    with pytest.raises(git.exc.BadName):
+        git_checkout(test_repository, "-f")
+
+
+def test_git_diff_allows_valid_refs(test_repository):
+    """git_diff should work normally with valid git refs."""
+    # Get the default branch name
+    default_branch = test_repository.active_branch.name
+
+    # Create a branch with a commit for diffing
+    test_repository.git.checkout("-b", "valid-diff-branch")
+    file_path = Path(test_repository.working_dir) / "test.txt"
+    file_path.write_text("valid diff content")
+    test_repository.index.add(["test.txt"])
+    test_repository.index.commit("valid diff commit")
+
+    # Test with branch name
+    result = git_diff(test_repository, default_branch)
+    assert "test.txt" in result
+
+    # Test with HEAD~1
+    result = git_diff(test_repository, "HEAD~1")
+    assert "test.txt" in result
+
+    # Test with commit hash
+    commit_sha = test_repository.head.commit.hexsha
+    result = git_diff(test_repository, commit_sha)
+    assert result is not None
+
+
+def test_git_checkout_allows_valid_branches(test_repository):
+    """git_checkout should work normally with valid branch names."""
+    # Get the default branch name
+    default_branch = test_repository.active_branch.name
+
+    # Create a branch to checkout
+    test_repository.git.branch("valid-checkout-branch")
+
+    result = git_checkout(test_repository, "valid-checkout-branch")
+    assert "Switched to branch 'valid-checkout-branch'" in result
+    assert test_repository.active_branch.name == "valid-checkout-branch"
+
+    # Checkout back to default branch
+    result = git_checkout(test_repository, default_branch)
+    assert "Switched to branch" in result
+    assert test_repository.active_branch.name == default_branch
+
+
+def test_git_diff_rejects_malicious_refs(test_repository):
+    """git_diff should reject refs starting with '-' even if they exist.
+
+    This tests defense in depth against an attacker who creates malicious
+    refs via filesystem manipulation (e.g. using mcp-filesystem to write
+    to .git/refs/heads/--output=...).
+    """
+    import os
+
+    # Manually create a malicious ref by writing directly to .git/refs
+    sha = test_repository.head.commit.hexsha
+    refs_dir = Path(test_repository.git_dir) / "refs" / "heads"
+    malicious_ref_path = refs_dir / "--output=evil.txt"
+    malicious_ref_path.write_text(sha)
+
+    # Even though the ref exists, it should be rejected
+    with pytest.raises(git.exc.BadName):
+        git_diff(test_repository, "--output=evil.txt")
+
+    # Verify no file was created (the attack was blocked)
+    assert not os.path.exists("evil.txt")
+
+    # Cleanup
+    malicious_ref_path.unlink()
+
+
+def test_git_checkout_rejects_malicious_refs(test_repository):
+    """git_checkout should reject refs starting with '-' even if they exist."""
+    # Manually create a malicious ref
+    sha = test_repository.head.commit.hexsha
+    refs_dir = Path(test_repository.git_dir) / "refs" / "heads"
+    malicious_ref_path = refs_dir / "--orphan=evil"
+    malicious_ref_path.write_text(sha)
+
+    # Even though the ref exists, it should be rejected
+    with pytest.raises(git.exc.BadName):
+        git_checkout(test_repository, "--orphan=evil")
+
+    # Cleanup
+    malicious_ref_path.unlink()