diff --git a/README.md b/README.md index e73ce0b4..c72349ea 100644 --- a/README.md +++ b/README.md @@ -350,7 +350,7 @@ Official integrations are maintained by companies building production ready MCP - **[Momento](https://github.com/momentohq/mcp-momento)** - Momento Cache lets you quickly improve your performance, reduce costs, and handle load at any scale. - Monday.com Logo **[Monday.com](https://github.com/mondaycom/mcp)** - Interact with Monday.com boards, items, accounts and work forms. - **[MongoDB](https://github.com/mongodb-js/mongodb-mcp-server)** - Both MongoDB Community Server and MongoDB Atlas are supported. -- Moorcheh Logo **[Moorcheh](https://github.com/moorcheh-ai/moorcheh-mcp)** - Embed, store, and search your documents, and build secure chatbots and RAG systems with Moorcheh's information-theoretic semantic search engine +- Moorcheh Logo **[Moorcheh](https://github.com/moorcheh-ai/moorcheh-mcp)** - Provides seamless integration with Moorcheh's Embedding, Vector Store, Search, and Gen AI Answer services. - MotherDuck Logo **[MotherDuck](https://github.com/motherduckdb/mcp-server-motherduck)** - Query and analyze data with MotherDuck and local DuckDB - Mulesoft Logo **[Mulesoft](https://www.npmjs.com/package/@mulesoft/mcp-server)** - Build, deploy, and manage MuleSoft applications with natural language, directly inside any compatible IDE. - Multiplayer Logo **[Multiplayer](https://www.multiplayer.app/docs/ai/mcp-server)** - Analyze your full stack session recordings easily. Record a bug with Multiplayer, analyze and fix it with LLM @@ -516,7 +516,7 @@ Official integrations are maintained by companies building production ready MCP - Todoist Logo **[Todoist](https://github.com/doist/todoist-ai)** - Search, add, and update [Todoist](https://todoist.com) tasks, projects, sections, comments, and more. - Token Metrics Logo **[Token Metrics](https://github.com/token-metrics/mcp)** - [Token Metrics](https://www.tokenmetrics.com/) integration for fetching real-time crypto market data, trading signals, price predictions, and advanced analytics. - TomTom Logo **[TomTom-MCP](https://github.com/tomtom-international/tomtom-mcp)** - The [TomTom](https://www.tomtom.com/) MCP Server simplifies geospatial development by providing seamless access to TomTom's location services, including search, routing, traffic and static maps data. -- Trade Agent Logo **[Trade Agent](https://github.com/Trade-Agent/trade-agent-mcp)** - Execute stock and crypto trades on your brokerage via [Trade Agent](https://thetradeagent.ai) +- Trade It Logo **[Trade It](https://github.com/trade-it-inc/trade-it-mcp)** - Execute stock, crypto, and options trades on your brokerage via [Trade It](https://tradeit.app). Supports Robinhood, ETrade, Charles Schwab, Webull, Coinbase, and Kraken. - Twelvedata Logo **[Twelve Data](https://github.com/twelvedata/mcp)** — Integrate your AI agents with real-time and historical financial market data through our official [Twelve Data](https://twelvedata.com) MCP server. - Twilio Logo **[Twilio](https://github.com/twilio-labs/mcp)** - Interact with [Twilio](https://www.twilio.com/en-us) APIs to send SMS messages, manage phone numbers, configure your account, and more. - TCSAS Logo **[TCSAS](https://github.com/TCMPP-Team/tcsas-devtools-mcp-server)** - Built on the Tencent Mini Program technical framework and fully following the development, powered by [Tencent Cloud Super App as a Service](https://www.tencentcloud.com/products/tcsas?lang=en&pg=). diff --git a/src/git/src/mcp_server_git/server.py b/src/git/src/mcp_server_git/server.py index 1968ded2..23e9b53f 100644 --- a/src/git/src/mcp_server_git/server.py +++ b/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}'" @@ -207,6 +217,27 @@ def git_show(repo: git.Repo, revision: str) -> str: output.append(d.diff) return "".join(output) +def validate_repo_path(repo_path: Path, allowed_repository: Path | None) -> None: + """Validate that repo_path is within the allowed repository path.""" + if allowed_repository is None: + return # No restriction configured + + # Resolve both paths to handle symlinks and relative paths + try: + resolved_repo = repo_path.resolve() + resolved_allowed = allowed_repository.resolve() + except (OSError, RuntimeError): + raise ValueError(f"Invalid path: {repo_path}") + + # Check if repo_path is the same as or a subdirectory of allowed_repository + try: + resolved_repo.relative_to(resolved_allowed) + except ValueError: + raise ValueError( + f"Repository path '{repo_path}' is outside the allowed repository '{allowed_repository}'" + ) + + def git_branch(repo: git.Repo, branch_type: str, contains: str | None = None, not_contains: str | None = None) -> str: match contains: case None: @@ -349,6 +380,9 @@ async def serve(repository: Path | None) -> None: async def call_tool(name: str, arguments: dict) -> list[TextContent]: repo_path = Path(arguments["repo_path"]) + # Validate repo_path is within allowed repository + 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 c852d1f7..3dba7387 100644 --- a/src/git/tests/test_server.py +++ b/src/git/tests/test_server.py @@ -13,7 +13,8 @@ from mcp_server_git.server import ( git_reset, git_log, git_create_branch, - git_show + git_show, + validate_repo_path, ) import shutil @@ -39,7 +40,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 +249,176 @@ def test_git_show_initial_commit(test_repository): assert "Commit:" in result assert "initial commit" in result assert "test.txt" in result + + +# Tests for validate_repo_path (repository scoping security fix) + +def test_validate_repo_path_no_restriction(): + """When no repository restriction is configured, any path should be allowed.""" + validate_repo_path(Path("/any/path"), None) # Should not raise + + +def test_validate_repo_path_exact_match(tmp_path: Path): + """When repo_path exactly matches allowed_repository, validation should pass.""" + allowed = tmp_path / "repo" + allowed.mkdir() + validate_repo_path(allowed, allowed) # Should not raise + + +def test_validate_repo_path_subdirectory(tmp_path: Path): + """When repo_path is a subdirectory of allowed_repository, validation should pass.""" + allowed = tmp_path / "repo" + allowed.mkdir() + subdir = allowed / "subdir" + subdir.mkdir() + validate_repo_path(subdir, allowed) # Should not raise + + +def test_validate_repo_path_outside_allowed(tmp_path: Path): + """When repo_path is outside allowed_repository, validation should raise ValueError.""" + allowed = tmp_path / "allowed_repo" + allowed.mkdir() + outside = tmp_path / "other_repo" + outside.mkdir() + + with pytest.raises(ValueError) as exc_info: + validate_repo_path(outside, allowed) + assert "outside the allowed repository" in str(exc_info.value) + + +def test_validate_repo_path_traversal_attempt(tmp_path: Path): + """Path traversal attempts (../) should be caught and rejected.""" + allowed = tmp_path / "allowed_repo" + allowed.mkdir() + # Attempt to escape via ../ + traversal_path = allowed / ".." / "other_repo" + + with pytest.raises(ValueError) as exc_info: + validate_repo_path(traversal_path, allowed) + assert "outside the allowed repository" in str(exc_info.value) + + +def test_validate_repo_path_symlink_escape(tmp_path: Path): + """Symlinks pointing outside allowed_repository should be rejected.""" + allowed = tmp_path / "allowed_repo" + allowed.mkdir() + outside = tmp_path / "outside" + outside.mkdir() + + # Create a symlink inside allowed that points outside + symlink = allowed / "escape_link" + symlink.symlink_to(outside) + + with pytest.raises(ValueError) as exc_info: + validate_repo_path(symlink, allowed) + assert "outside the allowed repository" in str(exc_info.value) +# 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()