mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-15 23:31:29 +00:00
fix(mcp): warn when allowed_response_headers cannot be honored by the transport
Only streamable-HTTP exposes the upstream tools/call HTTP response, so the setting was silently inert on the other transports: the SDK's sse_client calls the httpx factory without event_hooks, and stdio carries no HTTP at all. An operator saw no headers, no error and no explanation. Warns once at config load, naming the server and its transport, and documents the restriction on the field. Covers every non-http transport rather than only the SSE path.
This commit is contained in:
parent
dee3d58a6b
commit
e0d32ea7f6
3 changed files with 110 additions and 1 deletions
|
|
@ -726,6 +726,30 @@ def _warn_internal_delegate_pkce_if_applicable(server: MCPServer, *, source: str
|
|||
)
|
||||
|
||||
|
||||
def _warn_response_headers_unsupported_transport_if_applicable(server: MCPServer, *, source: str) -> None:
|
||||
"""Surface an ``allowed_response_headers`` config that the server's transport cannot honor.
|
||||
|
||||
Only the streamable-HTTP transport exposes the upstream ``tools/call`` HTTP response. stdio has no
|
||||
HTTP at all, and the SSE transport delivers the result over a stream whose headers are committed
|
||||
before the tool runs (its POST returns only an ack), so on both the setting is inert. Without this
|
||||
line an operator would see no headers, no error, and no explanation.
|
||||
"""
|
||||
if not server.allowed_response_headers:
|
||||
return
|
||||
if server.transport == MCPTransport.http:
|
||||
return
|
||||
label = get_server_prefix(server)
|
||||
verbose_logger.warning(
|
||||
"MCP server %r (id=%s, source=%s): allowed_response_headers is set but the transport is %s. "
|
||||
"Upstream response headers are only observable on the streamable-HTTP transport, so no headers "
|
||||
"will be surfaced on the tool result's _meta.",
|
||||
label,
|
||||
server.server_id,
|
||||
source,
|
||||
server.transport,
|
||||
)
|
||||
|
||||
|
||||
def _deserialize_json_dict(data: Any) -> Optional[dict[str, str]]:
|
||||
"""
|
||||
Deserialize optional JSON mappings stored in the database.
|
||||
|
|
@ -1354,6 +1378,7 @@ class MCPServerManager:
|
|||
)
|
||||
self._assign_unique_short_prefix(new_server)
|
||||
_warn_internal_delegate_pkce_if_applicable(new_server, source="config")
|
||||
_warn_response_headers_unsupported_transport_if_applicable(new_server, source="config")
|
||||
self.config_mcp_servers[server_id] = new_server
|
||||
|
||||
# Check if this is an OpenAPI-based server
|
||||
|
|
|
|||
|
|
@ -64,7 +64,12 @@ class MCPServer(BaseModel):
|
|||
Opt-in allowlist of header names; empty or unset forwards nothing. The gateway answers the caller
|
||||
over a stream whose HTTP headers are already committed before the tool runs, so these are relayed
|
||||
as protocol metadata rather than as HTTP response headers. Credential, cookie, upstream-session and
|
||||
hop-by-hop headers are never forwarded (see ``MCP_RESPONSE_HEADER_DENYLIST``)."""
|
||||
hop-by-hop headers are never forwarded (see ``MCP_RESPONSE_HEADER_DENYLIST``).
|
||||
|
||||
Honored only on the ``http`` (streamable-HTTP) transport, the one that exposes the upstream
|
||||
``tools/call`` HTTP response. stdio carries no HTTP, and SSE returns only an ack on its POST and
|
||||
delivers the result over a pre-committed stream, so on those the setting is inert and loading a
|
||||
server configured this way logs a warning."""
|
||||
# Admin-configured env vars. Each entry is {name, value, scope, description}.
|
||||
# scope=="global" values are interpolated into static_headers using ${NAME}.
|
||||
# scope=="user" values must be supplied per-user.
|
||||
|
|
|
|||
|
|
@ -6482,6 +6482,85 @@ class TestInternalDelegatePkceWarningLog:
|
|||
assert "internal-only" not in combined
|
||||
|
||||
|
||||
class TestWarnResponseHeadersUnsupportedTransport:
|
||||
"""allowed_response_headers is inert off streamable-HTTP, so loading such a server must say so."""
|
||||
|
||||
def _server(self, transport, allowed):
|
||||
return MCPServer(
|
||||
server_id="hdr-1",
|
||||
name="hdr_server",
|
||||
url="https://example.com/mcp",
|
||||
transport=transport,
|
||||
allowed_response_headers=allowed,
|
||||
)
|
||||
|
||||
def _warn(self, server, caplog):
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
|
||||
_warn_response_headers_unsupported_transport_if_applicable,
|
||||
)
|
||||
|
||||
_warn_response_headers_unsupported_transport_if_applicable(server, source="test")
|
||||
return " ".join(r.getMessage() for r in caplog.records)
|
||||
|
||||
@pytest.mark.parametrize("transport", [MCPTransport.sse, MCPTransport.stdio])
|
||||
def test_warns_on_every_non_http_transport(self, caplog, transport):
|
||||
caplog.set_level(logging.WARNING, logger="LiteLLM")
|
||||
|
||||
combined = self._warn(self._server(transport, ["X-Example-Header"]), caplog)
|
||||
|
||||
assert "allowed_response_headers is set but the transport is" in combined
|
||||
assert "hdr-1" in combined
|
||||
assert transport.value in combined
|
||||
|
||||
def test_no_warning_on_http_where_the_feature_works(self, caplog):
|
||||
caplog.set_level(logging.WARNING, logger="LiteLLM")
|
||||
|
||||
combined = self._warn(self._server(MCPTransport.http, ["X-Example-Header"]), caplog)
|
||||
|
||||
assert "allowed_response_headers" not in combined
|
||||
|
||||
@pytest.mark.parametrize("allowed", [None, []])
|
||||
def test_no_warning_when_the_feature_is_not_configured(self, caplog, allowed):
|
||||
caplog.set_level(logging.WARNING, logger="LiteLLM")
|
||||
|
||||
combined = self._warn(self._server(MCPTransport.sse, allowed), caplog)
|
||||
|
||||
assert "allowed_response_headers" not in combined
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_loading_such_a_server_from_config_emits_the_warning(self, caplog):
|
||||
"""Pins the call site: the check is only useful if config load actually runs it."""
|
||||
manager = MCPServerManager()
|
||||
config = {
|
||||
"sse_server": {
|
||||
"url": "https://example.com/sse",
|
||||
"transport": MCPTransport.sse,
|
||||
"allowed_response_headers": ["X-Example-Header"],
|
||||
}
|
||||
}
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="LiteLLM"):
|
||||
await manager.load_servers_from_config(config)
|
||||
|
||||
assert any("allowed_response_headers is set but the transport is" in m for m in caplog.messages)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_loading_an_http_server_from_config_stays_quiet(self, caplog):
|
||||
manager = MCPServerManager()
|
||||
config = {
|
||||
"http_server": {
|
||||
"url": "https://example.com/mcp",
|
||||
"transport": MCPTransport.http,
|
||||
"allowed_response_headers": ["X-Example-Header"],
|
||||
}
|
||||
}
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="LiteLLM"):
|
||||
await manager.load_servers_from_config(config)
|
||||
|
||||
assert not any("allowed_response_headers" in m for m in caplog.messages)
|
||||
|
||||
|
||||
class TestHasClientCredentialsOAuth2Flow:
|
||||
"""
|
||||
Regression tests for the M2M auto-detection bug.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue