* fix(mcp): reject initialize with 403 when the key grants no MCP servers
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* test(mcp): e2e expects 403 initialize for a key with no MCP servers
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(mcp): mention IP filtering in the no-servers initialize denial and keep zero-grant tool coverage
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: mateo <mateo@berri.ai>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(mcp): honor an explicit null on toolset update, cover MCP lifecycle e2e
PUT /v1/mcp/toolset dumped its payload with exclude_none, so a field sent as
null looked exactly like one the caller left out and the stored value
survived. An admin could not clear a toolset's description: the save reported
success and the old text came straight back. It now dumps with exclude_unset,
so absent keeps and null clears, which is what PUT /v1/mcp/server already did.
A null tools list clears the selection to empty, and a null toolset_name is
ignored because a toolset always has a name.
Adds create, read, partial-update, clear and delete e2e coverage for MCP
servers and toolsets, with every read-back polled on every replica so an edit
that lands on one replica and not another fails the test, plus an enforcement
test proving a key granted a toolset lists exactly that toolset's tools
against the real Datadog upstream.
* fix(e2e): refuse a read-back that no replica serves
A read-back over an empty replica mapping satisfied every predicate and
returned as if it had converged, so it would have asserted nothing and
passed. No wiring can produce that today, since the replica list always
falls back to at least one URL, but a helper whose whole job is proving a
write reached every replica should not have a shape that passes vacuously.
* fix(mcp): keep a null tools list a no-op on toolset update
Treating a null tools list as a clear meant an existing client that sends
tools=null during a partial update, meaning "leave the selection alone",
silently lost every tool the toolset grants. That is a permission surface,
so the quiet version of it is the worst version.
A toolset always has a tool list, the same way it always has a name, so a
null on either is now a no-op. Emptying the selection is an explicit [],
which cannot be confused with a field the caller left out, and which is
what the dashboard already sends.
* fix(e2e): keep MCP admin routes on the data plane
/v1/mcp/* is a lazily mounted feature, so a gateway registers it on the first
matching request, which happens after the startup route trim that drops
management endpoints. Routing it to the control plane therefore sent every MCP
call to the one backend process: the new lifecycle read-backs proved a single
process rather than every replica, and mcp_client's await_registered barrier
waited on a registry that does not serve the tools/list call it guards, so the
existing MCP suites polled a gateway that had not synced yet until poll_timeout
Verified against a two-gateway split stack (backend on 4001, gateways on 4010
and 4011, one postgres): both gateways answer /v1/mcp/server and /v1/mcp/toolset,
and each served 6 server reads and 7 toolset reads over the run
* fix(e2e): grant the toolset by the tool's own name, not the wire name
tools/list serves a tool as <prefix><tool_name>, but a toolset grants by the
tool's own name: resolve_toolset_permissions reads toolset.tools[].tool_name
straight through, and the prefix is added on the way out. The test built the
toolset from the names tools/list reported, so the grant matched nothing, the
scoped key listed no tools, and await_tools ran out its whole poll_timeout
before failing
Measure the prefix off search_datadog_logs, whose own name is known, rather than
guessing it from the alias, since the proxy can be configured to prefix with a
short server id instead. The expectation compared against tools/list stays in
wire names; only what the toolset stores crosses back
* test(mcp): build immutable lifecycle updates and replica results
* test: validate opaque stream IDs and hide log-reader credentials
* test: isolate auto-router scenarios and clean partial setup
* test: honor Datadog search rate-limit reset headers
* test: share the Datadog read-back deadline across retries
* test: preserve captured MCP toolset update fields
* test(mcp): exercise /mcp/proxy authorization against the real registry instead of patched manager methods
* fix(mcp): preserve proxy logging and authorization coverage
* test(mcp): respect the proxy FastAPI import boundary
* feat(otel v2): send a key's or team's whole trace to its own destination
A key or team that configures its own Langfuse, Arize, Weave or New Relic
credentials used to get a single detached span in its account while the rest of
the request trace stayed on the operator's backend, so neither side held a
complete trace. Resolve the destination during auth, forward every span of the
request to it, and hold the same request back from the operator's exporter for
that backend, so the tenant gets the tree the operator would have seen and the
operator gets nothing for that request.
Also let a credential-mandatory preset build without the operator's own env
credentials. Without that, a proxy whose teams each bring their own account fell
back to the legacy integration and never ran a line of the v2 path.
* fix(otel v2): validate tenant destinations and match each backend's own endpoint
Review round on the tenant destination routing.
- A key/team Langfuse host is user-supplied input, so it goes through the
proxy's SSRF guard. A private address is refused, the operator keeps the
trace, and the warning names user_url_allowed_hosts. The operator's own
LANGFUSE_HOST is not checked.
- Arize and Weave destinations now resolve their endpoint and transport
through the backend's own config, so an ARIZE_HTTP_ENDPOINT collector and
a self-hosted WANDB_HOST are honoured instead of the cloud default.
- A half-configured backend no longer resolves: several dynamic header
builders gate each credential separately, so an api key with no space id
produced a non-empty but unusable header set that suppressed the
operator's exporter.
- A callback_type of "failure" no longer takes over the trace. The
destination is resolved during auth, before the outcome is known.
- The fan-out cache evicts without shutting the processor down, matching
ArizePhoenixLogger: a concurrent on_end may still hold it.
- The stdout placeholder is identified by what it does rather than by
equality with an import-time default, so an operator's OTEL_EXPORTER_OTLP_*
collector survives the credential-less path.
* refactor(otel v2): reuse the proxy's own destination allowlist for tenant hosts
A tenant-supplied Langfuse host is the same threat as a URL-valued `model`, so
it now goes through `is_url_destination_allowed_by_host` against
`provider_url_destination_allowed_hosts` instead of a second, DNS-based check
of its own. The DNS lookup would have blocked the asyncio auth path on a
hostname the caller picked, and its cached verdicts could blackhole a real host
after one resolver blip.
Evicting a destination processor now retires it to drain rather than shutting it
down, since `on_end` hands a processor back and exports outside the lock. The
retirees are capped so they cannot accumulate a thread each.
`credential_gated_exporters` tells the synthesized stdout placeholder from a
real exporter by transport rather than by the literal kind `console`, so an
unrecognized kind is not mistaken for a configured collector, and an exporter
the operator did configure survives. That also stops a weave test's env writes
from making this look like a real OTLP exporter later in the same CI worker.
* fix(otel v2): read the tenant's stored callback config the way the sibling parser does
Three divergences between the destination resolver and
`convert_key_logging_metadata_to_callback`, which read the same stored config:
- A key whose callbacks are disabled stores an empty list, and `or` treated that
as "the key configured nothing", so the request inherited the team's
destination. The sibling parser treats an empty list as configured.
- Two entries naming one backend now merge their `callback_vars` last-wins,
matching the sibling, instead of the resolver taking the first entry and the
per-request tracer routing taking the last.
- `credential_gated_exporters` dropped any exporter whose kind had no transport,
which also dropped an `in_memory` exporter the operator asked for. The
placeholder is the spec with every field still at its default, so that is what
the predicate now says.
Arize's `allow_missing_credentials` branch was unreachable: `get_arize_config`
resolves every credential with `os.environ.get` and always supplies an endpoint,
so it never raises. Dropped it and corrected the protocol docstring.
* fix(otel v2): keep the destination merge immutable
The per-backend var merge seeded a plain dict and the gated exporter list a plain
list, both of which the LIT budget counts. Wrap the merge in MappingProxyType and
hand the exporters back as a tuple.
* fix(otel v2): scope the fan-out to its own backend and close shed processors off the export path
Three problems in the fan-out, two of them in the eviction added last round:
- Every v2 logger carries its own provider and emits its own copy of a gen-AI
span, so a proxy running two of them handed the tenant the same model call
twice. A provider now forwards only destinations for the backend it speaks
for; the tenant's own backend always has a logger, since naming it in the key
or team config is what builds one. Reproduced live against a self-hosted
Langfuse on an arize-only proxy and on the bare `otel` callback.
- Eviction could close a processor another thread was still exporting through,
which drops that span. Exports are now counted, and a retired processor is
closed only once its count reaches zero.
- That close ran inside `on_end`, where `shutdown` flushes over the network, so
one unreachable tenant collector stalled every other tenant's spans. It now
runs on a short-lived thread, which also retires the retiree cap: a retiree
drains as soon as its export finishes.
* fix(otel v2): deliver tenant destinations from the published global provider
Scoping the fan-out by callback name in the previous commit left every backend
that is not the canonical logger with a one-span trace: only the published
global provider sees the FastAPI server span, the auth span and the post-call
database spans, so an arize-only proxy handed a team's Langfuse just the model
call. Attach the fan-out once, to that provider, and let it forward every
destination.
An overridden backend now skips per-request tracer routing outright rather than
only clearing its credential headers, since a key or team otel_service_name was
still enough to detach the model call onto a second provider. The destination
carries that service name as a resource attribute instead.
Shed processors drain on a two-thread pool rather than a thread each, so a
tenant cycling its destination config cannot spawn threads as fast as it sends
requests.
* fix(otel v2): drain shed destination processors on daemon workers
A ThreadPoolExecutor joins its workers at interpreter exit, so one unreachable
tenant collector would hold the whole proxy open for its export timeout on the
way down. Two long-lived daemon workers off a queue keep the thread count
bounded without blocking shutdown.
* fix(otel v2): give the fan-out its own drain pool instead of a lazy singleton
functools.lru_cache does not hold a lock across the call it caches, so
concurrent first evictions each finish building a queue and start its
workers, and every queue but the winner is abandoned with two daemon
threads blocked on it forever.
* fix(otel v2): close no destination processor under a span still in flight
The fan-out now refuses new work once shutdown starts and waits out the
spans already being forwarded, so teardown neither drops a trace mid-forward
nor hands the next caller an exporter nothing will ever close. The wait is
bounded so a dead collector cannot hold the proxy open.
* fix(otel v2): retire the drain workers with the fan-out that started them
A proxy that rebuilds its telemetry builds another fan-out, so workers that
outlive the one that started them are two more threads per reload. Shutdown
now retires them once everything queued is closed, and a processor shed
afterwards is closed inline rather than queued to nobody.
* fix(otel v2): guard the fan-out's closed state with the lock that gates it
An Event read on its own leaves room for shutdown to run in the gap. A cache
miss then inserted a live exporter into a map that had been cleared, and a
shed processor landed behind sentinels every drain worker had exited on.
The drain pool takes its queue by injection so both interleavings are
reachable from a test without patching.
* feat(otel v2): let a tenant destination export alongside the operator's own
Override stays the default: a key or team destination replaces the operator's
exporter for that backend. Operators running one org-wide backend across every
team set litellm_settings.otel_tenant_destination_mode to additive, and the
same trace lands in both places. A team that names the operator's own project
is still written once, since the fan-out skips a destination the operator's
exporter is already sending that span to.
* fix(otel v2): let a straggling export close its own destination processor
Shutdown waits out the exports in flight, but the wait has to be bounded or a
tenant collector that stops answering holds the proxy open on the way down.
Past the bound it closed everything anyway, which is the case it was written to
avoid: a processor closed under the span it is carrying loses that span.
Keep the bound and retire the stragglers instead. The thread still exporting one
closes it through the drain as soon as its export returns, so teardown stays
bounded and no span is dropped mid-forward.
* fix(otel v2): identify a destination account by its credentials, not its header names
Under additive the fan-out skips a destination the operator's own exporter
already writes to, so the same account is not written twice. It compared header
names as well as values, and one account answers to more than one spelling:
the operator's Arize exporter sends space_id where a team destination sends
arize-space-id, so every span landed in the operator's own space twice.
The credentials are the identity. Compare those and leave the spelling to each
backend.
* fix(otel v2): keep the credential's role in a destination's account identity
Comparing values alone folds two accounts together whenever they hold the same
strings in different roles, and the second team would then get no trace at all.
Compare the credential under a normalized name instead, and fold the one alias
that actually exists: Arize's space_id and arize-space-id.
* fix(otel v2): build one destination processor per destination, not per racing span
Building outside the cache lock meant a cold cache met by a burst of concurrent
requests constructed an exporter per thread, kept one, and handed the rest to the
drain, so a batch worker and a connection pool per losing thread sat in a queue
two workers service.
Build under the lock that reads the cache. Opening an exporter connects to
nothing, so the lock is held for a constructor, once per destination, and the
race it was avoiding stops existing.
* fix(otel v2): bound the teardown that closes a destination, not the one that never blocks
The five-second bound guarded the wait for spans still inside on_end, but a
batching processor's on_end only queues the span and returns, so that counter is
empty and the bound engaged against nothing. The blocking half was the serial
close, which flushes over the network and joins the SDK's own worker thread with
no timeout of its own, so a single tenant collector that answers and never
finishes held process teardown open for as long as it liked.
Hand every close to the drain, whose workers are daemons, and give the whole
teardown one deadline.
* fix(otel v2): preserve operator spans on destination failure
* fix(otel v2): anchor destinations off the published provider, refuse headerless tenant transports
set_tracer_provider keeps the first provider it is handed, so a process whose
OTel global was claimed before the proxy published (auto-instrumentation, a
legacy logger) had no fan-out on the global and auth anchored no destination.
Auth now reads the fan-out off the registered logger's own provider.
A destination whose protocol maps to a headerless exporter kind is no longer
buildable: the console fallback would drop the tenant's credentials and print
the spans to stdout while the operator's exporter stood down for them.
* fix(otel): anchor tenant fan-out to the published provider
A legacy v1 logger can occupy proxy_server.open_telemetry_logger, in which case
the proxy publishes with registered=None and the fan-out lands on a v2 logger
taken from _in_memory_loggers. Reading the registered slot found no v2 logger
and the OTel global belonged to v1, so auth refused every tenant destination.
* fix(otel): preserve registered provider fallback
* test(otel): cover pre-publish provider fallback
* fix(otel): attach fan-out on fallback provider
* fix(otel): serialize first fan-out attach
* fix(otel): keep the operator's database endpoint out of tenant traces
A database span forwarded to a key or team destination carried the proxy's own
Postgres host, port and schema, and on failure the Prisma error text naming them.
The fan-out now hands tenants a view of each database span without those keys,
its events or its status text, while the operator's own copy is untouched and
model endpoints such as server.address on the LLM span still travel
* fix(otel): keep relabelled spans in the fan-out and honour disabled callbacks for destinations
A key or team otel_service_name used to move a backend's span onto a second
provider even when another backend had a destination, so the fan-out never saw
the model call and the tenant's trace lost it. A service name alone now stays on
the published provider whenever the request has a destination; credential and
project routing to a tenant's own account is unchanged
Destinations now skip a backend the request disabled dynamically, reading the
x-litellm-disable-callbacks header and the key's litellm_disabled_callbacks with
the same precedence and premium gate dispatch applies, so a disabled backend is
neither delivered to nor withheld from the operator
* test(otel): project routing survives a sibling backend destination
* docs(otel): state why a disabled backend still routes its own span
* fix(otel): keep a degraded backend's spans off a collector another v2 logger already serves
* test(otel): a credentialed preset beside another v2 logger keeps every exporter
* fix(otel): keep credentialless fallback on base path
* test(otel): cover legacy callback carrier rejection
* fix(otel): preserve valid exporter beside gated preset
* fix(otel): avoid console export without operator destination
* refactor(otel): share the console placeholder check with the presets
* fix(otel): bound shed destination processors waiting on a dead collector
* fix(otel): preserve explicit console exporters
* fix(otel): avoid mutable field-set construction
* fix(otel): close drain saturation race
* fix(otel): drop captured request headers from tenant spans
* test(otel v2): give the newrelic dispatch tests operator credentials, since a credential-less preset now falls back
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(otel): rebuild an anchored destination's processor past drain saturation
A destination deliverable() accepted at auth can be evicted by other tenants' auths
before its request's spans end, and that eviction is what tips the drain over. The
saturation gate then refused the rebuild at on_end, and with the operator's exporter
already stood down for that backend the span went nowhere. The gate now applies only
while a request decides whether to anchor
* fix(otel): hold destination eviction while the drain is saturated
An anchored destination evicted by other tenants' auths is rebuilt on its
next span, and that rebuild evicted another anchored one, so with more
destinations in flight than the cache holds every span cost one more
processor, one more batch thread and one more close queued behind a collector
that never answers. Eviction now holds while the drain is saturated, so the
cache keeps one entry per destination in flight and trims back to its cap on
the next hit or build once the drain has room
* fix(otel): keep the proxy's own error text out of tenant traces
A tenant destination received every span the request produced, error text
included, so a Prisma failure during auth handed a team admin's collector the
operator's Postgres endpoint, and the exception event on any failed span carried
a stack trace naming the proxy's install paths.
Spans the tenant's own call produced (the model call, MCP, guardrails) keep their
error text. Every other span keeps the failure without the prose: its type, its
provider error code and its status code, with the message, the events and the
status description dropped. Stack traces come off every span, attribute and event
alike.
A destination's resource attributes now merge onto the span's resource instead of
rebuilding one per span, which was re-running resource detection on every export.
* fix(otel): redact tenant URL query parameters
* fix(otel): close final tenant routing gaps
* fix(otel): refresh destinations for stateful MCP messages
---------
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
An aggregate gateway DCR authorize whose RFC 8707 resource resolves to exactly one
gateway-managed oauth2 server sealed that server into the flow and then sent the browser to
the generic connect grid anyway, so the user had to find the server the client had already
named and click Connect.
The connect URL now carries only the flow handle. GET /authorize/flow classifies the sealed
flow as unscoped, interactive, M2M, or stale, and returns the matching state to the page.
Interactive flows require a live per-user vendor credential before minting and do not burn the
flow on an early submit. M2M flows use the gateway's configured service credential and finish
without an interactive OAuth trip. Stale flows fail closed instead of becoming unscoped.
The existing explicit Finish action and a new Cancel path preserve deliberate user intent.
Take staging's test_bedrock_knowledgebase_hook.py, which drops the duplicate
embedding_executor parameter that turned the lint check red, and make the two
cross-module helpers this branch added public (raise_denied_scoped_mcp_access
and routes_through_gateway) so the private-usage budget stays at its base count
Encrypt secret maps at the shared persistence boundary, preserve plaintext API/runtime views, and extend rotation and migration scanning to legacy rows.
Co-authored-by: Claude Code <noreply@anthropic.com>
* feat(skills): semantic search over the LiteLLM-hosted skill registry
Adds GET /v1/skills?query= (custom_llm_provider=litellm_proxy) and a
skill_search MCP virtual tool, ranking the caller's accessible skills by
semantic similarity, mirroring the A2A agent registry search (LIT-6309).
Also fixes a pre-existing bug where create_skill() dropped description and
instructions for the litellm_proxy provider, which left every LiteLLM-hosted
skill with no searchable text.
* fix(mcp): coerce skill_search top_k instead of raising 500 on malformed input
The MCP-REST skill_search dispatch validated raw tool arguments through a
pydantic model directly, so a non-numeric top_k raised a ValidationError
that the endpoint's catch-all turned into an HTTP 500. Mirrors the
agent_search branch's tolerant coerce_top_k handling instead.
* fix(skills): enforce key limits on search embeddings and bound the semantic index
Semantic search embeddings now run the same pre_call_hook the /embeddings
route runs, so key rate limits, budgets and guardrails apply before the
embedding model is called. The shared SemanticTextIndex caps cached vectors
and evicts the least recently searched entries, and each skill's embedded
text is capped so one skill cannot inflate the embedding batch
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(skills): surface proxy 429s from search embeddings instead of a 503
ProxyRateLimitError is also an OpenAIError, so the search engine was folding
a key rate limit into skill_search_unavailable. Proxy HTTPExceptions now
propagate so the caller gets the same 429 the /embeddings route returns
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(skills): import assert_never from typing_extensions for Python 3.10
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(skills): embed the request as the pre-call hooks returned it, not the original text
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* refactor(skills): keep the litellm_proxy provider check for GET /v1/skills?query= inside llms/
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* test(skills): move the GET /v1/skills?query= endpoint tests under tests/test_litellm/proxy
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* feat(proxy): resolve root_path per request from SERVER_ROOT_PATHS
One deployment can encode exactly one client-visible URL path prefix
today: SERVER_ROOT_PATH is a scalar stamped onto the app at startup, so
a pod fronting several ingress prefixes 404s every prefix but one before
any handler runs, and MCP OAuth discovery can emit only one prefix's
URLs (RFC 9728 section 3 exact-match fails for the rest).
Add an opt-in outermost ASGI middleware that matches the request path
against a configured prefix list (SERVER_ROOT_PATHS, comma-separated) on
a segment boundary and sets scope["root_path"] for that request only.
Everything downstream is stock Starlette: route matching strips
root_path so routes stay registered root-relative, and request.base_url
re-includes it, so the discovery documents' resource and the 401
challenges' resource_metadata land under the prefix the client actually
called — with no discovery-builder changes.
LazyFeatureMiddleware now strips the scope root_path (falling back to
the cached SERVER_ROOT_PATH scalar) before feature prefix matching, so
lazily-registered routers — the MCP OAuth discovery router among them —
load under per-request prefixes.
Follow-up to the routing discussion on #35226; composes with, but does
not depend on, #35576.
* fix(proxy): import Sequence from collections.abc (ruff UP035 strict-budget gate)
* review(greptile): trim implementation commentary; fixture-own MCP registry state in tests
Addresses both P2s from the first Greptile pass:
- per_request_root_path_middleware.py (and the related _lazy_features /
proxy_server comments) cut down to the constraints the code cannot
express, per repo comment guidance
- the new discovery tests no longer clear/repopulate the shared MCP
registry inline; a fixture snapshots it, hands the test an empty
registry, and restores it afterwards so no state leaks between cases
* fix(lint): mutable-ok marker on the prefix accumulator (LIT002 type-discipline gate)
* fix(proxy): tie 401 challenges and get_custom_url to the per-request root_path
The per-request root_path middleware sets scope["root_path"] to the
prefix the client actually called, but the OAuth 401 challenges
(raise_user_oauth_challenge / raise_token_exchange_challenge) still
built their resource_metadata from SERVER_ROOT_PATH. On a pod fronting
several prefixes, the challenge advertised a discovery URL under a
different prefix than the discovery document served — the two
disagreed on where the resource metadata lives, and a strict RFC 9728
client refused the challenge. Route the challenges through a small
ContextVar the middleware populates so they read the same effective
root_path Starlette resolves the request under.
The same accessor fixes get_custom_url: when a request lives under a
SERVER_ROOT_PATHS-matched prefix, request.base_url already carries it,
so appending the SERVER_ROOT_PATH scalar on top produced e.g.
/tenant-a/legacy/sso/callback — a path that does not exist. Reading
the per-request prefix instead (and relying on join_paths's tail-dedup)
keeps SSO login/callback URLs under one prefix — the one the request
actually arrived on.
Fallback: outside a request (module-load-time UI URL builders,
background tasks) the ContextVar is unset and the accessor reads
SERVER_ROOT_PATH, matching get_server_root_path() so scalar-only
deployments are byte-identical.
* fix(mcp): challenge URL under per-request prefix must route, and mock parity
Two follow-ups to the review fix that made the 401 challenge use the
per-request root_path:
1. oauth_protected_resource_path must pick the URL structure that
actually routes for the mechanism in use:
- The scalar SERVER_ROOT_PATH deployment registers the well-known
routes with the prefix INSERTED (via well_known_root_suffix at
import time), matching RFC 8414 §3. The challenge URL must use the
same insertion or a client fetching it 404s.
- The per-request SERVER_ROOT_PATHS deployment can't register routes
per prefix; PerRequestRootPathMiddleware strips the prefix from
scope["path"] and the router matches the un-inserted route. The
URL must place the prefix BEFORE .well-known so the strip leaves a
matching path.
The previous fix used the insertion form for both, which 404'd the
discovery fetch on the per-request path — the discovery doc and the
challenge would then disagree on where the resource metadata lives,
the very failure the review flagged. End-to-end verified: the URL
the challenge advertises routes and the doc's `resource` field
equals the URL the client originally called (RFC 9728 §3).
2. get_request_root_path now delegates its fallback through
get_server_root_path() instead of reading the env directly, so every
existing `monkeypatch.setattr("litellm.proxy.utils.get_server_root_path"`
test override keeps working. This unstubbed the mock on the /v2/login
test that failed on the last CI run.
Plus the lint budget: annotate the local accumulator Final, tag the
scope["root_path"] rewrite as an intentional ASGI-contract mutation,
tag the reused `path`/`root_path` rebinds in LazyFeatureMiddleware, and
add reason strings to the two new PLC0415 lazy-import noqas.
* test(mcp): pin the reviewer's expected end-state — challenge URL routes, resource matches called URL
End-to-end regression test that mounts the discoverable router + the
per-request root_path middleware, hits an MCP endpoint that raises
raise_user_oauth_challenge, fetches the resource_metadata URL the
challenge advertises, and checks the returned document's `resource`
equals the URL the client originally called (RFC 9728 §3 exact match).
Covers /tenant-a, /tenant-b, and the unprefixed path on the same app so
a regression on any prefix — challenge URL 404s, or doc emits a
different prefix than the client called — fails at this test rather
than in a strict MCP client's discovery.
---------
Co-authored-by: gym-cmd <186399764+gym-cmd@users.noreply.github.com>
* fix(mcp): apply key and team guardrails to MCP tool calls
Guardrails attached to a virtual key or team were only enforced on LLM
routes. The synthetic request built for MCP tool call guardrail hooks
carried no guardrails in its metadata, so a guardrail with default_on
false never ran on tools/call even when the key explicitly listed it.
Resolve key, team, and project guardrails onto the synthetic request
with the same helper the chat path uses.
* fix(mcp): pass project metadata through without a mutable default
* fix(mcp): mark the request dict parameter mutable-ok with a reason
* test(mcp): explain the premium_user patch and tighten the helper docstring
The sidebar and header were still keyed on legacy ?page= ids and mapped
back and forth through MIGRATED_PAGES, legacyPageHref and
legacyKeyForPathname. Leaves are now plain Next links to their path
route, the active item and breadcrumb come from usePathname, and the
setPage/defaultSelectedKey prop chain is gone.
The id-to-route table moves next to the dashboard root page as its only
consumer. That redirect now forwards the remaining query params instead
of dropping them, so deep links such as the proxy's MCP env-var setup
link (?page=mcp-servers&fill_env_vars=) no longer rely on the target page
reading the pre-redirect URL during its first render. The proxy builds
that link as /ui/mcp-servers?fill_env_vars= directly, and the Playground
warnings link to the real routes instead of relative ?page= URLs.
migratedHref is renamed uiHref, the /ui base-path helper it always was.
* fix(mcp): scan and mask MCP tool call arguments in unified guardrails
A guardrail configured with mode pre_mcp_call was handed only a synthetic
tool definition (name plus an empty parameters schema), so it never saw the
argument values it was configured to inspect, and any rewrite it returned was
discarded. Detection could not fire and masking could not take effect, while
the applied-guardrails metadata still reported the guardrail as having run.
Pass every string leaf of the tool call arguments as texts, and fold the
guardrail's rewritten leaves back into modified_arguments, which is the channel
the MCP call path reads to decide what to send upstream. The leaf walk reuses
the json_string_leaves / with_json_string_leaves helpers the tool result path
already uses, so both directions share one bounded traversal.
Two guardrails running concurrently under run_in_parallel scan the same payload
snapshot, so each returns a full replacement derived from the original leaf.
Rewrites of the same leaf to different values are rejected rather than silently
losing one redaction; a leaf that already holds this guardrail's own replacement
is convergent and still masks, which is what the bundled content filter does
when it rewrites the arguments itself as well as through texts.
* fix(mcp): annotate guardrail argument rewrites
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(tests): isolate MCP guardrail callback state
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* chore: ratchet LIT010 budget after merge
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(tests): remove duplicate Bedrock hook parameter
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(mcp): fail closed when guardrail rewrites cannot be mapped to MCP arguments
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(tests): patch the guardrail translation mappings cache where staging now keeps it
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* feat(mcp): warn when an oauth2_id_jag server outruns the SSO provider's assertion capture
Only the generic OIDC login path captures the IdP id_token that an oauth2_id_jag MCP
server spends as its RFC 8693 subject token. Under Google, Microsoft, SAML or no SSO at
all, registration succeeds and then every ID-JAG credential resolution fails for every
user, with nothing in the logs, the config or the API response to say why.
Report the mismatch from the two places it is knowable: when an oauth2_id_jag server is
created or updated through the management endpoint, and at SSO callback time when a login
hands the arm nothing while such a server is registered. Provider selection mirrors the
callback's precedence, so a generic client id sitting behind GOOGLE_CLIENT_ID does not
clear the warning.
* test(sso): update merged CLI diagnostic patch target
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* feat(mcp): warn about the ID-JAG capture gap for config-declared servers and on the SSO debug page (#39350)
* feat(sso): surface the ID-JAG capture gap on the SSO debug page
/sso/debug/callback is where an operator lands when they are already trying to work out
why ID-JAG is failing, so the reason belongs on it. The annotation appears only when the
active SSO provider captures no identity assertion AND an oauth2_id_jag server is
registered for that gap to break; a deployment without both renders the page it rendered
before, byte for byte. Only the provider name and the remedy are rendered, never a
configured value, and an unreachable MCP table costs the page its annotation rather than
the page itself.
The payload carries the one mutable-ok in this work. Conditionally including a member of a
JSON document has to construct a mapping, and the rejected alternatives are recorded on the
helper so the next reader does not rediscover them.
Held out of the diagnosability PR deliberately: that PR is already reviewed and green, and
this surface ships with the remaining config-load warning as one follow-up.
* feat(mcp): warn at config load when an oauth2_id_jag server outruns the SSO provider's assertion capture
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* refactor(sso): trim comments on the ID-JAG debug page diagnostic
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(sso): clean up merged imports
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(sso): satisfy type discipline for diagnostic payload
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* style(sso): keep the optional ID-JAG payload member on one line for ruff format
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(sso): use Python 3.10-compatible assert_never
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: Yassin Kortam <yassin@berri.ai>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(sso): keep the ID-JAG capture-gap diagnostic out of the unauthenticated debug page
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* test(sso): inject the retention check and log via caplog so the ID-JAG tests pass the test-quality gate
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* test(sso): keep the debug-page outage test on the capture-gap path
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(sso): annotate the retention check type alias
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* feat(mcp): renew the stored SSO identity assertion behind ID-JAG
The oauth2_id_jag arm asserts the id_token captured at the user's last
interactive SSO login, and nothing ever renewed it, so an agent holding a
brokered LiteLLM key could act for that user only until that token's exp.
The assertion already carried the IdP refresh token beside it; this
redeems it.
RefreshingSSOAssertionStore wraps the database reader and satisfies the
same protocol, so the egress arm is unchanged. Renewal is lazy and
single-flighted per user through the same RefreshCoordinator the
authorization_code arm uses, since an IdP that rotates refresh tokens
treats two concurrent redemptions as replay. A refusal leaves the expired
assertion in place so the reader still challenges the user; an
unreachable IdP surfaces as a store outage instead.
* fix(mcp): let a cross-replica loser settle the SSO assertion renewal itself
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(mcp): satisfy type discipline lint budget
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* chore(ci): rerun checks after docs main added the missing router setting row
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(mcp): answer a cross-replica loser retryable instead of re-electing it
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(mcp): bypass stale assertion cache during renewal
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* chore: ratchet type-discipline budget after merge
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: Yassin Kortam <yassin.kortam@gmail.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(mcp): let config.yaml MCP servers pin server_id
A config-defined MCP server's id is a hash of server_name|url|transport|
auth_type|alias, recomputed on every config load, so editing any of those
fields mints a new id. Every key and team granted the old id via
object_permission.mcp_servers keeps pointing at an id that no longer exists,
and the server disappears from tools/list for them with nothing logged.
load_servers_from_config now uses an explicit server_id from the server's
config entry when present and falls back to the existing hash otherwise, so
grants survive url/name/alias edits. Rejected at config load: a blank or
non-string server_id, two entries claiming the same id, a pinned id already
held by a database-backed server, and a pinned id that is another entry's
server_name or alias (expand_permission_list matches ids before names, so
that one would capture the other server's grants). Because the database
registry loads after the config on startup, a database row that lands on a
pinned config id is reported as a warning from the database reload instead,
where it is decidable; the warning is latched on the shadowed set so the
config-reload timer does not reprint it every interval.
Deployments that do not set server_id keep the exact id they have today.
* fix(mcp): close two more pinned-id capture paths
A pinned server_id equal to an alias supplied through litellm_settings
mcp_aliases was accepted, because the collision index only held the entry's
own alias field. expand_permission_list matches ids before names, so grants
written for the aliased server resolved to the pinning one. mcp_aliases keys
whose target is a config server are now reserved the same way.
A pinned server_id equal to a database-backed server's name, server_name or
alias had the same effect against the database side, and could not be
rejected at config load because the database registry is not loaded yet. The
database reload now warns about it, latched like the existing shadow warning.
* fix(mcp): reserve only the aliases the loader actually assigns
Reserving every mcp_aliases key targeting a config server was too broad in
two ways: the mapping is ignored when the entry sets its own alias, and only
the first mapping for a server is ever applied. Both cases made a pinned
server_id that could never have collided abort proxy startup. Reserve only
the name load_servers_from_config will really assign.
The database capture warning also fired for a database server whose own id is
the config server_id. There the database row wins the id outright through
get_registry precedence, so the shadow warning above it is the accurate one
and the capture message contradicted it. Skip those rows.
Also mark the two litellm-internal patches in the reload test helper, which
the test-quality gate counts; the database reload has no other seam.
* fix(mcp): match the loader's alias check exactly, is None not falsiness
load_servers_from_config consults mcp_aliases only when the entry has no
alias key at all, so an entry setting alias: "" gets no mapped alias. The
collision index used falsiness and reserved the mapped name anyway, which
failed startup on a pinned server_id that could never have collided with it.
* fix(mcp): skip one identifier, not the whole database row
A database row can shadow one config server_id by id and capture another by
name at the same time. Skipping the entire row when its id shadowed a config
entry dropped the second warning, leaving the operator with half a diagnosis.
Skip only the identifier equal to the row's own id.
* fix(mcp): reject conflicting self-pinned server ids
* fix(mcp): validate config server names before building the identifier index
The collision check reads every entry's body up front, so a malformed
entry under an invalid name surfaced as an AttributeError instead of the
name validation error the loader gave before this change.
* perf(mcp): cache SSO identity assertion reads on the ID-JAG path
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(mcp): guard sso assertion cache against stale relogin reads
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(mcp): keep sso assertion cache entries and generation markers in separate namespaces
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* test(router): rename duplicate get_configured_mode test so ruff F811 passes
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(mcp): use a process-wide epoch for sso assertion cache invalidation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
---------
Co-authored-by: yassin <yassin@berri.ai>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
