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.
This commit is contained in:
Tin Chi Lo 2026-07-17 11:34:08 -07:00
parent 56cda9f674
commit cf23df9431
2 changed files with 27 additions and 10 deletions

View file

@ -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]:

View file

@ -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!")