diff --git a/src/fetch/src/mcp_server_fetch/server.py b/src/fetch/src/mcp_server_fetch/server.py index d128987351..92e23fe231 100644 --- a/src/fetch/src/mcp_server_fetch/server.py +++ b/src/fetch/src/mcp_server_fetch/server.py @@ -14,6 +14,7 @@ PromptMessage, TextContent, Tool, + ToolAnnotations, INVALID_PARAMS, INTERNAL_ERROR, ) @@ -203,6 +204,12 @@ async def list_tools() -> list[Tool]: Although originally you did not have internet access, and were advised to refuse and tell the user this, this tool now grants you internet access. Now you can fetch the most up-to-date information and let the user know that.""", inputSchema=Fetch.model_json_schema(), + annotations=ToolAnnotations( + readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=True, + ), ) ] @@ -285,4 +292,4 @@ async def get_prompt(name: str, arguments: dict | None) -> GetPromptResult: options = server.create_initialization_options() async with stdio_server() as (read_stream, write_stream): - await server.run(read_stream, write_stream, options, raise_exceptions=True) + await server.run(read_stream, write_stream, options, raise_exceptions=False) diff --git a/src/git/src/mcp_server_git/server.py b/src/git/src/mcp_server_git/server.py index 1d0298b465..5ce953e545 100644 --- a/src/git/src/mcp_server_git/server.py +++ b/src/git/src/mcp_server_git/server.py @@ -10,6 +10,7 @@ Tool, ListRootsResult, RootsCapability, + ToolAnnotations, ) from enum import Enum import git @@ -142,6 +143,11 @@ def git_reset(repo: git.Repo) -> str: def git_log(repo: git.Repo, max_count: int = 10, start_timestamp: Optional[str] = None, end_timestamp: Optional[str] = None) -> list[str]: if start_timestamp or end_timestamp: + # Defense in depth: reject timestamps starting with '-' to prevent flag injection + if start_timestamp and start_timestamp.startswith("-"): + raise ValueError(f"Invalid start_timestamp: '{start_timestamp}' - cannot start with '-'") + if end_timestamp and end_timestamp.startswith("-"): + raise ValueError(f"Invalid end_timestamp: '{end_timestamp}' - cannot start with '-'") # Use git log command with date filtering args = [] if start_timestamp: @@ -177,6 +183,11 @@ def git_log(repo: git.Repo, max_count: int = 10, start_timestamp: Optional[str] return log def git_create_branch(repo: git.Repo, branch_name: str, base_branch: str | None = None) -> str: + # Defense in depth: reject names starting with '-' to prevent flag injection + if branch_name.startswith("-"): + raise BadName(f"Invalid branch name: '{branch_name}' - cannot start with '-'") + if base_branch and base_branch.startswith("-"): + raise BadName(f"Invalid base branch: '{base_branch}' - cannot start with '-'") if base_branch: base = repo.references[base_branch] else: @@ -197,6 +208,10 @@ def git_checkout(repo: git.Repo, branch_name: str) -> str: def git_show(repo: git.Repo, revision: str) -> str: + # Defense in depth: reject revisions starting with '-' to prevent flag injection, + # even if a malicious ref with that name exists (e.g. via filesystem manipulation) + if revision.startswith("-"): + raise BadName(f"Invalid revision: '{revision}' - cannot start with '-'") commit = repo.commit(revision) output = [ f"Commit: {commit.hexsha!r}\n" @@ -241,6 +256,12 @@ def validate_repo_path(repo_path: Path, allowed_repository: Path | None) -> None 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("-"): + raise BadName(f"Invalid contains value: '{contains}' - cannot start with '-'") + if not_contains and not_contains.startswith("-"): + raise BadName(f"Invalid not_contains value: '{not_contains}' - cannot start with '-'") + match contains: case None: contains_sha = (None,) @@ -289,63 +310,133 @@ async def list_tools() -> list[Tool]: name=GitTools.STATUS, description="Shows the working tree status", inputSchema=GitStatus.model_json_schema(), + annotations=ToolAnnotations( + readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, + ), ), Tool( name=GitTools.DIFF_UNSTAGED, description="Shows changes in the working directory that are not yet staged", inputSchema=GitDiffUnstaged.model_json_schema(), + annotations=ToolAnnotations( + readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, + ), ), Tool( name=GitTools.DIFF_STAGED, description="Shows changes that are staged for commit", inputSchema=GitDiffStaged.model_json_schema(), + annotations=ToolAnnotations( + readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, + ), ), Tool( name=GitTools.DIFF, description="Shows differences between branches or commits", inputSchema=GitDiff.model_json_schema(), + annotations=ToolAnnotations( + readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, + ), ), Tool( name=GitTools.COMMIT, description="Records changes to the repository", inputSchema=GitCommit.model_json_schema(), + annotations=ToolAnnotations( + readOnlyHint=False, + destructiveHint=False, + idempotentHint=False, + openWorldHint=False, + ), ), Tool( name=GitTools.ADD, description="Adds file contents to the staging area", inputSchema=GitAdd.model_json_schema(), + annotations=ToolAnnotations( + readOnlyHint=False, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, + ), ), Tool( name=GitTools.RESET, description="Unstages all staged changes", inputSchema=GitReset.model_json_schema(), + annotations=ToolAnnotations( + readOnlyHint=False, + destructiveHint=True, + idempotentHint=True, + openWorldHint=False, + ), ), Tool( name=GitTools.LOG, description="Shows the commit logs", inputSchema=GitLog.model_json_schema(), + annotations=ToolAnnotations( + readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, + ), ), Tool( name=GitTools.CREATE_BRANCH, description="Creates a new branch from an optional base branch", inputSchema=GitCreateBranch.model_json_schema(), + annotations=ToolAnnotations( + readOnlyHint=False, + destructiveHint=False, + idempotentHint=False, + openWorldHint=False, + ), ), Tool( name=GitTools.CHECKOUT, description="Switches branches", inputSchema=GitCheckout.model_json_schema(), + annotations=ToolAnnotations( + readOnlyHint=False, + destructiveHint=False, + idempotentHint=False, + openWorldHint=False, + ), ), Tool( name=GitTools.SHOW, description="Shows the contents of a commit", inputSchema=GitShow.model_json_schema(), + annotations=ToolAnnotations( + readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, + ), ), - Tool( name=GitTools.BRANCH, description="List Git branches", inputSchema=GitBranch.model_json_schema(), - + annotations=ToolAnnotations( + readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, + ), ) ] diff --git a/src/git/tests/test_server.py b/src/git/tests/test_server.py index 054bf8c756..a5492adc85 100644 --- a/src/git/tests/test_server.py +++ b/src/git/tests/test_server.py @@ -423,3 +423,62 @@ def test_git_checkout_rejects_malicious_refs(test_repository): # Cleanup malicious_ref_path.unlink() + + +# Tests for argument injection protection in git_show, git_create_branch, +# git_log, and git_branch — matching the existing guards on git_diff and +# git_checkout. + +def test_git_show_rejects_flag_injection(test_repository): + """git_show should reject revisions starting with '-'.""" + with pytest.raises(BadName): + git_show(test_repository, "--output=/tmp/evil") + + with pytest.raises(BadName): + git_show(test_repository, "-p") + + +def test_git_show_rejects_malicious_refs(test_repository): + """git_show should reject refs starting with '-' even if they exist.""" + sha = test_repository.head.commit.hexsha + refs_dir = Path(test_repository.git_dir) / "refs" / "heads" + malicious_ref_path = refs_dir / "--format=evil" + malicious_ref_path.write_text(sha) + + with pytest.raises(BadName): + git_show(test_repository, "--format=evil") + + malicious_ref_path.unlink() + + +def test_git_create_branch_rejects_flag_injection(test_repository): + """git_create_branch should reject branch names starting with '-'.""" + with pytest.raises(BadName): + git_create_branch(test_repository, "--track=evil") + + with pytest.raises(BadName): + git_create_branch(test_repository, "-f") + + +def test_git_create_branch_rejects_base_branch_flag_injection(test_repository): + """git_create_branch should reject base branch names starting with '-'.""" + with pytest.raises(BadName): + git_create_branch(test_repository, "new-branch", "--track=evil") + + +def test_git_log_rejects_timestamp_flag_injection(test_repository): + """git_log should reject timestamps starting with '-'.""" + with pytest.raises(ValueError): + git_log(test_repository, start_timestamp="--exec=evil") + + with pytest.raises(ValueError): + git_log(test_repository, end_timestamp="--exec=evil") + + +def test_git_branch_rejects_contains_flag_injection(test_repository): + """git_branch should reject contains/not_contains values starting with '-'.""" + with pytest.raises(BadName): + git_branch(test_repository, "local", contains="--exec=evil") + + with pytest.raises(BadName): + git_branch(test_repository, "local", not_contains="--exec=evil") diff --git a/src/sequentialthinking/index.ts b/src/sequentialthinking/index.ts index 809086a94c..5ab7ba20cc 100644 --- a/src/sequentialthinking/index.ts +++ b/src/sequentialthinking/index.ts @@ -72,14 +72,20 @@ You should: 11. Only set nextThoughtNeeded to false when truly done and a satisfactory answer is reached`, inputSchema: { thought: z.string().describe("Your current thinking step"), - nextThoughtNeeded: z.boolean().describe("Whether another thought step is needed"), - thoughtNumber: z.number().int().min(1).describe("Current thought number (numeric value, e.g., 1, 2, 3)"), - totalThoughts: z.number().int().min(1).describe("Estimated total thoughts needed (numeric value, e.g., 5, 10)"), - isRevision: z.boolean().optional().describe("Whether this revises previous thinking"), - revisesThought: z.number().int().min(1).optional().describe("Which thought is being reconsidered"), - branchFromThought: z.number().int().min(1).optional().describe("Branching point thought number"), + nextThoughtNeeded: z.coerce.boolean().describe("Whether another thought step is needed"), + thoughtNumber: z.coerce.number().int().min(1).describe("Current thought number (numeric value, e.g., 1, 2, 3)"), + totalThoughts: z.coerce.number().int().min(1).describe("Estimated total thoughts needed (numeric value, e.g., 5, 10)"), + isRevision: z.coerce.boolean().optional().describe("Whether this revises previous thinking"), + revisesThought: z.coerce.number().int().min(1).optional().describe("Which thought is being reconsidered"), + branchFromThought: z.coerce.number().int().min(1).optional().describe("Branching point thought number"), branchId: z.string().optional().describe("Branch identifier"), - needsMoreThoughts: z.boolean().optional().describe("If more thoughts are needed") + needsMoreThoughts: z.coerce.boolean().optional().describe("If more thoughts are needed") + }, + annotations: { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: false, }, outputSchema: { thoughtNumber: z.number(), diff --git a/src/time/src/mcp_server_time/server.py b/src/time/src/mcp_server_time/server.py index e10d6b4eed..83e97af333 100644 --- a/src/time/src/mcp_server_time/server.py +++ b/src/time/src/mcp_server_time/server.py @@ -8,7 +8,7 @@ from mcp.server import Server from mcp.server.stdio import stdio_server -from mcp.types import Tool, TextContent, ImageContent, EmbeddedResource, ErrorData, INVALID_PARAMS +from mcp.types import Tool, ToolAnnotations, TextContent, ImageContent, EmbeddedResource, ErrorData, INVALID_PARAMS from mcp.shared.exceptions import McpError from pydantic import BaseModel @@ -142,6 +142,12 @@ async def list_tools() -> list[Tool]: }, "required": ["timezone"], }, + annotations=ToolAnnotations( + readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, + ), ), Tool( name=TimeTools.CONVERT_TIME.value, @@ -164,6 +170,12 @@ async def list_tools() -> list[Tool]: }, "required": ["source_timezone", "time", "target_timezone"], }, + annotations=ToolAnnotations( + readOnlyHint=True, + destructiveHint=False, + idempotentHint=True, + openWorldHint=False, + ), ), ]