mirror of
https://github.com/modelcontextprotocol/servers.git
synced 2026-04-17 23:53:24 +02:00
Merge pull request #3139 from modelcontextprotocol/fix-git-server-pyright-errors
fix(git): import BadName directly to fix pyright errors
This commit is contained in:
@@ -13,6 +13,7 @@ from mcp.types import (
|
|||||||
)
|
)
|
||||||
from enum import Enum
|
from enum import Enum
|
||||||
import git
|
import git
|
||||||
|
from git.exc import BadName
|
||||||
from pydantic import BaseModel, Field
|
from pydantic import BaseModel, Field
|
||||||
|
|
||||||
# Default number of context lines to show in diff output
|
# Default number of context lines to show in diff output
|
||||||
@@ -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,
|
# 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)
|
# even if a malicious ref with that name exists (e.g. via filesystem manipulation)
|
||||||
if target.startswith("-"):
|
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
|
repo.rev_parse(target) # Validates target is a real git ref, throws BadName if not
|
||||||
return repo.git.diff(f"--unified={context_lines}", target)
|
return repo.git.diff(f"--unified={context_lines}", target)
|
||||||
|
|
||||||
@@ -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,
|
# 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)
|
# even if a malicious ref with that name exists (e.g. via filesystem manipulation)
|
||||||
if branch_name.startswith("-"):
|
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.rev_parse(branch_name) # Validates branch_name is a real git ref, throws BadName if not
|
||||||
repo.git.checkout(branch_name)
|
repo.git.checkout(branch_name)
|
||||||
return f"Switched to branch '{branch_name}'"
|
return f"Switched to branch '{branch_name}'"
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
import pytest
|
import pytest
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
import git
|
import git
|
||||||
|
from git.exc import BadName
|
||||||
from mcp_server_git.server import (
|
from mcp_server_git.server import (
|
||||||
git_checkout,
|
git_checkout,
|
||||||
git_branch,
|
git_branch,
|
||||||
@@ -40,7 +41,7 @@ def test_git_checkout_existing_branch(test_repository):
|
|||||||
|
|
||||||
def test_git_checkout_nonexistent_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")
|
git_checkout(test_repository, "nonexistent-branch")
|
||||||
|
|
||||||
def test_git_branch_local(test_repository):
|
def test_git_branch_local(test_repository):
|
||||||
@@ -316,25 +317,25 @@ def test_validate_repo_path_symlink_escape(tmp_path: Path):
|
|||||||
|
|
||||||
def test_git_diff_rejects_flag_injection(test_repository):
|
def test_git_diff_rejects_flag_injection(test_repository):
|
||||||
"""git_diff should reject flags that could be used for argument injection."""
|
"""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")
|
git_diff(test_repository, "--output=/tmp/evil")
|
||||||
|
|
||||||
with pytest.raises(git.exc.BadName):
|
with pytest.raises(BadName):
|
||||||
git_diff(test_repository, "--help")
|
git_diff(test_repository, "--help")
|
||||||
|
|
||||||
with pytest.raises(git.exc.BadName):
|
with pytest.raises(BadName):
|
||||||
git_diff(test_repository, "-p")
|
git_diff(test_repository, "-p")
|
||||||
|
|
||||||
|
|
||||||
def test_git_checkout_rejects_flag_injection(test_repository):
|
def test_git_checkout_rejects_flag_injection(test_repository):
|
||||||
"""git_checkout should reject flags that could be used for argument injection."""
|
"""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")
|
git_checkout(test_repository, "--help")
|
||||||
|
|
||||||
with pytest.raises(git.exc.BadName):
|
with pytest.raises(BadName):
|
||||||
git_checkout(test_repository, "--orphan=evil")
|
git_checkout(test_repository, "--orphan=evil")
|
||||||
|
|
||||||
with pytest.raises(git.exc.BadName):
|
with pytest.raises(BadName):
|
||||||
git_checkout(test_repository, "-f")
|
git_checkout(test_repository, "-f")
|
||||||
|
|
||||||
|
|
||||||
@@ -398,7 +399,7 @@ def test_git_diff_rejects_malicious_refs(test_repository):
|
|||||||
malicious_ref_path.write_text(sha)
|
malicious_ref_path.write_text(sha)
|
||||||
|
|
||||||
# Even though the ref exists, it should be rejected
|
# 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")
|
git_diff(test_repository, "--output=evil.txt")
|
||||||
|
|
||||||
# Verify no file was created (the attack was blocked)
|
# Verify no file was created (the attack was blocked)
|
||||||
@@ -417,7 +418,7 @@ def test_git_checkout_rejects_malicious_refs(test_repository):
|
|||||||
malicious_ref_path.write_text(sha)
|
malicious_ref_path.write_text(sha)
|
||||||
|
|
||||||
# Even though the ref exists, it should be rejected
|
# 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")
|
git_checkout(test_repository, "--orphan=evil")
|
||||||
|
|
||||||
# Cleanup
|
# Cleanup
|
||||||
|
|||||||
Reference in New Issue
Block a user