From f90382584bd9e632ccbd1b4b19af50aa03da6129 Mon Sep 17 00:00:00 2001 From: Tin Chi Lo Date: Mon, 13 Jul 2026 13:16:45 -0700 Subject: [PATCH 1/6] fix(mcp): relay upstream OAuth token and DCR rejections instead of a generic 500 An upstream token endpoint rejection (e.g. Google requiring client_secret even for PKCE web clients) escaped exchange_token_with_server as a raw httpx.HTTPStatusError, which the global exception handler turned into an opaque 500 Internal server error in the create-flow UI. The RFC 6749 section 5.2 error body the IdP sent (error, error_description, error_uri) is now relayed with the upstream's own 400/401 status; rejections outside the section 5.2 contract map to 502 so a broken upstream is not misattributed to the caller. The same relay covers the non-bridge DCR registration arm, and a 200 token response without a usable access_token now answers 502 instead of a KeyError 500. The catch wraps the post call itself because litellm's AsyncHTTPHandler raises MaskedHTTPStatusError at call time, which also made the pre-existing bridge-relay status check unreachable in production. The dashboard's token exchange error message now composes error and error_description so the form shows the IdP's reason --- .../mcp_server/discoverable_endpoints.py | 94 +++++-- .../mcp_server/test_discoverable_endpoints.py | 253 +++++++++++++++++- .../src/components/networking.tsx | 6 +- 3 files changed, 328 insertions(+), 25 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py index 8d1713a5911..5c5a848a284 100644 --- a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py @@ -980,6 +980,50 @@ def _finish_bridge_mint( return JSONResponse(body, headers=TOKEN_NO_CACHE_HEADERS) +def _response_json_or_none(response: httpx.Response) -> object: + try: + return response.json() + except ValueError: + return None + + +def _upstream_token_fault_response(description: str) -> JSONResponse: + """502 token-endpoint response for an upstream fault, in the RFC 6749 §5.2 shape the gateway's + token callers parse, with the §5.1 no-store headers.""" + return JSONResponse( + status_code=502, + content={"error": "server_error", "error_description": description}, + headers=TOKEN_NO_CACHE_HEADERS, + ) + + +def _upstream_token_error_response(response: httpx.Response) -> JSONResponse: + """Relay an upstream token-endpoint rejection to the client instead of letting the raw + ``httpx.HTTPStatusError`` escape to the global handler as an opaque 500 (the IdP's own + ``error_description``, e.g. Google's "client_secret is missing.", is the message the caller needs). + Only the RFC 6749 §5.2 fields (``error`` / ``error_description`` / ``error_uri``) are relayed, + each bounded, on the upstream's own 400/401 status; a rejection outside the §5.2 contract + (no JSON ``error`` field, or a status §5.2 does not define) maps to 502 so a broken upstream + is not misattributed to the caller's request.""" + parsed = _response_json_or_none(response) + error_code = parsed.get("error") if isinstance(parsed, dict) else None + if not isinstance(error_code, str) or not error_code: + detail = (response.text or response.reason_phrase or "")[:_MAX_UPSTREAM_ERROR_CHARS] + return _upstream_token_fault_response(f"upstream token endpoint returned HTTP {response.status_code}: {detail}") + fields = parsed if isinstance(parsed, dict) else {} + relayed = { + key: value[:_MAX_UPSTREAM_ERROR_CHARS] + for key, value in ( + ("error", error_code), + ("error_description", fields.get("error_description")), + ("error_uri", fields.get("error_uri")), + ) + if isinstance(value, str) and value + } + status_code = response.status_code if response.status_code in (400, 401) else 502 + return JSONResponse(status_code=status_code, content=relayed, headers=TOKEN_NO_CACHE_HEADERS) + + async def exchange_token_with_server( request: Request, mcp_server: MCPServer, @@ -1064,19 +1108,14 @@ async def exchange_token_with_server( bridge_mint_ready = prepared async_client = get_async_httpx_client(llm_provider=httpxSpecialProvider.Oauth2Check) - response = await async_client.post( - mcp_server.token_url, - headers={"Accept": "application/json", **client_auth.headers}, - data=token_data, - ) - if response is None: - raise HTTPException( - status_code=502, - detail="MCP upstream token endpoint returned no response", - ) - try: - response.raise_for_status() + response = await async_client.post( + mcp_server.token_url, + headers={"Accept": "application/json", **client_auth.headers}, + data=token_data, + ) + if response is not None: + response.raise_for_status() except httpx.HTTPStatusError as exc: if "invalid_target" in exc.response.text: verbose_logger.warning( @@ -1085,7 +1124,12 @@ async def exchange_token_with_server( "does not send yet (tracked as LIT-4339)", mcp_server.server_id, ) - raise + return _upstream_token_error_response(exc.response) + if response is None: + raise HTTPException( + status_code=502, + detail="MCP upstream token endpoint returned no response", + ) token_response = response.json() # Validate token response against server-configured rules before any storage. @@ -1135,8 +1179,12 @@ async def exchange_token_with_server( minted = _finish_bridge_mint(bridge_mint_ready, mcp_server, token_response, datetime.now(timezone.utc)) return minted if isinstance(minted, JSONResponse) else _bridge_mint_error_response(minted) + raw_access_token = token_response.get("access_token") if isinstance(token_response, dict) else None + if not isinstance(raw_access_token, str) or not raw_access_token: + return _upstream_token_fault_response("the upstream token response has no usable access_token") + result = { - "access_token": token_response["access_token"], + "access_token": raw_access_token, "token_type": token_response.get("token_type", "Bearer"), } @@ -1466,11 +1514,18 @@ async def register_client_with_server( } async_client = get_async_httpx_client(llm_provider=httpxSpecialProvider.Oauth2Register) - response = await async_client.post( - mcp_server.registration_url, - headers=headers, - json=register_data, - ) + try: + response = await async_client.post( + mcp_server.registration_url, + headers=headers, + json=register_data, + ) + if response is not None: + response.raise_for_status() + except httpx.HTTPStatusError as exc: + raise HTTPException( + status_code=exc.response.status_code, detail=_safe_upstream_error_detail(exc.response) + ) from exc if response is None: raise HTTPException( status_code=502, @@ -1478,7 +1533,6 @@ async def register_client_with_server( ) if bridge_relay and response.status_code >= 400: raise HTTPException(status_code=response.status_code, detail=_safe_upstream_error_detail(response)) - response.raise_for_status() token_response = response.json() diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py index 68466e624ec..fd5e4b7d62b 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py @@ -4307,9 +4307,10 @@ async def test_register_bridge_relay_surfaces_upstream_error_not_500(): @pytest.mark.asyncio -async def test_register_non_bridge_upstream_error_still_raises_500(): - """Non-bridge DCR keeps its pre-change behavior: raise_for_status propagates so the flag-off - contract is byte-identical; only the bridge relay arm relays the upstream status.""" +async def test_register_non_bridge_upstream_error_relays_status_not_500(): + """A non-bridge DCR rejection must relay the upstream status and RFC 7591 error body just like + the bridge relay arm; a raw HTTPStatusError would escape to the global handler and surface as an + opaque 500 that hides the real reason from the create-flow UI.""" import httpx from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( @@ -4338,7 +4339,7 @@ async def test_register_non_bridge_upstream_error_still_raises_500(): return_value=False, ), ): - with pytest.raises(httpx.HTTPStatusError): + with pytest.raises(HTTPException) as exc: await register_client_with_server( request=_bridge_mock_request(), mcp_server=oauth2_server, @@ -4348,6 +4349,9 @@ async def test_register_non_bridge_upstream_error_still_raises_500(): token_endpoint_auth_method=None, ) + assert exc.value.status_code == 400 + assert "invalid_client_metadata" in str(exc.value.detail) + @pytest.mark.asyncio async def test_register_bridge_relay_never_persists(): @@ -5960,3 +5964,244 @@ async def test_token_exchange_pairs_client_secret_with_server_client_id(): sent = mock_async_client.post.call_args.kwargs["data"] assert sent["client_id"] == "persisted-client" assert "client_secret" not in sent + + +def _upstream_token_response(status_code: int, *, json_body: object = None, text_body: str = "") -> "httpx.Response": + import httpx + + request = httpx.Request("POST", "https://oauth2.googleapis.com/token") + if json_body is not None: + return httpx.Response(status_code, json=json_body, request=request) + return httpx.Response(status_code, text=text_body, request=request) + + +async def _exchange_with_upstream_response(upstream_response): + """Run the raw (non-bridge) authorization_code exchange against a canned upstream token-endpoint + response and return what the gateway would hand the client.""" + from fastapi import Request + + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + exchange_token_with_server, + ) + from litellm.proxy._types import MCPTransport + from litellm.types.mcp import MCPAuth + from litellm.types.mcp_server.mcp_server_manager import MCPServer + + server = MCPServer( + server_id="gcal", + name="gcal", + server_name="gcal", + alias="gcal", + transport=MCPTransport.http, + auth_type=MCPAuth.oauth2, + client_id="web-client.apps.googleusercontent.com", + authorization_url="https://accounts.google.com/o/oauth2/v2/auth", + token_url="https://oauth2.googleapis.com/token", + ) + mock_request = MagicMock(spec=Request) + mock_request.base_url = "https://litellm.example.com/" + mock_request.headers = {} + mock_async_client = MagicMock() + mock_async_client.post = AsyncMock(return_value=upstream_response) + + with patch( + "litellm.proxy._experimental.mcp_server.discoverable_endpoints.get_async_httpx_client", + return_value=mock_async_client, + ): + return await exchange_token_with_server( + request=mock_request, + mcp_server=server, + grant_type="authorization_code", + code="auth-code", + redirect_uri="https://litellm.example.com/ui/mcp/oauth/callback", + client_id="web-client.apps.googleusercontent.com", + client_secret=None, + code_verifier="verifier", + ) + + +@pytest.mark.asyncio +async def test_token_exchange_relays_upstream_rfc6749_rejection(): + """The IdP's own RFC 6749 §5.2 rejection is the message the caller needs (e.g. Google refusing a + secret-less web client); before the relay the raw HTTPStatusError escaped to the global handler + and every upstream rejection surfaced as an opaque 500 Internal server error.""" + response = await _exchange_with_upstream_response( + _upstream_token_response( + 401, json_body={"error": "invalid_client", "error_description": "The OAuth client was not found."} + ) + ) + + assert response.status_code == 401 + body = json.loads(response.body) + assert body == {"error": "invalid_client", "error_description": "The OAuth client was not found."} + assert response.headers["cache-control"] == "no-store" + + +@pytest.mark.asyncio +async def test_token_exchange_relays_only_rfc6749_error_fields(): + """Only error / error_description / error_uri cross the gateway; any other upstream body field is + dropped so an arbitrary rejection payload cannot ride the relay to the client.""" + response = await _exchange_with_upstream_response( + _upstream_token_response( + 400, + json_body={ + "error": "invalid_grant", + "error_description": "Code was already redeemed.", + "error_uri": "https://idp.example.com/errors/invalid_grant", + "internal_trace": "should never reach the client", + }, + ) + ) + + assert response.status_code == 400 + body = json.loads(response.body) + assert set(body.keys()) == {"error", "error_description", "error_uri"} + + +@pytest.mark.asyncio +async def test_token_exchange_maps_out_of_contract_rejection_to_502(): + """A rejection outside the §5.2 contract (no JSON error field, or a status the token-endpoint + contract does not define) is an upstream fault; 502 keeps it from being misread as a caller + mistake while the description still names the upstream status.""" + response = await _exchange_with_upstream_response( + _upstream_token_response(503, text_body="upstream maintenance") + ) + + assert response.status_code == 502 + body = json.loads(response.body) + assert body["error"] == "server_error" + assert "HTTP 503" in body["error_description"] + + +@pytest.mark.asyncio +async def test_token_exchange_bounds_relayed_error_fields(): + """Relayed §5.2 fields are length-bounded so a hostile or broken upstream cannot bloat the + gateway response.""" + response = await _exchange_with_upstream_response( + _upstream_token_response(400, json_body={"error": "invalid_request", "error_description": "x" * 5000}) + ) + + assert response.status_code == 400 + body = json.loads(response.body) + assert len(body["error_description"]) == 500 + + +@pytest.mark.asyncio +async def test_token_exchange_200_without_access_token_is_502_not_keyerror(): + """A 200 whose body has no usable access_token used to KeyError into a 500; the raw arm now + answers 502 with the same wording as the bridge arm's no_upstream_token rejection.""" + response = await _exchange_with_upstream_response( + _upstream_token_response(200, json_body={"token_type": "Bearer"}) + ) + + assert response.status_code == 502 + body = json.loads(response.body) + assert body["error"] == "server_error" + assert "access_token" in body["error_description"] + + +@pytest.mark.asyncio +async def test_token_exchange_relays_rejection_when_http_client_raises(): + """litellm's AsyncHTTPHandler.post raise_for_status()es internally and raises MaskedHTTPStatusError + at call time, so in production the rejection escapes from the post call itself rather than from the + explicit raise_for_status; the relay must catch it there too (proven live: a mock returning the + error response passed while the real proxy still 500ed).""" + import httpx + + rejection = _upstream_token_response( + 401, json_body={"error": "invalid_client", "error_description": "The OAuth client was not found."} + ) + raising_client = MagicMock() + raising_client.post = AsyncMock( + side_effect=httpx.HTTPStatusError("Client error '401 Unauthorized'", request=rejection.request, response=rejection) + ) + + from fastapi import Request + + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + exchange_token_with_server, + ) + from litellm.proxy._types import MCPTransport + from litellm.types.mcp import MCPAuth + from litellm.types.mcp_server.mcp_server_manager import MCPServer + + server = MCPServer( + server_id="gcal", + name="gcal", + server_name="gcal", + alias="gcal", + transport=MCPTransport.http, + auth_type=MCPAuth.oauth2, + client_id="web-client.apps.googleusercontent.com", + authorization_url="https://accounts.google.com/o/oauth2/v2/auth", + token_url="https://oauth2.googleapis.com/token", + ) + mock_request = MagicMock(spec=Request) + mock_request.base_url = "https://litellm.example.com/" + mock_request.headers = {} + + with patch( + "litellm.proxy._experimental.mcp_server.discoverable_endpoints.get_async_httpx_client", + return_value=raising_client, + ): + response = await exchange_token_with_server( + request=mock_request, + mcp_server=server, + grant_type="authorization_code", + code="auth-code", + redirect_uri="https://litellm.example.com/ui/mcp/oauth/callback", + client_id="web-client.apps.googleusercontent.com", + client_secret=None, + code_verifier="verifier", + ) + + assert response.status_code == 401 + body = json.loads(response.body) + assert body["error"] == "invalid_client" + + +@pytest.mark.asyncio +async def test_register_relays_rejection_when_http_client_raises(): + """Same live mechanism as the token exchange: the DCR rejection escapes from the post call itself, + so the register relay must catch it there, not only from the explicit raise_for_status.""" + import httpx + + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + register_client_with_server, + ) + + rejection = httpx.Response( + 400, + json={"error": "invalid_client_metadata"}, + request=httpx.Request("POST", "https://idp.example.com/register"), + ) + raising_client = MagicMock() + raising_client.post = AsyncMock( + side_effect=httpx.HTTPStatusError("Client error '400 Bad Request'", request=rejection.request, response=rejection) + ) + + oauth2_server = _bridge_server(auth_type=MCPAuth.oauth2, dcr_bridge=None) + + with ( + patch( + "litellm.proxy._experimental.mcp_server.discoverable_endpoints.get_async_httpx_client", + return_value=raising_client, + ), + patch( + "litellm.proxy._experimental.mcp_server.discoverable_endpoints._reuse_persisted_dcr_client_if_available", + new_callable=AsyncMock, + return_value=False, + ), + ): + with pytest.raises(HTTPException) as exc: + await register_client_with_server( + request=_bridge_mock_request(), + mcp_server=oauth2_server, + client_name="Claude", + grant_types=None, + response_types=None, + token_endpoint_auth_method=None, + ) + + assert exc.value.status_code == 400 + assert "invalid_client_metadata" in str(exc.value.detail) diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index 875a345a4c5..7e3af46b931 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -6847,7 +6847,11 @@ export const exchangeMcpOAuthToken = async ({ const data = await response.json(); if (!response.ok) { - const errorMessage = deriveErrorMessage(data) || data?.detail || "OAuth token exchange failed"; + const oauthErrorMessage = + typeof data?.error === "string" && typeof data?.error_description === "string" + ? `${data.error}: ${data.error_description}` + : undefined; + const errorMessage = oauthErrorMessage || deriveErrorMessage(data) || data?.detail || "OAuth token exchange failed"; throw new Error(errorMessage); } return data; From e940199a00aab8d178e306972042ab43ba4c1cc8 Mon Sep 17 00:00:00 2001 From: Tin Chi Lo Date: Mon, 13 Jul 2026 13:24:19 -0700 Subject: [PATCH 2/6] refactor(mcp): drop bridge relay status check made unreachable by the unified relay The try/except around the registration post now relays every upstream 4xx/5xx for both arms, so the bridge_relay status_code check could never fire; removing it addresses the Greptile P2 dead-code finding --- .../proxy/_experimental/mcp_server/discoverable_endpoints.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py index 5c5a848a284..13d514f16db 100644 --- a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py @@ -1531,8 +1531,6 @@ async def register_client_with_server( status_code=502, detail="MCP upstream registration endpoint returned no response", ) - if bridge_relay and response.status_code >= 400: - raise HTTPException(status_code=response.status_code, detail=_safe_upstream_error_detail(response)) token_response = response.json() From 0d9c3cf97bd70a14893c105961f1b98f8a3a87bf Mon Sep 17 00:00:00 2001 From: Tin Chi Lo Date: Mon, 13 Jul 2026 15:13:39 -0700 Subject: [PATCH 3/6] fix(mcp): keep out-of-contract upstream error bodies out of client responses The token and DCR relays serve unauthenticated OAuth clients, so only the RFC 6749/7591 error fields may cross the trust boundary. A rejection body outside those contracts (HTML error page, proxy banner, stack trace) is now logged server-side, bounded, and the client response names only the upstream status. Addresses the Veria information-exposure finding --- .../mcp_server/discoverable_endpoints.py | 41 ++++++++---- .../mcp_server/test_discoverable_endpoints.py | 66 +++++++++++++++++++ 2 files changed, 95 insertions(+), 12 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py index 13d514f16db..42677229add 100644 --- a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py @@ -1004,12 +1004,19 @@ def _upstream_token_error_response(response: httpx.Response) -> JSONResponse: Only the RFC 6749 §5.2 fields (``error`` / ``error_description`` / ``error_uri``) are relayed, each bounded, on the upstream's own 400/401 status; a rejection outside the §5.2 contract (no JSON ``error`` field, or a status §5.2 does not define) maps to 502 so a broken upstream - is not misattributed to the caller's request.""" + is not misattributed to the caller's request. Out-of-contract bodies (HTML error pages, proxy + banners, stack traces) never cross the trust boundary: these endpoints serve unauthenticated + OAuth clients, so the body is logged server-side and the client sees only the upstream status.""" parsed = _response_json_or_none(response) error_code = parsed.get("error") if isinstance(parsed, dict) else None if not isinstance(error_code, str) or not error_code: - detail = (response.text or response.reason_phrase or "")[:_MAX_UPSTREAM_ERROR_CHARS] - return _upstream_token_fault_response(f"upstream token endpoint returned HTTP {response.status_code}: {detail}") + verbose_logger.warning( + "MCP upstream token endpoint returned HTTP %s with a non-RFC6749 body (first %s chars): %s", + response.status_code, + _MAX_UPSTREAM_ERROR_CHARS, + (response.text or "")[:_MAX_UPSTREAM_ERROR_CHARS], + ) + return _upstream_token_fault_response(f"upstream token endpoint returned HTTP {response.status_code}") fields = parsed if isinstance(parsed, dict) else {} relayed = { key: value[:_MAX_UPSTREAM_ERROR_CHARS] @@ -1444,15 +1451,25 @@ _MAX_UPSTREAM_ERROR_CHARS = 500 def _safe_upstream_error_detail(response: httpx.Response) -> str: - """Bounded plaintext summary of an upstream registration failure for the client. - - RFC 7591 error bodies are small JSON objects (``error`` / ``error_description``); relaying the - text lets the client read the real reason instead of a bare 500, and the length bound keeps a - hostile or oversized upstream body from bloating the gateway response.""" - body = response.text - if not body: - return response.reason_phrase or "upstream registration failed" - return body[:_MAX_UPSTREAM_ERROR_CHARS] + """Client-safe summary of an upstream registration failure: only the RFC 7591 §3.2.2 fields + (``error`` / ``error_description``) cross the trust boundary, each bounded, since these endpoints + serve unauthenticated OAuth clients. A body outside that contract (HTML error pages, proxy + banners, stack traces) is logged server-side and summarized by status so upstream internals are + never relayed to callers.""" + parsed = _response_json_or_none(response) + error_code = parsed.get("error") if isinstance(parsed, dict) else None + if isinstance(error_code, str) and error_code: + description = parsed.get("error_description") if isinstance(parsed, dict) else None + if isinstance(description, str) and description: + return f"{error_code[:_MAX_UPSTREAM_ERROR_CHARS]}: {description[:_MAX_UPSTREAM_ERROR_CHARS]}" + return error_code[:_MAX_UPSTREAM_ERROR_CHARS] + verbose_logger.warning( + "MCP upstream registration endpoint returned HTTP %s with a non-RFC7591 body (first %s chars): %s", + response.status_code, + _MAX_UPSTREAM_ERROR_CHARS, + (response.text or "")[:_MAX_UPSTREAM_ERROR_CHARS], + ) + return f"upstream registration failed with HTTP {response.status_code}" async def register_client_with_server( diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py index fd5e4b7d62b..d494e1c6cb3 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py @@ -4274,6 +4274,9 @@ async def test_register_bridge_relay_surfaces_upstream_error_not_500(): error_response = MagicMock() error_response.status_code = 400 error_response.text = '{"error":"invalid_redirect_uri","error_description":"redirect_uri not allowed"}' + error_response.json = MagicMock( + return_value={"error": "invalid_redirect_uri", "error_description": "redirect_uri not allowed"} + ) error_response.raise_for_status = MagicMock( side_effect=httpx.HTTPStatusError("bad", request=MagicMock(), response=error_response) ) @@ -4320,6 +4323,7 @@ async def test_register_non_bridge_upstream_error_relays_status_not_500(): error_response = MagicMock() error_response.status_code = 400 error_response.text = '{"error":"invalid_client_metadata"}' + error_response.json = MagicMock(return_value={"error": "invalid_client_metadata"}) error_response.raise_for_status = MagicMock( side_effect=httpx.HTTPStatusError("bad", request=MagicMock(), response=error_response) ) @@ -6205,3 +6209,65 @@ async def test_register_relays_rejection_when_http_client_raises(): assert exc.value.status_code == 400 assert "invalid_client_metadata" in str(exc.value.detail) + + +@pytest.mark.asyncio +async def test_token_exchange_never_relays_out_of_contract_body_to_client(): + """These endpoints serve unauthenticated OAuth clients, so a non-RFC6749 upstream body (HTML + error page, proxy banner, stack trace) must stay in server logs; the client sees only the + upstream status.""" + response = await _exchange_with_upstream_response( + _upstream_token_response(404, text_body="Error 404 stack trace: secret internals") + ) + + assert response.status_code == 502 + body = json.loads(response.body) + assert body == {"error": "server_error", "error_description": "upstream token endpoint returned HTTP 404"} + + +@pytest.mark.asyncio +async def test_register_never_relays_out_of_contract_body_to_client(): + """Same trust boundary for DCR: a non-RFC7591 rejection body is logged server-side and the + client detail names only the status.""" + import httpx + + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + register_client_with_server, + ) + + rejection = httpx.Response( + 500, + text="Tomcat stack trace with internals", + request=httpx.Request("POST", "https://idp.example.com/register"), + ) + raising_client = MagicMock() + raising_client.post = AsyncMock( + side_effect=httpx.HTTPStatusError("Server error '500'", request=rejection.request, response=rejection) + ) + + oauth2_server = _bridge_server(auth_type=MCPAuth.oauth2, dcr_bridge=None) + + with ( + patch( + "litellm.proxy._experimental.mcp_server.discoverable_endpoints.get_async_httpx_client", + return_value=raising_client, + ), + patch( + "litellm.proxy._experimental.mcp_server.discoverable_endpoints._reuse_persisted_dcr_client_if_available", + new_callable=AsyncMock, + return_value=False, + ), + ): + with pytest.raises(HTTPException) as exc: + await register_client_with_server( + request=_bridge_mock_request(), + mcp_server=oauth2_server, + client_name="Claude", + grant_types=None, + response_types=None, + token_endpoint_auth_method=None, + ) + + assert exc.value.status_code == 500 + assert str(exc.value.detail) == "upstream registration failed with HTTP 500" + assert "Tomcat" not in str(exc.value.detail) From 9e94f2be14b410988c2d0a59268a014ff9c21b2d Mon Sep 17 00:00:00 2001 From: Tin Chi Lo Date: Mon, 13 Jul 2026 16:11:12 -0700 Subject: [PATCH 4/6] refactor(mcp): classify upstream OAuth faults once and derive status, code, and prose from the value Replaces the accreted relay helpers with a faults package (types, classify, render_oauth): every upstream token/DCR rejection is classified into exactly one fault value and the response status, wire error code, and prose are all derived from that value, so a caller-fault code can never ship on a server-fault status (the bugbot finding on invalid_grant over a 500). Classification takes the credential source into account: invalid_client and friends against the server's stored credentials are the operator's fault and render as 502 server_error with gateway-authored prose while the IdP's prose stays in server logs; the same codes against caller-supplied credentials relay on the status the code implies. Classifiers are total, so an unreadable rejection body (lying content-encoding, unconsumed stream) yields the same 502 fault instead of resurrecting the opaque 500 (the second bugbot finding); DCR rejections normalize to 400 per RFC 7591 regardless of the upstream's status --- .../mcp_server/discoverable_endpoints.py | 109 ++++-------------- .../mcp_server/faults/__init__.py | 36 ++++++ .../mcp_server/faults/classify.py | 107 +++++++++++++++++ .../mcp_server/faults/render_oauth.py | 74 ++++++++++++ .../_experimental/mcp_server/faults/types.py | 60 ++++++++++ .../mcp_server/faults/test_classify.py | 108 +++++++++++++++++ .../mcp_server/faults/test_render_oauth.py | 75 ++++++++++++ .../mcp_server/test_discoverable_endpoints.py | 80 +++++++++++-- 8 files changed, 553 insertions(+), 96 deletions(-) create mode 100644 litellm/proxy/_experimental/mcp_server/faults/__init__.py create mode 100644 litellm/proxy/_experimental/mcp_server/faults/classify.py create mode 100644 litellm/proxy/_experimental/mcp_server/faults/render_oauth.py create mode 100644 litellm/proxy/_experimental/mcp_server/faults/types.py create mode 100644 tests/test_litellm/proxy/_experimental/mcp_server/faults/test_classify.py create mode 100644 tests/test_litellm/proxy/_experimental/mcp_server/faults/test_render_oauth.py diff --git a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py index 42677229add..6435db37645 100644 --- a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py @@ -24,6 +24,14 @@ from litellm.proxy._experimental.mcp_server.auth.token_endpoint_auth import ( TokenEndpointAuthConfigError, build_token_endpoint_client_auth, ) +from litellm.proxy._experimental.mcp_server.faults import ( + CredentialSource, + UpstreamProtocolFault, + classify_upstream_dcr_rejection, + classify_upstream_token_rejection, + dcr_fault_detail, + render_token_fault, +) from litellm.proxy._experimental.mcp_server.oauth_utils import ( TOKEN_NO_CACHE_HEADERS, get_request_base_url, @@ -980,55 +988,11 @@ def _finish_bridge_mint( return JSONResponse(body, headers=TOKEN_NO_CACHE_HEADERS) -def _response_json_or_none(response: httpx.Response) -> object: - try: - return response.json() - except ValueError: - return None - - -def _upstream_token_fault_response(description: str) -> JSONResponse: - """502 token-endpoint response for an upstream fault, in the RFC 6749 §5.2 shape the gateway's - token callers parse, with the §5.1 no-store headers.""" - return JSONResponse( - status_code=502, - content={"error": "server_error", "error_description": description}, - headers=TOKEN_NO_CACHE_HEADERS, - ) - - -def _upstream_token_error_response(response: httpx.Response) -> JSONResponse: - """Relay an upstream token-endpoint rejection to the client instead of letting the raw - ``httpx.HTTPStatusError`` escape to the global handler as an opaque 500 (the IdP's own - ``error_description``, e.g. Google's "client_secret is missing.", is the message the caller needs). - Only the RFC 6749 §5.2 fields (``error`` / ``error_description`` / ``error_uri``) are relayed, - each bounded, on the upstream's own 400/401 status; a rejection outside the §5.2 contract - (no JSON ``error`` field, or a status §5.2 does not define) maps to 502 so a broken upstream - is not misattributed to the caller's request. Out-of-contract bodies (HTML error pages, proxy - banners, stack traces) never cross the trust boundary: these endpoints serve unauthenticated - OAuth clients, so the body is logged server-side and the client sees only the upstream status.""" - parsed = _response_json_or_none(response) - error_code = parsed.get("error") if isinstance(parsed, dict) else None - if not isinstance(error_code, str) or not error_code: - verbose_logger.warning( - "MCP upstream token endpoint returned HTTP %s with a non-RFC6749 body (first %s chars): %s", - response.status_code, - _MAX_UPSTREAM_ERROR_CHARS, - (response.text or "")[:_MAX_UPSTREAM_ERROR_CHARS], - ) - return _upstream_token_fault_response(f"upstream token endpoint returned HTTP {response.status_code}") - fields = parsed if isinstance(parsed, dict) else {} - relayed = { - key: value[:_MAX_UPSTREAM_ERROR_CHARS] - for key, value in ( - ("error", error_code), - ("error_description", fields.get("error_description")), - ("error_uri", fields.get("error_uri")), - ) - if isinstance(value, str) and value - } - status_code = response.status_code if response.status_code in (400, 401) else 502 - return JSONResponse(status_code=status_code, content=relayed, headers=TOKEN_NO_CACHE_HEADERS) +def _token_credential_source(mcp_server: MCPServer) -> CredentialSource: + """Mirrors the resolved-client rule in :func:`exchange_token_with_server`: when the server has a + stored client_id the gateway presents its own credentials upstream, so a credential rejection is + the operator's fault, not the caller's.""" + return "gateway_stored" if mcp_server.client_id else "caller_supplied" async def exchange_token_with_server( @@ -1124,14 +1088,13 @@ async def exchange_token_with_server( if response is not None: response.raise_for_status() except httpx.HTTPStatusError as exc: - if "invalid_target" in exc.response.text: - verbose_logger.warning( - "MCP server %s: the upstream authorization server rejected the token request with " - "invalid_target; it may require RFC 8707 resource indicators, which the gateway " - "does not send yet (tracked as LIT-4339)", - mcp_server.server_id, + return render_token_fault( + classify_upstream_token_rejection( + exc.response, + credential_source=_token_credential_source(mcp_server), + log_context=mcp_server.server_id, ) - return _upstream_token_error_response(exc.response) + ) if response is None: raise HTTPException( status_code=502, @@ -1188,7 +1151,7 @@ async def exchange_token_with_server( raw_access_token = token_response.get("access_token") if isinstance(token_response, dict) else None if not isinstance(raw_access_token, str) or not raw_access_token: - return _upstream_token_fault_response("the upstream token response has no usable access_token") + return render_token_fault(UpstreamProtocolFault(note="the upstream token response has no usable access_token")) result = { "access_token": raw_access_token, @@ -1447,31 +1410,6 @@ async def _persist_dcr_client_registration( return "failed" -_MAX_UPSTREAM_ERROR_CHARS = 500 - - -def _safe_upstream_error_detail(response: httpx.Response) -> str: - """Client-safe summary of an upstream registration failure: only the RFC 7591 §3.2.2 fields - (``error`` / ``error_description``) cross the trust boundary, each bounded, since these endpoints - serve unauthenticated OAuth clients. A body outside that contract (HTML error pages, proxy - banners, stack traces) is logged server-side and summarized by status so upstream internals are - never relayed to callers.""" - parsed = _response_json_or_none(response) - error_code = parsed.get("error") if isinstance(parsed, dict) else None - if isinstance(error_code, str) and error_code: - description = parsed.get("error_description") if isinstance(parsed, dict) else None - if isinstance(description, str) and description: - return f"{error_code[:_MAX_UPSTREAM_ERROR_CHARS]}: {description[:_MAX_UPSTREAM_ERROR_CHARS]}" - return error_code[:_MAX_UPSTREAM_ERROR_CHARS] - verbose_logger.warning( - "MCP upstream registration endpoint returned HTTP %s with a non-RFC7591 body (first %s chars): %s", - response.status_code, - _MAX_UPSTREAM_ERROR_CHARS, - (response.text or "")[:_MAX_UPSTREAM_ERROR_CHARS], - ) - return f"upstream registration failed with HTTP {response.status_code}" - - async def register_client_with_server( request: Request, mcp_server: MCPServer, @@ -1540,9 +1478,10 @@ async def register_client_with_server( if response is not None: response.raise_for_status() except httpx.HTTPStatusError as exc: - raise HTTPException( - status_code=exc.response.status_code, detail=_safe_upstream_error_detail(exc.response) - ) from exc + status_code, detail = dcr_fault_detail( + classify_upstream_dcr_rejection(exc.response, log_context=mcp_server.server_id) + ) + raise HTTPException(status_code=status_code, detail=detail) from exc if response is None: raise HTTPException( status_code=502, diff --git a/litellm/proxy/_experimental/mcp_server/faults/__init__.py b/litellm/proxy/_experimental/mcp_server/faults/__init__.py new file mode 100644 index 00000000000..ccbba308012 --- /dev/null +++ b/litellm/proxy/_experimental/mcp_server/faults/__init__.py @@ -0,0 +1,36 @@ +"""Typed fault values for upstream OAuth/DCR failures (phase 1 of the MCP error-handling framework). + +The invariant this package exists to enforce: an upstream failure is classified ONCE into a single +fault value, and the response status, wire error code, and prose are all derived from that value. +Deriving all three from one classification makes contradictory pairings (a caller-fault error code on +a server-fault status) unrepresentable, and gives the trust-boundary rule one enforcement point: +spec-defined machine fields may cross to callers, upstream prose and raw bodies go to server logs. +""" + +from litellm.proxy._experimental.mcp_server.faults.classify import ( + classify_upstream_dcr_rejection, + classify_upstream_token_rejection, +) +from litellm.proxy._experimental.mcp_server.faults.render_oauth import ( + dcr_fault_detail, + render_token_fault, +) +from litellm.proxy._experimental.mcp_server.faults.types import ( + CallerRejected, + CredentialSource, + GatewayCredentialsRejected, + UpstreamOAuthFault, + UpstreamProtocolFault, +) + +__all__ = [ + "CallerRejected", + "CredentialSource", + "GatewayCredentialsRejected", + "UpstreamOAuthFault", + "UpstreamProtocolFault", + "classify_upstream_dcr_rejection", + "classify_upstream_token_rejection", + "dcr_fault_detail", + "render_token_fault", +] diff --git a/litellm/proxy/_experimental/mcp_server/faults/classify.py b/litellm/proxy/_experimental/mcp_server/faults/classify.py new file mode 100644 index 00000000000..a816b8e2a96 --- /dev/null +++ b/litellm/proxy/_experimental/mcp_server/faults/classify.py @@ -0,0 +1,107 @@ +"""The single place that reads upstream OAuth/DCR failure responses. + +Every accessor here is total: an upstream that lies about its content encoding, sends an undecodable +body, or omits the spec fields yields a classified fault, never an exception. Nothing outside this +module should touch a failed upstream response's body. +""" + +from __future__ import annotations + +import httpx + +from litellm._logging import verbose_logger +from litellm.proxy._experimental.mcp_server.faults.types import ( + GATEWAY_RESPONSIBILITY_CODES, + MAX_WIRE_FIELD_CHARS, + CallerRejected, + CredentialSource, + GatewayCredentialsRejected, + UpstreamOAuthFault, + UpstreamProtocolFault, +) + + +def _safe_text(response: httpx.Response) -> str: + try: + return response.text + except Exception: + return "" + + +def _safe_json(response: httpx.Response) -> object: + try: + return response.json() + except Exception: + return None + + +def _bounded_field(value: object) -> "str | None": + if not isinstance(value, str) or not value: + return None + return value[:MAX_WIRE_FIELD_CHARS] + + +def _log_out_of_contract(endpoint_kind: str, response: httpx.Response, log_context: str) -> None: + verbose_logger.warning( + "MCP upstream %s endpoint (%s) returned HTTP %s outside the OAuth error contract (first %s chars): %s", + endpoint_kind, + log_context, + response.status_code, + MAX_WIRE_FIELD_CHARS, + _safe_text(response)[:MAX_WIRE_FIELD_CHARS], + ) + + +def classify_upstream_token_rejection( + response: httpx.Response, + credential_source: CredentialSource, + log_context: str, +) -> UpstreamOAuthFault: + """Classify a token-endpoint rejection into exactly one fault. + + A body with an RFC 6749 §5.2 ``error`` field is a contract-conformant rejection: it indicts the + gateway when the code blames the client credentials the gateway itself presented + (``GATEWAY_RESPONSIBILITY_CODES`` with ``gateway_stored`` credentials), and the caller otherwise. + The upstream's HTTP status is deliberately not consulted for blame: status is derived from the + classification at render time, which is what keeps status and code from contradicting each other. + Anything without a usable ``error`` field is an upstream protocol fault.""" + parsed = _safe_json(response) + fields = parsed if isinstance(parsed, dict) else {} + code = _bounded_field(fields.get("error")) + if code is None: + _log_out_of_contract("token", response, log_context) + return UpstreamProtocolFault(note=f"upstream token endpoint returned HTTP {response.status_code}") + if code == "invalid_target": + verbose_logger.warning( + "MCP server %s: the upstream authorization server rejected the token request with " + "invalid_target; it may require RFC 8707 resource indicators, which the gateway " + "does not send yet (tracked as LIT-4339)", + log_context, + ) + if credential_source == "gateway_stored" and code in GATEWAY_RESPONSIBILITY_CODES: + verbose_logger.warning( + "MCP server %s: upstream authorization server rejected the gateway's configured client " + "credentials (%s): %s", + log_context, + code, + _bounded_field(fields.get("error_description")) or "", + ) + return GatewayCredentialsRejected(code=code) + return CallerRejected( + code=code, + description=_bounded_field(fields.get("error_description")), + error_uri=_bounded_field(fields.get("error_uri")), + ) + + +def classify_upstream_dcr_rejection(response: httpx.Response, log_context: str) -> UpstreamOAuthFault: + """Classify a dynamic-client-registration rejection. RFC 7591 §3.2.2 errors carry + ``error`` / ``error_description`` and are caller-actionable (the registration metadata was + rejected); anything else is an upstream protocol fault.""" + parsed = _safe_json(response) + fields = parsed if isinstance(parsed, dict) else {} + code = _bounded_field(fields.get("error")) + if code is None: + _log_out_of_contract("registration", response, log_context) + return UpstreamProtocolFault(note=f"upstream registration failed with HTTP {response.status_code}") + return CallerRejected(code=code, description=_bounded_field(fields.get("error_description"))) diff --git a/litellm/proxy/_experimental/mcp_server/faults/render_oauth.py b/litellm/proxy/_experimental/mcp_server/faults/render_oauth.py new file mode 100644 index 00000000000..5fc82819dc0 --- /dev/null +++ b/litellm/proxy/_experimental/mcp_server/faults/render_oauth.py @@ -0,0 +1,74 @@ +"""Render upstream OAuth/DCR faults onto the wire. The only place that chooses statuses and bodies +for these faults, so every consumer emits the same contract: RFC 6749 §5.2-shaped JSON with the §5.1 +no-store headers on token endpoints, HTTPException details on registration. Status, code, and prose +all derive from the fault tag; exhaustive matches keep a new fault arm from shipping unrendered. +""" + +from __future__ import annotations + +from fastapi.responses import JSONResponse +from typing_extensions import assert_never + +from litellm.proxy._experimental.mcp_server.faults.types import UpstreamOAuthFault +from litellm.proxy._experimental.mcp_server.oauth_utils import TOKEN_NO_CACHE_HEADERS + + +def _gateway_credentials_description(code: str) -> str: + if code == "invalid_target": + return ( + "the upstream authorization server rejected the token request (invalid_target); " + "it may require RFC 8707 resource indicators, which the gateway does not send yet" + ) + return ( + f"the upstream authorization server rejected the gateway's configured client credentials " + f"({code}); verify the MCP server's client_id and client_secret" + ) + + +def render_token_fault(fault: UpstreamOAuthFault) -> JSONResponse: + """RFC 6749 §5.2 response for a token-endpoint fault. Caller-actionable rejections relay the + upstream's code on the status that code implies (401 for invalid_client per §5.2, else 400); + gateway-side faults are 502 ``server_error`` with gateway-authored prose so a caller is never + blamed for, or shown the internals of, a failure only the operator can fix.""" + match fault.tag: + case "caller_rejected": + content = { + "error": fault.code, + **({"error_description": fault.description} if fault.description else {}), + **({"error_uri": fault.error_uri} if fault.error_uri else {}), + } + status_code = 401 if fault.code == "invalid_client" else 400 + return JSONResponse(status_code=status_code, content=content, headers=TOKEN_NO_CACHE_HEADERS) + case "gateway_credentials_rejected": + return JSONResponse( + status_code=502, + content={ + "error": "server_error", + "error_description": _gateway_credentials_description(fault.code), + }, + headers=TOKEN_NO_CACHE_HEADERS, + ) + case "upstream_protocol_fault": + return JSONResponse( + status_code=502, + content={"error": "server_error", "error_description": fault.note}, + headers=TOKEN_NO_CACHE_HEADERS, + ) + case _: + assert_never(fault.tag) + + +def dcr_fault_detail(fault: UpstreamOAuthFault) -> "tuple[int, str]": + """Status and detail string for a registration fault, raised as HTTPException by the caller. + RFC 7591 §3.2.2 defines registration errors as 400, so a contract-conformant rejection is 400 + regardless of the status the upstream chose; everything else is a 502 upstream fault.""" + match fault.tag: + case "caller_rejected": + detail = f"{fault.code}: {fault.description}" if fault.description else fault.code + return 400, detail + case "gateway_credentials_rejected": + return 502, _gateway_credentials_description(fault.code) + case "upstream_protocol_fault": + return 502, fault.note + case _: + assert_never(fault.tag) diff --git a/litellm/proxy/_experimental/mcp_server/faults/types.py b/litellm/proxy/_experimental/mcp_server/faults/types.py new file mode 100644 index 00000000000..2a200323024 --- /dev/null +++ b/litellm/proxy/_experimental/mcp_server/faults/types.py @@ -0,0 +1,60 @@ +"""Fault taxonomy for upstream OAuth token and DCR registration failures. + +Each fault is a frozen model on a ``tag`` literal. The tag alone decides the HTTP status, the wire +error code, and whose prose the caller sees, so those three facts can never disagree the way they can +when an upstream's status and error code are relayed independently. +""" + +from __future__ import annotations + +from typing import Literal, Optional, TypeAlias + +from pydantic import BaseModel, ConfigDict + +MAX_WIRE_FIELD_CHARS = 500 +"""Bound on every upstream-derived string that crosses to a caller or into a log line.""" + +CredentialSource: TypeAlias = Literal["gateway_stored", "caller_supplied"] +"""Whose client credentials the gateway presented upstream: the MCP server's stored configuration or +credentials the caller supplied on the request. Decides whether a credential rejection is the +caller's problem to fix or the gateway operator's.""" + +GATEWAY_RESPONSIBILITY_CODES: frozenset[str] = frozenset({"invalid_client", "unauthorized_client", "invalid_target"}) +"""RFC 6749 error codes that indict the OAuth client itself (its credentials, its grant +authorization, or a gateway capability such as RFC 8707 resource indicators). When the gateway +presented its own stored credentials, these are gateway-side faults the caller cannot act on.""" + + +class CallerRejected(BaseModel): + """The upstream spoke the OAuth error contract and the failure is actionable by our caller + (e.g. ``invalid_grant``: re-run authorization). The code and its bounded prose relay on the + 4xx status the code itself implies.""" + + model_config = ConfigDict(frozen=True) + tag: Literal["caller_rejected"] = "caller_rejected" + code: str + description: Optional[str] = None + error_uri: Optional[str] = None + + +class GatewayCredentialsRejected(BaseModel): + """The upstream rejected the gateway's own stored client credentials or a gateway capability. + Not actionable by the caller: rendered as 502 with gateway-authored prose naming the code; + the upstream's prose goes to server logs only.""" + + model_config = ConfigDict(frozen=True) + tag: Literal["gateway_credentials_rejected"] = "gateway_credentials_rejected" + code: str + + +class UpstreamProtocolFault(BaseModel): + """The upstream broke the error contract: no JSON ``error`` field, an undecodable body, or a + success response without a usable token. Rendered as 502 with a gateway-authored note; the + upstream body never crosses to the caller.""" + + model_config = ConfigDict(frozen=True) + tag: Literal["upstream_protocol_fault"] = "upstream_protocol_fault" + note: str + + +UpstreamOAuthFault: TypeAlias = CallerRejected | GatewayCredentialsRejected | UpstreamProtocolFault diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/faults/test_classify.py b/tests/test_litellm/proxy/_experimental/mcp_server/faults/test_classify.py new file mode 100644 index 00000000000..32148e7a247 --- /dev/null +++ b/tests/test_litellm/proxy/_experimental/mcp_server/faults/test_classify.py @@ -0,0 +1,108 @@ +"""Classification matrix for upstream OAuth/DCR rejections: who is blamed depends only on the §5.2 +code and whose credentials the gateway presented, never on the upstream's HTTP status.""" + +import httpx + +from litellm.proxy._experimental.mcp_server.faults.classify import ( + classify_upstream_dcr_rejection, + classify_upstream_token_rejection, +) +from litellm.proxy._experimental.mcp_server.faults.types import ( + CallerRejected, + GatewayCredentialsRejected, + UpstreamProtocolFault, +) + + +def _response(status_code: int, *, json_body: object = None, text_body: str = "", headers: dict = None) -> httpx.Response: + request = httpx.Request("POST", "https://idp.example.com/token") + if json_body is not None: + return httpx.Response(status_code, json=json_body, request=request) + return httpx.Response(status_code, text=text_body, headers=headers or {}, request=request) + + +def test_caller_fault_code_classifies_as_caller_rejected_regardless_of_status(): + fault = classify_upstream_token_rejection( + _response(500, json_body={"error": "invalid_grant", "error_description": "Code expired."}), + credential_source="gateway_stored", + log_context="srv", + ) + assert isinstance(fault, CallerRejected) + assert fault.code == "invalid_grant" + assert fault.description == "Code expired." + + +def test_credential_code_with_gateway_stored_credentials_indicts_gateway(): + fault = classify_upstream_token_rejection( + _response(401, json_body={"error": "invalid_client", "error_description": "not found"}), + credential_source="gateway_stored", + log_context="srv", + ) + assert isinstance(fault, GatewayCredentialsRejected) + assert fault.code == "invalid_client" + + +def test_credential_code_with_caller_supplied_credentials_stays_caller_fault(): + fault = classify_upstream_token_rejection( + _response(401, json_body={"error": "invalid_client"}), + credential_source="caller_supplied", + log_context="srv", + ) + assert isinstance(fault, CallerRejected) + assert fault.code == "invalid_client" + + +def test_unknown_code_relays_as_caller_rejected(): + fault = classify_upstream_token_rejection( + _response(400, json_body={"error": "slow_down", "error_description": "Polling too fast."}), + credential_source="gateway_stored", + log_context="srv", + ) + assert isinstance(fault, CallerRejected) + assert fault.code == "slow_down" + + +def test_body_without_error_field_is_protocol_fault(): + fault = classify_upstream_token_rejection( + _response(404, text_body="not here"), + credential_source="gateway_stored", + log_context="srv", + ) + assert isinstance(fault, UpstreamProtocolFault) + assert fault.note == "upstream token endpoint returned HTTP 404" + + +def test_unreadable_body_is_protocol_fault_not_exception(): + unreadable = httpx.Response( + 400, + stream=httpx.ByteStream(b"\x1f\x8bnot-gzip"), + headers={"content-encoding": "gzip"}, + request=httpx.Request("POST", "https://idp.example.com/token"), + ) + fault = classify_upstream_token_rejection(unreadable, credential_source="gateway_stored", log_context="srv") + assert isinstance(fault, UpstreamProtocolFault) + + +def test_wire_fields_are_bounded(): + fault = classify_upstream_token_rejection( + _response(400, json_body={"error": "invalid_request", "error_description": "x" * 5000}), + credential_source="gateway_stored", + log_context="srv", + ) + assert isinstance(fault, CallerRejected) + assert len(fault.description) == 500 + + +def test_dcr_rejection_with_rfc7591_code_is_caller_rejected(): + fault = classify_upstream_dcr_rejection( + _response(400, json_body={"error": "invalid_redirect_uri", "error_description": "not allowed"}), + log_context="srv", + ) + assert isinstance(fault, CallerRejected) + assert fault.code == "invalid_redirect_uri" + + +def test_dcr_rejection_without_code_is_protocol_fault(): + fault = classify_upstream_dcr_rejection(_response(500, text_body="trace"), log_context="srv") + assert isinstance(fault, UpstreamProtocolFault) + assert fault.note == "upstream registration failed with HTTP 500" diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/faults/test_render_oauth.py b/tests/test_litellm/proxy/_experimental/mcp_server/faults/test_render_oauth.py new file mode 100644 index 00000000000..a025cbe2b01 --- /dev/null +++ b/tests/test_litellm/proxy/_experimental/mcp_server/faults/test_render_oauth.py @@ -0,0 +1,75 @@ +"""Rendering contract: status, wire code, and prose all derive from the fault tag, so a caller-fault +code can never ship on a server-fault status and gateway-side faults never carry provider prose.""" + +import json + +from litellm.proxy._experimental.mcp_server.faults.render_oauth import ( + dcr_fault_detail, + render_token_fault, +) +from litellm.proxy._experimental.mcp_server.faults.types import ( + CallerRejected, + GatewayCredentialsRejected, + UpstreamProtocolFault, +) + + +def test_caller_rejected_renders_code_derived_status(): + response = render_token_fault(CallerRejected(code="invalid_grant", description="Code expired.")) + assert response.status_code == 400 + assert json.loads(response.body) == {"error": "invalid_grant", "error_description": "Code expired."} + assert response.headers["cache-control"] == "no-store" + + +def test_caller_rejected_invalid_client_renders_401(): + response = render_token_fault(CallerRejected(code="invalid_client")) + assert response.status_code == 401 + assert json.loads(response.body) == {"error": "invalid_client"} + + +def test_caller_rejected_includes_error_uri_only_when_present(): + response = render_token_fault( + CallerRejected(code="invalid_scope", description="bad scope", error_uri="https://idp.example.com/e") + ) + assert json.loads(response.body) == { + "error": "invalid_scope", + "error_description": "bad scope", + "error_uri": "https://idp.example.com/e", + } + + +def test_gateway_credentials_rejected_renders_502_with_gateway_prose(): + response = render_token_fault(GatewayCredentialsRejected(code="invalid_client")) + assert response.status_code == 502 + body = json.loads(response.body) + assert body["error"] == "server_error" + assert "invalid_client" in body["error_description"] + assert "client_id and client_secret" in body["error_description"] + + +def test_gateway_invalid_target_prose_names_resource_indicators(): + response = render_token_fault(GatewayCredentialsRejected(code="invalid_target")) + body = json.loads(response.body) + assert response.status_code == 502 + assert "RFC 8707" in body["error_description"] + + +def test_protocol_fault_renders_502_note(): + response = render_token_fault(UpstreamProtocolFault(note="upstream token endpoint returned HTTP 503")) + assert response.status_code == 502 + assert json.loads(response.body) == { + "error": "server_error", + "error_description": "upstream token endpoint returned HTTP 503", + } + + +def test_dcr_caller_rejection_is_400_per_rfc7591_regardless_of_upstream_status(): + status_code, detail = dcr_fault_detail(CallerRejected(code="invalid_client_metadata", description="bad grant types")) + assert status_code == 400 + assert detail == "invalid_client_metadata: bad grant types" + + +def test_dcr_protocol_fault_is_502(): + status_code, detail = dcr_fault_detail(UpstreamProtocolFault(note="upstream registration failed with HTTP 500")) + assert status_code == 502 + assert detail == "upstream registration failed with HTTP 500" diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py index d494e1c6cb3..a67579c79ca 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py @@ -5979,9 +5979,10 @@ def _upstream_token_response(status_code: int, *, json_body: object = None, text return httpx.Response(status_code, text=text_body, request=request) -async def _exchange_with_upstream_response(upstream_response): +async def _exchange_with_upstream_response(upstream_response, *, server_client_id="web-client.apps.googleusercontent.com"): """Run the raw (non-bridge) authorization_code exchange against a canned upstream token-endpoint - response and return what the gateway would hand the client.""" + response and return what the gateway would hand the client. ``server_client_id=None`` models the + caller-supplied-credentials flow (no stored client on the server).""" from fastapi import Request from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( @@ -5998,7 +5999,7 @@ async def _exchange_with_upstream_response(upstream_response): alias="gcal", transport=MCPTransport.http, auth_type=MCPAuth.oauth2, - client_id="web-client.apps.googleusercontent.com", + client_id=server_client_id, authorization_url="https://accounts.google.com/o/oauth2/v2/auth", token_url="https://oauth2.googleapis.com/token", ) @@ -6025,20 +6026,55 @@ async def _exchange_with_upstream_response(upstream_response): @pytest.mark.asyncio -async def test_token_exchange_relays_upstream_rfc6749_rejection(): - """The IdP's own RFC 6749 §5.2 rejection is the message the caller needs (e.g. Google refusing a - secret-less web client); before the relay the raw HTTPStatusError escaped to the global handler - and every upstream rejection surfaced as an opaque 500 Internal server error.""" +async def test_token_exchange_gateway_credential_rejection_is_502_with_gateway_prose(): + """When the gateway presented the server's stored client credentials and the IdP rejected them + (Google refusing a secret-less or unknown client), the fault is the operator's, not the caller's: + 502 server_error with gateway-authored prose naming the code, and the IdP's own prose stays in + server logs. Before the framework this either 500ed raw or relayed provider prose verbatim.""" response = await _exchange_with_upstream_response( _upstream_token_response( 401, json_body={"error": "invalid_client", "error_description": "The OAuth client was not found."} ) ) + assert response.status_code == 502 + body = json.loads(response.body) + assert body["error"] == "server_error" + assert "invalid_client" in body["error_description"] + assert "client_id and client_secret" in body["error_description"] + assert "The OAuth client was not found." not in body["error_description"] + assert response.headers["cache-control"] == "no-store" + + +@pytest.mark.asyncio +async def test_token_exchange_caller_supplied_credential_rejection_relays_code(): + """When the caller supplied the client credentials themselves (no stored client on the server), + an invalid_client rejection is theirs to act on: the §5.2 code relays on the 401 that code + implies.""" + response = await _exchange_with_upstream_response( + _upstream_token_response( + 401, json_body={"error": "invalid_client", "error_description": "The OAuth client was not found."} + ), + server_client_id=None, + ) + assert response.status_code == 401 body = json.loads(response.body) assert body == {"error": "invalid_client", "error_description": "The OAuth client was not found."} - assert response.headers["cache-control"] == "no-store" + + +@pytest.mark.asyncio +async def test_token_exchange_status_derives_from_error_code_not_upstream_status(): + """An upstream that pairs a caller-fault code with a server-fault status (invalid_grant on a 500) + must not produce a contradictory response: status derives from the classified fault, so the + caller sees 400 invalid_grant and knows to re-authorize rather than blaming the gateway.""" + response = await _exchange_with_upstream_response( + _upstream_token_response(500, json_body={"error": "invalid_grant", "error_description": "Code expired."}) + ) + + assert response.status_code == 400 + body = json.loads(response.body) + assert body == {"error": "invalid_grant", "error_description": "Code expired."} @pytest.mark.asyncio @@ -6159,9 +6195,10 @@ async def test_token_exchange_relays_rejection_when_http_client_raises(): code_verifier="verifier", ) - assert response.status_code == 401 + assert response.status_code == 502 body = json.loads(response.body) - assert body["error"] == "invalid_client" + assert body["error"] == "server_error" + assert "invalid_client" in body["error_description"] @pytest.mark.asyncio @@ -6268,6 +6305,27 @@ async def test_register_never_relays_out_of_contract_body_to_client(): token_endpoint_auth_method=None, ) - assert exc.value.status_code == 500 + assert exc.value.status_code == 502 assert str(exc.value.detail) == "upstream registration failed with HTTP 500" assert "Tomcat" not in str(exc.value.detail) + + +@pytest.mark.asyncio +async def test_token_exchange_unreadable_body_still_renders_oauth_fault(): + """An upstream whose failure body cannot be read (unconsumed stream, lying content-encoding) + makes response.text/.json raise; the classifier must stay total so the caller still gets the + §5.2-shaped 502 instead of the opaque 500 this change set out to remove.""" + import httpx + + unreadable = httpx.Response( + 400, + stream=httpx.ByteStream(b"\x1f\x8bnot-actually-gzip"), + headers={"content-encoding": "gzip"}, + request=httpx.Request("POST", "https://oauth2.googleapis.com/token"), + ) + + response = await _exchange_with_upstream_response(unreadable) + + assert response.status_code == 502 + body = json.loads(response.body) + assert body == {"error": "server_error", "error_description": "upstream token endpoint returned HTTP 400"} From e8090028f3718373a09e7ecfd9087e59523c5b0d Mon Sep 17 00:00:00 2001 From: Tin Chi Lo Date: Mon, 13 Jul 2026 16:14:54 -0700 Subject: [PATCH 5/6] style(mcp): unquote annotations and use PEP 604 unions in the faults package --- litellm/proxy/_experimental/mcp_server/faults/classify.py | 2 +- .../proxy/_experimental/mcp_server/faults/render_oauth.py | 2 +- litellm/proxy/_experimental/mcp_server/faults/types.py | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/faults/classify.py b/litellm/proxy/_experimental/mcp_server/faults/classify.py index a816b8e2a96..cf4d022d33b 100644 --- a/litellm/proxy/_experimental/mcp_server/faults/classify.py +++ b/litellm/proxy/_experimental/mcp_server/faults/classify.py @@ -35,7 +35,7 @@ def _safe_json(response: httpx.Response) -> object: return None -def _bounded_field(value: object) -> "str | None": +def _bounded_field(value: object) -> str | None: if not isinstance(value, str) or not value: return None return value[:MAX_WIRE_FIELD_CHARS] diff --git a/litellm/proxy/_experimental/mcp_server/faults/render_oauth.py b/litellm/proxy/_experimental/mcp_server/faults/render_oauth.py index 5fc82819dc0..4856e6fbe52 100644 --- a/litellm/proxy/_experimental/mcp_server/faults/render_oauth.py +++ b/litellm/proxy/_experimental/mcp_server/faults/render_oauth.py @@ -58,7 +58,7 @@ def render_token_fault(fault: UpstreamOAuthFault) -> JSONResponse: assert_never(fault.tag) -def dcr_fault_detail(fault: UpstreamOAuthFault) -> "tuple[int, str]": +def dcr_fault_detail(fault: UpstreamOAuthFault) -> tuple[int, str]: """Status and detail string for a registration fault, raised as HTTPException by the caller. RFC 7591 §3.2.2 defines registration errors as 400, so a contract-conformant rejection is 400 regardless of the status the upstream chose; everything else is a 502 upstream fault.""" diff --git a/litellm/proxy/_experimental/mcp_server/faults/types.py b/litellm/proxy/_experimental/mcp_server/faults/types.py index 2a200323024..8cc291459a0 100644 --- a/litellm/proxy/_experimental/mcp_server/faults/types.py +++ b/litellm/proxy/_experimental/mcp_server/faults/types.py @@ -7,7 +7,7 @@ when an upstream's status and error code are relayed independently. from __future__ import annotations -from typing import Literal, Optional, TypeAlias +from typing import Literal, TypeAlias from pydantic import BaseModel, ConfigDict @@ -33,8 +33,8 @@ class CallerRejected(BaseModel): model_config = ConfigDict(frozen=True) tag: Literal["caller_rejected"] = "caller_rejected" code: str - description: Optional[str] = None - error_uri: Optional[str] = None + description: str | None = None + error_uri: str | None = None class GatewayCredentialsRejected(BaseModel): From 9335edeb85fb072e248b4a006d76a835b9e6954f Mon Sep 17 00:00:00 2001 From: Tin Chi Lo Date: Mon, 13 Jul 2026 16:37:49 -0700 Subject: [PATCH 6/6] fix(mcp): keep upstream self-blame codes and gateway capability gaps off the caller Extends the fault matrix per review: server_error and temporarily_unavailable are codes by which the upstream blames itself, so they classify as a new UpstreamReportedFault arm rendering 502/503 with a matching wire code instead of a 400 that blames the caller; invalid_target is a gateway capability gap (RFC 8707 resource indicators, LIT-4339) and is gateway-blamed regardless of whose credentials were presented; the DCR classifier shares the same blame assignment. The gateway-fault arm is renamed GatewayRejected since it now covers capability gaps as well as stored-credential rejections --- .../mcp_server/faults/__init__.py | 6 +- .../mcp_server/faults/classify.py | 88 ++++++++++++------- .../mcp_server/faults/render_oauth.py | 27 ++++-- .../_experimental/mcp_server/faults/types.py | 39 +++++--- .../mcp_server/faults/test_classify.py | 43 ++++++++- .../mcp_server/faults/test_render_oauth.py | 29 +++++- 6 files changed, 177 insertions(+), 55 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/faults/__init__.py b/litellm/proxy/_experimental/mcp_server/faults/__init__.py index ccbba308012..da078f0e242 100644 --- a/litellm/proxy/_experimental/mcp_server/faults/__init__.py +++ b/litellm/proxy/_experimental/mcp_server/faults/__init__.py @@ -18,17 +18,19 @@ from litellm.proxy._experimental.mcp_server.faults.render_oauth import ( from litellm.proxy._experimental.mcp_server.faults.types import ( CallerRejected, CredentialSource, - GatewayCredentialsRejected, + GatewayRejected, UpstreamOAuthFault, UpstreamProtocolFault, + UpstreamReportedFault, ) __all__ = [ "CallerRejected", "CredentialSource", - "GatewayCredentialsRejected", + "GatewayRejected", "UpstreamOAuthFault", "UpstreamProtocolFault", + "UpstreamReportedFault", "classify_upstream_dcr_rejection", "classify_upstream_token_rejection", "dcr_fault_detail", diff --git a/litellm/proxy/_experimental/mcp_server/faults/classify.py b/litellm/proxy/_experimental/mcp_server/faults/classify.py index cf4d022d33b..8b3a09f8d8d 100644 --- a/litellm/proxy/_experimental/mcp_server/faults/classify.py +++ b/litellm/proxy/_experimental/mcp_server/faults/classify.py @@ -11,13 +11,15 @@ import httpx from litellm._logging import verbose_logger from litellm.proxy._experimental.mcp_server.faults.types import ( - GATEWAY_RESPONSIBILITY_CODES, + GATEWAY_CAPABILITY_CODES, + GATEWAY_CREDENTIAL_CODES, MAX_WIRE_FIELD_CHARS, CallerRejected, CredentialSource, - GatewayCredentialsRejected, + GatewayRejected, UpstreamOAuthFault, UpstreamProtocolFault, + UpstreamReportedFault, ) @@ -52,56 +54,80 @@ def _log_out_of_contract(endpoint_kind: str, response: httpx.Response, log_conte ) +def _classify_oauth_error_code( + code: str, + description: str | None, + error_uri: str | None, + credential_source: CredentialSource, + log_context: str, +) -> UpstreamOAuthFault: + """Blame assignment for a contract-conformant OAuth error code, shared by the token and DCR + classifiers. Codes by which the upstream blames itself keep that blame; ``invalid_target`` is a + gateway capability gap (RFC 8707 resource indicators, LIT-4339) no matter whose credentials were + presented; credential-indicting codes follow the credential source; everything else, including + codes we do not recognize, is the caller's to act on. The upstream's HTTP status is deliberately + never consulted: status derives from this classification at render time, which is what keeps + status and code from contradicting each other.""" + if code == "server_error" or code == "temporarily_unavailable": + return UpstreamReportedFault(code=code) + if code in GATEWAY_CAPABILITY_CODES: + verbose_logger.warning( + "MCP server %s: the upstream authorization server rejected the request with " + "invalid_target; it may require RFC 8707 resource indicators, which the gateway " + "does not send yet (tracked as LIT-4339)", + log_context, + ) + return GatewayRejected(code=code) + if credential_source == "gateway_stored" and code in GATEWAY_CREDENTIAL_CODES: + verbose_logger.warning( + "MCP server %s: upstream authorization server rejected the gateway's configured client " + "credentials (%s): %s", + log_context, + code, + description or "", + ) + return GatewayRejected(code=code) + return CallerRejected(code=code, description=description, error_uri=error_uri) + + def classify_upstream_token_rejection( response: httpx.Response, credential_source: CredentialSource, log_context: str, ) -> UpstreamOAuthFault: - """Classify a token-endpoint rejection into exactly one fault. - - A body with an RFC 6749 §5.2 ``error`` field is a contract-conformant rejection: it indicts the - gateway when the code blames the client credentials the gateway itself presented - (``GATEWAY_RESPONSIBILITY_CODES`` with ``gateway_stored`` credentials), and the caller otherwise. - The upstream's HTTP status is deliberately not consulted for blame: status is derived from the - classification at render time, which is what keeps status and code from contradicting each other. - Anything without a usable ``error`` field is an upstream protocol fault.""" + """Classify a token-endpoint rejection into exactly one fault: a body with an RFC 6749 §5.2 + ``error`` field goes through blame assignment (:func:`_classify_oauth_error_code`); anything + without a usable ``error`` field is an upstream protocol fault.""" parsed = _safe_json(response) fields = parsed if isinstance(parsed, dict) else {} code = _bounded_field(fields.get("error")) if code is None: _log_out_of_contract("token", response, log_context) return UpstreamProtocolFault(note=f"upstream token endpoint returned HTTP {response.status_code}") - if code == "invalid_target": - verbose_logger.warning( - "MCP server %s: the upstream authorization server rejected the token request with " - "invalid_target; it may require RFC 8707 resource indicators, which the gateway " - "does not send yet (tracked as LIT-4339)", - log_context, - ) - if credential_source == "gateway_stored" and code in GATEWAY_RESPONSIBILITY_CODES: - verbose_logger.warning( - "MCP server %s: upstream authorization server rejected the gateway's configured client " - "credentials (%s): %s", - log_context, - code, - _bounded_field(fields.get("error_description")) or "", - ) - return GatewayCredentialsRejected(code=code) - return CallerRejected( - code=code, + return _classify_oauth_error_code( + code, description=_bounded_field(fields.get("error_description")), error_uri=_bounded_field(fields.get("error_uri")), + credential_source=credential_source, + log_context=log_context, ) def classify_upstream_dcr_rejection(response: httpx.Response, log_context: str) -> UpstreamOAuthFault: """Classify a dynamic-client-registration rejection. RFC 7591 §3.2.2 errors carry - ``error`` / ``error_description`` and are caller-actionable (the registration metadata was - rejected); anything else is an upstream protocol fault.""" + ``error`` / ``error_description`` and go through the same blame assignment as token errors + (registration sends no client credentials, so credential codes stay caller-actionable); anything + without a usable ``error`` field is an upstream protocol fault.""" parsed = _safe_json(response) fields = parsed if isinstance(parsed, dict) else {} code = _bounded_field(fields.get("error")) if code is None: _log_out_of_contract("registration", response, log_context) return UpstreamProtocolFault(note=f"upstream registration failed with HTTP {response.status_code}") - return CallerRejected(code=code, description=_bounded_field(fields.get("error_description"))) + return _classify_oauth_error_code( + code, + description=_bounded_field(fields.get("error_description")), + error_uri=None, + credential_source="caller_supplied", + log_context=log_context, + ) diff --git a/litellm/proxy/_experimental/mcp_server/faults/render_oauth.py b/litellm/proxy/_experimental/mcp_server/faults/render_oauth.py index 4856e6fbe52..89ce5011830 100644 --- a/litellm/proxy/_experimental/mcp_server/faults/render_oauth.py +++ b/litellm/proxy/_experimental/mcp_server/faults/render_oauth.py @@ -13,10 +13,10 @@ from litellm.proxy._experimental.mcp_server.faults.types import UpstreamOAuthFau from litellm.proxy._experimental.mcp_server.oauth_utils import TOKEN_NO_CACHE_HEADERS -def _gateway_credentials_description(code: str) -> str: +def _gateway_rejected_description(code: str) -> str: if code == "invalid_target": return ( - "the upstream authorization server rejected the token request (invalid_target); " + "the upstream authorization server rejected the request (invalid_target); " "it may require RFC 8707 resource indicators, which the gateway does not send yet" ) return ( @@ -25,6 +25,12 @@ def _gateway_credentials_description(code: str) -> str: ) +def _upstream_reported_status_and_description(code: str) -> tuple[int, str]: + if code == "temporarily_unavailable": + return 503, "the upstream authorization server is temporarily unavailable; retry shortly" + return 502, "the upstream authorization server reported an internal error" + + def render_token_fault(fault: UpstreamOAuthFault) -> JSONResponse: """RFC 6749 §5.2 response for a token-endpoint fault. Caller-actionable rejections relay the upstream's code on the status that code implies (401 for invalid_client per §5.2, else 400); @@ -39,15 +45,22 @@ def render_token_fault(fault: UpstreamOAuthFault) -> JSONResponse: } status_code = 401 if fault.code == "invalid_client" else 400 return JSONResponse(status_code=status_code, content=content, headers=TOKEN_NO_CACHE_HEADERS) - case "gateway_credentials_rejected": + case "gateway_rejected": return JSONResponse( status_code=502, content={ "error": "server_error", - "error_description": _gateway_credentials_description(fault.code), + "error_description": _gateway_rejected_description(fault.code), }, headers=TOKEN_NO_CACHE_HEADERS, ) + case "upstream_reported_fault": + status_code, description = _upstream_reported_status_and_description(fault.code) + return JSONResponse( + status_code=status_code, + content={"error": fault.code, "error_description": description}, + headers=TOKEN_NO_CACHE_HEADERS, + ) case "upstream_protocol_fault": return JSONResponse( status_code=502, @@ -66,8 +79,10 @@ def dcr_fault_detail(fault: UpstreamOAuthFault) -> tuple[int, str]: case "caller_rejected": detail = f"{fault.code}: {fault.description}" if fault.description else fault.code return 400, detail - case "gateway_credentials_rejected": - return 502, _gateway_credentials_description(fault.code) + case "gateway_rejected": + return 502, _gateway_rejected_description(fault.code) + case "upstream_reported_fault": + return _upstream_reported_status_and_description(fault.code) case "upstream_protocol_fault": return 502, fault.note case _: diff --git a/litellm/proxy/_experimental/mcp_server/faults/types.py b/litellm/proxy/_experimental/mcp_server/faults/types.py index 8cc291459a0..128b5e3e6cf 100644 --- a/litellm/proxy/_experimental/mcp_server/faults/types.py +++ b/litellm/proxy/_experimental/mcp_server/faults/types.py @@ -19,10 +19,19 @@ CredentialSource: TypeAlias = Literal["gateway_stored", "caller_supplied"] credentials the caller supplied on the request. Decides whether a credential rejection is the caller's problem to fix or the gateway operator's.""" -GATEWAY_RESPONSIBILITY_CODES: frozenset[str] = frozenset({"invalid_client", "unauthorized_client", "invalid_target"}) -"""RFC 6749 error codes that indict the OAuth client itself (its credentials, its grant -authorization, or a gateway capability such as RFC 8707 resource indicators). When the gateway -presented its own stored credentials, these are gateway-side faults the caller cannot act on.""" +GATEWAY_CREDENTIAL_CODES: frozenset[str] = frozenset({"invalid_client", "unauthorized_client"}) +"""RFC 6749 error codes that indict the OAuth client's credentials or grant authorization. When the +gateway presented its own stored credentials, these are gateway-side faults the caller cannot act on; +when the caller supplied the credentials, they are the caller's to fix.""" + +GATEWAY_CAPABILITY_CODES: frozenset[str] = frozenset({"invalid_target"}) +"""Codes that indict a gateway capability regardless of whose credentials were presented: +``invalid_target`` means the upstream wants RFC 8707 resource indicators, which the gateway does not +send yet (LIT-4339). Never the caller's fault.""" + +UPSTREAM_FAULT_CODES: frozenset[str] = frozenset({"server_error", "temporarily_unavailable"}) +"""Codes by which the upstream blames itself. Relaying them as caller faults would invert blame, so +they classify as upstream-reported faults and render on the 5xx their meaning implies.""" class CallerRejected(BaseModel): @@ -37,16 +46,26 @@ class CallerRejected(BaseModel): error_uri: str | None = None -class GatewayCredentialsRejected(BaseModel): - """The upstream rejected the gateway's own stored client credentials or a gateway capability. - Not actionable by the caller: rendered as 502 with gateway-authored prose naming the code; - the upstream's prose goes to server logs only.""" +class GatewayRejected(BaseModel): + """The upstream rejected the request for a cause only the gateway operator can address: the + server's stored client credentials or a gateway capability gap. Not actionable by the caller: + rendered as 502 with gateway-authored prose naming the code; the upstream's prose goes to + server logs only.""" model_config = ConfigDict(frozen=True) - tag: Literal["gateway_credentials_rejected"] = "gateway_credentials_rejected" + tag: Literal["gateway_rejected"] = "gateway_rejected" code: str +class UpstreamReportedFault(BaseModel): + """The upstream blamed itself in the OAuth vocabulary. Rendered on the 5xx the code implies + (``server_error`` 502, ``temporarily_unavailable`` 503) so blame and status agree.""" + + model_config = ConfigDict(frozen=True) + tag: Literal["upstream_reported_fault"] = "upstream_reported_fault" + code: Literal["server_error", "temporarily_unavailable"] + + class UpstreamProtocolFault(BaseModel): """The upstream broke the error contract: no JSON ``error`` field, an undecodable body, or a success response without a usable token. Rendered as 502 with a gateway-authored note; the @@ -57,4 +76,4 @@ class UpstreamProtocolFault(BaseModel): note: str -UpstreamOAuthFault: TypeAlias = CallerRejected | GatewayCredentialsRejected | UpstreamProtocolFault +UpstreamOAuthFault: TypeAlias = CallerRejected | GatewayRejected | UpstreamReportedFault | UpstreamProtocolFault diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/faults/test_classify.py b/tests/test_litellm/proxy/_experimental/mcp_server/faults/test_classify.py index 32148e7a247..dc20d664a53 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/faults/test_classify.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/faults/test_classify.py @@ -9,8 +9,9 @@ from litellm.proxy._experimental.mcp_server.faults.classify import ( ) from litellm.proxy._experimental.mcp_server.faults.types import ( CallerRejected, - GatewayCredentialsRejected, + GatewayRejected, UpstreamProtocolFault, + UpstreamReportedFault, ) @@ -38,7 +39,7 @@ def test_credential_code_with_gateway_stored_credentials_indicts_gateway(): credential_source="gateway_stored", log_context="srv", ) - assert isinstance(fault, GatewayCredentialsRejected) + assert isinstance(fault, GatewayRejected) assert fault.code == "invalid_client" @@ -106,3 +107,41 @@ def test_dcr_rejection_without_code_is_protocol_fault(): fault = classify_upstream_dcr_rejection(_response(500, text_body="trace"), log_context="srv") assert isinstance(fault, UpstreamProtocolFault) assert fault.note == "upstream registration failed with HTTP 500" + + +def test_upstream_self_blame_codes_stay_upstream_faults(): + fault = classify_upstream_token_rejection( + _response(400, json_body={"error": "server_error", "error_description": "boom"}), + credential_source="caller_supplied", + log_context="srv", + ) + assert isinstance(fault, UpstreamReportedFault) + assert fault.code == "server_error" + + +def test_temporarily_unavailable_is_upstream_fault(): + fault = classify_upstream_token_rejection( + _response(503, json_body={"error": "temporarily_unavailable"}), + credential_source="gateway_stored", + log_context="srv", + ) + assert isinstance(fault, UpstreamReportedFault) + assert fault.code == "temporarily_unavailable" + + +def test_invalid_target_is_gateway_fault_even_with_caller_credentials(): + fault = classify_upstream_token_rejection( + _response(400, json_body={"error": "invalid_target"}), + credential_source="caller_supplied", + log_context="srv", + ) + assert isinstance(fault, GatewayRejected) + assert fault.code == "invalid_target" + + +def test_dcr_server_error_code_is_not_blamed_on_caller(): + fault = classify_upstream_dcr_rejection( + _response(500, json_body={"error": "server_error"}), + log_context="srv", + ) + assert isinstance(fault, UpstreamReportedFault) diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/faults/test_render_oauth.py b/tests/test_litellm/proxy/_experimental/mcp_server/faults/test_render_oauth.py index a025cbe2b01..78513e315a7 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/faults/test_render_oauth.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/faults/test_render_oauth.py @@ -9,8 +9,9 @@ from litellm.proxy._experimental.mcp_server.faults.render_oauth import ( ) from litellm.proxy._experimental.mcp_server.faults.types import ( CallerRejected, - GatewayCredentialsRejected, + GatewayRejected, UpstreamProtocolFault, + UpstreamReportedFault, ) @@ -38,8 +39,8 @@ def test_caller_rejected_includes_error_uri_only_when_present(): } -def test_gateway_credentials_rejected_renders_502_with_gateway_prose(): - response = render_token_fault(GatewayCredentialsRejected(code="invalid_client")) +def test_gateway_rejected_renders_502_with_gateway_prose(): + response = render_token_fault(GatewayRejected(code="invalid_client")) assert response.status_code == 502 body = json.loads(response.body) assert body["error"] == "server_error" @@ -48,7 +49,7 @@ def test_gateway_credentials_rejected_renders_502_with_gateway_prose(): def test_gateway_invalid_target_prose_names_resource_indicators(): - response = render_token_fault(GatewayCredentialsRejected(code="invalid_target")) + response = render_token_fault(GatewayRejected(code="invalid_target")) body = json.loads(response.body) assert response.status_code == 502 assert "RFC 8707" in body["error_description"] @@ -73,3 +74,23 @@ def test_dcr_protocol_fault_is_502(): status_code, detail = dcr_fault_detail(UpstreamProtocolFault(note="upstream registration failed with HTTP 500")) assert status_code == 502 assert detail == "upstream registration failed with HTTP 500" + + +def test_upstream_reported_server_error_renders_502_with_matching_code(): + response = render_token_fault(UpstreamReportedFault(code="server_error")) + assert response.status_code == 502 + assert json.loads(response.body)["error"] == "server_error" + + +def test_upstream_reported_temporarily_unavailable_renders_503_with_matching_code(): + response = render_token_fault(UpstreamReportedFault(code="temporarily_unavailable")) + assert response.status_code == 503 + body = json.loads(response.body) + assert body["error"] == "temporarily_unavailable" + assert "retry" in body["error_description"] + + +def test_dcr_upstream_reported_fault_maps_to_5xx(): + status_code, detail = dcr_fault_detail(UpstreamReportedFault(code="server_error")) + assert status_code == 502 + assert "internal error" in detail