From 3672fce64c17284b941dcbeac5ad6578a5ea54a9 Mon Sep 17 00:00:00 2001 From: xiefuzheng713-alt <266900198+xiefuzheng713-alt@users.noreply.github.com> Date: Fri, 5 Jun 2026 04:03:45 +0800 Subject: [PATCH] Reject unexpected MCP read tool arguments --- app/mcp_tools.py | 13 +++++++++++++ tests/test_api_mcp.py | 38 ++++++++++++++++++++++++++++++++++++++ tests/test_mcp_tools.py | 22 ++++++++++++++++++++++ 3 files changed, 73 insertions(+) diff --git a/app/mcp_tools.py b/app/mcp_tools.py index 0c3e4521..a6490f99 100644 --- a/app/mcp_tools.py +++ b/app/mcp_tools.py @@ -126,6 +126,11 @@ def optional_bool_arg(field: str, default: bool = False) -> bool: raise ValueError(f"{field} must be a boolean") return value + def reject_unexpected_args(allowed: set[str]) -> None: + unexpected = sorted(set(args) - allowed) + if unexpected: + raise ValueError(f"unexpected argument: {unexpected[0]}") + def bounty_by_issue_number(repo_selector: str | None) -> Bounty | None: issue_query = select(Bounty).where(Bounty.issue_number == positive_int_arg("issue_number")) if repo_selector is not None: @@ -193,6 +198,7 @@ def selected_bounty(internal_id_field: str) -> Bounty | None: ) return json.dumps(sorted_bounties[:limit]) if name == "get_bounty": + reject_unexpected_args({"id", "include_awards", "issue_number", "repo"}) bounty = selected_bounty("id") if bounty is None: return "bounty not found" @@ -201,6 +207,9 @@ def selected_bounty(internal_id_field: str) -> Bounty | None: bounty_data["awards"] = bounty_awards_to_dict(session, bounty.id) return json.dumps(bounty_data) if name == "list_bounty_attempts": + reject_unexpected_args( + {"bounty_id", "include_expired", "issue_number", "limit", "repo"} + ) bounty = selected_bounty("bounty_id") if bounty is None: return "bounty not found" @@ -218,6 +227,7 @@ def selected_bounty(internal_id_field: str) -> Bounty | None: "attempts": attempt_listing["attempts"], } if name == "get_balance": + reject_unexpected_args({"account"}) account = normalized_account(str_arg("account")) return f"{account}: {format_mrwk(get_balance(session, account))} MRWK" if name == "register_wallet": @@ -228,6 +238,7 @@ def selected_bounty(internal_id_field: str) -> Bounty | None: ) return json.dumps(wallet_to_dict(session, wallet)) if name == "get_wallet": + reject_unexpected_args({"address"}) wallet_row = session.get(Wallet, normalized_wallet_address(str_arg("address"))) if wallet_row is None: return "wallet not found" @@ -244,11 +255,13 @@ def selected_bounty(internal_id_field: str) -> Bounty | None: ) return json.dumps(wallet_transfer_to_dict(transfer)) if name == "get_ledger_entry": + reject_unexpected_args({"sequence"}) entry = ledger_entry_to_dict(session, positive_int_arg("sequence")) if entry is None: return "ledger entry not found" return json.dumps(entry) if name == "get_proof": + reject_unexpected_args({"hash"}) proof = session.get(Proof, proof_hash_from_path(str_arg("hash"))) if proof is None: return "proof not found" diff --git a/tests/test_api_mcp.py b/tests/test_api_mcp.py index 8a3326f5..48d5aaf6 100644 --- a/tests/test_api_mcp.py +++ b/tests/test_api_mcp.py @@ -814,6 +814,44 @@ def test_mcp_rejects_unknown_tool_name(sqlite_url: str) -> None: } +@pytest.mark.parametrize( + ("tool_name", "arguments", "request_id"), + [ + ("get_bounty", {"id": 1, "bounty": 1}, 13), + ("list_bounty_attempts", {"bounty_id": 1, "attempt_status": "active"}, 14), + ("get_balance", {"account": "treasury:mrwk", "acount": "typo"}, 15), + ("get_wallet", {"address": "mrwk1" + ("a" * 40), "wallet": "typo"}, 16), + ("get_ledger_entry", {"sequence": 1, "seq": 2}, 17), + ("get_proof", {"hash": "a" * 64, "proof": "typo"}, 18), + ], +) +def test_mcp_read_tools_reject_unexpected_arguments( + sqlite_url: str, tool_name: str, arguments: dict[str, object], request_id: int +) -> None: + create_schema(sqlite_url) + with session_scope(sqlite_url) as session: + ensure_genesis(session) + + client = TestClient(create_app(database_url=sqlite_url, webhook_secret="secret")) + + response = client.post( + "/mcp", + json={ + "jsonrpc": "2.0", + "id": request_id, + "method": "tools/call", + "params": {"name": tool_name, "arguments": arguments}, + }, + ) + + assert response.status_code == 200 + assert response.json() == { + "jsonrpc": "2.0", + "id": request_id, + "error": {"code": -32602, "message": "invalid tool arguments"}, + } + + def test_mcp_get_bounty_can_include_accepted_awards(sqlite_url: str) -> None: create_schema(sqlite_url) with session_scope(sqlite_url) as session: diff --git a/tests/test_mcp_tools.py b/tests/test_mcp_tools.py index b588f951..a16bf17f 100644 --- a/tests/test_mcp_tools.py +++ b/tests/test_mcp_tools.py @@ -83,6 +83,28 @@ def test_call_mcp_tool_rejects_c1_status_before_normalizing(sqlite_url: str) -> call_mcp_tool(sqlite_url, "list_bounties", {"status": "\u0085open"}) +@pytest.mark.parametrize( + ("tool_name", "arguments", "unexpected"), + [ + ("get_bounty", {"id": 1, "bounty": 1}, "bounty"), + ("list_bounty_attempts", {"bounty_id": 1, "attempt_status": "active"}, "attempt_status"), + ("get_balance", {"account": "treasury:mrwk", "acount": "typo"}, "acount"), + ("get_wallet", {"address": "mrwk1" + ("a" * 40), "wallet": "typo"}, "wallet"), + ("get_ledger_entry", {"sequence": 1, "seq": 2}, "seq"), + ("get_proof", {"hash": "a" * 64, "proof": "typo"}, "proof"), + ], +) +def test_call_mcp_tool_rejects_unexpected_read_tool_arguments( + sqlite_url: str, tool_name: str, arguments: dict[str, object], unexpected: str +) -> None: + create_schema(sqlite_url) + with session_scope(sqlite_url) as session: + ensure_genesis(session) + + with pytest.raises(ValueError, match=f"unexpected argument: {unexpected}"): + call_mcp_tool(sqlite_url, tool_name, arguments) + + def test_call_mcp_tool_rejects_c1_nonce_before_integer_parsing(sqlite_url: str) -> None: create_schema(sqlite_url) with session_scope(sqlite_url) as session: