mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-25 01:02:15 +00:00
security(mcp): strip Authorization in call_tool when LiteLLM admission used legacy header
Mirror the OAuth pass-through admission check from _prepare_mcp_server_headers (list-tools path) in _call_regular_mcp_tool (tool-call path): when the server is OAuth pass-through and the caller did not supply x-litellm-api-key, Authorization on the inbound request may itself be the LiteLLM API key — so strip it before forwarding instead of leaking the gateway credential upstream. When x-litellm-api-key is present, admission is unambiguous and Authorization continues to carry the upstream OAuth bearer (transparent pass-through).
This commit is contained in:
parent
7dfef63a02
commit
ec3c67084f
2 changed files with 183 additions and 5 deletions
|
|
@ -2762,6 +2762,7 @@ class MCPServerManager:
|
|||
proxy_logging_obj: Optional[ProxyLogging],
|
||||
host_progress_callback: Optional[Callable] = None,
|
||||
hook_extra_headers: Optional[Dict[str, str]] = None,
|
||||
user_api_key_auth: Optional[UserAPIKeyAuth] = None,
|
||||
) -> CallToolResult:
|
||||
"""
|
||||
Call a regular MCP tool using the MCP client.
|
||||
|
|
@ -2828,14 +2829,29 @@ class MCPServerManager:
|
|||
normalized_raw_headers = {
|
||||
str(k).lower(): v for k, v in raw_headers.items() if isinstance(k, str)
|
||||
}
|
||||
has_explicit_litellm_admission_header = (
|
||||
normalized_raw_headers.get("x-litellm-api-key") is not None
|
||||
)
|
||||
admission_consumed_authorization_as_litellm_key = (
|
||||
user_api_key_auth is not None
|
||||
and bool(getattr(user_api_key_auth, "api_key", None))
|
||||
and not has_explicit_litellm_admission_header
|
||||
)
|
||||
|
||||
for header in mcp_server.extra_headers:
|
||||
if not isinstance(header, str):
|
||||
continue
|
||||
if (
|
||||
mcp_server.has_client_credentials
|
||||
and header.lower() == "authorization"
|
||||
):
|
||||
continue
|
||||
if header.lower() == "authorization":
|
||||
if mcp_server.has_client_credentials:
|
||||
continue
|
||||
if mcp_server.is_oauth_passthrough and (
|
||||
admission_consumed_authorization_as_litellm_key
|
||||
or (
|
||||
user_api_key_auth is None
|
||||
and not has_explicit_litellm_admission_header
|
||||
)
|
||||
):
|
||||
continue
|
||||
header_value = normalized_raw_headers.get(header.lower())
|
||||
if header_value is None:
|
||||
continue
|
||||
|
|
@ -3126,6 +3142,7 @@ class MCPServerManager:
|
|||
proxy_logging_obj=proxy_logging_obj,
|
||||
host_progress_callback=host_progress_callback,
|
||||
hook_extra_headers=hook_result.get("extra_headers"),
|
||||
user_api_key_auth=user_api_key_auth,
|
||||
)
|
||||
|
||||
return await self._gather_openapi_tool_tasks(tasks, proxy_logging_obj)
|
||||
|
|
|
|||
|
|
@ -480,6 +480,167 @@ class TestMCPServerManager:
|
|||
assert captured_extra_headers == {"Authorization": "Bearer token"}
|
||||
assert isinstance(result, CallToolResult)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_call_regular_mcp_tool_passthrough_strips_authorization_when_admission_consumed_litellm_key(
|
||||
self,
|
||||
):
|
||||
"""OAuth pass-through must not forward the caller's Authorization to upstream
|
||||
when LiteLLM admission consumed the bearer as its API key — otherwise the
|
||||
LiteLLM key the caller used for admission would leak upstream."""
|
||||
from litellm.proxy._types import UserAPIKeyAuth
|
||||
|
||||
manager = MCPServerManager()
|
||||
server = MCPServer(
|
||||
server_id="server-passthrough-call",
|
||||
name="passthrough-server",
|
||||
url="https://example.com",
|
||||
transport=MCPTransport.http,
|
||||
auth_type=MCPAuth.none,
|
||||
extra_headers=["Authorization", "x-request-id"],
|
||||
delegate_auth_to_upstream=True,
|
||||
)
|
||||
|
||||
mock_client = AsyncMock()
|
||||
mock_client.call_tool = AsyncMock(
|
||||
return_value=CallToolResult(content=[], isError=False)
|
||||
)
|
||||
captured_extra_headers = None
|
||||
|
||||
async def capture_create_mcp_client(
|
||||
server, mcp_auth_header, extra_headers, stdio_env, subject_token=None
|
||||
): # pragma: no cover - helper
|
||||
nonlocal captured_extra_headers
|
||||
captured_extra_headers = extra_headers
|
||||
return mock_client
|
||||
|
||||
manager._create_mcp_client = AsyncMock(side_effect=capture_create_mcp_client)
|
||||
|
||||
await manager._call_regular_mcp_tool(
|
||||
mcp_server=server,
|
||||
original_tool_name="tool",
|
||||
arguments={},
|
||||
tasks=[],
|
||||
mcp_auth_header=None,
|
||||
mcp_server_auth_headers=None,
|
||||
oauth2_headers=None,
|
||||
raw_headers={
|
||||
"authorization": "Bearer sk-litellm-key",
|
||||
"x-request-id": "req-123",
|
||||
},
|
||||
proxy_logging_obj=None,
|
||||
user_api_key_auth=UserAPIKeyAuth(api_key="sk-litellm-key"),
|
||||
)
|
||||
|
||||
assert captured_extra_headers == {"x-request-id": "req-123"}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_call_regular_mcp_tool_passthrough_forwards_authorization_with_admission_header(
|
||||
self,
|
||||
):
|
||||
"""OAuth pass-through forwards Authorization upstream when x-litellm-api-key
|
||||
provides admission — in that case Authorization carries the upstream OAuth
|
||||
bearer, not the LiteLLM key."""
|
||||
from litellm.proxy._types import UserAPIKeyAuth
|
||||
|
||||
manager = MCPServerManager()
|
||||
server = MCPServer(
|
||||
server_id="server-passthrough-call-admission",
|
||||
name="passthrough-server",
|
||||
url="https://example.com",
|
||||
transport=MCPTransport.http,
|
||||
auth_type=MCPAuth.none,
|
||||
extra_headers=["Authorization"],
|
||||
delegate_auth_to_upstream=True,
|
||||
)
|
||||
|
||||
mock_client = AsyncMock()
|
||||
mock_client.call_tool = AsyncMock(
|
||||
return_value=CallToolResult(content=[], isError=False)
|
||||
)
|
||||
captured_extra_headers = None
|
||||
|
||||
async def capture_create_mcp_client(
|
||||
server, mcp_auth_header, extra_headers, stdio_env, subject_token=None
|
||||
): # pragma: no cover - helper
|
||||
nonlocal captured_extra_headers
|
||||
captured_extra_headers = extra_headers
|
||||
return mock_client
|
||||
|
||||
manager._create_mcp_client = AsyncMock(side_effect=capture_create_mcp_client)
|
||||
|
||||
await manager._call_regular_mcp_tool(
|
||||
mcp_server=server,
|
||||
original_tool_name="tool",
|
||||
arguments={},
|
||||
tasks=[],
|
||||
mcp_auth_header=None,
|
||||
mcp_server_auth_headers=None,
|
||||
oauth2_headers=None,
|
||||
raw_headers={
|
||||
"x-litellm-api-key": "Bearer sk-litellm-key",
|
||||
"authorization": "Bearer upstream-oauth-bearer",
|
||||
},
|
||||
proxy_logging_obj=None,
|
||||
user_api_key_auth=UserAPIKeyAuth(api_key="sk-litellm-key"),
|
||||
)
|
||||
|
||||
assert captured_extra_headers == {
|
||||
"Authorization": "Bearer upstream-oauth-bearer"
|
||||
}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_call_regular_mcp_tool_passthrough_forwards_authorization_for_anonymous_admission(
|
||||
self,
|
||||
):
|
||||
"""OAuth pass-through cold-start return (RFC 9728): the caller's only
|
||||
credential is the upstream bearer in Authorization, and LiteLLM admission
|
||||
is anonymous (no api_key on user_api_key_auth). Authorization must be
|
||||
forwarded so the delegated flow can complete."""
|
||||
from litellm.proxy._types import UserAPIKeyAuth
|
||||
|
||||
manager = MCPServerManager()
|
||||
server = MCPServer(
|
||||
server_id="server-passthrough-call-anon",
|
||||
name="passthrough-server",
|
||||
url="https://example.com",
|
||||
transport=MCPTransport.http,
|
||||
auth_type=MCPAuth.none,
|
||||
extra_headers=["Authorization"],
|
||||
delegate_auth_to_upstream=True,
|
||||
)
|
||||
|
||||
mock_client = AsyncMock()
|
||||
mock_client.call_tool = AsyncMock(
|
||||
return_value=CallToolResult(content=[], isError=False)
|
||||
)
|
||||
captured_extra_headers = None
|
||||
|
||||
async def capture_create_mcp_client(
|
||||
server, mcp_auth_header, extra_headers, stdio_env, subject_token=None
|
||||
): # pragma: no cover - helper
|
||||
nonlocal captured_extra_headers
|
||||
captured_extra_headers = extra_headers
|
||||
return mock_client
|
||||
|
||||
manager._create_mcp_client = AsyncMock(side_effect=capture_create_mcp_client)
|
||||
|
||||
await manager._call_regular_mcp_tool(
|
||||
mcp_server=server,
|
||||
original_tool_name="tool",
|
||||
arguments={},
|
||||
tasks=[],
|
||||
mcp_auth_header=None,
|
||||
mcp_server_auth_headers=None,
|
||||
oauth2_headers=None,
|
||||
raw_headers={"authorization": "Bearer upstream-oauth-bearer"},
|
||||
proxy_logging_obj=None,
|
||||
user_api_key_auth=UserAPIKeyAuth(api_key=None),
|
||||
)
|
||||
|
||||
assert captured_extra_headers == {
|
||||
"Authorization": "Bearer upstream-oauth-bearer"
|
||||
}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_prompts_from_server_success(self):
|
||||
"""Ensure prompts are fetched and prefixed when requested."""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue