diff --git a/src/git/src/mcp_server_git/server.py b/src/git/src/mcp_server_git/server.py index 5ce953e545..7f2305ffac 100644 --- a/src/git/src/mcp_server_git/server.py +++ b/src/git/src/mcp_server_git/server.py @@ -255,6 +255,26 @@ def validate_repo_path(repo_path: Path, allowed_repository: Path | None) -> None ) +def validate_repo_path_against_allowed_repositories( + repo_path: Path, allowed_repositories: Sequence[Path] +) -> None: + """Validate that repo_path is within at least one allowed repository path.""" + if not allowed_repositories: + raise ValueError("Repository path is not within the current allowed roots") + + for allowed_repository in allowed_repositories: + try: + validate_repo_path(repo_path, allowed_repository) + return + except ValueError: + continue + + allowed_list = ", ".join(str(path) for path in allowed_repositories) + raise ValueError( + f"Repository path '{repo_path}' is outside the allowed repositories: {allowed_list}" + ) + + 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("-"): @@ -472,9 +492,17 @@ def by_commandline() -> Sequence[str]: @server.call_tool() async def call_tool(name: str, arguments: dict) -> list[TextContent]: repo_path = Path(arguments["repo_path"]) + allowed_repositories: list[Path] = [] + if repository is not None: + allowed_repositories.append(repository) + allowed_repositories.extend(Path(path) for path in await by_roots()) - # Validate repo_path is within allowed repository - validate_repo_path(repo_path, repository) + if allowed_repositories: + validate_repo_path_against_allowed_repositories(repo_path, allowed_repositories) + else: + # Preserve the legacy unrestricted behavior when no command-line + # repository restriction or root-derived allowlist is available. + validate_repo_path(repo_path, repository) # For all commands, we need an existing repo repo = git.Repo(repo_path) diff --git a/src/git/tests/test_server.py b/src/git/tests/test_server.py index a5492adc85..857069eb62 100644 --- a/src/git/tests/test_server.py +++ b/src/git/tests/test_server.py @@ -16,6 +16,7 @@ git_create_branch, git_show, validate_repo_path, + validate_repo_path_against_allowed_repositories, ) import shutil @@ -313,6 +314,38 @@ def test_validate_repo_path_symlink_escape(tmp_path: Path): with pytest.raises(ValueError) as exc_info: validate_repo_path(symlink, allowed) assert "outside the allowed repository" in str(exc_info.value) + + +def test_validate_repo_path_against_allowed_repositories_exact_match(tmp_path: Path): + allowed_a = tmp_path / "allowed_a" + allowed_b = tmp_path / "allowed_b" + allowed_a.mkdir() + allowed_b.mkdir() + + validate_repo_path_against_allowed_repositories(allowed_b, [allowed_a, allowed_b]) + + +def test_validate_repo_path_against_allowed_repositories_rejects_removed_root(tmp_path: Path): + allowed_a = tmp_path / "allowed_a" + allowed_b = tmp_path / "allowed_b" + stale_repo = allowed_a / "repo" + allowed_a.mkdir() + allowed_b.mkdir() + stale_repo.mkdir() + + with pytest.raises(ValueError) as exc_info: + validate_repo_path_against_allowed_repositories(stale_repo, [allowed_b]) + assert "outside the allowed repositories" in str(exc_info.value) + + +def test_validate_repo_path_against_allowed_repositories_rejects_empty_roots(tmp_path: Path): + repo = tmp_path / "repo" + repo.mkdir() + + with pytest.raises(ValueError) as exc_info: + validate_repo_path_against_allowed_repositories(repo, []) + assert "current allowed roots" in str(exc_info.value) + # Tests for argument injection protection def test_git_diff_rejects_flag_injection(test_repository):