mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-25 01:02:15 +00:00
refactor(mcp): centralize caller Authorization strip decision
Extracted the security-sensitive logic that decides whether the caller's Authorization header is forwarded to (or stripped from) an outgoing MCP request into a single helper, _should_strip_caller_authorization, in mcp_server_manager.py. Previously the same condition was duplicated across _call_regular_mcp_tool (mcp_server_manager.py) and _prepare_mcp_server_headers (server.py). Keeping two copies of this check risked future divergence and credential-leak / broken-passthrough bugs. Both call sites now share the helper, preserving exact behavior. Co-authored-by: Yassin Kortam <yassin@berri.ai>
This commit is contained in:
Cursor Agent
parent
ec3c67084f
commit
b80a91ffc5
2 changed files with 63 additions and 55 deletions
|
|
@ -118,6 +118,52 @@ _AZURE_ENTRA_HOSTS = {
|
|||
}
|
||||
|
||||
|
||||
def _should_strip_caller_authorization(
|
||||
mcp_server: MCPServer,
|
||||
raw_headers: Optional[Dict[str, str]],
|
||||
user_api_key_auth: Optional[UserAPIKeyAuth],
|
||||
) -> bool:
|
||||
"""Decide whether the caller's ``Authorization`` header must NOT be
|
||||
forwarded upstream when populating ``extra_headers`` for an MCP server.
|
||||
|
||||
Centralized so ``_call_regular_mcp_tool`` (this module) and
|
||||
``_prepare_mcp_server_headers`` (``server.py``) cannot drift apart on
|
||||
this security-sensitive decision.
|
||||
|
||||
Strip rules:
|
||||
- **M2M (client_credentials) servers**: never forward the caller's
|
||||
``Authorization`` — the proxy fetches its own upstream token.
|
||||
- **OAuth pass-through servers**: strip when the ``Authorization``
|
||||
header is actually the LiteLLM API key — either because admission
|
||||
validated it (``user_api_key_auth.api_key`` is set) and the caller
|
||||
did NOT also supply ``x-litellm-api-key`` to disambiguate, or
|
||||
because the legacy ``user_api_key_auth is None`` call sites did
|
||||
not supply an explicit admission header. In the anonymous /
|
||||
pass-through cold-start case (RFC 9728) the bearer in
|
||||
``Authorization`` is the upstream OAuth token and must be
|
||||
forwarded, so we keep it.
|
||||
"""
|
||||
if mcp_server.has_client_credentials:
|
||||
return True
|
||||
if not mcp_server.is_oauth_passthrough:
|
||||
return False
|
||||
|
||||
normalized_raw_headers = {
|
||||
str(k).lower(): v for k, v in (raw_headers or {}).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
|
||||
)
|
||||
return admission_consumed_authorization_as_litellm_key or (
|
||||
user_api_key_auth is None and not has_explicit_litellm_admission_header
|
||||
)
|
||||
|
||||
|
||||
def _extract_upstream_auth_failure(
|
||||
exc: BaseException,
|
||||
) -> Optional[Tuple[int, Optional[str]]]:
|
||||
|
|
@ -2829,29 +2875,17 @@ 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
|
||||
strip_caller_authorization = _should_strip_caller_authorization(
|
||||
mcp_server=mcp_server,
|
||||
raw_headers=raw_headers,
|
||||
user_api_key_auth=user_api_key_auth,
|
||||
)
|
||||
|
||||
for header in mcp_server.extra_headers:
|
||||
if not isinstance(header, str):
|
||||
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
|
||||
if header.lower() == "authorization" and strip_caller_authorization:
|
||||
continue
|
||||
header_value = normalized_raw_headers.get(header.lower())
|
||||
if header_value is None:
|
||||
continue
|
||||
|
|
|
|||
|
|
@ -160,6 +160,7 @@ if MCP_AVAILABLE:
|
|||
)
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
|
||||
MCPServerManager,
|
||||
_should_strip_caller_authorization,
|
||||
global_mcp_server_manager,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import (
|
||||
|
|
@ -1175,48 +1176,21 @@ if MCP_AVAILABLE:
|
|||
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 a LiteLLM key only when
|
||||
# auth produced a validated ``api_key`` AND the caller did not
|
||||
# supply ``x-litellm-api-key``. When admission was anonymous
|
||||
# (e.g. pass-through cold-start return per RFC 9728), the bearer
|
||||
# in ``Authorization`` is the upstream OAuth token and must be
|
||||
# forwarded — not stripped — for the delegated flow to work.
|
||||
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
|
||||
# Centralized strip decision shared with
|
||||
# ``MCPServerManager._call_regular_mcp_tool`` so the two
|
||||
# code paths cannot drift on this security-sensitive choice.
|
||||
# See ``_should_strip_caller_authorization`` for the rules.
|
||||
strip_caller_authorization = _should_strip_caller_authorization(
|
||||
mcp_server=server,
|
||||
raw_headers=raw_headers,
|
||||
user_api_key_auth=user_api_key_auth,
|
||||
)
|
||||
|
||||
for header in server.extra_headers:
|
||||
if not isinstance(header, str):
|
||||
continue
|
||||
if header.lower() == "authorization":
|
||||
# M2M servers fetch their own upstream token via the
|
||||
# client_credentials flow — never forward the caller's
|
||||
# Authorization header.
|
||||
if server.has_client_credentials:
|
||||
continue
|
||||
# Transparent OAuth pass-through: forward the caller's
|
||||
# Authorization header only when LiteLLM admission used
|
||||
# a different header (`x-litellm-api-key`) or when
|
||||
# admission was anonymous (delegated to upstream).
|
||||
# Without that signal `Authorization` may itself be the
|
||||
# LiteLLM key — strip it to avoid leaking the gateway
|
||||
# credential upstream. The legacy ``user_api_key_auth
|
||||
# is None`` callers keep the conservative pre-PR
|
||||
# behavior of stripping when no explicit admission
|
||||
# header was supplied.
|
||||
if 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
|
||||
if header.lower() == "authorization" and strip_caller_authorization:
|
||||
continue
|
||||
header_value = normalized_raw_headers.get(header.lower())
|
||||
if header_value is None:
|
||||
continue
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue