From cf23df94313ae308484a41772e1e8aab23ded4d6 Mon Sep 17 00:00:00 2001 From: Tin Chi Lo Date: Fri, 17 Jul 2026 11:34:08 -0700 Subject: [PATCH] fix(mcp): require every reference to opt in before auto-executing tools _should_auto_execute_tools returned True as soon as any MCP reference set require_approval="never", so a request that mixed a "never" reference with an "always" or "manual" one auto-executed every tool call the model produced, including the approval-gated ones. A prompt could name the approval-required tool and have it run with no approval. Make the gate fail closed: auto-execute only when every reference opts in with "never". A single approval-required reference (including the object form or an unset value) returns the model's tool calls to the caller instead of running them, so an approval-gated tool can never be auto-invoked. This is the shared decision behind /chat/completions, /responses, the streaming iterator and the new /v1/messages path, so all four fail closed from one change. The common case, every reference "never", is unchanged. The alternative, executing the "never" calls and returning only the approval-required ones, needs partial execution that the Anthropic tool loop cannot express without fabricating tool_result blocks for the calls it withheld, so the whole-request fail-closed gate is the safe minimum. A future change can add per-call partial execution if a caller needs it. Test covers the mixed and manual cases; reverting to "any never" fails it. --- .../mcp/litellm_proxy_mcp_handler.py | 28 ++++++++++++------- .../mcp_tests/test_aresponses_api_with_mcp.py | 9 ++++++ 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/litellm/responses/mcp/litellm_proxy_mcp_handler.py b/litellm/responses/mcp/litellm_proxy_mcp_handler.py index d2c9f220690..a94cd2413d8 100644 --- a/litellm/responses/mcp/litellm_proxy_mcp_handler.py +++ b/litellm/responses/mcp/litellm_proxy_mcp_handler.py @@ -478,17 +478,25 @@ class LiteLLM_Proxy_MCP_Handler: ) -> bool: """Check if we should auto-execute tool calls. - Only auto-execute tools if user passed a MCP tool with require_approval set to "never". - - + Auto-execution requires EVERY MCP reference to opt in with + ``require_approval="never"``. A single reference that requires approval + ("always", "manual", the object form, or an unset value) disables + auto-execution for the whole request. This fails closed: when an + approval-required reference shares a request with a "never" one, the + model's tool calls are returned to the caller instead of being run, so + an approval-gated tool can never be invoked without approval. Returns + False for an empty list. """ - for tool in mcp_tools_with_litellm_proxy: - if isinstance(tool, dict): - if tool.get("require_approval") == "never": - return True - elif getattr(tool, "require_approval", None) == "never": - return True - return False + references = list(mcp_tools_with_litellm_proxy or []) + if not references: + return False + for tool in references: + approval = ( + tool.get("require_approval") if isinstance(tool, dict) else getattr(tool, "require_approval", None) + ) + if approval != "never": + return False + return True @staticmethod def _extract_tool_calls_from_response(response: ResponsesAPIResponse) -> List[Any]: diff --git a/tests/mcp_tests/test_aresponses_api_with_mcp.py b/tests/mcp_tests/test_aresponses_api_with_mcp.py index 9cd45f3d6fc..32295310005 100644 --- a/tests/mcp_tests/test_aresponses_api_with_mcp.py +++ b/tests/mcp_tests/test_aresponses_api_with_mcp.py @@ -86,6 +86,15 @@ async def test_mcp_helper_methods(): LiteLLM_Proxy_MCP_Handler._should_auto_execute_tools(mcp_tools_always) == False ) + # A single approval-required reference must disable auto-execution for the + # whole request; otherwise a "never" reference alongside an "always" one + # would let the approval-gated tool run without approval. + mcp_tools_mixed = [{"require_approval": "never"}, {"require_approval": "always"}] + assert LiteLLM_Proxy_MCP_Handler._should_auto_execute_tools(mcp_tools_mixed) == False + mcp_tools_manual = [{"require_approval": "never"}, {"require_approval": "manual"}] + assert LiteLLM_Proxy_MCP_Handler._should_auto_execute_tools(mcp_tools_manual) == False + assert LiteLLM_Proxy_MCP_Handler._should_auto_execute_tools([]) == False + print("✓ MCP helper methods test passed!")