fix(mcp): harden the bridge token mint (multi-lens review pass)

Findings from a full adversarial review of the mint path across security,
correctness, error-handling, concurrency, and OAuth-protocol dimensions.

- expires_in coercion is now total: int(float(...)) can raise OverflowError on
  Infinity / a giant numeric string, which escaped the ValueError/TypeError catch
  and 500'd the token endpoint. Unified to catch OverflowError too.
- Resolve the litellm identity BEFORE exchanging the single-use upstream code, so
  a missing or transiently-unresolvable identity fails closed with invalid_request
  without burning the code (the mint re-resolves via a cache hit).
- The no-identity failure is now an RFC 6749 5.2-shaped invalid_request
  (JSONResponse, top-level error, no-store) instead of a detail-wrapped
  HTTPException, matching the BYOK OAuth endpoint.
- EnvelopeTooLarge (upstream token too big to seal) surfaces a 502, not a 500.
- The upstream refresh_token is no longer sealed into the envelope: the edge
  never consumes it, so it was dead weight embedding a long-lived upstream
  credential in the client bearer and enlarging the envelope; refresh is a
  follow-up (a dedicated refresh-envelope).

Security review found no exploitable defect (forgery, cross-server/user replay,
leakage, confused-deputy all closed). Regression tests cover the OverflowError,
the code-not-burned path, the RFC-shaped error, the 502, and the dropped refresh.

litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py

@@ -373,7 +373,7 @@ def _active_key_user_id(key_obj: "UserAPIKeyAuth") -> str | None:
     return key_obj.user_id if _key_is_active(key_obj) else None
 
 
