From 4e4344f779e1b6e108391b33bbd629f6842e0ab0 Mon Sep 17 00:00:00 2001 From: lester076 <2665063734@qq.com> Date: Mon, 2 Nov 2026 13:19:22 +0800 Subject: [PATCH 1/4] fix(mcp): propagate is_error from MCP tool execution to Anthropic tool_result blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an MCP tool call fails — the server returns CallToolResult.isError=True, a guardrail blocks the call, or execution raises — execute_tool_calls drops the failure flag, so _build_tool_result_message renders the error text as a successful tool return and Anthropic never triggers its native error-handling or retry logic. Add an optional is_error key to MCPToolResult, set it on the isError result path (accepting both is_error and isError attribute names across mcp SDK versions) and on all four exception paths, and pass is_error=True through to the AnthropicMessagesToolResultParam in the native /v1/messages MCP handler. Fixes #49359 --- .../anthropic/messages/native/mcp_handler.py | 25 ++--- .../mcp/litellm_proxy_mcp_handler.py | 28 ++++-- .../messages/native/test_mcp_handler.py | 42 +++++++- .../mcp/test_litellm_proxy_mcp_handler.py | 98 ++++++++++++++++++- 4 files changed, 166 insertions(+), 27 deletions(-) diff --git a/litellm/llms/anthropic/messages/native/mcp_handler.py b/litellm/llms/anthropic/messages/native/mcp_handler.py index 8fe18d79f24..aaa16469236 100644 --- a/litellm/llms/anthropic/messages/native/mcp_handler.py +++ b/litellm/llms/anthropic/messages/native/mcp_handler.py @@ -48,17 +48,20 @@ def _get_stop_reason(response: AnthropicMessagesResponse) -> str | None: def _build_tool_result_message(tool_results: Sequence[Mapping[str, object]]) -> AnthropicMessagesUserMessageParam: """Turn executed tool results into the user message Anthropic expects.""" - return AnthropicMessagesUserMessageParam( - role="user", - content=[ - AnthropicMessagesToolResultParam( - type="tool_result", - tool_use_id=str(result.get("tool_call_id") or ""), - content=str(result.get("result") or ""), - ) - for result in tool_results - ], - ) + content: list[AnthropicMessagesToolResultParam] = [] + for result in tool_results: + tool_result = AnthropicMessagesToolResultParam( + type="tool_result", + tool_use_id=str(result.get("tool_call_id") or ""), + content=str(result.get("result") or ""), + ) + if result.get("is_error"): + # Propagate MCP failures (CallToolResult.isError or an execution + # exception) so Anthropic's native error-handling triggers instead + # of reading the failure string as a successful tool return (#49359). + tool_result["is_error"] = True + content.append(tool_result) + return AnthropicMessagesUserMessageParam(role="user", content=content) async def anthropic_messages_with_mcp( diff --git a/litellm/responses/mcp/litellm_proxy_mcp_handler.py b/litellm/responses/mcp/litellm_proxy_mcp_handler.py index 1104aee353e..a2b60ff8b4d 100644 --- a/litellm/responses/mcp/litellm_proxy_mcp_handler.py +++ b/litellm/responses/mcp/litellm_proxy_mcp_handler.py @@ -58,10 +58,15 @@ ToolParam: TypeAlias = Mapping[str, object] SplitTools: TypeAlias = tuple[list[ToolParam], list[Any]] -class MCPToolResult(TypedDict): +class MCPToolResult(TypedDict, total=False): tool_call_id: str | None result: str name: str | None + # True when the MCP server returned CallToolResult.isError or execution + # raised (guardrail block, HTTP failure, unexpected exception). Anthropic + # native MCP consumers pass is_error=True on the tool_result block so the + # model can react to the failure instead of reading it as a success (#49359). + is_error: bool class _AresponsesOptions(ExtraItemsTypedDict, total=False, extra_items=object): @@ -948,13 +953,16 @@ class LiteLLM_Proxy_MCP_Handler: # Format result for inclusion in response result_text = LiteLLM_Proxy_MCP_Handler._parse_mcp_result(result) - tool_results.append( - { - "tool_call_id": tool_call_id, - "result": result_text, - "name": tool_name, - } - ) + tool_result: MCPToolResult = { + "tool_call_id": tool_call_id, + "result": result_text, + "name": tool_name, + } + # mcp>=2.3.0 names the flag is_error (constructor alias isError); + # accept both so older SDK versions keep working. + if getattr(result, "is_error", False) or getattr(result, "isError", False): + tool_result["is_error"] = True + tool_results.append(tool_result) except BlockedPiiEntityError as e: await LiteLLM_Proxy_MCP_Handler._log_mcp_tool_failure( @@ -970,6 +978,7 @@ class LiteLLM_Proxy_MCP_Handler: "tool_call_id": tool_call_id, "result": error_message, "name": tool_name, + "is_error": True, } ) except GuardrailRaisedException as e: @@ -988,6 +997,7 @@ class LiteLLM_Proxy_MCP_Handler: "tool_call_id": tool_call_id, "result": error_message, "name": tool_name, + "is_error": True, } ) except HTTPException as e: @@ -1004,6 +1014,7 @@ class LiteLLM_Proxy_MCP_Handler: "tool_call_id": tool_call_id, "result": error_message, "name": tool_name, + "is_error": True, } ) except Exception as e: @@ -1019,6 +1030,7 @@ class LiteLLM_Proxy_MCP_Handler: "tool_call_id": tool_call_id, "result": f"Error executing tool: {e}", "name": tool_name, + "is_error": True, } ) diff --git a/tests/unit/llms/anthropic/messages/native/test_mcp_handler.py b/tests/unit/llms/anthropic/messages/native/test_mcp_handler.py index 73c2edbce19..701f9aa2937 100644 --- a/tests/unit/llms/anthropic/messages/native/test_mcp_handler.py +++ b/tests/unit/llms/anthropic/messages/native/test_mcp_handler.py @@ -258,9 +258,7 @@ async def test_anthropic_messages_with_mcp_hands_execution_the_requests_served_t ] process = AsyncMock(return_value=(served, {"read_wiki_structure": "deepwiki"})) - execute = AsyncMock( - return_value=[{"tool_call_id": "toolu_1", "result": "ok", "name": "read_wiki_structure"}] - ) + execute = AsyncMock(return_value=[{"tool_call_id": "toolu_1", "result": "ok", "name": "read_wiki_structure"}]) responses = [ { "stop_reason": "tool_use", @@ -454,3 +452,41 @@ async def test_anthropic_messages_with_mcp_loads_a_filtered_routes_beta_map_off_ @pytest.mark.asyncio async def test_anthropic_messages_with_mcp_never_loads_the_beta_map_for_first_party_anthropic(): assert await _mcp_safeguards_turn_recording_beta_map_loads("anthropic", "claude-sonnet-4-6") == () + + +def test_build_tool_result_message_marks_is_error_results(): + """ + Regression test (#49359): a tool result flagged is_error by the MCP gateway + must reach Anthropic as is_error=True so the model reacts to the failure. + + Given: tool results where one carries is_error=True (MCP CallToolResult.isError + or an execution exception path in execute_tool_calls) + When: _build_tool_result_message renders them + Then: Only the flagged result gets is_error; the success stays clean. + """ + message = _build_tool_result_message( + [ + {"tool_call_id": "call_ok", "result": "9 sections", "name": "read_wiki"}, + {"tool_call_id": "call_bad", "result": "Error executing tool: boom", "name": "read_wiki", "is_error": True}, + ] + ) + + assert message["role"] == "user" + assert message["content"][0] == {"type": "tool_result", "tool_use_id": "call_ok", "content": "9 sections"} + assert message["content"][1] == { + "type": "tool_result", + "tool_use_id": "call_bad", + "content": "Error executing tool: boom", + "is_error": True, + } + + +def test_build_tool_result_message_omits_is_error_for_successes(): + """ + Successful tool results must not carry is_error at all (Anthropic treats + is_error: false the same, but omitting keeps the payload minimal and the + chat-completions bridge unaffected). + """ + message = _build_tool_result_message([{"tool_call_id": "call_1", "result": "5", "name": "add", "is_error": False}]) + + assert message["content"][0] == {"type": "tool_result", "tool_use_id": "call_1", "content": "5"} diff --git a/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py b/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py index a16661c0174..9a96550a231 100644 --- a/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py +++ b/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py @@ -49,7 +49,14 @@ class _DummyMCPResult: def _setup_mcp_call_environment(monkeypatch: pytest.MonkeyPatch) -> AsyncMock: """Patch MCP globals so _execute_tool_calls can run in tests.""" - proxy_module = types.SimpleNamespace(proxy_logging_obj=object(), prisma_client=None) + proxy_logging_obj = MagicMock() + + async def _passthrough_post_mcp_call_hook(response, **_): + return response + + proxy_logging_obj.post_mcp_call_hook = AsyncMock(side_effect=_passthrough_post_mcp_call_hook) + proxy_logging_obj.post_call_failure_hook = AsyncMock() + proxy_module = types.SimpleNamespace(proxy_logging_obj=proxy_logging_obj, prisma_client=None) monkeypatch.setitem(sys.modules, "litellm.proxy.proxy_server", proxy_module) fake_manager = types.SimpleNamespace( @@ -146,9 +153,7 @@ def test_chat_follow_up_answers_only_tool_calls_in_its_assistant_turn(): "message": { "role": "assistant", "content": None, - "tool_calls": [ - {"id": call_id, "type": "function", "function": {"name": "foo", "arguments": "{}"}} - ], + "tool_calls": [{"id": call_id, "type": "function", "function": {"name": "foo", "arguments": "{}"}}], }, "finish_reason": "tool_calls", } @@ -621,7 +626,9 @@ async def test_execute_tool_calls_applies_post_call_hook_content(monkeypatch): user_api_key_auth=None, ) - assert results == [{"tool_call_id": "call-1", "result": "[REDACTED]", "name": tool_name}] + # The mocked async_post_mcp_tool_call_hook returns a result flagged is_error=True, + # so the gateway must propagate that flag (#49359) alongside the redacted text. + assert results == [{"tool_call_id": "call-1", "result": "[REDACTED]", "name": tool_name, "is_error": True}] assert logging_obj.async_success_handler.await_args.kwargs["result"].content[0].text == "[REDACTED]" assert logging_obj.async_success_handler.await_args.kwargs["result"].structured_content is None @@ -1909,3 +1916,84 @@ async def test_apply_toolset_permissions_pins_the_auth_to_explicit_grants_only(m assert scoped.object_permission is not None assert scoped.object_permission.mcp_servers == ["srv-1"] assert scoped.object_permission.mcp_tool_permissions == {"srv-1": ["add"]} + + +@pytest.mark.asyncio +async def test_execute_tool_calls_propagates_mcp_is_error_flag(monkeypatch): + """ + Regression test (#49359): CallToolResult.isError=True must surface as + is_error=True on the MCPToolResult so Anthropic's native MCP loop can mark + the tool_result block as failed. + """ + call_tool_mock = _setup_mcp_call_environment(monkeypatch) + call_tool_mock.return_value = CallToolResult( + content=[TextContent(type="text", text="Database connection refused")], + isError=True, + ) + tool_name = "read_wiki_structure" + tool_calls = [{"id": "call-err", "function": {"name": tool_name, "arguments": "{}"}}] + + results = await LiteLLM_Proxy_MCP_Handler.execute_tool_calls( + tool_server_map={tool_name: "deepwiki"}, + tool_calls=tool_calls, + user_api_key_auth=None, + ) + + assert results == [ + { + "tool_call_id": "call-err", + "result": "Database connection refused", + "name": tool_name, + "is_error": True, + } + ] + + +@pytest.mark.asyncio +async def test_execute_tool_calls_success_has_no_is_error(monkeypatch): + """A successful CallToolResult must not carry the is_error key at all.""" + call_tool_mock = _setup_mcp_call_environment(monkeypatch) + call_tool_mock.return_value = CallToolResult( + content=[TextContent(type="text", text="9 sections")], + isError=False, + ) + tool_name = "read_wiki_structure" + tool_calls = [{"id": "call-ok", "function": {"name": tool_name, "arguments": "{}"}}] + + results = await LiteLLM_Proxy_MCP_Handler.execute_tool_calls( + tool_server_map={tool_name: "deepwiki"}, + tool_calls=tool_calls, + user_api_key_auth=None, + ) + + assert results == [ + { + "tool_call_id": "call-ok", + "result": "9 sections", + "name": tool_name, + } + ] + + +@pytest.mark.asyncio +async def test_execute_tool_calls_marks_exception_results_as_error(monkeypatch): + """An unexpected exception in tool execution must set is_error=True.""" + call_tool_mock = _setup_mcp_call_environment(monkeypatch) + call_tool_mock.side_effect = RuntimeError("boom") + tool_name = "read_wiki_structure" + tool_calls = [{"id": "call-boom", "function": {"name": tool_name, "arguments": "{}"}}] + + results = await LiteLLM_Proxy_MCP_Handler.execute_tool_calls( + tool_server_map={tool_name: "deepwiki"}, + tool_calls=tool_calls, + user_api_key_auth=None, + ) + + assert results == [ + { + "tool_call_id": "call-boom", + "result": "Error executing tool: boom", + "name": tool_name, + "is_error": True, + } + ] From 8ce5e5c313d0f0f33bec3bdd83c994481de7209d Mon Sep 17 00:00:00 2001 From: lester076 <2665063734@qq.com> Date: Mon, 2 Nov 2026 18:20:38 +0800 Subject: [PATCH 2/4] fix(mcp): keep MCPToolResult's original keys required via NotRequired MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first cut declared the whole TypedDict total=False, which made tool_call_id, result, and name optional to static type checkers and weakened the existing contract — the exact PEP 655 regression flagged in the issue review. Revert to the default total=True and mark only the new is_error key NotRequired, and pin the invariant with a static negative control (required_keys / optional_keys assertions). --- .../mcp/litellm_proxy_mcp_handler.py | 7 ++++--- .../mcp/test_litellm_proxy_mcp_handler.py | 21 +++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/litellm/responses/mcp/litellm_proxy_mcp_handler.py b/litellm/responses/mcp/litellm_proxy_mcp_handler.py index a2b60ff8b4d..abdc7e82614 100644 --- a/litellm/responses/mcp/litellm_proxy_mcp_handler.py +++ b/litellm/responses/mcp/litellm_proxy_mcp_handler.py @@ -6,7 +6,7 @@ from typing import TYPE_CHECKING, Any, Final, Literal, Optional, TypeAlias, Type from openai.types.chat import ChatCompletionToolParam from openai.types.responses.function_tool_param import FunctionToolParam from typing_extensions import TypedDict as ExtraItemsTypedDict -from typing_extensions import Unpack +from typing_extensions import NotRequired, Unpack from litellm._logging import verbose_logger from litellm.constants import MAXIMUM_TRACEBACK_LINES_TO_LOG @@ -58,7 +58,7 @@ ToolParam: TypeAlias = Mapping[str, object] SplitTools: TypeAlias = tuple[list[ToolParam], list[Any]] -class MCPToolResult(TypedDict, total=False): +class MCPToolResult(TypedDict): tool_call_id: str | None result: str name: str | None @@ -66,7 +66,8 @@ class MCPToolResult(TypedDict, total=False): # raised (guardrail block, HTTP failure, unexpected exception). Anthropic # native MCP consumers pass is_error=True on the tool_result block so the # model can react to the failure instead of reading it as a success (#49359). - is_error: bool + # Optional via NotRequired (PEP 655) so the three original keys stay required. + is_error: NotRequired[bool] class _AresponsesOptions(ExtraItemsTypedDict, total=False, extra_items=object): diff --git a/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py b/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py index 9a96550a231..69569eb4853 100644 --- a/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py +++ b/tests/unit/responses/mcp/test_litellm_proxy_mcp_handler.py @@ -1997,3 +1997,24 @@ async def test_execute_tool_calls_marks_exception_results_as_error(monkeypatch): "is_error": True, } ] + + +def test_mcp_tool_result_keeps_original_keys_required_statically(): + """ + PEP 655 negative control (#49359 review): ``is_error`` is the only optional + key; ``tool_call_id``, ``result``, and ``name`` must stay required, so a + bare ``{"is_error": True}`` literal is rejected at type-check time. This + pins the static contract the runtime tests cannot see. + """ + from typing_extensions import assert_type + + from litellm.responses.mcp.litellm_proxy_mcp_handler import MCPToolResult + + # The three original keys keep Required status: total=False was reverted. + assert not MCPToolResult.__required_keys__ < {"tool_call_id", "result", "name"} + assert MCPToolResult.__required_keys__ == {"tool_call_id", "result", "name"} + assert MCPToolResult.__optional_keys__ == {"is_error"} + + # Runtime shape of a full result is unchanged. + full: MCPToolResult = {"tool_call_id": "call-1", "result": "ok", "name": "echo"} + assert_type(full, MCPToolResult) From 51d36e86cdec2ef968edd16f55a75b05616d4be0 Mon Sep 17 00:00:00 2001 From: lester076 <2665063734@qq.com> Date: Mon, 2 Nov 2026 18:32:59 +0800 Subject: [PATCH 3/4] fix(a2a): validate inline pushNotificationConfig on message/send and message/stream tasks/pushNotificationConfig/set runs _validate_push_notification_url (HTTPS + SSRF guard) before forwarding a webhook, but the same config registered inline via message/send or message/stream (A2A 1.0 configuration. pushNotificationConfig, spec 3.2.1/6.6) was passed through _build_message_ send_params unchecked, so a key holder could make the agent POST task data to an http:// or internal address the set method refuses. Run the same validation on both inline paths: the direct-validate build checks the built MessageSendConfiguration.push_notification_config, and the proto-fallback path (whose 1.0->0.3 conversion drops the config) validates the raw wire configuration instead. A refused URL now gets the same 400 shape as set, and nothing reaches the agent. Fixes #49622 --- .../proxy/agent_endpoints/a2a_endpoints.py | 33 +++++- .../agent_endpoints/test_a2a_endpoints.py | 100 ++++++++++++++++++ 2 files changed, 131 insertions(+), 2 deletions(-) diff --git a/litellm/proxy/agent_endpoints/a2a_endpoints.py b/litellm/proxy/agent_endpoints/a2a_endpoints.py index 3dfaa7f213e..f3acce9b640 100644 --- a/litellm/proxy/agent_endpoints/a2a_endpoints.py +++ b/litellm/proxy/agent_endpoints/a2a_endpoints.py @@ -149,7 +149,7 @@ def _build_message_send_params(params: dict[str, Any]) -> "MessageSendParams": from a2a.compat.v0_3.types import MessageSendParams try: - return MessageSendParams(**params) + built = MessageSendParams(**params) except ValidationError: from a2a.compat.v0_3.conversions import pb2_v10, to_compat_send_message_request from google.protobuf.json_format import ParseDict, ParseError @@ -159,7 +159,36 @@ def _build_message_send_params(params: dict[str, Any]) -> "MessageSendParams": ParseDict(params, pb, ignore_unknown_fields=True) except ParseError as e: raise ValueError(f"Invalid message/send params: {e}") from e - return to_compat_send_message_request(pb, "").params + built = to_compat_send_message_request(pb, "").params + # The 1.0->0.3 conversion drops pushNotificationConfig, so validate the + # raw wire config on this path. + _validate_inline_push_notification_url(params) + return built + # A push config registered inline via message/send or message/stream must + # clear the same SSRF guard the tasks/pushNotificationConfig/set method + # applies, otherwise message/send is a way around it (#49622). + push_config = built.configuration.push_notification_config if built.configuration else None + callback_url = push_config.url if push_config is not None else None + if callback_url: + _validate_push_notification_url(str(callback_url)) + return built + + +def _validate_inline_push_notification_url(params: Mapping[str, Any]) -> None: + """Reject an unsafe inline pushNotificationConfig carried in wire params. + + Covers the A2A 1.0 shape (``configuration.pushNotificationConfig.url``) on the + proto-fallback path, where the built params no longer carry the config. + """ + configuration = params.get("configuration") + if not isinstance(configuration, Mapping): + return + push_config = configuration.get("pushNotificationConfig") or configuration.get("push_notification_config") + if not isinstance(push_config, Mapping): + return + callback_url = push_config.get("url") + if isinstance(callback_url, str) and callback_url: + _validate_push_notification_url(callback_url) def _served_version(agent: "AgentResponse", request: Request, original_method: str | None = None) -> A2AVersion: diff --git a/tests/unit/proxy/agent_endpoints/test_a2a_endpoints.py b/tests/unit/proxy/agent_endpoints/test_a2a_endpoints.py index 88da07c612b..d2f65caa73b 100644 --- a/tests/unit/proxy/agent_endpoints/test_a2a_endpoints.py +++ b/tests/unit/proxy/agent_endpoints/test_a2a_endpoints.py @@ -3196,3 +3196,103 @@ def test_a2a_http_refusals_answer_as_jsonrpc_errors_with_a_null_id_for_unparseab assert body["error"]["code"] == -32000 assert body["error"]["message"] == "Agent 'agent-1' is not allowed" assert body["error"]["data"]["code"] == "403" + + +def test_message_send_params_rejects_inline_http_push_url(): + """Regression (#49622): message/send's inline pushNotificationConfig must clear the same SSRF guard as tasks/pushNotificationConfig/set.""" + from fastapi import HTTPException + + from litellm.proxy.agent_endpoints.a2a_endpoints import _build_message_send_params + + params = { + "message": { + "messageId": "msg-1", + "role": "ROLE_USER", + "parts": [{"text": "hello"}], + }, + "configuration": { + "pushNotificationConfig": { + "url": "http://webhook.example.com/hook", + "token": "tok", + } + }, + } + + with pytest.raises(HTTPException) as exc_info: + _build_message_send_params(params) + + assert exc_info.value.status_code == 400 + assert "HTTPS" in exc_info.value.detail + + +def test_message_send_params_rejects_inline_private_address_push_url(): + """Regression (#49622): a link-local inline push URL is refused before anything reaches the agent.""" + import socket as socket_module + + from fastapi import HTTPException + + from litellm.proxy.agent_endpoints.a2a_endpoints import _build_message_send_params + + params = { + "message": { + "messageId": "msg-1", + "role": "ROLE_USER", + "parts": [{"text": "hello"}], + }, + "configuration": { + "pushNotificationConfig": { + "url": "https://169.254.169.254/latest/meta-data/", + } + }, + } + + with pytest.raises(HTTPException) as exc_info: + _build_message_send_params(params) + + assert exc_info.value.status_code == 400 + + +def test_message_send_params_accepts_https_public_push_url(): + """A compliant inline push config passes through unchanged.""" + from litellm.proxy.agent_endpoints.a2a_endpoints import _build_message_send_params + + params = { + "message": { + "messageId": "msg-1", + "role": "user", + "parts": [{"text": "hello"}], + }, + "configuration": { + "pushNotificationConfig": { + "url": "https://webhook.example.com/hook", + "token": "tok", + } + }, + } + + # The public-address SSRF check itself is covered by the dedicated + # _validate_push_notification_url tests; keep this test offline by + # letting the (already scheme-validated) https URL through. + with patch("litellm.proxy.agent_endpoints.a2a_endpoints.validate_url", return_value=None): + result = _build_message_send_params(params) + assert result.message.role.value == "user" + assert result.configuration is not None + assert result.configuration.push_notification_config is not None + assert result.configuration.push_notification_config.url == "https://webhook.example.com/hook" + + +def test_message_send_params_without_push_config_is_untouched(): + """No inline push config: nothing extra is validated, plain sends behave as before.""" + from litellm.proxy.agent_endpoints.a2a_endpoints import _build_message_send_params + + result = _build_message_send_params( + { + "message": { + "messageId": "msg-1", + "role": "ROLE_USER", + "parts": [{"text": "hello"}], + }, + "configuration": {"blocking": True}, + } + ) + assert result.message.role.value == "user" From 324b28dddd0044d31f2240b281ab57520b78730e Mon Sep 17 00:00:00 2001 From: lester076 <2665063734@qq.com> Date: Mon, 2 Nov 2026 23:57:05 +0800 Subject: [PATCH 4/4] fix(a2a): close the taskPushNotificationConfig inline bypass The A2A 1.0 taskPushNotificationConfig spelling (camelCase or snake_case) survives the 1.0->0.3 conversion into built.configuration.push_notification_ config, but the fallback path returned before the built-object check ran, so an authenticated caller could still register an http:// or loopback webhook via message/send or message/stream. Drop the early return so the built-object validation runs on both paths, and extend the raw wire check with the taskPushNotificationConfig / task_push_notification_config spellings for the configs the conversion does drop. Reviewed-by: veria-ai[bot] (flagged at a2a_endpoints.py:166) --- .../proxy/agent_endpoints/a2a_endpoints.py | 27 +++++++--- .../agent_endpoints/test_a2a_endpoints.py | 53 +++++++++++++++++++ 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/litellm/proxy/agent_endpoints/a2a_endpoints.py b/litellm/proxy/agent_endpoints/a2a_endpoints.py index f3acce9b640..9ccca4298c1 100644 --- a/litellm/proxy/agent_endpoints/a2a_endpoints.py +++ b/litellm/proxy/agent_endpoints/a2a_endpoints.py @@ -161,12 +161,13 @@ def _build_message_send_params(params: dict[str, Any]) -> "MessageSendParams": raise ValueError(f"Invalid message/send params: {e}") from e built = to_compat_send_message_request(pb, "").params # The 1.0->0.3 conversion drops pushNotificationConfig, so validate the - # raw wire config on this path. + # raw wire config on this path too. _validate_inline_push_notification_url(params) - return built # A push config registered inline via message/send or message/stream must # clear the same SSRF guard the tasks/pushNotificationConfig/set method - # applies, otherwise message/send is a way around it (#49622). + # applies, otherwise message/send is a way around it (#49622). Runs on both + # paths: the 1.0 taskPushNotificationConfig spelling survives the + # conversion into built.configuration.push_notification_config. push_config = built.configuration.push_notification_config if built.configuration else None callback_url = push_config.url if push_config is not None else None if callback_url: @@ -177,14 +178,26 @@ def _build_message_send_params(params: dict[str, Any]) -> "MessageSendParams": def _validate_inline_push_notification_url(params: Mapping[str, Any]) -> None: """Reject an unsafe inline pushNotificationConfig carried in wire params. - Covers the A2A 1.0 shape (``configuration.pushNotificationConfig.url``) on the - proto-fallback path, where the built params no longer carry the config. + Covers the A2A 1.0 shapes (``configuration.pushNotificationConfig.url`` and + ``configuration.taskPushNotificationConfig.url``, camelCase or snake_case) + on the proto-fallback path, where a dropped config no longer exists in the + built params. """ configuration = params.get("configuration") if not isinstance(configuration, Mapping): return - push_config = configuration.get("pushNotificationConfig") or configuration.get("push_notification_config") - if not isinstance(push_config, Mapping): + push_config: Any = None + for key in ( + "pushNotificationConfig", + "push_notification_config", + "taskPushNotificationConfig", + "task_push_notification_config", + ): + value = configuration.get(key) + if isinstance(value, Mapping): + push_config = value + break + if push_config is None: return callback_url = push_config.get("url") if isinstance(callback_url, str) and callback_url: diff --git a/tests/unit/proxy/agent_endpoints/test_a2a_endpoints.py b/tests/unit/proxy/agent_endpoints/test_a2a_endpoints.py index d2f65caa73b..e8b4e3139ba 100644 --- a/tests/unit/proxy/agent_endpoints/test_a2a_endpoints.py +++ b/tests/unit/proxy/agent_endpoints/test_a2a_endpoints.py @@ -3296,3 +3296,56 @@ def test_message_send_params_without_push_config_is_untouched(): } ) assert result.message.role.value == "user" + + +def test_message_send_params_rejects_a2a_10_task_push_url_loopback(): + """Regression (#49637 review): A2A 1.0 taskPushNotificationConfig survives the + 1.0->0.3 conversion into built.configuration.push_notification_config, so the + built-object check must run on the fallback path too — a loopback webhook on + message/send is refused before reaching the agent.""" + from fastapi import HTTPException + + from litellm.proxy.agent_endpoints.a2a_endpoints import _build_message_send_params + + params = { + "message": { + "messageId": "msg-1", + "role": "ROLE_USER", + "parts": [{"text": "hello"}], + }, + "configuration": { + "taskPushNotificationConfig": { + "url": "http://127.0.0.1:8080/hook", + } + }, + } + + with pytest.raises(HTTPException) as exc_info: + _build_message_send_params(params) + + assert exc_info.value.status_code == 400 + + +def test_message_send_params_rejects_snake_case_task_push_url(): + """The snake_case task_push_notification_config wire spelling is covered too.""" + from fastapi import HTTPException + + from litellm.proxy.agent_endpoints.a2a_endpoints import _build_message_send_params + + params = { + "message": { + "messageId": "msg-1", + "role": "ROLE_USER", + "parts": [{"text": "hello"}], + }, + "configuration": { + "task_push_notification_config": { + "url": "http://10.0.0.9/hook", + } + }, + } + + with pytest.raises(HTTPException) as exc_info: + _build_message_send_params(params) + + assert exc_info.value.status_code == 400