From 9335edeb85fb072e248b4a006d76a835b9e6954f Mon Sep 17 00:00:00 2001 From: Tin Chi Lo Date: Mon, 13 Jul 2026 16:37:49 -0700 Subject: [PATCH] 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