From 10a7bd57e9610672b740ab0474f7de53fe2b8ffa Mon Sep 17 00:00:00 2001 From: Shivani Bhandari Date: Thu, 27 Aug 2026 00:56:25 +0530 Subject: [PATCH 1/2] Python: Add origin-scoped headers for MCP connect authentication Allow MCPStreamableHTTPTool to authenticate initialize/handshake with static headers while keeping header_provider for per-call overlays, so kwargs-only providers no longer leave connect unauthenticated. --- python/packages/core/AGENTS.md | 8 + python/packages/core/agent_framework/_mcp.py | 104 ++++++---- .../packages/core/agent_framework/security.py | 8 +- python/packages/core/tests/core/test_mcp.py | 196 ++++++++++++++++++ .../samples/02-agents/mcp/mcp_api_key_auth.py | 5 + 5 files changed, 275 insertions(+), 46 deletions(-) diff --git a/python/packages/core/AGENTS.md b/python/packages/core/AGENTS.md index 593aa1e4b54..83e9cb0c430 100644 --- a/python/packages/core/AGENTS.md +++ b/python/packages/core/AGENTS.md @@ -103,6 +103,14 @@ 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. +- **`MCPStreamableHTTPTool` headers** - ``headers=`` are static same-origin headers applied to + ``connect()`` / initialize, discovery, pings, reconnects, and tool calls (origin-scoped, so they + are not leaked on cross-origin redirects). ``header_provider`` overlays per-call headers from + runtime kwargs (``FunctionInvocationContext.kwargs``). Ambient requests call + ``header_provider({})``; kwargs-only providers that raise ``KeyError`` are tolerated so connect + can succeed against servers that do not authenticate the handshake. Servers that *do* require + handshake auth need ``headers=`` or a provider that returns credentials for ``{}``. Prefer + ``headers=`` over baking tokens into a custom ``http_client`` so the origin check still applies. - **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. - **`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. diff --git a/python/packages/core/agent_framework/_mcp.py b/python/packages/core/agent_framework/_mcp.py index 9abafdad277..0b8af9abe8c 100644 --- a/python/packages/core/agent_framework/_mcp.py +++ b/python/packages/core/agent_framework/_mcp.py @@ -2940,6 +2940,7 @@ def __init__( sampling_max_requests: int | None = _DEFAULT_SAMPLING_MAX_REQUESTS, additional_properties: dict[str, Any] | None = None, http_client: AsyncClient | None = None, + headers: Mapping[str, str] | None = None, header_provider: Callable[[dict[str, Any]], dict[str, str]] | None = None, task_options: MCPTaskOptions | None = None, additional_tool_argument_names: Sequence[str] | Mapping[str, Sequence[str]] | None = None, @@ -3013,23 +3014,35 @@ def __init__( and pass your own ``asyncClient`` instance. Security: when you attach sensitive headers (e.g. authentication tokens) via a custom ``http_client``, you are responsible for enforcing the same - origin-scoped header policy that the built-in ``header_provider`` hook - applies. The framework only injects ``header_provider`` headers on requests - whose origin (scheme, host, port) matches the configured ``url``, so tokens - are not leaked to third-party origins on cross-origin redirects. A custom - client that sets headers unconditionally (e.g. via ``AsyncClient(headers=...)`` - or ``follow_redirects=True`` without an origin check) can leak those headers - to other origins; scope them to the target origin yourself. + origin-scoped header policy that the built-in ``headers`` / + ``header_provider`` hook applies. The framework only injects those + headers on requests whose origin (scheme, host, port) matches the + configured ``url``, so tokens are not leaked to third-party origins on + cross-origin redirects. A custom client that sets headers unconditionally + (e.g. via ``AsyncClient(headers=...)`` or ``follow_redirects=True`` + without an origin check) can leak those headers to other origins; scope + them to the target origin yourself. + headers: Optional static HTTP headers attached to every same-origin request, + including ``connect()`` / initialize, discovery, pings, reconnects, and + tool calls. Use this when the MCP server authenticates the handshake + itself. Prefer ``headers`` over baking tokens into a custom + ``http_client`` so the origin-scoped injection policy still applies. + Per-call ``header_provider`` values overlay these static headers. header_provider: Optional callable that receives the runtime keyword arguments (from ``FunctionInvocationContext.kwargs``) and returns a ``dict[str, str]`` - of HTTP headers to inject into every outbound request to the MCP server. + of HTTP headers to inject into outbound requests to the MCP server. Use this to forward per-request context (e.g. authentication tokens set in agent middleware) without creating a separate ``httpx.AsyncClient``. - The framework attaches these headers only to requests whose origin (scheme, - host, port) matches the configured ``url``, so they are not leaked to other - 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. + Ambient requests outside ``call_tool`` (the initialize handshake, + discovery, and pings) invoke the provider with ``{}``. Providers that + authenticate connect must either tolerate empty kwargs and return + handshake credentials, or the caller must also supply ``headers``. + A ``KeyError`` from a kwargs-only provider is tolerated on ambient + requests so connect can still succeed against servers that do not + require handshake auth. The framework attaches these headers only to + requests whose origin (scheme, host, port) matches the configured + ``url``. If you instead supply sensitive headers through a custom + ``http_client``, you must enforce this same origin-scoped policy yourself. 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 @@ -3073,6 +3086,7 @@ def __init__( self.url = url self.terminate_on_close = terminate_on_close self._httpx_client: AsyncClient | None = http_client + self._static_headers: dict[str, str] = dict(headers) if headers else {} self._header_provider = header_provider # Headers for the in-flight call_tool invocation. The streamable HTTP transport # sends requests from tasks spawned at connect time, whose contexts never observe @@ -3101,6 +3115,38 @@ def _mcp_base_span_attributes(self) -> dict[str, Any]: logger.debug("Failed to parse URL for MCP span transport attributes", exc_info=True) return attrs + def _resolve_outbound_headers(self) -> dict[str, str]: + """Resolve origin-scoped headers for the current HTTP request. + + Static ``headers`` always apply. Per-call ``header_provider`` values overlay + them when a tool call is in flight. Ambient requests (connect, discovery, + pings) call ``header_provider({})`` so static providers can authenticate the + handshake; ``KeyError`` is tolerated for kwargs-only providers. + """ + resolved = dict(self._static_headers) + call_headers = _mcp_call_headers.get(None) + if call_headers is None: + call_headers = self._active_call_headers + if call_headers is None: + # Ambient request made outside call_tool (the initialize handshake, + # load_tools/load_prompts discovery, or background pings). + if self._header_provider is not None: + try: + call_headers = self._header_provider({}) + except KeyError: + # A kwargs-dependent provider raises on every ambient request. + logger.debug( + "header_provider raised KeyError for MCP server %r on an ambient " + "request (missing per-call kwargs); proceeding with static headers.", + self.name, + exc_info=True, + ) + call_headers = {} + else: + call_headers = {} + resolved.update(call_headers) + return resolved + def get_mcp_client(self) -> _AsyncGeneratorContextManager[Any, None]: """Get an MCP streamable HTTP client. @@ -3110,7 +3156,7 @@ def get_mcp_client(self) -> _AsyncGeneratorContextManager[Any, None]: from httpx import URL, AsyncClient, Request, Timeout http_client = self._httpx_client - if self._header_provider is not None: + if self._header_provider is not None or self._static_headers: target_origin = _url_origin(URL(self.url)) if http_client is None: http_client = AsyncClient( @@ -3129,34 +3175,8 @@ async def _inject_headers(request: Request) -> None: # ruff:ignore[unused-async # instance-level snapshot of the active call's headers. Both are None # only when this is an ambient request outside call_tool; an active # call that legitimately produced no headers yields an empty dict and - # must not trigger the ambient fallback below. - headers = _mcp_call_headers.get(None) - if headers is None: - headers = self._active_call_headers - if headers is None: - # Ambient request made outside call_tool (the initialize handshake, - # load_tools/load_prompts discovery, or background pings). Invoke the - # provider with empty kwargs so static providers can authenticate these - # requests too. A provider that indexes a required per-call kwarg (e.g. - # kwargs["api_key"]) raises KeyError on the empty dict; that specific - # case is tolerated so connect still succeeds. Any other error is a - # genuine provider failure and is left to propagate, matching the - # call_tool path which does not catch header_provider exceptions. - if self._header_provider is None: - raise RuntimeError("Header injection hook invoked without a header_provider.") - try: - headers = self._header_provider({}) - except KeyError: - # A kwargs-dependent provider raises on every ambient request - # (initialize, discovery, and recurring pings). - logger.debug( - "header_provider raised KeyError for MCP server %r on an ambient " - "request (missing per-call kwargs); proceeding without headers.", - self.name, - exc_info=True, - ) - headers = {} - for key, value in headers.items(): + # must not trigger the ambient header_provider({}) fallback. + for key, value in self._resolve_outbound_headers().items(): request.headers[key] = value self._inject_headers_hook = _inject_headers diff --git a/python/packages/core/agent_framework/security.py b/python/packages/core/agent_framework/security.py index 397f9af62fc..37764ad1145 100644 --- a/python/packages/core/agent_framework/security.py +++ b/python/packages/core/agent_framework/security.py @@ -3425,10 +3425,10 @@ def __init__( static_headers = dict(headers or {}) # Pass headers via an AsyncClient so they are included on ALL requests - # (including session.initialize()), not just tool calls. Using - # header_provider alone only sets headers via a ContextVar that is - # populated during call_tool() and would be empty during initialization, - # causing 401s that silently manifest as anyio cancel-scope errors. + # (including session.initialize()), not just tool calls. ``headers=`` on + # MCPStreamableHTTPTool is the preferred origin-scoped equivalent; + # SecureMCPToolProxy still uses a dedicated client because it exposes a + # static ``headers`` mapping of its own. http_client = ( AsyncClient( headers=static_headers, diff --git a/python/packages/core/tests/core/test_mcp.py b/python/packages/core/tests/core/test_mcp.py index 028cac027b3..ee56688aec1 100644 --- a/python/packages/core/tests/core/test_mcp.py +++ b/python/packages/core/tests/core/test_mcp.py @@ -6059,6 +6059,100 @@ async def test_mcp_streamable_http_tool_header_provider_injects_on_ambient_reque await tool._httpx_client.aclose() # type: ignore[union-attr] +async def test_mcp_streamable_http_tool_static_headers_inject_on_ambient_request(): + """Regression test for #7841: static headers must authenticate connect/initialize. + + A kwargs-only header_provider cannot supply handshake credentials because + connect runs before FunctionInvocationContext kwargs exist. ``headers=`` is the + connect-time hook and must be attached even when no header_provider is set. + """ + import httpx + + tool = MCPStreamableHTTPTool( + name="test", + url="http://example.com/mcp", + headers={"Authorization": "Bearer connect-token"}, + ) + + try: + with patch("agent_framework._mcp.streamable_http_client"): + tool.get_mcp_client() + + assert tool._httpx_client is not None + hooks = tool._httpx_client.event_hooks.get("request", []) + assert len(hooks) == 1 + + request = httpx.Request("POST", "http://example.com/mcp") + await hooks[0](request) + assert request.headers.get("Authorization") == "Bearer connect-token" + finally: + if getattr(tool, "_httpx_client", None) is not None: + await tool._httpx_client.aclose() # type: ignore[union-attr] + + +async def test_mcp_streamable_http_tool_static_headers_overlay_kwargs_provider_on_ambient_request(): + """Static headers still authenticate connect when header_provider requires per-call kwargs.""" + import httpx + + tool = MCPStreamableHTTPTool( + name="test", + url="http://example.com/mcp", + headers={"Authorization": "Bearer connect-token"}, + header_provider=lambda kw: {"Authorization": f"Bearer {kw['mcp_api_key']}"}, + ) + + try: + with patch("agent_framework._mcp.streamable_http_client"): + tool.get_mcp_client() + + assert tool._httpx_client is not None + hooks = tool._httpx_client.event_hooks.get("request", []) + assert len(hooks) == 1 + + request = httpx.Request("POST", "http://example.com/mcp") + await hooks[0](request) + assert request.headers.get("Authorization") == "Bearer connect-token" + finally: + if getattr(tool, "_httpx_client", None) is not None: + await tool._httpx_client.aclose() # type: ignore[union-attr] + + +async def test_mcp_streamable_http_tool_header_provider_overlays_static_headers_on_call(): + """Per-call header_provider values overlay static headers during call_tool.""" + import httpx + + from agent_framework._mcp import _mcp_call_headers + + tool = MCPStreamableHTTPTool( + name="test", + url="http://example.com/mcp", + headers={"Authorization": "Bearer connect-token", "X-Static": "static"}, + header_provider=lambda kw: {"Authorization": f"Bearer {kw.get('mcp_api_key', '')}"}, + ) + + try: + with patch("agent_framework._mcp.streamable_http_client"): + tool.get_mcp_client() + + assert tool._httpx_client is not None + hooks = tool._httpx_client.event_hooks.get("request", []) + assert len(hooks) == 1 + + token = _mcp_call_headers.set({"Authorization": "Bearer call-token"}) + tool._active_call_headers = {"Authorization": "Bearer call-token"} + try: + request = httpx.Request("POST", "http://example.com/mcp") + await hooks[0](request) + assert request.headers.get("Authorization") == "Bearer call-token" + assert request.headers.get("X-Static") == "static" + finally: + tool._active_call_headers = None + _mcp_call_headers.reset(token) + finally: + if getattr(tool, "_httpx_client", None) is not None: + await tool._httpx_client.aclose() # type: ignore[union-attr] + + async def test_mcp_streamable_http_tool_header_provider_ambient_request_tolerates_kwargs_provider(): """A header_provider that requires per-call kwargs must not crash ambient requests. @@ -6244,6 +6338,36 @@ async def test_mcp_streamable_http_tool_header_provider_skips_cross_origin_redir await tool._httpx_client.aclose() # type: ignore[union-attr] +async def test_mcp_streamable_http_tool_static_headers_skip_cross_origin_redirect(): + """Static connect headers must not leak to a different origin after a redirect.""" + import httpx + + tool = MCPStreamableHTTPTool( + name="test", + url="http://example.com/mcp", + headers={"Authorization": "Bearer connect-token"}, + ) + + try: + with patch("agent_framework._mcp.streamable_http_client"): + tool.get_mcp_client() + + assert tool._httpx_client is not None + hooks = tool._httpx_client.event_hooks.get("request", []) + assert len(hooks) == 1 + + same_origin = httpx.Request("POST", "http://example.com/mcp") + await hooks[0](same_origin) + assert same_origin.headers.get("Authorization") == "Bearer connect-token" + + cross_origin = httpx.Request("POST", "http://attacker.example/capture") + await hooks[0](cross_origin) + assert "Authorization" not in cross_origin.headers + finally: + if getattr(tool, "_httpx_client", None) is not None: + await tool._httpx_client.aclose() # type: ignore[union-attr] + + async def test_mcp_streamable_http_tool_header_provider_with_user_httpx_client(): """Test that header_provider works when the user provides their own httpx client.""" import httpx @@ -6435,6 +6559,78 @@ async def handler(request: httpx.Request) -> httpx.Response: assert call_headers[0].get("authorization") == "Bearer secret-token" +async def test_mcp_streamable_http_tool_static_headers_apply_to_initialize(): + """Regression test for #7841: static headers must reach the initialize handshake. + + A kwargs-only header_provider cannot authenticate connect. ``headers=`` is applied + to initialize, tools/list, and tools/call; the provider overlays Authorization + on the in-flight tools/call. + """ + import httpx + + captured_requests: list[tuple[str, str, dict[str, str]]] = [] + + async def handler(request: httpx.Request) -> httpx.Response: + if request.method == "DELETE": + return httpx.Response(200) + if request.method == "GET": + return httpx.Response(405) + body = json.loads(request.content.decode()) + method = body.get("method", "") + captured_requests.append((request.method, method, {k.lower(): v for k, v in request.headers.items()})) + if method == "initialize": + result = { + "protocolVersion": body["params"]["protocolVersion"], + "capabilities": {"tools": {}}, + "serverInfo": {"name": "mock-server", "version": "1.0.0"}, + } + return httpx.Response( + 200, + headers={"mcp-session-id": "test-session"}, + json={"jsonrpc": "2.0", "id": body["id"], "result": result}, + ) + if method == "tools/list": + result = { + "tools": [ + { + "name": "greet", + "description": "Says hello", + "inputSchema": {"type": "object", "properties": {"name": {"type": "string"}}}, + } + ] + } + return httpx.Response(200, json={"jsonrpc": "2.0", "id": body["id"], "result": result}) + if method == "tools/call": + result = {"content": [{"type": "text", "text": "Hello!"}], "isError": False} + return httpx.Response(200, json={"jsonrpc": "2.0", "id": body["id"], "result": result}) + if "id" in body: + return httpx.Response(200, json={"jsonrpc": "2.0", "id": body["id"], "result": {}}) + return httpx.Response(202) + + http_client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) + tool = MCPStreamableHTTPTool( + name="test", + url="http://127.0.0.1:8000/mcp", + load_prompts=False, + http_client=http_client, + headers={"Authorization": "Bearer connect-token"}, + header_provider=lambda kw: {"Authorization": f"Bearer {kw['api_key']}"}, + ) + try: + async with tool: + initialize_headers = [headers for _, method, headers in captured_requests if method == "initialize"] + assert len(initialize_headers) == 1 + assert initialize_headers[0].get("authorization") == "Bearer connect-token" + + await tool.call_tool("greet", name="Alice", api_key="secret-token") + finally: + await http_client.aclose() + + call_headers = [headers for _, method, headers in captured_requests if method == "tools/call"] + assert len(call_headers) == 1 + assert call_headers[0].get("authorization") == "Bearer secret-token" + + async def test_mcp_streamable_http_tool_header_provider_snapshot_restored_after_call(): """Test that the instance-level header snapshot is set during a call and cleared after.""" observed_snapshots: list[dict[str, str] | None] = [] 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 456db2878c0..b8a78e4aa42 100644 --- a/python/samples/02-agents/mcp/mcp_api_key_auth.py +++ b/python/samples/02-agents/mcp/mcp_api_key_auth.py @@ -18,6 +18,11 @@ ``function_invocation_kwargs`` passed to ``Agent.run(...)`` so the API key stays in runtime context instead of being baked into a shared ``httpx.AsyncClient``. +If the MCP server authenticates the ``connect()`` handshake itself, also pass +``headers={"Authorization": f"Bearer {api_key}"}`` (or use a ``header_provider`` +that returns credentials for empty kwargs). ``header_provider`` alone cannot see +``function_invocation_kwargs`` until a tool call is in flight. + Replace the ``url`` parameter in the ``MCPStreamableHTTPTool`` with your authenticated server URL and run the sample with your API key as a command-line argument: python mcp_api_key_auth.py From 7d6952d1887fe717df19c386af1b41fa4948bbed Mon Sep 17 00:00:00 2001 From: Shivani Bhandari Date: Thu, 27 Aug 2026 00:56:25 +0530 Subject: [PATCH 2/2] Python: Strip MCP injected headers on cross-origin redirects Track headers injected by the MCP request hook and remove them on redirected cross-origin requests, route SecureMCPToolProxy through headers=, and harden the redirect regression tests with X-API-Key. --- python/packages/core/AGENTS.md | 2 + python/packages/core/agent_framework/_mcp.py | 16 +++++++- .../packages/core/agent_framework/security.py | 26 +++---------- python/packages/core/tests/core/test_mcp.py | 39 +++++++++++++------ 4 files changed, 50 insertions(+), 33 deletions(-) diff --git a/python/packages/core/AGENTS.md b/python/packages/core/AGENTS.md index 83e9cb0c430..fa036c92e4b 100644 --- a/python/packages/core/AGENTS.md +++ b/python/packages/core/AGENTS.md @@ -111,6 +111,8 @@ agent_framework/ can succeed against servers that do not authenticate the handshake. Servers that *do* require handshake auth need ``headers=`` or a provider that returns credentials for ``{}``. Prefer ``headers=`` over baking tokens into a custom ``http_client`` so the origin check still applies. + The request hook records keys it injected and strips those keys from subsequent cross-origin + requests (HTTPX already strips ``Authorization``; other secrets such as ``X-API-Key`` are removed here). - **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. - **`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. diff --git a/python/packages/core/agent_framework/_mcp.py b/python/packages/core/agent_framework/_mcp.py index 0b8af9abe8c..20ae64a9b76 100644 --- a/python/packages/core/agent_framework/_mcp.py +++ b/python/packages/core/agent_framework/_mcp.py @@ -3028,6 +3028,9 @@ def __init__( itself. Prefer ``headers`` over baking tokens into a custom ``http_client`` so the origin-scoped injection policy still applies. Per-call ``header_provider`` values overlay these static headers. + On a cross-origin redirect, headers previously injected by this hook + are stripped from the redirected request (HTTPX already strips + ``Authorization``; other secrets such as ``X-API-Key`` are removed here). header_provider: Optional callable that receives the runtime keyword arguments (from ``FunctionInvocationContext.kwargs``) and returns a ``dict[str, str]`` of HTTP headers to inject into outbound requests to the MCP server. @@ -3096,6 +3099,10 @@ def __init__( # otherwise overwrite each other's snapshot and attach the wrong per-call headers. self._active_call_headers: dict[str, str] | None = None self._call_headers_lock = asyncio.Lock() + # Keys last injected by the request hook. HTTPX may copy non-Authorization secrets + # onto a cross-origin redirect; the hook strips these keys when the next request + # leaves the configured origin. + self._injected_header_keys: set[str] = set() def _mcp_base_span_attributes(self) -> dict[str, Any]: attrs = super()._mcp_base_span_attributes() @@ -3169,6 +3176,11 @@ def get_mcp_client(self) -> _AsyncGeneratorContextManager[Any, None]: async def _inject_headers(request: Request) -> None: # ruff:ignore[unused-async] if _url_origin(request.url) != target_origin: + # Strip secrets this hook previously injected. HTTPX removes + # Authorization on cross-origin redirects, but other credentials + # (e.g. X-API-Key) can remain on the redirected request. + for key in self._injected_header_keys: + request.headers.pop(key, None) return # The transport may send this request from a task whose context was # captured before call_tool set the ContextVar; fall back to the @@ -3176,7 +3188,9 @@ async def _inject_headers(request: Request) -> None: # ruff:ignore[unused-async # only when this is an ambient request outside call_tool; an active # call that legitimately produced no headers yields an empty dict and # must not trigger the ambient header_provider({}) fallback. - for key, value in self._resolve_outbound_headers().items(): + outbound = self._resolve_outbound_headers() + self._injected_header_keys = set(outbound) + for key, value in outbound.items(): request.headers[key] = value self._inject_headers_hook = _inject_headers diff --git a/python/packages/core/agent_framework/security.py b/python/packages/core/agent_framework/security.py index 37764ad1145..38433748e70 100644 --- a/python/packages/core/agent_framework/security.py +++ b/python/packages/core/agent_framework/security.py @@ -3419,29 +3419,15 @@ def __init__( raise ValueError("Provide either 'mcp_tool' (an MCPTool instance) or 'url' (a remote MCP server URL).") if url is not None: - from httpx import AsyncClient, Timeout - - from ._mcp import MCP_DEFAULT_SSE_READ_TIMEOUT, MCP_DEFAULT_TIMEOUT, MCPStreamableHTTPTool - - static_headers = dict(headers or {}) - # Pass headers via an AsyncClient so they are included on ALL requests - # (including session.initialize()), not just tool calls. ``headers=`` on - # MCPStreamableHTTPTool is the preferred origin-scoped equivalent; - # SecureMCPToolProxy still uses a dedicated client because it exposes a - # static ``headers`` mapping of its own. - http_client = ( - AsyncClient( - headers=static_headers, - follow_redirects=True, - timeout=Timeout(MCP_DEFAULT_TIMEOUT, read=MCP_DEFAULT_SSE_READ_TIMEOUT), - ) - if static_headers - else None - ) + from ._mcp import MCPStreamableHTTPTool + + # Prefer origin-scoped ``headers=`` over baking tokens into an + # ``AsyncClient(headers=..., follow_redirects=True)``, which can leak + # non-Authorization credentials on cross-origin redirects. mcp_tool = MCPStreamableHTTPTool( name=name or "mcp", url=url, - http_client=http_client, + headers=dict(headers) if headers else None, description=description, ) diff --git a/python/packages/core/tests/core/test_mcp.py b/python/packages/core/tests/core/test_mcp.py index ee56688aec1..cdf61e7dc6b 100644 --- a/python/packages/core/tests/core/test_mcp.py +++ b/python/packages/core/tests/core/test_mcp.py @@ -6303,7 +6303,11 @@ def failing_provider(kw: dict[str, Any]) -> dict[str, str]: async def test_mcp_streamable_http_tool_header_provider_skips_cross_origin_redirect(): - """The request hook must not re-add caller headers after a cross-origin redirect.""" + """The request hook must strip previously injected secrets after a cross-origin redirect. + + HTTPX already strips Authorization on cross-origin redirects, so this uses X-API-Key + and preserves that header on the redirected request the way a real redirect copy would. + """ import httpx from agent_framework._mcp import _mcp_call_headers @@ -6311,7 +6315,7 @@ async def test_mcp_streamable_http_tool_header_provider_skips_cross_origin_redir tool = MCPStreamableHTTPTool( name="test", url="http://example.com/mcp", - header_provider=lambda kw: {"Authorization": f"Bearer {kw.get('token', '')}"}, + header_provider=lambda kw: {"X-API-Key": kw.get("token", "")}, ) try: @@ -6322,15 +6326,20 @@ async def test_mcp_streamable_http_tool_header_provider_skips_cross_origin_redir hooks = tool._httpx_client.event_hooks.get("request", []) assert len(hooks) == 1 - token = _mcp_call_headers.set({"Authorization": "Bearer secret"}) + token = _mcp_call_headers.set({"X-API-Key": "secret"}) try: same_origin = httpx.Request("POST", "http://example.com/redirected") await hooks[0](same_origin) - assert same_origin.headers.get("Authorization") == "Bearer secret" + assert same_origin.headers.get("X-API-Key") == "secret" - cross_origin = httpx.Request("POST", "http://attacker.example/capture") + # Simulate HTTPX copying non-Authorization credentials onto the redirect. + cross_origin = httpx.Request( + "POST", + "http://attacker.example/capture", + headers={"X-API-Key": "secret"}, + ) await hooks[0](cross_origin) - assert "Authorization" not in cross_origin.headers + assert "X-API-Key" not in cross_origin.headers finally: _mcp_call_headers.reset(token) finally: @@ -6339,13 +6348,13 @@ async def test_mcp_streamable_http_tool_header_provider_skips_cross_origin_redir async def test_mcp_streamable_http_tool_static_headers_skip_cross_origin_redirect(): - """Static connect headers must not leak to a different origin after a redirect.""" + """Static connect headers must be stripped from a cross-origin redirect request.""" import httpx tool = MCPStreamableHTTPTool( name="test", url="http://example.com/mcp", - headers={"Authorization": "Bearer connect-token"}, + headers={"X-API-Key": "connect-token"}, ) try: @@ -6358,11 +6367,17 @@ async def test_mcp_streamable_http_tool_static_headers_skip_cross_origin_redirec same_origin = httpx.Request("POST", "http://example.com/mcp") await hooks[0](same_origin) - assert same_origin.headers.get("Authorization") == "Bearer connect-token" - - cross_origin = httpx.Request("POST", "http://attacker.example/capture") + assert same_origin.headers.get("X-API-Key") == "connect-token" + + # Simulate HTTPX copying the secret onto the redirected request. Authorization + # would already be stripped by HTTPX; X-API-Key would not. + cross_origin = httpx.Request( + "POST", + "http://attacker.example/capture", + headers={"X-API-Key": "connect-token"}, + ) await hooks[0](cross_origin) - assert "Authorization" not in cross_origin.headers + assert "X-API-Key" not in cross_origin.headers finally: if getattr(tool, "_httpx_client", None) is not None: await tool._httpx_client.aclose() # type: ignore[union-attr]