-async def _resolve_active_litellm_key(request: Request) -> Tuple[str, "UserAPIKeyAuth"] | None:
+async def _resolve_active_litellm_key(request: Request) -> tuple[str, "UserAPIKeyAuth"] | None:
     """Resolve the presented litellm key to ``(its hash, the live active key record)``, or ``None``
     when the key is absent, unresolvable, or blocked/expired.
 
@@ -714,19 +714,17 @@ def _coerce_positive_expires_in(value: object) -> int | None:
     usable number. IdPs return it as an int, a float (``3600.0``), or a numeric string (``"3600"``);
     accepting only ``int`` would drop the float/string cases to ``None`` and fall back to the
     envelope's 1h cap, which can outlive a shorter-lived upstream token and forward a stale bearer.
-    ``bool`` is excluded (it is an ``int`` subclass but never a real lifetime)."""
-    if isinstance(value, bool):
+    ``bool`` is excluded (it is an ``int`` subclass but never a real lifetime). Total over hostile
+    input: a non-numeric string, ``NaN``, ``Infinity``, or an over-large value all resolve to
+    ``None`` rather than raising (``int(float(...))`` can raise ``ValueError`` or ``OverflowError``),
+    so a malformed upstream ``expires_in`` never surfaces as a 500 from the token endpoint."""
+    if isinstance(value, bool) or not isinstance(value, (int, float, str)):
         return None
-    if isinstance(value, (int, float)):
-        seconds = int(value)
-        return seconds if seconds > 0 else None
-    if isinstance(value, str):
-        try:
-            seconds = int(float(value.strip()))
-        except (ValueError, TypeError):
-            return None
-        return seconds if seconds > 0 else None
-    return None
+    try:
+        seconds = int(float(value))
+    except (ValueError, TypeError, OverflowError):
+        return None
+    return seconds if seconds > 0 else None
 
 
 def _bridge_grant_from_token_response(token_response: object) -> Optional["UpstreamTokenGrant"]:
@@ -744,17 +742,38 @@ def _bridge_grant_from_token_response(token_response: object) -> Optional["Upstr
     if not isinstance(access, str) or not access:
         return None
     token_type = token_response.get("token_type")
-    refresh = token_response.get("refresh_token")
     scope = token_response.get("scope")
     return UpstreamTokenGrant(
         access_token=SecretStr(access),
         token_type=token_type if isinstance(token_type, str) and token_type else "Bearer",
-        refresh_token=SecretStr(refresh) if isinstance(refresh, str) and refresh else None,
+        # The upstream refresh_token is deliberately NOT sealed: the edge never consumes it (it forwards
+        # only token_type + access_token), so it would be dead weight embedding a long-lived upstream
+        # credential in the client-held bearer, and it enlarges the envelope. Refresh support is a
+        # follow-up (a dedicated refresh-envelope); the client re-runs authorization_code at the cap.
+        refresh_token=None,
         scope=scope if isinstance(scope, str) and scope else None,
         expires_in=_coerce_positive_expires_in(token_response.get("expires_in")),
     )
 
 
+def _bridge_invalid_request_response() -> JSONResponse:
+    """RFC 6749 §5.2-shaped ``invalid_request`` for a bridge token exchange that carries no resolvable
+    litellm identity. Returned (not raised) so the OAuth error members sit at the top level rather than
+    wrapped in FastAPI's ``detail``, with the no-store token-endpoint headers, matching the BYOK OAuth
+    endpoint and what a strict DCR client parses per RFC 6749 §5.2."""
+    return JSONResponse(
+        status_code=400,
+        content={
+            "error": "invalid_request",
+            "error_description": (
+                "this server issues a gateway-bound credential; send a litellm credential "
+                "(x-litellm-api-key or Authorization) on the token request"
+            ),
+        },
+        headers=TOKEN_NO_CACHE_HEADERS,
+    )
+
+
 async def _mint_bridge_delegate_token_response(
     request: Request, mcp_server: MCPServer, token_response: object
 ) -> JSONResponse:
@@ -784,16 +803,7 @@ async def _mint_bridge_delegate_token_response(
 
     key_hash = await _extract_active_key_hash_from_request(request)
     if not key_hash:
-        raise HTTPException(
-            status_code=400,
-            detail={
-                "error": "invalid_request",
-                "error_description": (
-                    "this server issues a gateway-bound credential; send a litellm credential "
-                    "(x-litellm-api-key or Authorization) on the token request"
-                ),
-            },
-        )
+        return _bridge_invalid_request_response()
 
     grant = _bridge_grant_from_token_response(token_response)
     if grant is None:
@@ -804,7 +814,11 @@ async def _mint_bridge_delegate_token_response(
     identity = EnvelopeIdentity(server_id=mcp_server.server_id, key_hash=key_hash)
     sealed = build_bridge_token_response(identity, grant, keys, now)
     if not isinstance(sealed, SealedEnvelope):
-        raise HTTPException(status_code=500, detail="Failed to mint the gateway-bound credential")
+        # build_bridge_token_response returns EnvelopeTooLarge as a value when the upstream token is
+        # too large to seal; that is an upstream-payload condition, so surface a 502, not a 500.
+        raise HTTPException(
+            status_code=502, detail="Upstream token is too large to seal into a gateway-bound credential"
+        )
 
     expires_in = max(1, int((sealed.expires_at - now).total_seconds()))
     body = {"access_token": sealed.token.get_secret_value(), "token_type": "Bearer", "expires_in": expires_in}
@@ -884,6 +898,16 @@ async def exchange_token_with_server(
         if code_verifier:
             token_data["code_verifier"] = code_verifier
 
+        # For a bridge oauth_delegate mint, resolve the litellm identity BEFORE exchanging the
+        # single-use upstream code. A missing or transiently-unresolvable identity then fails closed
+        # with invalid_request without consuming the code, so the client can retry the same code
+        # instead of being forced back through the full interactive authorize. The mint below
+        # re-resolves authoritatively; get_key_object is cache-first, so that second call is a cache
+        # hit and this adds no extra database round-trip.
+        if mcp_server.is_oauth_delegate and mcp_server.is_dcr_bridge:
+            if not await _extract_active_key_hash_from_request(request):
+                return _bridge_invalid_request_response()
+
     async_client = get_async_httpx_client(llm_provider=httpxSpecialProvider.Oauth2Check)
     response = await async_client.post(
         mcp_server.token_url,

tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py

@@ -4365,7 +4365,7 @@ async def test_register_bridge_relay_never_persists():
 _BRIDGE_MASTER_KEY = "sk-bridge-producer-master-key-0123456789abcdef"
 
 
-async def _exchange_for_bridge_server(server, upstream_body, key_hash):
+async def _exchange_for_bridge_server(server, upstream_body, key_hash, fake_client_out=None):
     from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
         exchange_token_with_server,
     )
@@ -4375,6 +4375,8 @@ async def _exchange_for_bridge_server(server, upstream_body, key_hash):
     fake_http_response.raise_for_status = MagicMock()
     fake_http_client = MagicMock()
     fake_http_client.post = AsyncMock(return_value=fake_http_response)
+    if fake_client_out is not None:
+        fake_client_out["client"] = fake_http_client
 
     with (
         patch(
@@ -4436,17 +4438,69 @@ async def test_oauth_delegate_bridge_token_exchange_mints_envelope_not_raw_token
 @pytest.mark.asyncio
 async def test_oauth_delegate_bridge_token_exchange_fails_closed_without_litellm_identity():
     """Without a resolvable litellm identity on the token request, the exchange must not mint an
-    identity-less envelope; it returns an OAuth invalid_request so the client sends a credential."""
+    identity-less envelope. It returns an RFC 6749 §5.2-shaped invalid_request (error at the top
+    level, not wrapped in detail) BEFORE exchanging the upstream code, so the single-use code is not
+    burned and the client can retry."""
     from litellm.types.mcp import MCPAuth
 
     server = _bridge_server(auth_type=MCPAuth.oauth_delegate)
     upstream = {"access_token": "UPSTREAM-SECRET-TOKEN", "token_type": "Bearer", "expires_in": 3600}
+    captured: dict = {}
+    response = await _exchange_for_bridge_server(server, upstream, key_hash=None, fake_client_out=captured)
 
+    assert response.status_code == 400
+    assert json.loads(response.body)["error"] == "invalid_request"
+    # identity resolution failed first, so the upstream single-use code was never exchanged (not burned)
+    captured["client"].post.assert_not_called()
+
+
+@pytest.mark.asyncio
+async def test_bridge_envelope_too_large_upstream_token_is_502():
+    """An upstream token too large to seal into the envelope is an upstream-payload condition, so the
+    mint surfaces a 502 rather than a 500 (build_bridge_token_response returns EnvelopeTooLarge as a
+    value, and the caller maps it to a truthful status)."""
+    from litellm.types.mcp import MCPAuth
+
+    server = _bridge_server(auth_type=MCPAuth.oauth_delegate)
+    upstream = {"access_token": "x" * 40000, "token_type": "Bearer", "expires_in": 3600}
     with pytest.raises(HTTPException) as exc:
-        await _exchange_for_bridge_server(server, upstream, key_hash=None)
+        await _exchange_for_bridge_server(server, upstream, key_hash="hashed-litellm-key-77")
+    assert exc.value.status_code == 502
 
-    assert exc.value.status_code == 400
-    assert exc.value.detail["error"] == "invalid_request"
+
+@pytest.mark.asyncio
+async def test_bridge_envelope_does_not_seal_upstream_refresh_token():
+    """The upstream refresh_token is never sealed into the client-held envelope: the edge never
+    consumes it and a long-lived upstream credential should not live in the client bearer. The opened
+    envelope's grant carries no refresh token even when the upstream returned one, and neither does
+    the response body."""
+    from datetime import datetime, timezone
+
+    from litellm.proxy._experimental.mcp_server.outbound_credentials.bridge_credentials import (
+        envelope_keys_from_master_key,
+    )
+    from litellm.proxy._experimental.mcp_server.outbound_credentials.envelope import (
+        OpenedEnvelope,
+        open_envelope,
+    )
+    from litellm.types.mcp import MCPAuth
+
+    server = _bridge_server(auth_type=MCPAuth.oauth_delegate)
+    upstream = {
+        "access_token": "UP",
+        "token_type": "Bearer",
+        "expires_in": 3600,
+        "refresh_token": "UPSTREAM-REFRESH",
+    }
+    response = await _exchange_for_bridge_server(server, upstream, key_hash="hashed-litellm-key-77")
+
+    body = json.loads(response.body)
+    assert "refresh_token" not in body
+    assert "UPSTREAM-REFRESH" not in body["access_token"]
+    keys = envelope_keys_from_master_key(_BRIDGE_MASTER_KEY)
+    opened = open_envelope(body["access_token"], keys, datetime.now(timezone.utc))
+    assert isinstance(opened, OpenedEnvelope)
+    assert opened.grant.refresh_token is None
 
 
 def test_bridge_grant_coerces_numeric_expires_in():
@@ -4470,6 +4524,13 @@ def test_bridge_grant_coerces_numeric_expires_in():
     assert ei(0) is None
     assert ei(-5) is None
     assert ei(None) is None
+    # hostile numerics must not raise (int(float(...)) can OverflowError) -> None
+    assert ei("inf") is None
+    assert ei("1e999") is None
+    assert ei("-inf") is None
+    assert ei("nan") is None
+    assert ei(float("inf")) is None
+    assert ei(10**400) is None
 
 
 @pytest.mark.asyncio