diff --git a/python/packages/hosting-mcp/AGENTS.md b/python/packages/hosting-mcp/AGENTS.md index b4b403280e0..3417431241c 100644 --- a/python/packages/hosting-mcp/AGENTS.md +++ b/python/packages/hosting-mcp/AGENTS.md @@ -23,10 +23,17 @@ authentication, authorization, session-key policy, concurrency policy, or outbound delivery. Applications compose the adapter and conversion helpers with native MCP SDK constructs. -`AgentMCPTool` owns only the schema for its single generated agent tool. It -does not register that schema with a server. Applications call -`await adapter.list_tools()` and `await adapter.call_tool(...)` from native MCP -handlers. +`AgentMCPTool` generates and validates the schema for its single agent tool. +`call_tool(...)` rejects schema-invalid arguments before conversion, agent +execution, or session-store access. It does not register that schema with a +server. Applications call `await adapter.list_tools()` and +`await adapter.call_tool(...)` from native MCP handlers. Direct +`mcp_to_run(...)` conversion does not perform schema validation. + +Parameter schemas are trusted application configuration. Validation is +synchronous and has no execution timeout; applications must choose regex +patterns with predictable matching time for caller-controlled values and +property names. Schema enforcement does not provide execution isolation. `WorkflowMCPTool` owns only the schema derived from the start executor. It requires exactly one input type. Workflow factories and continuation policy diff --git a/python/packages/hosting-mcp/README.md b/python/packages/hosting-mcp/README.md index 8b077efb99e..880f2e1bda2 100644 --- a/python/packages/hosting-mcp/README.md +++ b/python/packages/hosting-mcp/README.md @@ -24,6 +24,11 @@ contract. Define it once and use the same value in both the native tool schema and `mcp_to_run(...)`: ```python +from jsonschema import validate +from mcp.types import Tool + +from agent_framework_hosting_mcp import mcp_to_run + agent_input_argument = "task" chat_option_arguments = { "reasoning_effort": { @@ -34,7 +39,7 @@ chat_option_arguments = { tool = Tool( name="run_agent", - inputSchema={ + input_schema={ "type": "object", "properties": { agent_input_argument: {"type": "string"}, @@ -43,6 +48,7 @@ tool = Tool( "required": [agent_input_argument], }, ) +validate(instance=arguments, schema=tool.input_schema) run = mcp_to_run( arguments, argument_name=agent_input_argument, @@ -52,13 +58,19 @@ run = mcp_to_run( Only names listed in `chat_option_arguments` are copied to `run["options"]`; other MCP arguments remain available in the message's raw representation but -are not forwarded to the model client. The native MCP schema remains -responsible for validating exposed option types and ranges. +are not forwarded to the model client. Applications using the conversion +helper directly must validate arguments against their tool schema before +conversion. Declaring a schema does not enforce it in the low-level MCP server. For an agent exposed as one MCP tool, use the adapter so the schema and conversion cannot drift: ```python +from mcp import types +from mcp.server import Server, ServerRequestContext + +from agent_framework_hosting_mcp import AgentMCPTool + agent_tool = AgentMCPTool( agent, name="run_agent", @@ -72,13 +84,22 @@ agent_tool = AgentMCPTool( }, ) -@server.list_tools() -async def list_tools(): + +async def list_tools( + _ctx: ServerRequestContext[dict[str, object]], + _params: types.PaginatedRequestParams | None, +) -> types.ListToolsResult: return await agent_tool.list_tools() -@server.call_tool() -async def call_tool(name, arguments): - return await agent_tool.call_tool(name, arguments) + +async def call_tool( + _ctx: ServerRequestContext[dict[str, object]], + params: types.CallToolRequestParams, +) -> types.CallToolResult: + return await agent_tool.call_tool(params.name, params.arguments) + + +server = Server("agent-server", on_list_tools=list_tools, on_call_tool=call_tool) ``` `AgentMCPTool` uses the agent's name and description unless overridden. @@ -86,6 +107,24 @@ async def call_tool(name, arguments): raw MCP arguments. `chat_option_parameters` adds properties and explicitly copies their values into Agent Framework chat options. +`call_tool(...)` validates the complete generated input schema, including +nested constraints, before conversion, agent execution, or session-store +access. Invalid arguments raise `MCPError` with `INVALID_PARAMS`. Valid exposed +chat options still override the agent's defaults. The adapter's +`mcp_to_run(...)` method remains a conversion helper, not a schema validator. + +Treat `parameters` and `chat_option_parameters` as trusted application +configuration, not client-supplied schemas. Validation runs synchronously on +the server event loop and has no execution timeout. JSON Schema `pattern` and +`patternProperties` use Python's backtracking regular-expression engine. +Unsafe configured patterns can let caller-controlled values or property names +stall unrelated requests. Choose and test patterns with predictable matching +time for untrusted inputs; request-size limits alone do not make a +backtracking pattern safe. Applications requiring a hard validation deadline +must use an execution boundary that can be terminated, such as a separate +process. An asyncio timeout or worker thread cannot interrupt a running +standard-library regex match. + For a workflow exposed as one MCP tool, use `WorkflowMCPTool`: ```python diff --git a/python/packages/hosting-mcp/agent_framework_hosting_mcp/_agent_tool.py b/python/packages/hosting-mcp/agent_framework_hosting_mcp/_agent_tool.py index 4a551e5a5ba..5a0f35b27e0 100644 --- a/python/packages/hosting-mcp/agent_framework_hosting_mcp/_agent_tool.py +++ b/python/packages/hosting-mcp/agent_framework_hosting_mcp/_agent_tool.py @@ -11,6 +11,7 @@ from agent_framework import AgentResponse, Message, SupportsAgentRun from agent_framework._telemetry import mark_feature_used from agent_framework_hosting import AgentRunArgs, AgentState +from jsonschema import ValidationError, validate from mcp import MCPError, types from ._conversion import mcp_from_run, mcp_to_run @@ -22,10 +23,10 @@ class AgentMCPTool(Generic[AgentT]): """Expose one Agent Framework agent through an app-owned MCP server. - The adapter generates the native MCP tool definition and keeps its argument - parsing aligned with agent execution. It does not create an MCP server, - register handlers, derive trusted session identifiers, or choose a - transport. + The adapter generates the native MCP tool definition and validates its + arguments before agent execution or session-store access. It does not + create an MCP server, register handlers, derive trusted session identifiers, + or choose a transport. """ def __init__( @@ -146,7 +147,8 @@ async def call_tool( Native MCP ``CallToolResult`` for the completed tool result. Raises: - MCPError: If the tool name or configured session id is invalid. + MCPError: If the tool name or arguments are invalid. + jsonschema.SchemaError: If the configured input schema is invalid. """ mark_feature_used(FeatureIndex.HOSTING_MCP) target = await self.state.get_target() @@ -154,6 +156,11 @@ async def call_tool( if name != tool.name: raise MCPError(types.INVALID_PARAMS, f"Unknown MCP tool: {name}") + try: + validate(instance=dict(arguments or {}), schema=tool.input_schema) + except ValidationError as exc: + raise MCPError(types.INVALID_PARAMS, f"Invalid MCP tool arguments: {exc.message}") from exc + run = self.mcp_to_run(arguments) if self.session_id_parameter is None: result = cast( diff --git a/python/packages/hosting-mcp/pyproject.toml b/python/packages/hosting-mcp/pyproject.toml index 2123a74575f..cb3cc23f5e8 100644 --- a/python/packages/hosting-mcp/pyproject.toml +++ b/python/packages/hosting-mcp/pyproject.toml @@ -25,6 +25,7 @@ classifiers = [ dependencies = [ "agent-framework-core>=1.19.0,<2", "agent-framework-hosting==1.0.0a261030", + "jsonschema>=4.20.0,<5", "mcp>=2.2.0,<3", "pydantic>=2,<3", ] diff --git a/python/packages/hosting-mcp/tests/hosting_mcp/test_agent_tool.py b/python/packages/hosting-mcp/tests/hosting_mcp/test_agent_tool.py index 5dee3253088..fca6082b159 100644 --- a/python/packages/hosting-mcp/tests/hosting_mcp/test_agent_tool.py +++ b/python/packages/hosting-mcp/tests/hosting_mcp/test_agent_tool.py @@ -2,7 +2,10 @@ from __future__ import annotations -from collections.abc import Awaitable, Mapping, Sequence +from collections.abc import Awaitable, Callable, Mapping, Sequence +from contextlib import AsyncExitStack, nullcontext +from copy import deepcopy +from types import MappingProxyType from typing import Any from unittest.mock import AsyncMock, patch @@ -18,12 +21,40 @@ ResponseStream, ) from agent_framework_hosting import AgentState +from jsonschema import SchemaError from mcp import Client, MCPError, types from mcp.server import Server, ServerRequestContext from pytest import raises from agent_framework_hosting_mcp import AgentMCPTool +_VALID_TOOL_ARGUMENTS: dict[str, Any] = { + "task": "hello", + "audience": "developers", + "model": "approved-model", + "max_tokens": 8, + "store": False, +} +_TOOL_PARAMETERS: dict[str, dict[str, Any]] = { + "audience": {"type": "string", "minLength": 1}, + "metadata": { + "type": "object", + "properties": { + "tags": {"type": "array", "minItems": 1, "items": {"type": "string", "minLength": 1}}, + "priority": {"$ref": "#/properties/metadata/$defs/priority"}, + }, + "required": ["tags", "priority"], + "additionalProperties": False, + "$defs": {"priority": {"enum": ["low", "high"]}}, + }, + "selection": {"oneOf": [{"type": "integer", "minimum": 1, "maximum": 3}, {"const": "auto"}]}, +} +_CHAT_OPTION_PARAMETERS: dict[str, dict[str, Any]] = { + "model": {"type": "string", "enum": ["approved-model"]}, + "max_tokens": {"type": "integer", "minimum": 1, "maximum": 8}, + "store": {"type": "boolean", "const": False}, +} + class RecordingClient(BaseChatClient[ChatOptions[None]]): """Record messages and return their latest text.""" @@ -167,29 +198,50 @@ async def test_agent_tool_always_requires_session_parameter() -> None: async def test_agent_tool_unknown_tool_name() -> None: - agent = Agent(client=RecordingClient(), name="agent") + client = RecordingClient() + agent = Agent(client=client, name="agent") tool: AgentMCPTool[Any] = AgentMCPTool( agent, parameters={"session_id": {"type": "string"}}, session_id_parameter="session_id", name="available_tool" ) - with raises(MCPError, match="Unknown MCP tool") as exc_info: + with ( + patch.object(agent, "run", wraps=agent.run) as run_agent, + patch.object(tool.state.session_store, "get", wraps=tool.state.session_store.get) as get_session, + patch.object(tool.state.session_store, "set", wraps=tool.state.session_store.set) as set_session, + raises(MCPError, match="Unknown MCP tool") as exc_info, + ): await tool.call_tool("made_up_tool", None) assert exc_info.value.code == types.INVALID_PARAMS + run_agent.assert_not_called() + get_session.assert_not_awaited() + set_session.assert_not_awaited() + assert not client.calls -async def test_agent_tool_rejects_missing_runtime_session_id() -> None: - agent = Agent(client=RecordingClient(), name="agent") +@pytest.mark.parametrize("arguments", [{"task": "hello"}, {"task": "hello", "session_id": ""}]) +async def test_agent_tool_rejects_invalid_runtime_session_id(arguments: dict[str, Any]) -> None: + client = RecordingClient() + agent = Agent(client=client, name="agent") tool: AgentMCPTool[Any] = AgentMCPTool( agent, parameters={"session_id": {"type": "string"}}, session_id_parameter="session_id", ) - with raises(MCPError) as exc_info: - await tool.call_tool("agent", {"task": "hello"}) + with ( + patch.object(agent, "run", wraps=agent.run) as run_agent, + patch.object(tool.state.session_store, "get", wraps=tool.state.session_store.get) as get_session, + patch.object(tool.state.session_store, "set", wraps=tool.state.session_store.set) as set_session, + raises(MCPError) as exc_info, + ): + await tool.call_tool("agent", arguments) assert exc_info.value.code == types.INVALID_PARAMS + run_agent.assert_not_called() + get_session.assert_not_awaited() + set_session.assert_not_awaited() + assert not client.calls async def test_agent_tool_propagates_agent_execution_failure() -> None: @@ -206,39 +258,201 @@ async def test_agent_tool_propagates_agent_execution_failure() -> None: @pytest.mark.parametrize( ("mode", "expected_version"), [ + ("direct", None), ("auto", "2026-07-28"), ("legacy", "2025-11-25"), ], ) -async def test_agent_tool_serves_both_protocol_eras( +@pytest.mark.parametrize("session_mode", ["none", "new", "existing"]) +@pytest.mark.parametrize( + ("arguments", "valid"), + [ + pytest.param(_VALID_TOOL_ARGUMENTS, True, id="valid-overrides"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "max_tokens": 1}, True, id="valid-minimum"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "max_tokens": 3.0}, True, id="valid-whole-number-float"), + pytest.param( + { + **_VALID_TOOL_ARGUMENTS, + "metadata": {"tags": ["first"], "priority": "low"}, + "selection": "auto", + }, + True, + id="valid-nested-arguments", + ), + pytest.param({**_VALID_TOOL_ARGUMENTS, "selection": 2}, True, id="valid-composition"), + pytest.param(None, False, id="no-arguments"), + pytest.param({}, False, id="empty-arguments"), + *( + pytest.param( + {key: value for key, value in _VALID_TOOL_ARGUMENTS.items() if key != missing}, + False, + id=f"missing-{missing}", + ) + for missing in ("task", "audience", "model", "max_tokens", "store") + ), + pytest.param({**_VALID_TOOL_ARGUMENTS, "task": None}, False, id="null-task"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "task": 1}, False, id="non-string-task"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "audience": ""}, False, id="empty-audience"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "model": "other-model"}, False, id="invalid-enum"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "max_tokens": 0}, False, id="below-minimum"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "max_tokens": 9}, False, id="above-maximum"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "max_tokens": "8"}, False, id="numeric-string"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "max_tokens": True}, False, id="boolean-integer"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "max_tokens": 3.5}, False, id="fractional-integer"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "store": True}, False, id="invalid-const"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "store": 0}, False, id="non-boolean-store"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "undeclared": True}, False, id="unknown-property"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "metadata": {}}, False, id="nested-required"), + pytest.param( + {**_VALID_TOOL_ARGUMENTS, "metadata": {"tags": [], "priority": "low"}}, + False, + id="nested-array-length", + ), + pytest.param( + {**_VALID_TOOL_ARGUMENTS, "metadata": {"tags": [1], "priority": "low"}}, + False, + id="nested-array-item-type", + ), + pytest.param( + {**_VALID_TOOL_ARGUMENTS, "metadata": {"tags": ["first"], "priority": "other"}}, + False, + id="local-reference", + ), + pytest.param( + {**_VALID_TOOL_ARGUMENTS, "metadata": {"tags": ["first"], "priority": "low", "undeclared": True}}, + False, + id="nested-unknown-property", + ), + pytest.param({**_VALID_TOOL_ARGUMENTS, "selection": 0}, False, id="composition"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "session_id": None}, False, id="null-session-id"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "session_id": 1}, False, id="non-string-session-id"), + pytest.param({**_VALID_TOOL_ARGUMENTS, "session_id": "other"}, False, id="session-id-pattern"), + ], +) +async def test_agent_tool_validates_schema_before_execution( mode: str, - expected_version: str, + expected_version: str | None, + session_mode: str, + arguments: dict[str, Any] | None, + valid: bool, ) -> None: - agent = Agent(client=RecordingClient(), name="agent") - agent_tool: AgentMCPTool[Any] = AgentMCPTool(agent) - - async def list_tools( - _ctx: ServerRequestContext[dict[str, Any]], _params: types.PaginatedRequestParams | None - ) -> types.ListToolsResult: - return await agent_tool.list_tools() - - async def call_tool( - _ctx: ServerRequestContext[dict[str, Any]], params: types.CallToolRequestParams - ) -> types.CallToolResult: - return await agent_tool.call_tool(params.name, params.arguments or {}) - - server = Server( - "test-server", - on_list_tools=list_tools, - on_call_tool=call_tool, + client = RecordingClient() + default_options: ChatOptions[None] = {"model": "default-model", "max_tokens": 2, "store": True} + agent = Agent( + client=client, + name="agent", + default_options=default_options, + context_providers=[InMemoryHistoryProvider()], + ) + state = AgentState(agent) + if session_mode == "existing": + session = agent.create_session(session_id="session-1") + session.state["marker"] = {"values": ["unchanged"]} + await state.set_session("session-1", session) + previous_session = await state.session_store.get("session-1") + previous_snapshot = previous_session.to_dict() if previous_session is not None else None + parameters = dict(_TOOL_PARAMETERS) + if session_mode != "none": + parameters["session_id"] = {"type": "string", "pattern": "^session-[0-9]+$"} + agent_tool: AgentMCPTool[Any] = AgentMCPTool( + state, + parameters=parameters, + required_parameters=("audience", "model", "max_tokens", "store"), + chat_option_parameters=_CHAT_OPTION_PARAMETERS, + session_id_parameter="session_id" if session_mode != "none" else None, + ) + call_arguments = deepcopy(arguments) + if session_mode != "none" and call_arguments is not None: + call_arguments.setdefault("session_id", "session-1") + arguments_snapshot = deepcopy(call_arguments) + result: types.CallToolResult | None = None + + with ( + patch.object(agent, "run", wraps=agent.run) as run_agent, + patch.object(client, "_inner_get_response", wraps=client._inner_get_response) as provider, + patch.object(state.session_store, "get", wraps=state.session_store.get) as get_session, + patch.object(state.session_store, "set", wraps=state.session_store.set) as set_session, + ): + async with AsyncExitStack() as stack: + invoke: Callable[[str, dict[str, Any] | None], Awaitable[types.CallToolResult]] = agent_tool.call_tool + if mode != "direct": + + async def list_tools( + _ctx: ServerRequestContext[dict[str, Any]], _params: types.PaginatedRequestParams | None + ) -> types.ListToolsResult: + return await agent_tool.list_tools() + + async def call_tool( + _ctx: ServerRequestContext[dict[str, Any]], params: types.CallToolRequestParams + ) -> types.CallToolResult: + return await agent_tool.call_tool(params.name, params.arguments) + + server = Server("test-server", on_list_tools=list_tools, on_call_tool=call_tool) + mcp_client = await stack.enter_async_context(Client(server, mode=mode)) + tools = await mcp_client.list_tools() + assert mcp_client.protocol_version == expected_version + assert [tool.name for tool in tools.tools] == ["agent"] + assert tools.tools[0].input_schema == (await agent_tool.list_tools()).tools[0].input_schema + invoke = mcp_client.call_tool + + with nullcontext() if valid else raises(MCPError) as exc_info: + result = await invoke("agent", call_arguments) + + if valid: + assert result is not None + assert result.result_type == "complete" + assert not result.is_error + assert result.content + run_agent.assert_called_once() + provider.assert_called_once() + assert call_arguments is not None + options = provider.call_args.kwargs["options"] + for option in _CHAT_OPTION_PARAMETERS: + assert options[option] == call_arguments[option] + assert type(options[option]) is type(call_arguments[option]) + assert not {"audience", "metadata", "selection", "session_id"} & options.keys() + if session_mode == "none": + get_session.assert_not_awaited() + set_session.assert_not_awaited() + else: + get_session.assert_awaited_once_with("session-1") + assert set_session.await_count == (2 if session_mode == "new" else 1) + else: + assert exc_info is not None + assert exc_info.value.code == types.INVALID_PARAMS + run_agent.assert_not_called() + provider.assert_not_called() + get_session.assert_not_awaited() + set_session.assert_not_awaited() + assert not client.calls + + assert call_arguments == arguments_snapshot + if not valid: + stored_session = await state.session_store.get("session-1") + assert (stored_session.to_dict() if stored_session is not None else None) == previous_snapshot + + +@pytest.mark.parametrize("argument_name", ["task", "prompt"]) +async def test_agent_tool_accepts_mapping_arguments(argument_name: str) -> None: + client = RecordingClient() + agent = Agent(client=client, name="agent") + tool: AgentMCPTool[Any] = AgentMCPTool(agent, argument_name=argument_name) + + result = await tool.call_tool("agent", MappingProxyType({argument_name: "hello"})) + + assert not result.is_error + assert client.calls == [["hello"]] + + +async def test_agent_tool_propagates_invalid_schema_configuration() -> None: + client = RecordingClient() + agent = Agent(client=client, name="agent") + tool: AgentMCPTool[Any] = AgentMCPTool( + agent, + chat_option_parameters={"max_tokens": {"type": "unsupported"}}, ) - async with Client(server, mode=mode) as mcp_client: - tools = await mcp_client.list_tools() - result = await mcp_client.call_tool("agent", {"task": "hello"}) + with raises(SchemaError): + await tool.call_tool("agent", {"task": "hello", "max_tokens": 1}) - assert mcp_client.protocol_version == expected_version - assert [tool.name for tool in tools.tools] == ["agent"] - assert result.result_type == "complete" - assert not result.is_error - assert result.content + assert not client.calls diff --git a/python/uv.lock b/python/uv.lock index 089e8481beb..e913077c596 100644 --- a/python/uv.lock +++ b/python/uv.lock @@ -826,6 +826,7 @@ source = { editable = "packages/hosting-mcp" } dependencies = [ { name = "agent-framework-core" }, { name = "agent-framework-hosting" }, + { name = "jsonschema" }, { name = "mcp" }, { name = "pydantic" }, ] @@ -834,6 +835,7 @@ dependencies = [ requires-dist = [ { name = "agent-framework-core", editable = "packages/core" }, { name = "agent-framework-hosting", editable = "packages/hosting" }, + { name = "jsonschema", specifier = ">=4.20.0,<5" }, { name = "mcp", specifier = ">=2.2.0,<3" }, { name = "pydantic", specifier = ">=2,<3" }, ] diff --git a/skills/agent-framework-builder-python/references/hosting-and-deployment.md b/skills/agent-framework-builder-python/references/hosting-and-deployment.md index 480c09fc374..a0bec55e2ba 100644 --- a/skills/agent-framework-builder-python/references/hosting-and-deployment.md +++ b/skills/agent-framework-builder-python/references/hosting-and-deployment.md @@ -26,6 +26,11 @@ into application code. checkpoint restoration. - Make session, checkpoint, response, identity, approval, storage, and cleanup ownership explicit. +For MCP agent tools, `AgentMCPTool.call_tool(...)` validates the generated input schema before execution or +session-store access. Applications using `mcp_to_run(...)` directly must validate their own schema before conversion. +Keep schemas in trusted application configuration. Regex validation is synchronous and has no execution timeout; +choose patterns with predictable matching time for untrusted values and property names. + Read the selected host's public docstrings and maintained sample for constructor options, parsers, history modes, storage providers, and protocol details.