* fix(mcp-oauth): add PROXY_BASE_URL escape hatch + diagnostic logging for invalid_request
Customers hitting "{"detail":"invalid_request"}" on the MCP /authorize
endpoint had no way to recover when their ingress mangles X-Forwarded-*
headers (the same-origin check in validate_trusted_redirect_uri compares
the browser-supplied redirect_uri against get_request_base_url, which is
reconstructed from those headers).
Two contained changes:
1. get_request_base_url now honours PROXY_BASE_URL as the canonical
public origin when set, bypassing the X-Forwarded-* trust gate
entirely. Operators who know their public URL can set it once
instead of debugging ingress header rewrites.
2. The rejection path in validate_trusted_redirect_uri emits a WARN
log carrying the redirect_uri, computed proxy base, and the
X-Forwarded-* / Host headers seen. A bare 400 was undiagnosable;
this turns it into a one-line root-cause.
* test(mcp-oauth): capture warnings from correct logger ("LiteLLM")
Co-authored-by: Yassin Kortam <yassin@berri.ai>
* fix(mcp-oauth): reject malformed PROXY_BASE_URL with one-shot diagnostic
A scheme-less PROXY_BASE_URL (e.g. "litellm.example.com" instead of
"https://litellm.example.com") would sail through urlparse with empty
scheme + netloc, silently breaking every same-origin compare in
validate_trusted_redirect_uri and leaving the operator staring at the
same opaque 400 the env var was meant to fix.
Validate it once at read time: only honour values that parse as
http(s) URLs with a non-empty netloc; otherwise log a one-shot WARN
naming the bad value and fall through to the request-derived origin
so the proxy still serves traffic.
* fix(mcp/oauth): normalize PROXY_BASE_URL to strip query/fragment
Match the X-Forwarded-* path's normalization so a configured
PROXY_BASE_URL containing a query string or fragment does not break
downstream f-string concatenation like f"{base_url}/callback".
Co-authored-by: Yassin Kortam <yassin@berri.ai>
* refactor(mcp-oauth): drop non-essential comments from PROXY_BASE_URL changes
Strip narrative comments and verbose docstrings added in this PR; the
code is intuitive enough on its own and the log messages already carry
their own diagnostic context. Pre-existing comments are left untouched.
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Yassin Kortam <yassin@berri.ai>
_build_mcp_server_table omitted delegate_auth_to_upstream, so GET /v1/mcp/server always returned the default false while the registry kept the DB value.
Co-authored-by: Cursor <cursoragent@cursor.com>
Log verbose_logger.warning when loading oauth2 interactive servers with
available_on_public_internet=false and delegate_auth_to_upstream=true
(config + DB). Dashboard Alert for the same combo. CLAUDE note for
operators. Tests for log and M2M skip.
Remove available_on_public_internet gating from delegate-auth-to-upstream
paths so oauth2 + delegate_auth_to_upstream interactive servers behave
the same when marked internal. Keeps M2M exclusion. Updates tests.
2026-05-15 08:32:09 +05:30
Dennis HenryGitHubveria-ai[bot] <224490171+veria-ai[bot]@users.noreply.github.com>
* feat(mcp): add delegate_auth_to_upstream flag for PKCE passthrough
Adds an opt-in per-server flag that lets clients (e.g. VS Code) complete
PKCE directly with an upstream OAuth2 MCP server, instead of LiteLLM
double-gating with its own API-key/SSO check. Only honored when
auth_type=oauth2 and the operator explicitly sets the flag; mixed-target
or non-oauth2 requests fail closed.
- Adds the field to Pydantic models, Prisma schema, and a migration
- New MCPRequestHandler._target_servers_delegate_auth_to_upstream gate
that runs only when no x-litellm-api-key is present, so authenticated
users still get user_id resolution + stored-credential lookup
- Anonymous callers now see delegate servers in get_allowed_mcp_servers
(scoped to delegate servers only; the upstream still enforces auth)
- mcp_management_endpoints: allow anonymous /authorize and /token for
delegate servers so VS Code can complete PKCE without a LiteLLM session
- UI toggle (shown only for oauth2) + payload/view wiring
- Tests covering: oauth2 on/off, non-oauth2 with flag, mixed targets,
no resolvable target, explicit key precedence, and 401 emission
Co-authored-by: Cursor <cursoragent@cursor.com>
* Enforce oauth2 for delegated MCP auth bypass
Co-authored-by: Yassin Kortam <yassin@berri.ai>
* fix(mcp): close secondary Authorization bypass for delegate servers
The delegate-auth bypass gated only on the primary `x-litellm-api-key`
header, so a LiteLLM key sent via `Authorization: Bearer sk-...` (the
secondary header) was silently dropped — skipping spend tracking and
rate limiting. Gate on the resolved litellm_api_key (which considers
both headers) so the bypass fires only when neither is present.
Also update the existing "Authorization header present" test to reflect
that an upstream OAuth token now flows through the existing oauth2
fallback (LiteLLM auth attempt → fail → anonymous), not via the
delegate branch.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Avoid duplicate MCP OAuth credential lookup
Co-authored-by: Yassin Kortam <yassin@berri.ai>
* fix(mcp): block delegate bypass for M2M and internal-only servers
Two security issues flagged in code review:
1. High – client_credentials (M2M) servers must not be delegatable:
LiteLLM auto-fetches the upstream token using stored credentials, so
allowing anonymous bypass would let any external caller invoke tools
authenticated as LiteLLM's service account.
Fix: check `server.has_client_credentials` in
`_target_servers_delegate_auth_to_upstream`, the anonymous
allow-list in `get_allowed_mcp_servers`, and `_mcp_oauth_user_api_key_auth`.
2. Medium – internal-only servers exposed to public internet:
The anonymous delegate allow-list was not filtering by
`available_on_public_internet`, so external callers with an upstream
OAuth token could invoke tools on servers marked internal-only.
Fix: add `available_on_public_internet` guard to the anonymous
delegate server list in `get_allowed_mcp_servers`.
Tests added for both cases.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Require public MCP delegate auth servers
Co-authored-by: Yassin Kortam <yassin@berri.ai>
* fix(mcp): align delegate auth path parsing with downstream routing
`_extract_target_server_names_from_path` used a naive segments-based
split while `server.py::_get_mcp_servers_in_path` uses a regex that
allows server names with one embedded slash and comma-separated lists.
With the old parser, a request to `/mcp/<delegated>/<garbage>` was
parsed as targeting `<delegated>` by the auth gate (bypassing LiteLLM
auth) while the routing layer parsed it as `<delegated>/<garbage>` —
when that name did not resolve, the request fell back to the anonymous
allow-list, which can include `allow_all_keys` servers that normally
require a LiteLLM key.
Replace the parser with the same regex logic as
`_get_mcp_servers_in_path` so auth gating sees the exact target name(s)
downstream routing sees. Add regression tests covering parser parity
and the specific extra-path-segment bypass attempt.
https://claude.ai/code/session_01SjyPmwfmrq8fveFgw9iHW9
* fix(mcp): close header/path TOCTOU in MCP delegate auth gate
`_target_servers_delegate_auth_to_upstream` and
`_target_servers_use_oauth2` trusted the `x-mcp-servers` header when
present, but `server.py::extract_mcp_auth_context` overrides that
header with the path-derived list for `/mcp/...` routes. An attacker
could set `x-mcp-servers: <delegated>` while pointing the URL path at
a non-delegate server, flipping the auth gate without changing the
target downstream routing actually uses.
Extract a shared `_resolve_target_server_names` helper that mirrors
the downstream override (path-derived names for `/mcp/...` routes,
header value otherwise). Add regression tests covering the TOCTOU
attempt and the helper's path-vs-header precedence.
https://claude.ai/code/session_01SjyPmwfmrq8fveFgw9iHW9
* Fix delegated MCP OAuth test mock
Co-authored-by: Yassin Kortam <yassin@berri.ai>
* fix(mcp): drop unreachable /{server}/mcp branch in auth path parser
`_extract_target_server_names_from_path` also matched the
``/{server_name}/mcp`` form, but the downstream parser
``_get_mcp_servers_in_path`` only handles ``/mcp/...`` — and
``dynamic_mcp_route`` in ``proxy_server`` rewrites ``/{name}/mcp``
to ``/mcp/{name}`` on the scope before the MCP handler runs. Parsing
the un-rewritten form on the auth side was therefore unreachable in
production, and contradicted the docstring's claim of mirroring the
downstream parser — exactly the kind of mismatch that risks a future
header/path TOCTOU if any new entry point skips the rewrite.
Drop the branch; the canonical ``/mcp/...`` path matches both
parsers. Update the regression test to assert the new behavior.
https://claude.ai/code/session_01SjyPmwfmrq8fveFgw9iHW9
* Fix MCP path auth target resolution
Co-authored-by: Yassin Kortam <yassin@berri.ai>
* fix(mcp): require auth for refresh_token grants on delegate-auth servers
`_mcp_oauth_user_api_key_auth` gates the unauthenticated PKCE flow for
``delegate_auth_to_upstream`` servers, but the bypass applied to BOTH
``/authorize`` and ``/token`` regardless of grant type. ``mcp_token``
accepts ``grant_type=refresh_token`` as well as ``authorization_code``,
and ``exchange_token_with_server`` attaches the server's stored
``client_secret`` to whatever is forwarded upstream. An unauthenticated
caller holding a refresh token issued to that OAuth client could mint
fresh upstream access tokens through LiteLLM.
Limit the anonymous bypass on ``/token`` to ``grant_type=authorization_code``
(the only grant PKCE actually protects via ``code_verifier``); fall
through to normal LiteLLM auth for ``refresh_token`` and any other grant.
``/authorize`` continues to allow anonymous PKCE redirects.
https://claude.ai/code/session_01SjyPmwfmrq8fveFgw9iHW9
* fix(ui): clear delegate_auth_to_upstream when switching off oauth2
The ``delegate_auth_to_upstream`` form field is rendered inside an
``isOAuth2 && (...)`` conditional, so the Form.Item unmounts when the
user changes ``auth_type`` away from ``oauth2``. The follow-up
``form.setFieldValue("delegate_auth_to_upstream", false)`` runs after
the field has already deregistered, so ``onFinish`` receives
``undefined`` and the fallback ``?? mcpServer.delegate_auth_to_upstream``
preserved the old ``true``. The flag then persisted in the database for
a non-oauth2 server and silently re-activated if ``auth_type`` was later
switched back to ``oauth2``.
In the edit payload, force the flag to ``false`` whenever
``auth_type !== oauth2``; only trust the form value (and the existing
DB fallback) when the server is actually oauth2. Backend defense-in-depth
already ignores the flag for non-oauth2 servers, but the DB state should
stay clean too.
https://claude.ai/code/session_01SjyPmwfmrq8fveFgw9iHW9
* Fix MCP delegate auth reset on edit
Co-authored-by: Yassin Kortam <yassin@berri.ai>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Yassin Kortam <yassin@berri.ai>
Co-authored-by: Claude <claude@anthropic.com>
* fix(mcp): surface upstream 401 for token-forwarding MCP servers
For MCP servers configured with extra_headers: [Authorization], the gateway
forwards the client token directly to the upstream. When that token is rejected
(expired or invalid) the upstream returns 401, but the MCP SDK starts the SSE
stream with 200 OK before calling handlers, so the 401 can't be returned
mid-stream.
Fix: add a pre-flight httpx probe in handle_streamable_http_mcp — before the
SDK opens the session — so the gateway can still return HTTP 401 with
WWW-Authenticate: Bearer authorization_uri=<gateway-discovery-url> when the
upstream rejects the token. The probe fails-open (returns 200) on network
errors so a transient hiccup does not block valid requests.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(mcp): parallelize pre-flight auth probes and use HEAD to avoid side effects
- Extract forwarded_auth outside the pass-through server loop (was called N times for the same scope value)
- Gather all upstream auth probes concurrently with asyncio.gather instead of sequentially; eliminates N×5 s worst-case latency
- Switch probe from POST+initialize JSON-RPC body to HEAD request; HEAD carries the Authorization header so the upstream rejects invalid tokens with 401 but never allocates a session or writes an audit entry
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(mcp): use get_async_httpx_client in _probe_upstream_auth
Replaces bare httpx.AsyncClient with the project-standard
get_async_httpx_client(httpxSpecialProvider.MCP) to satisfy the
ensure_async_clients_test code coverage check and avoid the +500 ms
per-request overhead of creating a new client on every probe call.
Co-authored-by: Cursor <cursoragent@cursor.com>
* refactor(mcp): extract pre-flight probe into _check_passthrough_upstream_auth
Moves the parallel upstream auth probe logic out of
handle_streamable_http_mcp into a dedicated helper to satisfy
Ruff PLR0915 (Too many statements > 50).
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(mcp): gate pre-flight probes on authorized server set to prevent bypass
_check_passthrough_upstream_auth was resolving user-supplied server names
directly before authorization ran, letting any permitted LiteLLM key
trigger an upstream HEAD probe to a server it was not allowed to use.
Changes:
- Call _get_allowed_mcp_servers inside the helper so only servers the
caller's key is authorized for are probed.
- Move the call site to after toolset scoping so the auth context is
fully resolved before the probe list is built.
- Thread user_api_key_auth into the helper signature (replaces the raw
mcp_servers name list).
Co-authored-by: Cursor <cursoragent@cursor.com>
* Add async HTTP HEAD support
Co-authored-by: Yassin Kortam <yassin@berri.ai>
* fix(mcp): use Scope type annotation in _get_forwarded_auth_from_scope
Co-authored-by: Cursor <cursoragent@cursor.com>
* Fix MCP upstream auth probe method
Co-authored-by: Yassin Kortam <yassin@berri.ai>
* Remove unused AsyncHTTPHandler head method
Co-authored-by: Yassin Kortam <yassin@berri.ai>
* fix(mcp): exclude has_client_credentials servers from pre-flight auth probe
_prepare_mcp_server_headers skips caller Authorization when the server
uses OAuth client-credentials (M2M), but the pre-flight probe was still
selecting those servers and forwarding the caller's raw token in the HEAD
request. Exclude servers with has_client_credentials from the probe list
to match the actual downstream header-preparation logic.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(mcp): propagate upstream 403 as 403, not 401 with WWW-Authenticate
Per RFC 9110, 401 means "go get new credentials." Mapping an upstream 403
to a gateway 401 causes OAuth clients to restart the authorization flow,
obtain a fresh token with identical scopes, hit 403 again, and loop
indefinitely.
401 from upstream → gateway 401 + WWW-Authenticate (re-authorize)
403 from upstream → gateway 403 (no WWW-Authenticate hint)
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(mcp): skip auth probe when Authorization may be the LiteLLM proxy key
The pre-flight upstream probe must not forward the caller's Authorization
header when it could itself be the LiteLLM proxy API key. Restrict the
probe to requests that supply x-litellm-api-key explicitly — only then is
the Authorization header unambiguously the upstream OAuth token the
caller wants forwarded.
* Fix MCP ASGI HTTPException propagation
Co-authored-by: Yassin Kortam <yassin@berri.ai>
* fix(mcp): use public AsyncHTTPHandler.post() in auth probe
Use AsyncHTTPHandler.post() and catch httpx.HTTPStatusError explicitly so
the 401/403 we want to surface is not silently swallowed by the broad
fail-open except Exception block. Avoids reaching into the handler's
private client attribute, which would silently regress to fail-open if
AsyncHTTPHandler is ever refactored.
* Fix MCP auth probe tests
Co-authored-by: Yassin Kortam <yassin@berri.ai>
* test(mcp): add coverage for httpx.HTTPStatusError path in auth probe
AsyncHTTPHandler.post() calls raise_for_status() internally, so a real
upstream 401/403 lands as httpx.HTTPStatusError. Add a test that exercises
that specific exception path so a regression that swallows the error in
the broad fail-open except Exception would be caught.
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Yassin Kortam <yassin@berri.ai>
Co-authored-by: claude-bot <claude-bot@anthropic.com>
* fix: invalidate cached tag object on tag budget reset (#27481) (#27572)
Squash-merged by litellm-agent from oss-agent-shin's PR.
* chore(mcp): tighten stdio server registration paths (#27570)
Squash-merged by litellm-agent from stuxf's PR.
* fix(proxy): clear MCP OpenAPI mappings on server eviction; widen budget cache invalidation
Evict OpenAPI tools from global_mcp_tool_registry and strip tool_name_to_mcp_server_name_mapping entries when a server leaves the runtime registry (remove_server and approval-status eviction). Invalidate user_api_key_cache for keys, orgs, and team members on budget-tier spend resets alongside tags.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(mcp): align update_server eviction with remove_server name fallback
Document budget-reset test assertion flip (cross-pod cache staleness).
Greptile: eviction now pops by server_id then server_name like remove_server;
test docstring explains assert_not_awaited -> assert_any_await change.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Fix org budget cache invalidation
---------
Co-authored-by: oss-agent-shin <ext-agent-shin@berri.ai>
Co-authored-by: stuxf <70670632+stuxf@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(mcp): forward extra_headers for OpenAPI MCP tools
OpenAPI-generated tools only applied static closure headers and BYOK
Authorization via ContextVar. Copy MCPServer.extra_headers from the
incoming MCP request into _request_extra_headers (set in server.py before
local tool dispatch), merge in openapi_to_mcp_generator via a small helper.
OAuth2 M2M: do not forward caller Authorization from raw_headers (same rule
as _prepare_mcp_server_headers for managed MCP).
Adds TestRequestExtraHeaders and clarifies mcp_server_manager registration
comment.
Fixes#26794
Co-authored-by: Cursor <cursoragent@cursor.com>
* refactor(mcp): access has_client_credentials on MCPServer directly
Greptile: getattr default was redundant; property exists on MCPServer and
mcp_server is non-None inside the extra_headers forwarding block.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
* fix(anthropic, mcp): sanitize tool names to match Anthropic's `^[a-zA-Z0-9_-]{1,128}$`
Tool names with characters like `/` or `.` (commonly produced by the
OpenAPI -> MCP generator from `operationId`s such as
`actions/download-job-logs-for-workflow-run`) caused Anthropic to reject
requests with `tools.N.custom.name: String should match pattern
'^[a-zA-Z0-9_-]{1,128}$'`.
Two layers of fix:
1. Anthropic transformation: build a per-request forward map (original ->
sanitized, disambiguated by suffix on collisions) and a reverse map
(only for names actually rewritten). Forward map is applied to tool
defs, `tool_choice`, and historical assistant tool_calls in messages.
Reverse map is threaded through both the non-streaming and streaming
response paths so callers continue to see their original tool names
in `tool_use` blocks.
2. OpenAPI -> MCP generator: sanitize `operationId` (and the
method+path fallback) at registration time so generated MCP tools are
valid for any strict-name provider, not just Anthropic. The dashboard
preview endpoint applies the same sanitization for parity.
Includes unit tests covering: collision disambiguation between
`foo_bar` and `foo/bar` in the same request, reverse-map only firing
for actually-rewritten names, message rewrite for historical tool_calls,
streaming chunk_parser reverse-mapping, and sanitization of OpenAPI
operationIds plus the preview endpoint output.
Made-with: Cursor
* fix(anthropic): build tool-name maps in transform_request, not optional_params
The previous patch stashed the per-request forward and reverse tool-name
maps under ``optional_params["_anthropic_tool_name_forward_map"]`` and
``optional_params["_anthropic_tool_name_map"]``. ``optional_params`` is
the dict that becomes the JSON body via ``data = {**optional_params}``,
so those internal keys leaked over the wire and Anthropic 400'd with:
_anthropic_tool_name_forward_map: Extra inputs are not permitted
Worse, this meant *every* request whose tool list contained any name with
an invalid character (the exact case the patch was meant to fix) regressed
into a confusing meta-error pointing at LiteLLM's internal map instead of
the offending tool.
Fix: move all tool-name sanitization into ``transform_request``, which is
the single chokepoint already shared by ``AnthropicConfig``,
``AmazonAnthropicConfig`` (Bedrock invoke), ``VertexAIAnthropicConfig``,
and ``AzureAnthropicConfig`` (all call ``super().transform_request`` /
``AnthropicConfig.transform_request(self, ...)``). New static helper
``_sanitize_tool_names_in_request`` walks the already-Anthropic-shaped
``optional_params["tools"]`` (only ``type=="custom"`` entries -- hosted
tool names are reserved by Anthropic and must not be touched), builds
the per-request forward/reverse maps, and applies the forward map in
place to ``tools[*].name`` and ``tool_choice.name``. The reverse map is
stashed exclusively on ``litellm_params`` (which is never serialized to
a provider) under ``_anthropic_tool_name_map`` for the response paths
to consume.
Side effect of this restructure: ``map_openai_params`` is now a pure
OpenAI->Anthropic param translator with no side-channel state, which
matches its contract everywhere else in the codebase.
Tests: replaced the now-incorrect "stashes maps in optional_params"
tests with regressions that assert no underscore-prefixed keys appear
in either ``optional_params`` after ``map_openai_params`` or in the
final ``transform_request`` body. Added end-to-end coverage for:
sanitization in ``transform_request``, ``tool_choice`` rewriting,
historical ``tool_calls`` rewriting in messages, and hosted-tool
passthrough.
Made-with: Cursor
* fix(anthropic): always sanitize empty text content blocks
Anthropic 400s on `{"role": "user", "content": ""}` with:
"messages: text content blocks must be non-empty"
LiteLLM already had `_sanitize_empty_text_content` to rewrite empty text
to a placeholder, but it was gated behind `litellm.modify_params=True`.
With that flag off (default), empty content from upstream agent
frameworks (e.g. pydantic-ai) flowed straight through and tripped the
Anthropic validator.
Fix:
- Always run `_sanitize_empty_text_content` at the top of
`anthropic_messages_pt`, independent of `modify_params`. There is no
way to "pass through" an empty text block, so this is non-optional.
The richer tool-call sanitizations (Cases A/B/D, which actually
mutate conversation structure) remain gated on `modify_params`.
- Extend `_sanitize_empty_text_content` to also handle list-of-blocks
content (`[{"type": "text", "text": ""}]`), not just string content.
Adds 3 regression tests covering string content, list-of-blocks
content, and the no-op case (non-empty messages with modify_params off).
Made-with: Cursor
* fix(anthropic): drop dead tool-name forward-map params, fix mypy + caller-mutation
- remove unused `name_forward_map` param from `_map_tool_choice`,
`_map_tool_helper`, `_map_tools` and the `_apply_anthropic_tool_name_forward`
helper. Production sanitization runs in `_sanitize_tool_names_in_request`
at `transform_request`; these params were never threaded through.
- handler.py: use `ANTHROPIC_TOOL_NAME_REVERSE_MAP_KEY` constant instead of
the hardcoded `"_anthropic_tool_name_map"` string.
- fix mypy `"object" has no attribute "__iter__"` in
`_rewrite_tool_names_in_messages` by guarding `tool_calls` with
`isinstance(..., list)`.
- `_sanitize_tool_names_in_request`: build a new tools list with copy-on-
change entries (and copy `tool_choice` on rewrite) so a caller reusing
the same tool list/dicts across requests doesn't see its inputs
permanently rewritten.
- doc-comment `_build_request_tool_name_maps` clarifying it operates on
OpenAI-format tools (vs `_sanitize_tool_names_in_request` which runs
on Anthropic-format tools post-`_map_tools`).
- tests: drop 3 tests pinning the now-removed param paths; add coverage
for tool_calls + None function_call rewrite and caller-dict immutability.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix(mcp): inherit stored credentials in test/tools/list for edit flow
When editing an existing MCP server, the Tool Configuration preview
calls POST /mcp-rest/test/tools/list with server_id but no credentials
(management API redacts them). The endpoint now calls
_inherit_credentials_from_existing_server() so stored bearer tokens
and OAuth2 M2M credentials are loaded from global_mcp_server_manager
automatically — tools load without re-entering credentials.
New servers (no server_id) and requests with explicit credentials are
unaffected (function is a no-op in both cases).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(mcp): show all tools in edit panel, not just allowed tools
Edit flow was passing externalTools (from GET /tools/list, filtered by
allowed_tools) to MCPToolConfiguration, disabling the internal hook.
Remove the external props so the internal hook fires via
POST /test/tools/list, which returns all tools unfiltered. Combined
with the credential inheritance fix, tools load automatically without
re-entering credentials and all tools are visible for re-configuration.
existingAllowedTools still pre-checks previously allowed tools.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix order-dependent collision in _build_anthropic_tool_name_maps
Use a two-pass approach: first pre-register all already-valid tool names
in the 'used' set, then sanitize/disambiguate names that need rewriting.
This ensures valid names always have priority regardless of input order,
preventing duplicate tool names on the wire when e.g. 'foo/bar' appears
before 'foo_bar' in the tool list.
Add regression test for the reversed ordering case.
* Fix OpenAPI tool name collision: disambiguate sanitized names with numeric suffixes
sanitize_openapi_tool_name replaces all invalid chars with '_', but when
two operationIds differ only by sanitized characters (e.g. 'foo/list' and
'foo.list' both become 'foo_list'), the second registration silently
overwrites the first in the tool registry.
Add collision disambiguation in register_tools_from_openapi that appends
_2, _3, ... suffixes when a sanitized name is already taken, mirroring
the existing logic in _build_anthropic_tool_name_maps.
* Fix preview endpoint missing collision disambiguation for tool names
Add used_names tracking and _2/_3 suffix disambiguation to
_preview_openapi_tools, matching the logic in register_tools_from_openapi.
Without this, two operationIds that sanitize to the same string (e.g.
'foo/list' and 'foo.list' both becoming 'foo_list') would show duplicate
names in the preview while registration would disambiguate them.
* Align preview HTTP method order with register_tools_from_openapi
The preview endpoint and register_tools_from_openapi both use
order-dependent collision disambiguation (_2, _3 suffixes). When the
iteration order differs, two operations on the same path with sanitized
names that collide get different suffixes in preview vs registration,
so the dashboard shows names that don't match what actually got
registered.
Also adds a regression test that fails on the swapped order.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
* Skip duplicate originals in _build_anthropic_tool_name_maps
If the same invalid tool name appeared twice in original_names (e.g.
['foo/bar', 'foo/bar']), the second occurrence overwrote the forward
map entry with a freshly-suffixed name (foo_bar_2), leaving foo_bar
orphaned in 'used' with no reverse mapping. _sanitize_tool_names_in_request
then rewrote both tool entries to foo_bar_2, and Anthropic 400'd on
duplicate tool names.
Skip the rewrite if forward already has the original mapped.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
Greptile flagged two follow-ups on the OpenAPI/local-registry pre-call
check:
1. **P1 runtime crash via None proxy_logging_obj.**
`kwargs.get("proxy_logging_obj")` is `None` on the MCP entry path,
and `pre_call_tool_check` calls `proxy_logging_obj._create_mcp_request_object_from_kwargs`
unconditionally after the security checks, which would have crashed
every legitimate call with `AttributeError`. Source the logging
object from `litellm.proxy.proxy_server` the same way
`_handle_managed_mcp_tool` already does.
2. **P2 authorization-bypass window when mcp_server is None.**
Previously the new check was guarded by `if mcp_server is not None`,
so any local tool whose registry entry had no resolvable server (a
startup-race window before `_initialize_tool_name_to_mcp_server_name_mapping`
completes, or an orphaned registry entry) ran without the security
check. Tools registered via openapi_to_mcp_generator are always tied
to a server, so a missing one is a configuration/timing fault — fail
the call with 503 instead of dispatching unguarded.
Tests: existing two pass with an added assertion that
`proxy_logging_obj` is non-None at the call site, plus a new test that
covers the 503 deny branch when the tool→server mapping is missing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`execute_mcp_tool` dispatches in two ways: managed MCP servers go
through `_handle_managed_mcp_tool`, which calls
`MCPServerManager.pre_call_tool_check` to enforce allowed/banned tool
lists, key/team `object_permission` tool grants, and parameter
validation. OpenAPI-backed tools, however, were resolved via
`global_mcp_tool_registry` and dispatched directly to
`_handle_local_mcp_tool` — entirely skipping `pre_call_tool_check`.
A caller could invoke any registered OpenAPI tool regardless of their
key/team permissions, including administrative or destructive
operations on the upstream API.
Run `pre_call_tool_check` before the local-registry dispatch whenever
the resolved server is set (the same condition used to surface server
context to the managed path). Honor any guardrail-modified arguments
the hook returns. Errors raised by the hook propagate up before
`_handle_local_mcp_tool` runs.
Tests cover both directions: the pre-call hook fires when the local
tool resolves alongside a server, and a hook-raised HTTPException
prevents the local handler from being invoked.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Apply organization object_permission as a ceiling on allowed MCP servers
and tool permissions, consistent with vector store org checks.
Includes unit tests for org ceiling, intersection, and tool filtering.
Made-with: Cursor
Three follow-ups to the OAuth-discovery SSRF guard:
1. Greptile P1 (redirect bypass): the validated origin could return a
3xx whose ``Location`` points at an internal address, and httpx
would follow without re-checking the new target. Pass
``follow_redirects=False`` to both gated httpx GETs. Spec-compliant
OAuth/OIDC metadata endpoints serve the JSON directly, so this
doesn't affect legitimate providers.
2. Greptile P2 (empty getaddrinfo): POSIX doesn't strictly forbid an
empty success-list from ``getaddrinfo``. Add an explicit
``if not infos: return False`` so the guard fails closed instead of
falling through to ``return True``.
3. Mypy: ``info[4][0]`` is typed ``str | int``; narrow at the
boundary with an ``isinstance`` check (fail-closed if non-str).
Adds two regression tests verifying ``follow_redirects=False`` is
passed at both gated fetch sites, and one verifying the empty-list
case rejects the URL.
The OAuth discovery code in mcp_server_manager followed two
attacker-influenceable URLs without validation: the
``resource_metadata`` URL parsed out of a ``WWW-Authenticate``
challenge, and the ``authorization_servers[0]`` field of the
PRM JSON returned by the resource server. A malicious MCP server
could point those at a cloud-instance-metadata service, an internal
admin panel, or a loopback debug endpoint and the proxy would issue
a blind GET on its behalf.
Add ``_is_safe_metadata_url(url, server_url)`` and gate both follow-
up fetch sites on it. A URL is allowed when:
- it shares scheme + host + port with ``server_url`` (well-known
endpoints constructed from the admin's URL, and PRM published at
the resource server itself per RFC 9728 §3.3), or
- it resolves to publicly-routable IPs only (covers federated
authorization servers — Azure Entra, Google, Okta, GitHub —
hosted cross-origin from the resource server).
URLs that resolve to private / loopback / link-local / cloud-metadata
addresses, or that don't resolve at all, are rejected. ``http`` and
``https`` are the only schemes accepted. The IP block list is
provided by the existing ``_is_blocked_ip`` helper from
``litellm_core_utils.url_utils`` so the policy stays consistent with
the rest of the proxy.
The guard does not protect against active DNS rebinding between
this resolution and the subsequent httpx GET — the same-authority
pin remains the primary mitigation; the IP check is defence in
depth. The surface only triggers on config load / add-server, not
per request, so the synchronous ``getaddrinfo`` is acceptable.
Threads ``server_url`` through ``_fetch_oauth_metadata_from_resource``,
``_fetch_authorization_server_metadata``, and
``_fetch_single_authorization_server_metadata``. Existing tests for
those helpers updated for the new signature; new
``TestOAuthDiscoverySSRFGuard`` covers same-authority allow,
private-IP rejection across IPv4 and IPv6, multi-A-record dual-
stack rejection, unresolvable hosts, non-http schemes, and
end-to-end "no network call when guard denies".
Greptile P1: this PR encrypts LiteLLM_MCPUserCredentials rows under the
salt key, but the /key/regenerate rotation endpoint had no
corresponding step for that table. Rotating the master key would
leave every BYOK and OAuth2 user credential permanently unreadable.
Adds rotate_mcp_user_credentials_master_key, mirroring the existing
rotate_mcp_server_credentials_master_key pattern: read each row with
the current key (via _decode_user_credential, which also handles
unmigrated legacy plaintext rows), re-encrypt under the new master
key, write back. One bad row is logged and skipped instead of
aborting the whole rotation.
Wired into key_management_endpoints.py as step 4b, alongside the
existing server-credentials rotation, with the same try/except shape
so a transient DB error on this table doesn't kill the whole
regenerate-key flow.
Tests cover: round-trip through rotation under a new key, automatic
re-encryption of legacy plaintext rows (rotation also acts as a
migration trigger), and a corrupt row not aborting the rotation.
Greptile P1: deployments that today have ``use_x_forwarded_for: true``
but never configured ``mcp_trusted_proxy_ranges`` would silently see
their MCP OAuth discovery URLs revert to the proxy's literal bind
address after this change, with no log line explaining why.
Emit a one-shot WARNING the first time the gate denies for that
specific reason, telling the operator exactly which setting to add.
The warning is module-scoped (not per-request) so the proxy log
stays quiet after the first hit.
get_request_base_url unconditionally honoured X-Forwarded-Proto / Host /
Port to build OAuth issuer / redirect_uri / authorization_endpoint
values for the MCP discovery endpoints. In a deployment where the
proxy is reachable from a caller that can send those headers (direct
internet exposure, or a reverse proxy that does not strip them), an
attacker could poison the OAuth metadata and steer MCP clients at an
attacker-controlled host.
Apply the same trusted-proxy gate the codebase already uses for
get_mcp_client_ip: only honour the headers when use_x_forwarded_for is
enabled in proxy settings AND the direct connection IP falls inside
mcp_trusted_proxy_ranges. When that's not configured, fall back to
the request's literal base_url, so an untrusted caller cannot poison
the discovery metadata.
The existing X-Forwarded-* parsing test cases now opt into a
trust_xff fixture (the parsing logic itself is unchanged). Adds a
matrix for the new gate covering: XFF disabled, XFF enabled with no
ranges, caller outside ranges, caller inside ranges, and the
loopback-dev-deployment case.
Three minor fixes from Greptile review:
1. _decode_user_credential now also catches TypeError so a null
credential_b64 value returns None instead of propagating, matching
the documented "returns None when neither path yields a valid
string" contract.
2. The OAuth2 BYOK guard error no longer claims the existing row is a
BYOK credential — after a salt-key rotation, an OAuth2 row can fail
to decrypt and reach the same guard. Reword to "could not be
verified as an OAuth2 token", which is accurate for both cases.
3. Drop the no-op sys.path.insert in the new test file (other tests
in the directory don't need it; pytest picks up the package via
the installed editable wheel).
Adds a regression test for the None-input case.
LiteLLM_MCPUserCredentials.credential_b64 stored both BYOK API keys and
OAuth2 access tokens as plain urlsafe-base64 of the raw value. Any DB
read could recover the upstream-provider key.
Run all writes through encrypt_value_helper (nacl SecretBox, the same
helper used for the server-level credentials column) and read back via
a small dual-path helper that tries decryption first, then falls back to
plain base64 so existing rows keep working until they get rewritten.
Folds the three near-identical "decode -> json.loads -> check type ==
oauth2" sites into _decode_oauth_payload, which simplifies the BYOK
guard inside store_user_oauth_credential.
- server.py: drop the redundant server_id append in
_get_filtered_mcp_servers_from_mcp_server_names. iter_known_server_prefixes
already yields server_id unconditionally, so the manual append (and its
misleading comment) was a no-op duplicate.
- utils.py: rewrite the SHORT_MCP_TOOL_PREFIX docstring to accurately
describe the collision behaviour. The previous wording said collisions
were 'cosmetic only', but a natural-hash collision IS a routing-correctness
issue, which is precisely why we already added _assign_unique_short_prefix
to rehash deterministically. The new comment cross-references that path.
- utils.py: restrict the first character of the short prefix to [A-Za-z]
via a 52-char alphabet for position 0 only. The remaining two positions
still use the full base62 alphabet. This keeps prefixes valid identifiers
on every backend and gives 52*62*62 = 199_888 distinct prefixes (still
comfortably more than any realistic deployment).
- tests: add coverage proving the first character of the prefix is always
alphabetic across many server_ids and rehash attempts.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
Two MCP servers can natural-hash to the same three-character base62
prefix. With 62**3 = 238_328 slots the birthday bound is ~488 servers
for 50% collision probability, so a single proxy hosting more than
~100 MCP servers has a non-trivial chance of seeing a collision in
practice — and a collision means tool names from two different servers
share a routing key, causing silent mis-routing.
Mitigation:
- compute_short_server_prefix(server_id, attempt=N) folds an attempt
counter into the SHA-256 seed, so rehashes are deterministic and
produce a fresh three-char prefix space per attempt.
- New MCPServer.short_prefix field caches the resolved (post-dedup)
prefix on the model so it stays stable across the process lifetime.
- MCPServerManager._assign_unique_short_prefix walks attempts 0..N
until it finds a prefix not already used by another server in the
combined registry. Logs an INFO line when a rehash happens so
operators have a breadcrumb if it ever does.
- Wired into every registration path: load_servers_from_config,
add_server, update_server, reload_servers_from_database. The
database reload path also carries the previously-resolved prefix
forward so reloads don't churn it.
- get_server_prefix prefers the cached short_prefix when set, so the
resolved value (not the raw natural hash) is used everywhere.
- iter_known_server_prefixes yields the cached short_prefix too, so
reverse-lookup tolerance covers the rehashed form.
No-op when LITELLM_USE_SHORT_MCP_TOOL_PREFIX is disabled — the field
stays None and behaviour is unchanged.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
Adds LITELLM_USE_SHORT_MCP_TOOL_PREFIX. When enabled, tool / prompt /
resource / resource-template names emitted from MCP servers are prefixed
with a deterministic three-character base62 ID derived from the server's
server_id (SHA-256 → base62) instead of the (potentially long)
alias / server_name. This keeps namespaced tool names well under the
60-character upper bound enforced by some model APIs while still letting
us distinguish MCP-routed tools from local tools.
Behavioural notes:
- Default off — when the env var is unset, the long-prefix behaviour
is unchanged. The plan is to flip the default in a future release
and remove the gate after a deprecation window.
- Prefix derivation is deterministic, so it is stable across processes,
workers and restarts without any persistence layer.
- Reverse-lookup is tolerant: _create_prefixed_tools registers every
known prefix form (alias / server_name / server_id / short ID) in
the routing map and _get_mcp_server_from_tool_name resolves any of
them. Old clients holding cached long-prefixed names continue to
route correctly even after the flag is enabled.
- _get_allowed_mcp_servers_from_mcp_server_names accepts the short
prefix in /mcp/{server_name}-style URLs.
- The OpenAPI tool-listing path now filters by the active server
prefix instead of server.name so spec-backed servers benefit too.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
Greptile review feedback (P2): the two negative `.well-known`-substring
tests fell through to `_target_servers_use_oauth2`, which queries
`global_mcp_server_manager.get_mcp_server_by_name`. Without an explicit
mock the tests passed only because the real registry happens to be empty
in the test process. Mock the manager to return None so the assertion
exercises the fail-closed path explicitly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Greptile flagged a regression introduced in the previous commit's merged
exception handler: ``ProxyException.__init__`` normalizes ``code`` via
``str(code)``, so a ``code=None`` (valid per the type signature) becomes
the string ``"None"``. Coercing that with ``int(...)`` raises
``ValueError``, which propagates uncaught and rewrites the auth error as
an unhandled 500 — degrading security posture compared to the pre-merge
``str(e.code) in ("401", "403")`` shape.
Compare against both int and str forms of the auth-error codes instead
of coercing. Adds a regression test for the ``code=None`` case.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two related issues in `MCPRequestHandler.process_mcp_request`:
1. Public-route detection used `".well-known" in str(request.url)`, a
substring match against the full URL. Attackers could smuggle the
marker via the query string, hostname, or a deeper path segment to
bypass authentication on any MCP route. Replaced with an exact path
prefix on `request.url.path` (`startswith("/.well-known/")`).
2. The OAuth2 passthrough fallback (added in #20602 to support
`auth_type=oauth2` upstream MCP servers like Atlassian) caught any
401/403 from `user_api_key_auth` and replaced the result with an
anonymous `UserAPIKeyAuth()`. That fallback fired regardless of the
target server's configured `auth_type`, so an attacker presenting a
garbage `Authorization` header could exchange a failed LiteLLM auth
for an anonymous session against any server. The fallback now runs
only when EVERY MCP server the request targets is operator-configured
for `auth_type=oauth2`. For any non-oauth2 server (api_key,
bearer_token, basic, etc.), the auth error propagates as before.
Target resolution prefers the `x-mcp-servers` header when present
(including the explicitly-empty case, which fails closed) and otherwise
parses the standard `/mcp/{server_name}` and `/{server_name}/mcp`
transport URL patterns. Routes that don't match either form fail closed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`_check_byok_credential` previously returned silently when `prisma_client`
was None, bypassing BYOK ownership validation during database-outage
windows. Any proxy-authenticated user could invoke BYOK-protected MCP
tools without a stored credential during the outage window.
Now raises HTTP 503 with a structured error so the flow fails closed.
Regression test asserts 503 is raised when `prisma_client` is None.
Reported by @brodmart in GHSA-6762-2m23-5mxp.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR feedback (greptile P1 / veria high): with the previous change, a team
storing mcp_tool_permissions={"my-alias": ["read_file"]} would pass the
server-access check (because the alias expanded to a concrete id in the
allowed-servers list) but the per-server tool lookup still did
dict.get(server_id) against the raw name-keyed dict — missing, returning
None, which callers treat as "no restrictions" → all tools allowed instead
of only the declared ones.
Add MCPServerManager.expand_tool_permissions() that rewrites the dict so
every key is a concrete server_id where possible (tool lists from keys
pointing at the same server are unioned). Unresolved keys pass through
unchanged so stale id-keyed restrictions still apply when the same string
is used for lookup. Wire the helper into the four dict-lookup sites:
get_allowed_tools_for_server (key + team paths), the agent tool lookup,
and the rest_endpoints.py tool filter.
Also switch expand_permission_list to pass through unresolved entries
(rather than dropping them) so existing test fixtures that use bare string
placeholders continue to work. The downstream access check denies unknown
entries when compared to the concrete request server_id, so security
posture is unchanged.
Sanitize the debug log to use %r formatting so an admin-controlled
identifier with newlines can't forge log entries (CodeQL log-injection
warning).
team.object_permission.mcp_servers (and the per-key equivalent) previously
only accepted server_id strings. For config-loaded MCP servers, the id is
derived from a hash that includes the server URL, so the same logical
server in two regions ends up with two different ids in a shared database.
Permission lists had to enumerate every region's id.
Add a single MCPServerManager.expand_permission_list() helper that resolves
each entry against the current region's config + DB registry union: entries
that match a server_id pass through, entries that match an alias/server_name/
name expand to every matching id, and unresolved entries drop with a debug
log so stale or typo entries are diagnosable. Wire it into the four
_get_allowed_mcp_servers_for_* helpers so direct server entries and
mcp_tool_permissions dict keys are both expanded before the intersection.
Access-check outcomes are unchanged for existing id-based permissions;
name-based entries now resolve instead of being silently denied.
