Repository navigation
fix(mcp): keep upstream OAuth Authorization when jwt signer hook injects one on tools/call - #38555
Conversation
…cts one on tools/call Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
|
PR #38555 (BerriAI/litellm, author devin-ai-integration[bot]) has no labels, so it lacks the |
Greptile SummaryThe latest revision refines tools/call header precedence so existing upstream Authorization credentials are preserved while non-conflicting signer headers continue to merge.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| litellm/proxy/_experimental/mcp_server/mcp_server_manager.py | The revised occupancy check now matches outbound credential mappings and resolves the previously reported non-Authorization credential conflict. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_hook_extra_headers.py | Regression tests cover the corrected header-precedence cases without indicating a blocking behavioral failure. |
Reviews (2): Last reviewed commit: "fix(mcp): only treat server credential a..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…n it maps to that header Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
@greptileai review the latest revision (3a91bd4). Changes since the 3/5 review: the Authorization-occupancy check now inspects the resolved server credential — dict credentials are scanned for an actual |
|
Addressed in 3a91bd4: capture dicts now use precise types instead of Dict[str, Any], with isinstance guards on the captured headers |
TLDR
Problem this solves:
How it solves it:
User Flow
Before: a user whose gateway has the MCP JWT signer enabled can list tools on an OAuth-backed MCP server but every tool call is rejected upstream
After: the same call succeeds because the gateway keeps their OAuth token in the Authorization header
Relevant issues
Fixes #31977
Linear ticket
Resolves LIT-6321
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Root cause in one sentence: the tools/call path merged pre_mcp_call hook headers last, letting the signer's Authorization JWT replace the user or server OAuth credential, while tools/list already skipped signer injection when the Authorization slot was occupied
Setup shared by both runs: local proxy on :4000 with an oauth2 authorization_code MCP server (
delegate_auth_to_upstream: true) pointing at an OAuth-protected MCP upstream on :9100 that accepts onlyBearer USER_OAUTH_TOKEN_abc123and logs the Authorization header it receives, plus themcp_jwt_signerguardrail as a default_onpre_mcp_callhook. The calling key's user has that OAuth token stored as their per-user credential for the server. Requests are byte-identical across runs; the serving build is identified by a log line that exists only in the fixed codeBefore (02dcc4d)
tools/list with OAuth credential and signer enabled (control, works)
curl -s "http://localhost:4000/mcp-rest/tools/list?server_id=9ea9ca1cd1f9241dcfd1f7a23ba5bfd3" -H "x-litellm-api-key: Bearer $USER_KEY"{"tools":[{"name":"whoami",...}],"error":null,"message":"Successfully retrieved tools"}UPSTREAM_AUTH_HEADER: Bearer USER_OAUTH_TOKEN_abc123tools/call with OAuth credential and signer enabled (bug)
curl -s -X POST "http://localhost:4000/mcp-rest/tools/call" -H "x-litellm-api-key: Bearer $USER_KEY" -H "Content-Type: application/json" -d '{"server_id":"9ea9ca1cd1f9241dcfd1f7a23ba5bfd3","name":"whoami","arguments":{}}'{"_meta":null,"content":[{"type":"text","text":"HTTPStatusError: Client error '401 Unauthorized' for url 'http://127.0.0.1:9100/mcp'..."}],"structuredContent":null,"isError":true}UPSTREAM_AUTH_HEADER: Bearer eyJhbGciOiJSUzI1NiIs...(decoded payload:iss=https://litellm.local aud=mcp-upstream act.sub=litellm-proxy)After (3a91bd4)
tools/list with OAuth credential and signer enabled (control, still works)
curl -s "http://localhost:4000/mcp-rest/tools/list?server_id=9ea9ca1cd1f9241dcfd1f7a23ba5bfd3" -H "x-litellm-api-key: Bearer $USER_KEY"{"tools":[{"name":"whoami",...}],"error":null,"message":"Successfully retrieved tools"}tools/call with OAuth credential and signer enabled (fixed)
curl -s -X POST "http://localhost:4000/mcp-rest/tools/call" -H "x-litellm-api-key: Bearer $USER_KEY" -H "Content-Type: application/json" -d '{"server_id":"9ea9ca1cd1f9241dcfd1f7a23ba5bfd3","name":"whoami","arguments":{}}'{"_meta":null,"content":[{"type":"text","text":"tool-ok","annotations":null,"_meta":null}],"structuredContent":{"result":"tool-ok"},"isError":false}UPSTREAM_AUTH_HEADER: Bearer USER_OAUTH_TOKEN_abc123grep -c "dropping hook-injected 'Authorization'" proxy.logprints1(this warning line exists only in the fixed code)Scope note: this run exercises the delegated per-user OAuth branch of the regular (SSE/HTTP) tools/call path. The same merge site also guards static_headers Authorization and configured Authorization-mapped authentication_token credentials, while api_key credentials (which map to X-API-Key) and per-server header dicts without Authorization do not block the signer; all covered by the new unit tests. Migrated (non-delegated) authorization_code, client_credentials, and token exchange servers already resolve their credential later and drop a conflicting hook JWT there; the OpenAPI-backed path ignores hook headers entirely and is unchanged
Type
🐛 Bug Fix
Caveats (if any)
Low
Link to Devin session: https://app.devin.ai/sessions/12cde377d365464f8379dd44698e3545
Open in Devin Desktop: https://app.devin.ai/desktop/session/12cde377d365464f8379dd44698e3545?variant=devin
Requested by: @yassin-berriai