mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-21 13:04:18 +02:00
* fix: prevent MCP transport auth header from leaking to downstream OpenAPI APIs (#3260) Two issues in OpenAPITool.run(): 1. get_http_headers() does not exclude 'authorization', so the MCP client's auth token is included in forwarded headers. 2. mcp_headers.update() overwrites existing request headers, including the Authorization header that was already set from the httpx client's configured API key. Fix: - Add 'authorization' to exclude_headers in get_http_headers() to prevent MCP transport credentials from being forwarded by default. - Change mcp_headers forwarding to use the same non-overwriting pattern as client headers (only set if key not already present), making the behavior consistent and preventing accidental overwrites. Fixes #3260 * Add include parameter to get_http_headers(); update proxy transports The authorization exclusion is correct for the default case (OpenAPI tools should not forward MCP transport credentials), but proxy transports need auth headers forwarded to upstream MCP servers. The new `include` parameter lets callers opt specific headers back in despite the default exclusion set. Proxy transports now explicitly request authorization forwarding. * Include authorization header in CurrentHeaders dependency CurrentHeaders is user-facing — tools use it to inspect the caller's auth token for custom logic. Reading a header in your own code is safe; the exclusion is meant to prevent blindly forwarding it to third-party APIs. --------- Co-authored-by: User <user@example.com> Co-authored-by: Jeremiah Lowin <153965+jlowin@users.noreply.github.com>
This commit is contained in:
parent
3ec7cfa0d5
commit
0d4580fef3
4 changed files with 26 additions and 10 deletions
|
|
@ -95,7 +95,7 @@ class StreamableHttpTransport(ClientTransport):
|
|||
# Load headers from an active HTTP request, if available. This will only be true
|
||||
# if the client is used in a FastMCP Proxy, in which case the MCP client headers
|
||||
# need to be forwarded to the remote server.
|
||||
headers = get_http_headers() | self.headers
|
||||
headers = get_http_headers(include={"authorization"}) | self.headers
|
||||
|
||||
# Configure timeout if provided, preserving MCP's 30s connect default
|
||||
timeout: httpx.Timeout | None = None
|
||||
|
|
|
|||
|
|
@ -69,7 +69,9 @@ class SSETransport(ClientTransport):
|
|||
# load headers from an active HTTP request, if available. This will only be true
|
||||
# if the client is used in a FastMCP Proxy, in which case the MCP client headers
|
||||
# need to be forwarded to the remote server.
|
||||
client_kwargs["headers"] = get_http_headers() | self.headers
|
||||
client_kwargs["headers"] = (
|
||||
get_http_headers(include={"authorization"}) | self.headers
|
||||
)
|
||||
|
||||
# sse_read_timeout has a default value set, so we can't pass None without overriding it
|
||||
# instead we simply leave the kwarg out if it's not provided
|
||||
|
|
|
|||
|
|
@ -434,14 +434,22 @@ def get_http_request() -> Request:
|
|||
return request
|
||||
|
||||
|
||||
def get_http_headers(include_all: bool = False) -> dict[str, str]:
|
||||
def get_http_headers(
|
||||
include_all: bool = False,
|
||||
include: set[str] | None = None,
|
||||
) -> dict[str, str]:
|
||||
"""Extract headers from the current HTTP request if available.
|
||||
|
||||
Never raises an exception, even if there is no active HTTP request (in which case
|
||||
an empty dict is returned).
|
||||
|
||||
By default, strips problematic headers like `content-length` that cause issues
|
||||
if forwarded to downstream clients. If `include_all` is True, all headers are returned.
|
||||
By default, strips problematic headers like `content-length` and `authorization`
|
||||
that cause issues if forwarded to downstream services. If `include_all` is True,
|
||||
all headers are returned.
|
||||
|
||||
The `include` parameter allows specific headers to be included even if they would
|
||||
normally be excluded. This is useful for proxy transports that need to forward
|
||||
authorization headers to upstream MCP servers.
|
||||
"""
|
||||
if include_all:
|
||||
exclude_headers: set[str] = set()
|
||||
|
|
@ -457,6 +465,7 @@ def get_http_headers(include_all: bool = False) -> dict[str, str]:
|
|||
"keep-alive",
|
||||
"expect",
|
||||
"accept",
|
||||
"authorization",
|
||||
# Proxy-related headers
|
||||
"proxy-authenticate",
|
||||
"proxy-authorization",
|
||||
|
|
@ -464,6 +473,8 @@ def get_http_headers(include_all: bool = False) -> dict[str, str]:
|
|||
# MCP-related headers
|
||||
"mcp-session-id",
|
||||
}
|
||||
if include:
|
||||
exclude_headers -= {h.lower() for h in include}
|
||||
# (just in case)
|
||||
if not all(h.lower() == h for h in exclude_headers):
|
||||
raise ValueError("Excluded headers must be lowercase")
|
||||
|
|
@ -1037,7 +1048,7 @@ class _CurrentHeaders(Dependency): # type: ignore[misc]
|
|||
"""Async context manager for HTTP Headers dependency."""
|
||||
|
||||
async def __aenter__(self) -> dict[str, str]:
|
||||
return get_http_headers()
|
||||
return get_http_headers(include={"authorization"})
|
||||
|
||||
async def __aexit__(self, *args: object) -> None:
|
||||
pass
|
||||
|
|
@ -1046,9 +1057,10 @@ class _CurrentHeaders(Dependency): # type: ignore[misc]
|
|||
def CurrentHeaders() -> dict[str, str]:
|
||||
"""Get the current HTTP request headers.
|
||||
|
||||
This dependency provides access to the HTTP headers for the current request.
|
||||
Returns an empty dictionary when no HTTP request is available, making it
|
||||
safe to use in code that might run over any transport.
|
||||
This dependency provides access to the HTTP headers for the current request,
|
||||
including the authorization header. Returns an empty dictionary when no HTTP
|
||||
request is available, making it safe to use in code that might run over any
|
||||
transport.
|
||||
|
||||
Returns:
|
||||
A dependency that resolves to a dictionary of header name -> value
|
||||
|
|
|
|||
|
|
@ -172,7 +172,9 @@ class OpenAPITool(Tool):
|
|||
|
||||
mcp_headers = get_http_headers()
|
||||
if mcp_headers:
|
||||
request.headers.update(mcp_headers)
|
||||
for key, value in mcp_headers.items():
|
||||
if key not in request.headers:
|
||||
request.headers[key] = value
|
||||
except Exception as e:
|
||||
raise ValueError(
|
||||
f"Error building request for {self._route.method.upper()} "
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue