fix(mcp): forward dcr_bridge-injected credential instead of discarding it for oauth_delegate - #36359
Conversation
…g it for oauth_delegate _admit_dcr_bridge_delegate opens the caller's signed DCR-bridge envelope during admission and injects the real unsealed upstream credential as mcp_auth_header. _create_mcp_client's v1-override exemption kept the v2 spec for PassthroughConfig unconditionally (which oauth_delegate maps to), discarding that injected credential and falling back to the v2 resolver's raw passthrough headers -- headers an external DCR client (e.g. claude.ai) has no way to populate, since it only ever presents the one envelope bearer that admission already strips before egress. The upstream MCP server then receives no Authorization header at all. dcr_bridge servers now always take the v1 override path here, since their mcp_auth_header is a cryptographically verified credential from admission, not an arbitrary caller-supplied bearer -- the security concern the PassthroughConfig exemption exists for doesn't apply to this origin. Fixes BerriAI#36358
|
|
Greptile SummaryThe PR changes MCP client authentication priority so an admitted DCR bridge credential is forwarded through the existing client path, and adds a regression test for that behavior.
Confidence Score: 4/5The PR appears safe to merge after the non-blocking source-comment concision issue is addressed. The credential-priority change matches the documented admission-to-egress flow, and the only accepted concern is the unnecessarily lengthy explanatory comment. Files Needing Attention: litellm/proxy/_experimental/mcp_server/mcp_server_manager.py
|
| Filename | Overview |
|---|---|
| litellm/proxy/_experimental/mcp_server/mcp_server_manager.py | Adjusts credential-resolution priority for DCR bridge servers; the behavior appears coherent, but the accompanying explanation violates the repository’s concise-comment convention. |
| tests/mcp_tests/test_mcp_auth_priority.py | Adds focused regression coverage showing that the supplied bridge credential reaches the constructed MCP client. |
Reviews (1): Last reviewed commit: "fix(mcp): forward dcr_bridge-injected cr..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # | ||
| # dcr_bridge is a different origin, not a caller override: for a client-forwarded-token | ||
| # server admitted through the gateway-DCR bridge, `_admit_dcr_bridge_delegate` has already | ||
| # opened the caller's signed envelope and injected the real upstream credential it sealed — | ||
| # `mcp_auth_header` here IS that unsealed credential, not an arbitrary bearer the caller | ||
| # attached. Keeping `spec` for PassthroughConfig in this case discards that credential and | ||
| # falls through to the v2 resolver's own passthrough headers, which the DCR caller has no | ||
| # way to populate (it only ever presents the one envelope bearer, which admission already | ||
| # strips before egress). So a dcr_bridge server always takes the v1 path here regardless of |
There was a problem hiding this comment.
This nine-line comment repeats the full admission and resolver flow instead of concisely documenting the credential-origin invariant, making this authentication branch harder to scan and maintain.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Thanks for the trace. The passthrough arm already forwards the injected credential; the failure is earlier, at access resolution. #37865 denies loudly at authorize |
3 participants
Summary
Fixes #36358.
For a
dcr_bridge+oauth_delegateMCP server (the gateway-DCR bridge mode that lets an external OAuth client like claude.ai's custom connector UI authenticate against an MCP server LiteLLM otherwise reaches with a static credential), the OAuth handshake completes successfully end-to-end, but the upstream MCP server never receives a validAuthorizationheader, so no tools are ever returned._admit_dcr_bridge_delegatecorrectly opens the caller's signed envelope during admission and injects the real, unsealed upstream credential asmcp_auth_header. But_create_mcp_client's v1-override exemption unconditionally kept the v2 resolver'sspecfor anyPassthroughConfig-mapped server (which is whatoauth_delegatemaps to inoutbound_credentials/adapter.py) — discarding that correctly-injected credential and falling through to the v2 resolver's own passthrough-header resolution instead. An external DCR client has no way to populate those headers itself; it only ever presents the one envelope bearer, and that gets stripped separately by_should_strip_caller_authorizationsince it lacks the LiteLLM-specificx-litellm-api-keydisambiguation header. Net result: the upstream server gets noAuthorizationheader at all.This PR makes
dcr_bridgeservers always take the v1 override path in_create_mcp_client, regardless ofspec.config, since theirmcp_auth_headeris a cryptographically verified credential recovered during admission — not an arbitrary caller-supplied bearer. The security concern thePassthroughConfigexemption exists for (a caller substituting another user's resolved credential) doesn't apply to this origin.Root cause
Traced end-to-end against a live reproduction: OAuth flow (
register→authorize→ SSO login →callback→token) all return200, the external client shows "Connected," but the upstream MCP server's own session count stays at0for the entire session — it never receives a request with a matchingAuthorizationheader. Full trace is in #36358.Test plan
test_dcr_bridge_oauth_delegate_uses_admission_injected_credentialtotests/mcp_tests/test_mcp_auth_priority.py, asserting_create_mcp_clientforwards the injectedmcp_auth_headerfor adcr_bridge=True, auth_type=oauth_delegateserver.AssertionError: assert None == 'Bearer unsealed_upstream_token') and passes with the fix.ruff format/ruff checkpass on both changed files (pre-existing unrelated lint findings in the test file's import block were left untouched).tests/mcp_tests/test_mcp_auth_priority.pysuite — all 4 tests pass.