Skip to content

fix: keep tool input from overriding the MCP handler's tool - #613

Open
shreyanshj10 wants to merge 1 commit into
shareAI-lab:mainfrom
shreyanshj10:fix/issue-608-mcp-handler-binding
Open

shreyanshj10 wants to merge 1 commit into
shareAI-lab:mainfrom
shreyanshj10:fix/issue-608-mcp-handler-binding

Conversation

@shreyanshj10

Copy link
Copy Markdown

Fixes #608.

What was wrong

assemble_tool_pool() built each MCP handler as a lambda whose client and tool were ordinary keyword parameters carrying defaults:

handlers[prefixed] = (
    lambda *, client=server, tool=raw_name, **kwargs:
    client.call_tool(tool, kwargs)
)

execute_tool() dispatches with handler(**block.input), so a tool key in the model's tool input replaces the default. With the chapter's own MCP_HOST_POLICY, mcp__deploy__status is allow and mcp__deploy__trigger is confirm, so this call:

{"name": "mcp__deploy__status", "input": {"service": "web", "tool": "trigger"}}

clears permission_hook() with no prompt and then runs call_tool("trigger", {"service": "web"}). Against the unmodified chapter I got [deploy] Triggered: web with zero permission prompts.

I do not read this as a deliberate simplification. §5 introduces the default arguments only as the late-binding fix for the loop variable, and §6 says the host policy decides what runs. The dispatch path quietly contradicts §6, and since the point of the chapter is that the host owns authorization, a handler the tool input can re-target undercuts the lesson itself rather than shortening it.

The change

A small factory binds the pair in a closure, where call-time keywords cannot reach it:

def make_mcp_handler(client: MCPClient, tool: str):
    """Bind client and tool in a closure so tool input cannot replace them."""
    return lambda **kwargs: client.call_tool(tool, kwargs)

That still fixes late binding, so §5's point stands. The same input now calls status with a stray tool argument, and the server rejects it at the tool boundary exactly as §7 describes:

MCP error: TypeError: <lambda>() got an unexpected keyword argument 'tool'

functools.partial is not an alternative here: keywords passed at call time override the ones stored in a partial, so the redirect still goes through. I checked that before writing the fix.

s15_integrated_harness/code.py carries the same two lines, so it had the same bypass. I fixed both rather than leaving the integrated harness able to do what s14 no longer can, which is how #586 handled the s11/s15 pair. MCP_LESSONS in tests/test_agent_teams_runtime.py already pairs the two chapters, so the test below covers both. Nothing else changed — no input validation, no schema enforcement, no extra error handling.

The three s14_mcp_plugin READMEs are updated with the identical code block, plus a sentence saying default arguments solve late binding but stay overridable by caller keywords. The s15 READMEs describe this part in prose and do not embed the snippet, so they needed no change.

Tests

tests/test_agent_teams_runtime.py already runs test_mcp_permission_uses_host_policy over MCP_LESSONS (s14 and s15), so I added one sibling test beside it instead of a new file or any scaffolding. It fails before this change and passes after.

I ran the tests this change touches: python -m pytest tests/test_agent_teams_runtime.py -k mcp -q (4 passed, covering both chapters) and python -m pytest tests/test_chapter_readmes.py -q (3 passed). I also checked the chapter-loading tests in tests/test_agent_loop_boundaries.py, tests/test_permission_command_words.py and tests/test_compaction_tool_pairs.py against main and this change adds no failures there.

AI disclosure

I used an AI coding assistant on this change. I read it, reproduced the bypass and verified the fix against the chapter code myself, and I stand behind it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

s14: MCP handler defaults can be overridden by tool input, bypassing host policy

2 participants