mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
fix(mcp): harden DCR bridge admission
Preserve standard Authorization key validation while preventing client MCP credentials from receiving anonymous bridge admission. Generated with AI Co-Authored-By: Codex
This commit is contained in:
Daniel Meismer
parent
a66e091cd7
commit
da036ad0f0
2 changed files with 144 additions and 21 deletions
|
|
@ -447,6 +447,8 @@ class MCPRequestHandler:
|
|||
)
|
||||
is not None
|
||||
and not oauth2_headers
|
||||
and not mcp_server_auth_headers
|
||||
and not mcp_auth_header
|
||||
):
|
||||
validated_user_api_key_auth = UserAPIKeyAuth()
|
||||
elif (
|
||||
|
|
@ -456,9 +458,13 @@ class MCPRequestHandler:
|
|||
client_ip=IPAddressUtils.get_mcp_client_ip(request),
|
||||
)
|
||||
) is not None and oauth2_headers:
|
||||
validated_user_api_key_auth, mcp_server_auth_headers = await MCPRequestHandler._admit_dcr_bridge_delegate(
|
||||
(
|
||||
validated_user_api_key_auth,
|
||||
mcp_server_auth_headers,
|
||||
) = await MCPRequestHandler._admit_dcr_bridge_authorization(
|
||||
server=bridge_delegate_target,
|
||||
authorization_value=oauth2_headers["Authorization"],
|
||||
litellm_api_key=litellm_api_key,
|
||||
mcp_server_auth_headers=mcp_server_auth_headers,
|
||||
request=request,
|
||||
route=request_route,
|
||||
|
|
@ -802,28 +808,57 @@ class MCPRequestHandler:
|
|||
new_headers: Final = {**(mcp_server_auth_headers or {}), **injected}
|
||||
return admitted, new_headers
|
||||
case BridgeEnvelopeInvalid() | NotBridgeEnvelope():
|
||||
resource_name: Final = server.alias or server.server_name
|
||||
if resource_name is None:
|
||||
raise HTTPException(
|
||||
status_code=500,
|
||||
detail="Server misconfigured: MCP server has no routable name",
|
||||
)
|
||||
raise HTTPException(
|
||||
status_code=401,
|
||||
detail="Invalid or expired credential",
|
||||
headers=MappingProxyType(
|
||||
{
|
||||
"www-authenticate": get_passthrough_www_authenticate(
|
||||
scope=request.scope,
|
||||
server_name=resource_name,
|
||||
invalid_token=True,
|
||||
)
|
||||
}
|
||||
),
|
||||
)
|
||||
raise MCPRequestHandler._dcr_bridge_invalid_token_challenge(server=server, request=request)
|
||||
case _:
|
||||
assert_never(result)
|
||||
|
||||
@staticmethod
|
||||
async def _admit_dcr_bridge_authorization(
|
||||
server: MCPServer,
|
||||
authorization_value: str,
|
||||
litellm_api_key: str,
|
||||
mcp_server_auth_headers: dict[str, dict[str, str]] | None,
|
||||
request: Request,
|
||||
route: str,
|
||||
) -> tuple[UserAPIKeyAuth, dict[str, dict[str, str]] | None]:
|
||||
if is_bridge_envelope_shaped(authorization_value):
|
||||
return await MCPRequestHandler._admit_dcr_bridge_delegate(
|
||||
server=server,
|
||||
authorization_value=authorization_value,
|
||||
mcp_server_auth_headers=mcp_server_auth_headers,
|
||||
request=request,
|
||||
route=route,
|
||||
)
|
||||
try:
|
||||
admitted: Final = await user_api_key_auth(api_key=litellm_api_key, request=request)
|
||||
except (HTTPException, ProxyException) as exc:
|
||||
if not _is_litellm_auth_admission_error(exc):
|
||||
raise
|
||||
raise MCPRequestHandler._dcr_bridge_invalid_token_challenge(server=server, request=request) from exc
|
||||
return admitted, mcp_server_auth_headers
|
||||
|
||||
@staticmethod
|
||||
def _dcr_bridge_invalid_token_challenge(server: MCPServer, request: Request) -> HTTPException:
|
||||
resource_name: Final = server.alias or server.server_name
|
||||
if resource_name is None:
|
||||
raise HTTPException(
|
||||
status_code=500,
|
||||
detail="Server misconfigured: MCP server has no routable name",
|
||||
)
|
||||
return HTTPException(
|
||||
status_code=401,
|
||||
detail="Invalid or expired credential",
|
||||
headers=MappingProxyType(
|
||||
{
|
||||
"www-authenticate": get_passthrough_www_authenticate(
|
||||
scope=request.scope,
|
||||
server_name=resource_name,
|
||||
invalid_token=True,
|
||||
)
|
||||
}
|
||||
),
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
async def _admit_gateway_session(
|
||||
authorization_value: str,
|
||||
|
|
|
|||
|
|
@ -5320,6 +5320,37 @@ class TestMCPDcrBridgeDelegateAdmission:
|
|||
assert auth_result == UserAPIKeyAuth()
|
||||
assert mcp_server_auth_headers == {}
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"headers",
|
||||
(
|
||||
[(b"x-mcp-auth", b"Bearer upstream-token")],
|
||||
[(b"x-mcp-bridge_delegate_server-authorization", b"Bearer upstream-token")],
|
||||
),
|
||||
ids=("deprecated-mcp-auth", "per-server-auth"),
|
||||
)
|
||||
async def test_client_mcp_credentials_do_not_receive_keyless_bridge_admission(self, headers):
|
||||
scope = {
|
||||
"type": "http",
|
||||
"method": "POST",
|
||||
"path": "/mcp/bridge_delegate_server",
|
||||
"headers": headers,
|
||||
}
|
||||
|
||||
with (
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp.user_api_key_auth",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=HTTPException(status_code=401, detail="Invalid key"),
|
||||
) as mock_auth,
|
||||
patch("litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager") as mock_mgr,
|
||||
):
|
||||
mock_mgr.get_mcp_server_by_name.return_value = self._bridge_delegate_server()
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await MCPRequestHandler.process_mcp_request(scope)
|
||||
|
||||
assert exc_info.value.status_code == 401
|
||||
mock_auth.assert_awaited_once()
|
||||
|
||||
async def test_valid_envelope_reloads_live_key_and_admits_its_authorization_context(self):
|
||||
"""A valid envelope admits under the LIVE key record the sealed key_hash references, not a
|
||||
blank identity: the reload is keyed by that exact hash, and the admitted auth carries the
|
||||
|
|
@ -6058,6 +6089,7 @@ class TestMCPDcrBridgeDelegateAdmission:
|
|||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp.user_api_key_auth",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=HTTPException(status_code=401, detail="Invalid key"),
|
||||
) as mock_auth,
|
||||
patch("litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager") as mock_mgr,
|
||||
patch("litellm.proxy.proxy_server.master_key", self._MASTER_KEY),
|
||||
|
|
@ -6067,7 +6099,7 @@ class TestMCPDcrBridgeDelegateAdmission:
|
|||
await MCPRequestHandler.process_mcp_request(scope)
|
||||
|
||||
assert exc_info.value.status_code == 401
|
||||
mock_auth.assert_not_called()
|
||||
mock_auth.assert_awaited_once()
|
||||
assert exc_info.value.headers == {
|
||||
"www-authenticate": (
|
||||
'Bearer error="invalid_token", '
|
||||
|
|
@ -6075,6 +6107,62 @@ class TestMCPDcrBridgeDelegateAdmission:
|
|||
)
|
||||
}
|
||||
|
||||
async def test_valid_litellm_authorization_key_uses_standard_admission(self):
|
||||
scope = {
|
||||
"type": "http",
|
||||
"method": "POST",
|
||||
"path": "/mcp/bridge_delegate_server",
|
||||
"headers": [(b"authorization", b"Bearer sk-valid-litellm-key")],
|
||||
}
|
||||
admitted = UserAPIKeyAuth(api_key="hashed-key", user_id="litellm-key-user")
|
||||
|
||||
with (
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp.user_api_key_auth",
|
||||
new_callable=AsyncMock,
|
||||
return_value=admitted,
|
||||
) as mock_auth,
|
||||
patch("litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager") as mock_mgr,
|
||||
patch("litellm.proxy.proxy_server.master_key", self._MASTER_KEY),
|
||||
):
|
||||
mock_mgr.get_mcp_server_by_name.return_value = self._bridge_delegate_server()
|
||||
(
|
||||
auth_result,
|
||||
_mcp_auth,
|
||||
_servers,
|
||||
mcp_server_auth_headers,
|
||||
_oauth,
|
||||
_raw,
|
||||
) = await MCPRequestHandler.process_mcp_request(scope)
|
||||
|
||||
assert auth_result is admitted
|
||||
assert mcp_server_auth_headers == {}
|
||||
assert mock_auth.await_args.kwargs["api_key"] == "Bearer sk-valid-litellm-key"
|
||||
|
||||
async def test_non_401_litellm_key_failure_is_not_converted_to_oauth_challenge(self):
|
||||
scope = {
|
||||
"type": "http",
|
||||
"method": "POST",
|
||||
"path": "/mcp/bridge_delegate_server",
|
||||
"headers": [(b"authorization", b"Bearer sk-blocked-litellm-key")],
|
||||
}
|
||||
|
||||
with (
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp.user_api_key_auth",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=HTTPException(status_code=403, detail="Key blocked"),
|
||||
),
|
||||
patch("litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager") as mock_mgr,
|
||||
patch("litellm.proxy.proxy_server.master_key", self._MASTER_KEY),
|
||||
):
|
||||
mock_mgr.get_mcp_server_by_name.return_value = self._bridge_delegate_server()
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await MCPRequestHandler.process_mcp_request(scope)
|
||||
|
||||
assert exc_info.value.status_code == 403
|
||||
assert not exc_info.value.headers
|
||||
|
||||
async def test_explicit_litellm_key_wins_over_envelope_arm(self):
|
||||
"""An explicit x-litellm-api-key is always a LiteLLM credential and its arm precedes the
|
||||
envelope arm: user_api_key_auth validates the key and NO inner token is injected, even
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue