Skip to content

Commit f04ab55

Browse files
authored
Require approval for callable MCP policies when agent is omitted (#2553)
1 parent b7cdd78 commit f04ab55

3 files changed

Lines changed: 29 additions & 3 deletions

File tree

src/agents/mcp/server.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ def _get_needs_approval_for_tool(
236236

237237
if callable(policy):
238238
if agent is None:
239-
return False
239+
return True
240240

241241
async def _needs_approval(
242242
run_context: RunContextWrapper[Any], _args: dict[str, Any], _call_id: str

src/agents/mcp/util.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,9 @@ def to_function_tool(
240240
241241
The ``agent`` parameter is optional for backward compatibility with older
242242
call sites that used ``MCPUtil.to_function_tool(tool, server, strict)``.
243-
When omitted, this helper preserves the historical behavior and leaves
244-
``needs_approval`` disabled.
243+
When omitted, this helper preserves the historical behavior for static
244+
policies. If the server uses a callable approval policy, approvals default
245+
to required to avoid bypassing dynamic checks.
245246
"""
246247
invoke_func_impl = functools.partial(cls.invoke_mcp_tool, server, tool)
247248
effective_failure_error_function = server._get_failure_error_function(

tests/mcp/test_mcp_util.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,31 @@ async def test_to_function_tool_legacy_call_without_agent_uses_server_policy():
322322
pytest.fail(f"Unexpected tool result type: {type(result).__name__}")
323323

324324

325+
@pytest.mark.asyncio
326+
async def test_to_function_tool_legacy_call_callable_policy_requires_approval():
327+
"""Legacy to_function_tool calls should default to approval for callable policies."""
328+
329+
server = FakeMCPServer()
330+
server.add_tool("legacy_callable_tool", {})
331+
332+
def require_approval(
333+
_run_context: RunContextWrapper[Any],
334+
_agent: Agent,
335+
_tool: MCPTool,
336+
) -> bool:
337+
return False
338+
339+
server._needs_approval_policy = require_approval # type: ignore[assignment]
340+
341+
function_tool = MCPUtil.to_function_tool(
342+
MCPTool(name="legacy_callable_tool", inputSchema={}),
343+
server,
344+
convert_schemas_to_strict=False,
345+
)
346+
347+
assert function_tool.needs_approval is True
348+
349+
325350
@pytest.mark.asyncio
326351
async def test_mcp_tool_failure_error_function_agent_default():
327352
"""Agent-level failure_error_function should handle MCP tool failures."""

0 commit comments

Comments
 (0)