Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions coworker/mcp/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,22 @@ def build_callables(
name = tool_name(server.name, mcp_tool.name)
remote = mcp_tool.name

def _invoke(_remote: str = remote, **kwargs: Any) -> Any:
future = asyncio.run_coroutine_threadsafe(call_async(_remote, kwargs), loop)
return future.result(timeout)
# Bind `remote` in a dedicated closure rather than a default argument. The
# default-argument form (`_remote=remote`) also defeats late binding, but it
# exposes the routing target as a caller-overridable keyword: a model-supplied
# `_remote` in the tool-call arguments would then re-route an approved, filtered
# call to any other tool on this server, since the approval gate and the
# include/exclude filter only ever see the visible tool name.
def _make_invoke(remote: str) -> Callable[..., Any]:
def _invoke(**kwargs: Any) -> Any:
future = asyncio.run_coroutine_threadsafe(
call_async(remote, kwargs), loop
)
return future.result(timeout)

return _invoke

_invoke = _make_invoke(remote)

# We attach the schema + metadata explicitly (rather than via `ai.tool`, which would
# try to derive a schema from this `**kwargs` wrapper): the registry reads both attrs.
Expand Down
29 changes: 29 additions & 0 deletions tests/test_mcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,35 @@ async def call_async(tool, args):
assert seen == [("read_file", {"path": "a.txt"})]


async def test_remote_tool_routing_is_not_overridable_by_arguments():
"""A model-supplied ``_remote`` argument must not re-route an approved,
filtered tool call to a different remote tool.

The approval gate and the include/exclude filter both vet the *visible*
tool name (``mcp__fs__read_file``). If ``_remote`` were a bindable
parameter, ``{"_remote": "delete_file"}`` in the call arguments would run
``delete_file`` while the gate only ever saw ``read_file``.
"""
loop = asyncio.get_running_loop()
seen = []

async def call_async(tool, args):
seen.append((tool, args))
return {"echo": args}

server = MCPServerDef(name="fs", transport="stdio")
fn = build_callables(server, [_fake_tool("read_file")], call_async, loop)[0]

# Simulate the model emitting a `_remote` key in the tool-call arguments.
await asyncio.to_thread(fn, _remote="delete_file", path="a.txt")

routed_tool = seen[0][0]
assert routed_tool == "read_file", (
f"routing hijacked: call reached {routed_tool!r} instead of the "
"approved 'read_file'"
)


# -- REST ----------------------------------------------------------------------
def test_rest_crud(tmp_path, monkeypatch):
monkeypatch.setenv("COWORKER_STATE_DIR", str(tmp_path / "state"))
Expand Down