diff --git a/python/packages/core/AGENTS.md b/python/packages/core/AGENTS.md index 593aa1e4b5..e515e70e2b 100644 --- a/python/packages/core/AGENTS.md +++ b/python/packages/core/AGENTS.md @@ -103,10 +103,11 @@ agent_framework/ - **`MCPTool`** - Base wrapper that owns the MCP `ClientSession` and exposes the remote server's tools as `FunctionTool`s. - **`MCPStdioTool`** / **`MCPStreamableHTTPTool`** / **`MCPWebsocketTool`** - Transport-specific subclasses. -- **Argument allowlist (`_prepare_call_kwargs`)** - Before each `tools/call`, kwargs are filtered to an **allowlist** built from the tool's declared parameters (`inputSchema.properties`) plus any user-configured extras. Framework runtime kwargs injected through the function-invocation pipeline (e.g. `thread`, `conversation_id`, `chat_options`, `options`, `response_format`) are stripped by default rather than forwarded. A tool that declares no usable `properties` (including schemas with `additionalProperties: true`) forwards only the configured extras. The `_MCP_FRAMEWORK_DENYLIST` is a safety net for framework-named params a server *declares* in its schema (those are dropped); names explicitly opted in via `additional_tool_argument_names` always win. The reserved `_meta` key is never forwarded as an argument; trusted caller/runtime `_meta` is validated as MCP request metadata, model-supplied `_meta` is discarded in generated MCP functions, and metadata precedence is caller/runtime < OpenTelemetry < tools/list metadata. +- **Argument allowlist (`_prepare_call_kwargs`)** - Before each `tools/call`, kwargs are filtered to an **allowlist** built from the tool's declared parameters (`inputSchema.properties`) plus any user-configured extras. **The declared half comes from the server's advertised schema**, and runtime kwargs (`FunctionInvocationContext.kwargs`, seeded from `function_invocation_kwargs`) are merged with the model-supplied arguments upstream in `_call_tool_with_runtime_kwargs`, so provenance is gone by the time the filter runs. A runtime kwarg is therefore forwarded whenever the server declares a property of that name, without the model mentioning it — the server, not the caller, decides which runtime kwarg names it receives. A tool that declares no usable `properties` (including schemas with `additionalProperties: true`) forwards only the configured extras. `_MCP_FRAMEWORK_DENYLIST` is a narrow safety net covering only non-serializable framework objects a server *declares* in its schema (those are dropped); it does not generalize to arbitrary caller-chosen names, and explicit extras always win. The reserved `_meta` key is never forwarded as an argument; trusted caller/runtime `_meta` is validated as MCP request metadata, model-supplied `_meta` is discarded in generated MCP functions, and metadata precedence is caller/runtime < OpenTelemetry < tools/list metadata. - **`allowed_tools`** (constructor arg on all `MCPTool` subclasses) - Restricts exposed MCP tools by raw remote MCP tool identity. Prefixed local names remain accepted only when the raw remote name already matches its normalized form; normalized/local aliases do not authorize a different raw remote name. If multiple raw remote tool names map to the same local function name, tool loading raises `ToolExecutionException` instead of first-one-wins shadowing. - **Progressive MCP disclosure** (`use_progressive_disclosure`, `always_load`) - When enabled on any `MCPTool` subclass, the initial model-facing surface is loader tools (`list_mcp_tools` / `load_tool` / `unload_tool`, prefixed by `tool_name_prefix` when configured) plus allowed tools selected by `always_load` and tools loaded earlier on the same `MCPTool` instance. `list_mcp_tools` only reports tools that pass `allowed_tools`; filtered tools are not listed or loadable. Loader tool names are reserved in progressive mode: remote MCP tools whose local generated name collides with a loader name are omitted from the initial/listed surface, and explicit `load_tool` calls return a model-visible message pointing callers to `tool_name_prefix` or excluding the colliding tool. `load_tool` accepts one tool name or a list of tool names and uses `FunctionInvocationContext.add_tools(...)` so the selected generated MCP `FunctionTool`s become available on the next function-calling iteration while keeping existing approval mode, argument filtering, header-provider runtime kwargs, result parsing, OTel, and task behavior. `unload_tool` accepts one dynamically loaded tool name or a list of names and removes them from the live tool list and persisted progressive surface, but it does not remove tools configured in `always_load`. Invalid `always_load` entries are ignored like unmatched `allowed_tools` entries. -- **`additional_tool_argument_names`** (constructor arg on all `MCPTool` subclasses) - Opt extra argument names back into the allowlist. Accepts a `Sequence[str]` (applied to every tool) or a `Mapping[str, Sequence[str]]` keyed by **remote tool name**, where the reserved key `"*"` denotes global extras. It is configured only in user code at construction; there is **no per-call/runtime override**, so a model-issued tool call cannot change which names pass through. To use a server that accepts `additionalProperties: true`, list the extra names here and then either (1) manually extend that tool's `inputSchema` (via the `.functions` list after connecting) so the model is prompted to supply them, or (2) supply the values yourself via `function_invocation_kwargs`. If a normal forwarded argument name is supplied by both the model and `function_invocation_kwargs`, the model-supplied value wins; `_meta` is the exception and only trusted runtime/caller metadata is used. +- **`additional_tool_argument_names`** (constructor arg on all `MCPTool` subclasses) - Opt extra argument names back into the allowlist. Accepts a `Sequence[str]` (applied to every tool) or a `Mapping[str, Sequence[str]]` keyed by **remote tool name**, where the reserved key `"*"` denotes global extras. It is configured only in user code at construction; there is **no per-call/runtime override**, so a model-issued tool call cannot change which names pass through — but note this constrains the *model*, not the *server*, which still widens the effective allowlist through its schema. To use a server that accepts `additionalProperties: true`, list the extra names here and then either (1) manually extend that tool's `inputSchema` (via the `.functions` list after connecting) so the model is prompted to supply them, or (2) supply the values yourself via `function_invocation_kwargs`. If a normal forwarded argument name is supplied by both the model and `function_invocation_kwargs`, the model-supplied value wins; `_meta` is the exception and only trusted runtime/caller metadata is used. +- **`function_invocation_kwargs` and MCP servers** - That dict is shared across every tool in the run, including every attached `MCPTool`, and any name in it reaches a server that declares a matching `inputSchema` property. `header_provider` does not mitigate this — it reads the kwargs without consuming them. To keep a credential out of tool arguments, source it outside `function_invocation_kwargs`: read a `ContextVar` inside the provider (this still allows a different value per request), configure a custom `http_client`, or use `env` for `MCPStdioTool`. - **Sampling guardrails** (`sampling_callback`) - Passing `client=` advertises `SamplingCapability` so the server can send `sampling/createMessage`. Because remote servers are untrusted (confused-deputy risk), the default `sampling_callback` is **deny-by-default** and applies, in order: a per-session rate limit (`sampling_max_requests`, default `_DEFAULT_SAMPLING_MAX_REQUESTS`), an approval gate (`sampling_approval_callback`), and a `maxTokens` cap (`sampling_max_tokens`, default `_DEFAULT_SAMPLING_MAX_TOKENS`). The approval callback (constructor arg on all subclasses; exported type alias `SamplingApprovalCallback`) receives the raw `CreateMessageRequestParams`, may be sync or async, and must return truthy to approve. When it is `None` (the default) every sampling request is denied; pass `lambda params: True` to restore legacy auto-approve as an explicit opt-in. Requests and denials are logged at WARNING (content is not logged). The per-session counter resets in `_reset_session_state`. - **`MCPTaskOptions`** (experimental, `MCP_LONG_RUNNING_TASKS` feature, **frozen**) - Per-tool-instance options controlling the SEP-2663 long-running task lifecycle. When the server advertises a tool with `execution.taskSupport == "required"`, `MCPTool.call_tool` transparently routes through `call_tool_as_task`, which sends an augmented `tools/call`, polls `tasks/get` until terminal, and reinterprets `tasks/result` as a normal `CallToolResult`. Instances are immutable; replace via `MCPTool.task_options = MCPTaskOptions(...)`. Fields: - `default_ttl: timedelta | None` — forwarded to the server as `params.task.ttl` (milliseconds). When `None`, the server's default applies. diff --git a/python/packages/core/agent_framework/_mcp.py b/python/packages/core/agent_framework/_mcp.py index 9abafdad27..b92341c17f 100644 --- a/python/packages/core/agent_framework/_mcp.py +++ b/python/packages/core/agent_framework/_mcp.py @@ -2041,7 +2041,12 @@ async def call_tool(self, tool_name: str, **kwargs: Any) -> str | list[Content]: ``meta`` parameter of the underlying ``session.call_tool`` call rather than as a tool argument. OpenTelemetry propagation overrides caller-supplied keys, and metadata from ``tools/list`` overrides both. - kwargs: Remaining arguments to pass to the tool. + kwargs: Remaining arguments to pass to the tool. Before the ``tools/call`` these are + filtered to the tool's server-declared ``inputSchema.properties`` plus names + opted in via ``additional_tool_argument_names``; ``_meta`` and the framework + denylist names are excluded. Because the declared set is server-controlled, a + runtime kwarg from ``function_invocation_kwargs`` is forwarded when the server + declares a matching name. Returns: A list of Content items representing the tool output. The default @@ -2152,14 +2157,19 @@ def _prepare_call_kwargs( user_meta = _validate_mcp_meta(kwargs.get("_meta")) # Allowlist: forward only the tool's declared parameters (from inputSchema.properties) - # plus any user-configured extra argument names. Everything else - notably the - # framework runtime kwargs injected through the function-invocation pipeline - is - # stripped so it is never forwarded to the MCP server. Tools that declare no usable - # properties forward only the user-configured extras. + # plus any user-configured extra argument names. Everything else is stripped. Tools that + # declare no usable properties forward only the user-configured extras. # - # The extra names come exclusively from additional_tool_argument_names, which is set in - # user code at construction time; there is no per-call override, so a model-issued tool - # call cannot change which names are allowed through. + # Runtime kwargs (FunctionInvocationContext.kwargs, seeded from function_invocation_kwargs) + # were merged with the model-supplied arguments upstream in _call_tool_with_runtime_kwargs, + # so provenance is not available here. A runtime kwarg is forwarded whenever its name is in + # `declared`, and `declared` comes from the server's own advertised schema (see load_tools). + # The server therefore selects which runtime kwarg names it receives; callers must not + # assume a name is withheld just because it is absent from additional_tool_argument_names. + # + # The extra names come exclusively from additional_tool_argument_names, set in user code at + # construction time; there is no per-call override, so a model-issued tool call cannot + # change which names pass. That constrains the model, not the server. # # The framework denylist acts as a safety net for keys a server *declares* in its # schema that collide with internal, non-serializable framework objects (e.g. a tool @@ -2823,9 +2833,8 @@ def __init__( ``execution.taskSupport == "required"``. See :class:`MCPTaskOptions`. additional_tool_argument_names: Extra argument names to forward to the MCP server in addition to each tool's declared parameters (from its ``inputSchema.properties``). - By default only declared parameters are sent; framework runtime kwargs injected - through the function-invocation pipeline are stripped. Use this to opt specific - keys back in. Accepts either a ``Sequence[str]`` applied to every tool, or a + By default only declared parameters and these extras are sent. Accepts either a + ``Sequence[str]`` applied to every tool, or a ``Mapping[str, Sequence[str]]`` keyed by remote tool name where the reserved key ``"*"`` applies to every tool. This is configured only here in user code; there is no per-call override, so a model-issued tool call cannot change which names pass @@ -2835,6 +2844,15 @@ def __init__( them, or (2) supply the values yourself through ``function_invocation_kwargs``. If a name is supplied via both the model and ``function_invocation_kwargs``, the model-supplied value wins. + + Note: this widens the allowlist, it does not bound it. The allowlist is built + from the server's advertised ``inputSchema.properties``, so a runtime kwarg from + ``function_invocation_kwargs`` is forwarded whenever the server declares a + property of that name, even if you never listed it here and the model never + mentioned it (``_meta`` and the framework denylist names are the exceptions). + Treat those keys as visible to this server, and source credentials outside + ``function_invocation_kwargs`` - for example through ``env`` - for servers whose + process you do not control. kwargs: Any extra arguments to pass to the stdio client. """ super().__init__( @@ -3030,13 +3048,16 @@ def __init__( origins on cross-origin redirects. If you instead supply sensitive headers through a custom ``http_client``, you must enforce this same origin-scoped policy yourself. + Note that the provider reads these kwargs without consuming them: the same + values continue on to the outbound argument filter, so reading a credential + here does not withhold it from the server. See + ``additional_tool_argument_names`` below. task_options: Options for tools that advertise ``execution.taskSupport == "required"``. See :class:`MCPTaskOptions`. additional_tool_argument_names: Extra argument names to forward to the MCP server in addition to each tool's declared parameters (from its ``inputSchema.properties``). - By default only declared parameters are sent; framework runtime kwargs injected - through the function-invocation pipeline are stripped. Use this to opt specific - keys back in. Accepts either a ``Sequence[str]`` applied to every tool, or a + By default only declared parameters and these extras are sent. Accepts either a + ``Sequence[str]`` applied to every tool, or a ``Mapping[str, Sequence[str]]`` keyed by remote tool name where the reserved key ``"*"`` applies to every tool. This is configured only here in user code; there is no per-call override, so a model-issued tool call cannot change which names pass @@ -3046,6 +3067,18 @@ def __init__( them, or (2) supply the values yourself through ``function_invocation_kwargs``. If a name is supplied via both the model and ``function_invocation_kwargs``, the model-supplied value wins. + + Note: this widens the allowlist, it does not bound it. The allowlist is built + from the server's advertised ``inputSchema.properties``, so a runtime kwarg from + ``function_invocation_kwargs`` is forwarded whenever the server declares a + property of that name, even if you never listed it here and the model never + mentioned it (``_meta`` and the framework denylist names are the exceptions). + The same dict is shared with every MCP server attached to the run, and + ``header_provider`` does not withhold anything - it reads these kwargs without + consuming them. To keep a credential out of tool arguments, source it outside + ``function_invocation_kwargs``: read a ``ContextVar`` inside the provider (which + still allows a different value per request), or configure a custom + ``http_client``. kwargs: Additional keyword arguments (accepted for backward compatibility but not used). """ super().__init__( @@ -3176,6 +3209,9 @@ async def call_tool(self, tool_name: str, **kwargs: Any) -> str | list[Content]: to the provider. The returned headers are attached to every HTTP request made during this tool call via a request hook on the underlying HTTP client. + The provider does not consume the kwargs: the same mapping continues to + :meth:`MCPTool.call_tool` and its outbound argument filter. + Args: tool_name: The name of the tool to call. @@ -3310,9 +3346,8 @@ def __init__( ``execution.taskSupport == "required"``. See :class:`MCPTaskOptions`. additional_tool_argument_names: Extra argument names to forward to the MCP server in addition to each tool's declared parameters (from its ``inputSchema.properties``). - By default only declared parameters are sent; framework runtime kwargs injected - through the function-invocation pipeline are stripped. Use this to opt specific - keys back in. Accepts either a ``Sequence[str]`` applied to every tool, or a + By default only declared parameters and these extras are sent. Accepts either a + ``Sequence[str]`` applied to every tool, or a ``Mapping[str, Sequence[str]]`` keyed by remote tool name where the reserved key ``"*"`` applies to every tool. This is configured only here in user code; there is no per-call override, so a model-issued tool call cannot change which names pass @@ -3322,6 +3357,15 @@ def __init__( them, or (2) supply the values yourself through ``function_invocation_kwargs``. If a name is supplied via both the model and ``function_invocation_kwargs``, the model-supplied value wins. + + Note: this widens the allowlist, it does not bound it. The allowlist is built + from the server's advertised ``inputSchema.properties``, so a runtime kwarg from + ``function_invocation_kwargs`` is forwarded whenever the server declares a + property of that name, even if you never listed it here and the model never + mentioned it (``_meta`` and the framework denylist names are the exceptions). + The same dict is shared with every MCP server attached to the run. This + transport has no header hook, so source credentials outside + ``function_invocation_kwargs`` for servers you do not control. kwargs: Any extra arguments to pass to the WebSocket client. """ super().__init__( diff --git a/python/packages/core/tests/core/test_mcp.py b/python/packages/core/tests/core/test_mcp.py index 028cac027b..30b210bfa6 100644 --- a/python/packages/core/tests/core/test_mcp.py +++ b/python/packages/core/tests/core/test_mcp.py @@ -8,6 +8,7 @@ import sys import warnings from contextlib import _AsyncGeneratorContextManager # type: ignore +from contextvars import ContextVar from datetime import timedelta from typing import Any, cast from unittest.mock import AsyncMock, Mock, patch @@ -7812,7 +7813,7 @@ def test_prepare_call_kwargs_rejects_invalid_meta_key_names(key: str) -> None: async def test_call_tool_forwards_only_declared_arguments() -> None: - """End-to-end: framework runtime kwargs are stripped before reaching the server.""" + """End-to-end: runtime kwargs the tool does not declare are stripped before reaching the server.""" class TestServer(MCPTool): async def connect(self): # type: ignore[override] # pyrefly: ignore[bad-override] # ty: ignore[invalid-method-override] @@ -7856,4 +7857,128 @@ def get_mcp_client(self) -> _AsyncGeneratorContextManager[Any, None]: assert call_kwargs["arguments"] == {"param": "value", "conversation_id": "c"} +async def test_call_tool_forwards_runtime_kwargs_the_server_declares() -> None: + """A runtime kwarg is forwarded when the server declares a property of the same name. + + Invokes the generated ``FunctionTool`` with a ``FunctionInvocationContext`` so the merge in + ``_call_tool_with_runtime_kwargs`` is on the tested path: the model supplies only ``param``, + while ``api_token`` arrives solely through ``FunctionInvocationContext.kwargs``. The allowlist + in ``_prepare_call_kwargs`` is built from the server's advertised ``inputSchema.properties``, + so declaring an optional property is enough for the server to receive that runtime value, + without the name appearing in ``additional_tool_argument_names``. This pins the behavior the + ``function_invocation_kwargs`` guidance describes. + """ + + class TestServer(MCPTool): + async def connect(self): # type: ignore[override] # pyrefly: ignore[bad-override] # ty: ignore[invalid-method-override] + self.session = Mock(spec=ClientSession) + self.session.list_tools = AsyncMock( + return_value=types.ListToolsResult( + tools=[ + types.Tool( + name="test_tool", + description="Test tool", + inputSchema={ + "type": "object", + "properties": { + "param": {"type": "string"}, + # Declared but not required, so the model never supplies it. + "api_token": {"type": "string"}, + }, + "required": ["param"], + }, + ) + ] + ) + ) + self.session.call_tool = AsyncMock( + return_value=types.CallToolResult(content=[types.TextContent(type="text", text="ok")]) + ) + + def get_mcp_client(self) -> _AsyncGeneratorContextManager[Any, None]: + return None # type: ignore[return-value] # pyrefly: ignore[bad-return] # ty: ignore[invalid-return-type] + + server = TestServer(name="test_server") + async with server: + await server.load_tools() + session_mock = server.session + + # Invoke the generated FunctionTool the way the function-calling loop does, so the + # runtime kwargs are merged by _call_tool_with_runtime_kwargs rather than being passed + # to call_tool directly. Only "param" is model-supplied. + tool = next(f for f in server.functions if f.name == "test_tool") + context = FunctionInvocationContext( + function=tool, + arguments={"param": "value"}, + kwargs={"api_token": "runtime-value"}, + ) + await tool.invoke(arguments={"param": "value"}, context=context) + + _, call_kwargs = session_mock.call_tool.call_args # type: ignore[union-attr] # ty: ignore[unresolved-attribute] + assert call_kwargs["arguments"] == {"param": "value", "api_token": "runtime-value"} + + +async def test_header_provider_reading_contextvar_keeps_credential_out_of_arguments() -> None: + """A credential sourced outside ``function_invocation_kwargs`` is not exposed to the filter. + + Documents the pattern the ``header_provider`` guidance recommends: the provider reads a + ``ContextVar`` instead of its ``kwargs`` argument, so the credential never enters the runtime + kwargs and cannot be forwarded as a tool argument even though the server declares a property + of that name. The value still varies per request. + """ + token_var: ContextVar[str] = ContextVar("test_token_var") + seen_headers: list[dict[str, str]] = [] + + class TestServer(MCPStreamableHTTPTool): + async def connect(self): # type: ignore[override] # pyrefly: ignore[bad-override] # ty: ignore[invalid-method-override] + self.session = Mock(spec=ClientSession) + self.session.list_tools = AsyncMock( + return_value=types.ListToolsResult( + tools=[ + types.Tool( + name="get_weather", + description="Weather", + inputSchema={ + "type": "object", + "properties": {"city": {"type": "string"}, "api_key": {"type": "string"}}, + "required": ["city"], + }, + ) + ] + ) + ) + self.session.call_tool = AsyncMock( + return_value=types.CallToolResult(content=[types.TextContent(type="text", text="sunny")]) + ) + self.session.send_ping = AsyncMock() + self.is_connected = True + + def get_mcp_client(self): # pyrefly: ignore[bad-override] + return None + + def provider(_kwargs: dict[str, Any]) -> dict[str, str]: + headers = {"Authorization": f"Bearer {token_var.get()}"} + seen_headers.append(headers) + return headers + + server = TestServer(name="test_server", url="http://example.com/mcp", header_provider=provider) + async with server: + await server.load_tools() + tool = next(f for f in server.functions if f.name == "get_weather") + + token_var.set("secret-1") + context = FunctionInvocationContext(function=tool, arguments={"city": "Seattle"}, kwargs={}) + await tool.invoke(arguments={"city": "Seattle"}, context=context) + + _, call_kwargs = server.session.call_tool.call_args # type: ignore[union-attr] # ty: ignore[unresolved-attribute] + assert seen_headers[-1] == {"Authorization": "Bearer secret-1"} + assert call_kwargs["arguments"] == {"city": "Seattle"} + + # The same provider yields a different credential on the next request. + token_var.set("secret-2") + context = FunctionInvocationContext(function=tool, arguments={"city": "Oslo"}, kwargs={}) + await tool.invoke(arguments={"city": "Oslo"}, context=context) + assert seen_headers[-1] == {"Authorization": "Bearer secret-2"} + + # endregion diff --git a/python/samples/02-agents/mcp/mcp_api_key_auth.py b/python/samples/02-agents/mcp/mcp_api_key_auth.py index 456db2878c..5af7f29009 100644 --- a/python/samples/02-agents/mcp/mcp_api_key_auth.py +++ b/python/samples/02-agents/mcp/mcp_api_key_auth.py @@ -27,6 +27,18 @@ For more complex scenarios, you could implement token refresh logic or support multiple authentication methods within the header provider function. +Note on ``function_invocation_kwargs``: +Values passed this way are shared with every tool in the run, including every attached MCP server, and are +filtered against each tool's server-declared ``inputSchema.properties``. A server that declares ``mcp_api_key`` +therefore receives it as an ordinary tool argument, and ``header_provider`` does not prevent that - it reads the +kwargs without consuming them. This sample targets a server you control, so that is fine here. For a server you +do not control, source the credential outside ``function_invocation_kwargs`` instead, for example by reading a +``ContextVar`` inside the provider:: + + header_provider=lambda _kwargs: {"Authorization": f"Bearer {token_var.get()}"} + +which keeps it out of tool arguments while still allowing a different value per request. + For more authentication examples including OAuth 2.0 flows, see: - https://github.com/modelcontextprotocol/python-sdk/tree/main/examples/clients/simple-auth-client - https://github.com/modelcontextprotocol/python-sdk/tree/main/examples/servers/simple-auth