Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/git/src/mcp_server_git/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
)
from enum import Enum
import git
from git.exc import BadName
from pydantic import BaseModel, Field

# Default number of context lines to show in diff output
Expand Down Expand Up @@ -119,7 +120,7 @@ def git_diff(repo: git.Repo, target: str, context_lines: int = DEFAULT_CONTEXT_L
# 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 '-'")
raise 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)

Expand Down Expand Up @@ -187,7 +188,7 @@ 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 '-'")
raise 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}'"
Expand Down
19 changes: 10 additions & 9 deletions src/git/tests/test_server.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import pytest
from pathlib import Path
import git
from git.exc import BadName
from mcp_server_git.server import (
git_checkout,
git_branch,
Expand Down Expand Up @@ -40,7 +41,7 @@ def test_git_checkout_existing_branch(test_repository):

def test_git_checkout_nonexistent_branch(test_repository):

with pytest.raises(git.exc.BadName):
with pytest.raises(BadName):
git_checkout(test_repository, "nonexistent-branch")

def test_git_branch_local(test_repository):
Expand Down Expand Up @@ -316,25 +317,25 @@ def test_validate_repo_path_symlink_escape(tmp_path: Path):

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):
with pytest.raises(BadName):
git_diff(test_repository, "--output=/tmp/evil")

with pytest.raises(git.exc.BadName):
with pytest.raises(BadName):
git_diff(test_repository, "--help")

with pytest.raises(git.exc.BadName):
with pytest.raises(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):
with pytest.raises(BadName):
git_checkout(test_repository, "--help")

with pytest.raises(git.exc.BadName):
with pytest.raises(BadName):
git_checkout(test_repository, "--orphan=evil")

with pytest.raises(git.exc.BadName):
with pytest.raises(BadName):
git_checkout(test_repository, "-f")


Expand Down Expand Up @@ -398,7 +399,7 @@ def test_git_diff_rejects_malicious_refs(test_repository):
malicious_ref_path.write_text(sha)

# Even though the ref exists, it should be rejected
with pytest.raises(git.exc.BadName):
with pytest.raises(BadName):
git_diff(test_repository, "--output=evil.txt")

# Verify no file was created (the attack was blocked)
Expand All @@ -417,7 +418,7 @@ def test_git_checkout_rejects_malicious_refs(test_repository):
malicious_ref_path.write_text(sha)

# Even though the ref exists, it should be rejected
with pytest.raises(git.exc.BadName):
with pytest.raises(BadName):
git_checkout(test_repository, "--orphan=evil")

# Cleanup
Expand Down