- /connect now fails immediately with 503 if prisma_client is None, so
misconfigured deployments don't waste users through the full provider
consent flow before failing at /callback.
- Fix "LRU-style" → "FIFO" in _write_byok_cred_cache docstring since
eviction uses insertion order (first-inserted), not recency.
- Fix malformed authorization URL when base URL already has query params
(e.g. Google: ?access_type=offline). Use & instead of ? in that case.
- Simplify _make_state_token() to take no parameters since none were used.
Update call site and tests accordingly.
- Status endpoint now checks _byok_cred_cache before querying the DB,
avoiding a raw Prisma query on every 2-second poll from the UI.
The callback's _invalidate_byok_cred_cache call ensures the first poll
after successful auth always falls through to DB and returns connected=True.
- require_byok_credential_store now defaults to False so existing deployments
without a database are not broken on upgrade. Set True to opt into the strict
503-on-no-DB behavior.
- Add comment clarifying that get_user_credential() is the centralized DB helper
for LiteLLM_MCPUserCredentials, mitigated by 60s in-memory cache.
- Update tests to explicitly set the flag to True when asserting 503 behavior.
Add a feature flag `litellm.require_byok_credential_store` (default True)
that controls whether _get_byok_credential and _check_byok_credential raise
HTTP 503 or fall through to legacy behavior when prisma_client is None.
Default True preserves the new secure behavior (503 = infra problem, distinct
from 401 = missing credential). Set to False to restore legacy silent-bypass
for stateless/no-database deployments.
- _make_state_token now uses secrets.token_urlsafe(32) instead of an
HMAC digest that was never re-verified by the callback (dict lookup
only), making the intent explicit and removing misleading crypto.
- Fix misleading comment on managed MCP tool call: original_tool_name
is the unprefixed name, not the "full (potentially prefixed)" name.
- Document the known stale-token window trade-off in _BYOK_CRED_CACHE_TTL
so future contributors understand the failure mode.
- Update state-token tests to reflect the new implementation.
- _get_byok_credential now raises HTTP 503 when prisma_client is None,
matching _check_byok_credential behavior. Previously it returned None
silently, causing callers to surface a misleading 401 instead of the
correct 503 infrastructure error.
- execute_mcp_tool prefixed-name fallback now checks name doesn't already
start with the server prefix before calling add_server_prefix_to_name,
preventing double-prefixed names like "github-github-get_user".
- Add MCP_TOOL_PREFIX_SEPARATOR to server.py utils imports.
- Add regression tests for both fixes.
- _check_byok_credential now raises HTTP 503 when prisma_client is None
instead of silently returning; previously this bypassed BYOK identity
enforcement entirely when the database was not configured
- Add regression tests:
- test_check_byok_credential_raises_503_when_no_db: verifies 503 is
raised (not silent bypass) when credential store is unavailable
- test_spec_path_server_uses_tool_registry: documents spec_path
short-circuit invariant (OpenAPI servers bypass MCP client creation)
- test_mcp_server_byok_fields_propagated/default: cover the YAML
config byok-field propagation bug fix
_check_byok_credential wrote the raw credential from DB directly to
_byok_cred_cache without calling _extract_access_token. When the OAuth2
callback stores a JSON blob {"access_token": "...", "refresh_token": "..."},
a subsequent call that went through _check_byok_credential first (rather
than _get_byok_credential) would populate the cache with the blob. The
next _get_byok_credential call would return the blob as the Bearer token,
causing silent HTTP 401s on every upstream API call.
Fix: extract the access_token before writing to cache, consistent with
how _get_byok_credential already handles it.
- Replace _byok_cred_cache.clear() with single-entry eviction in
_write_byok_cred_cache: at capacity, evict only the oldest key
instead of wiping all 4096 entries (thundering-herd prevention)
- Add regression tests:
- Cache LRU eviction: verifies exactly 1 entry evicted at capacity
- MCPServer byok fields: is_byok/byok_description/byok_api_key_help_url
propagated correctly (regression for the load_servers_from_config
missing-fields bug that caused BYOK servers to lose their fields)
- Cache capacity: tests all paths in _write_byok_cred_cache
1. Separate try-blocks for store_user_credential and
_invalidate_byok_cred_cache: a cache-flush failure no longer
returns a 'Storage error' page when the write succeeded
2. Popup-close race fix in OAuth2ConnectButton: do one final status
check when popup is detected closed so fast OAuth flows (popup
auto-closes before the first 2s poll fires) are not silently missed
3. Fix test patches: `master_key` and `prisma_client` are imported
inline in the function body via `from proxy_server import X`;
patch `litellm.proxy.proxy_server.*` not the endpoint module
- Extract and store refresh_token alongside access_token when the
provider returns one; stored as JSON blob {"access_token": ...,
"refresh_token": ...} so users aren't forced back through OAuth2
consent when the access token expires
- Add _extract_access_token() helper in server.py that transparently
handles both the new JSON blob format and legacy plain-string
credentials (backward compatible)
- Add tests: refresh_token stored as JSON, plain token when no
refresh_token, _extract_access_token with all input shapes
- Add PKCE comment acknowledging RFC 9700 gap; confidential client
with client_secret is lower risk, full PKCE tracked as follow-up
When a server has is_byok=true and auth_type=oauth2, the Credentials
column in the MCP Servers table shows an OAuth2ConnectButton instead
of the static key entry modal.
- OAuth2ConnectButton: calls /v1/mcp/server/{id}/oauth2/connect,
opens the returned authorization_url in a popup, polls
/v1/mcp/server/{id}/oauth2/status every 2 s until connected=true,
then shows a Connected badge and calls onConnected() to refresh the table
- mcp_server_columns: branches on auth_type===oauth2 to render the
new button, passing accessToken and refreshServers
- mcp_servers: passes accessToken and refetch down to the columns factory
- networking: adds getMcpOAuth2ConnectUrl and getMcpOAuth2Status helpers
Three bugs prevented stored OAuth2 tokens from being injected as
Bearer headers when OpenAPI MCP tools were called:
1. _get_tools_from_server called _create_mcp_client before checking
spec_path, so for BYOK OAuth2 servers resolve_mcp_auth tried a
client_credentials token exchange (GitHub rejects this with a non-JSON
body), causing a JSON parse error that made tools/list return [].
Fix: check spec_path first and read from tool registry directly,
skipping MCP client creation entirely for OpenAPI servers.
2. REST tool call passed user_api_key_auth=data.get("user_api_key_auth")
which is None unless set in request metadata, so _get_byok_credential
couldn't look up the stored token by user_id.
Fix: fall back to user_api_key_dict from the route dependency.
3. OpenAPI tools are registered in the local registry under the prefixed
name (e.g. github_user-get_authenticated_user) but callers may use
the bare name (get_authenticated_user). get_tool(bare_name) returned
None and execution fell through to the managed MCP client path.
Fix: when local_tool is None and we know the server has a spec_path,
retry get_tool with the prefixed name.
Also fixes load_servers_from_config to propagate is_byok, byok_description,
and byok_api_key_help_url from YAML config into the MCPServer object.
Adds three new endpoints so users can authorize their own accounts
through a provider's OAuth2 consent screen (GitHub, Spotify, Linear, etc.)
instead of pasting static API keys for BYOK OpenAPI MCP servers.
- GET /v1/mcp/server/{server_id}/oauth2/connect — initiates the flow,
returns an authorization_url the UI opens as a popup
- GET /v1/mcp/oauth2/callback — receives code+state from provider,
exchanges for access token, stores in LiteLLM_MCPUserCredentials
- GET /v1/mcp/server/{server_id}/oauth2/status — returns connected:true/false
State tokens are HMAC-SHA256 signed with master_key and expire after 10 min.
The callback shows a success HTML page (with auto-close for popups) instead
of redirecting, avoiding 404s in environments without a full UI deploy.
Claude Code v2.1.69+ sends `custom: {defer_loading: true}` on tool
definitions. Anthropic's API accepts this field, but Bedrock rejects it
with "Extra inputs are not permitted", causing ~90% of requests to fail.
Strip the `custom` field from each tool in the request body before
sending to Bedrock, in both the Messages API and Chat API invoke paths.
Fixes#22847
Co-authored-by: Ishaan Jaff <ishaanjaffer0324@gmail.com>
* fix(proxy): readiness check returns 200 when database is unreachable
_db_health_readiness_check() catches health_check() exceptions but
never updates db_health_cache to "disconnected" and never re-raises.
The caller health_readiness() always returns 200 with "db": "connected"
hardcoded, regardless of actual DB state.
In Kubernetes, this means pods with dead database connections stay in
the Service endpoints and continue receiving traffic they cannot serve.
Changes:
- Set db_health_cache to "disconnected" and re-raise the exception on
health_check failure so health_readiness() returns 503
- Use actual db_health_status["status"] in the response instead of
hardcoding "db": "connected"
- Reduce cache TTL from 2 minutes to 15 seconds. The 2-minute window
is too wide for readiness probes (typically 10-15s intervals) and
means a pod can report healthy for up to 2 minutes after the DB dies
- Only serve cached results when status is "connected". The previous
condition (status != "unknown") would also cache "disconnected" for
2 minutes, delaying recovery detection after a DB comes back
* fix(proxy): add DB connection self-healing to readiness check
When the Prisma query engine's internal TCP connection pool holds dead
connections (caused by network blips, Cloud SQL proxy restarts, or
node-level issues), health_check() fails with httpx.ConnectError.
The engine never recovers on its own because nothing triggers a
disconnect/connect cycle to restart the subprocess with fresh
connections.
This leaves pods permanently failing readiness checks until they are
manually restarted, even after the underlying DB becomes reachable
again.
Add a reconnect attempt to _db_health_readiness_check() when
health_check() fails:
1. disconnect() - kills the query engine subprocess and closes all
connections (has built-in backoff retry: 3 tries, 10s max)
2. connect() - starts a new engine with fresh TCP connections (has
built-in backoff retry: 3 tries, 10s max)
3. health_check() - verifies the new connection works (has built-in
backoff retry: 3 tries, 10s max)
If reconnect succeeds, the pod immediately returns to service (200).
If it fails, the original exception is re-raised (503). Reconnect
attempts are rate-limited by probe frequency (~10-15s), so a
permanently unreachable DB gets one attempt per cycle with no retry
loops.
This uses the same disconnect/connect mechanism that
PrismaWrapper.recreate_prisma_client() uses for IAM token refresh,
and aligns with the community-documented pattern for Prisma connection
recovery in long-running processes (prisma/prisma#24718, #27024).
* Add poetry lock and modify test_health_endpoints
* Address allow_requests_on_db_unavailable regression
* Address comments
* resolve greptile issue
* Restore accidentally deleted UI HTML files
These were removed in an earlier commit but still exist on main.
Restoring to keep the PR diff clean.
* Guard reconnect with is_database_transport_error
Only attempt disconnect/connect/health_check cycle for transport-level
failures (unreachable DB, dropped connection). Data-layer errors like
UniqueViolationError indicate the DB is reachable, so reconnecting
would be pointless churn.
* Address greptile's comments
* Fix module alias after rebase and add adversarial test coverage
- Unify module alias to _health_endpoints_module after rebase conflict
- Add test for non-transport error with flag on (exercises is_database_transport_error guard)
- Add test for disconnect() failure during reconnect cycle
- Split non-transport error test into flag-off (re-raises) and flag-on (skips reconnect) variants
* Remove stale UI HTML files reintroduced during rebase
* fix: don't close HTTP/SDK clients on LLMClientCache eviction
Removing the _remove_key override that eagerly called aclose()/close()
on evicted clients. Evicted clients may still be held by in-flight
streaming requests; closing them causes:
RuntimeError: Cannot send a request, as the client has been closed.
This is a regression from commit fb72979432. Clients that are no longer
referenced will be garbage-collected naturally. Explicit shutdown cleanup
happens via close_litellm_async_clients().
Fixes production crashes after the 1-hour cache TTL expires.
* test: update LLMClientCache unit tests for no-close-on-eviction behavior
Flip the assertions: evicted clients must NOT be closed. Replace
test_remove_key_closes_async_client → test_remove_key_does_not_close_async_client
and equivalents for sync/eviction paths.
Add test_remove_key_removes_plain_values for non-client cache entries.
Remove test_background_tasks_cleaned_up_after_completion (no more _background_tasks).
Remove test_remove_key_no_event_loop variant that depended on old behavior.
* test: add e2e tests for OpenAI SDK client surviving cache eviction
Add two new e2e tests using real AsyncOpenAI clients:
- test_evicted_openai_sdk_client_stays_usable: verifies size-based eviction
doesn't close the client
- test_ttl_expired_openai_sdk_client_stays_usable: verifies TTL expiry
eviction doesn't close the client
Both tests sleep after eviction so any create_task()-based close would
have time to run, making the regression detectable.
Also expand the module docstring to explain why the sleep is required.
* docs(AGENTS.md): add rule — never close HTTP/SDK clients on cache eviction
* docs(CLAUDE.md): add HTTP client cache safety guideline
* Include user_email in new user creation within get_user_object
Enhance the get_user_object function to include user_email in the parameters when creating a new user. This change is accompanied by a new test to verify that user_email is correctly included during the upsert process.
* Improve error handling in test_get_user_object by logging exceptions
Updated the test_get_user_object_upsert_includes_user_email function to log exceptions when they occur, enhancing the visibility of potential issues during testing. This change helps in diagnosing failures related to the mock LiteLLM_UserTable.
* fix(passthrough): raise_for_status in _async_streaming to propagate Azure 429s
* address greptile review feedback (greploop iteration 1)
Guard data/json args when content is provided to avoid httpx ValueError
* address greptile review feedback (greploop iteration 2)
Use bare raise to preserve original traceback in _async_streaming exception handler
* address greptile review feedback (greploop iteration 3)
Close httpx streaming response on error to prevent connection pool exhaustion
* address greptile review feedback (greploop iteration 4)
Guard aclose() call to prevent masking original exception; add explicit test for content param forwarding
* address greptile review feedback (greploop iteration 5)
Pass content to sign_request so AWS body-hash signing is correct when content is the sole body source
* revert sign_request content change - request_data expects dict, not bytes
Bedrock's sign_request calls json.dumps(request_data) — passing content bytes
would TypeError. sign_request should only receive data/json (dict), not raw bytes.
All operational/diagnostic messages in WebSearchInterceptionLogger are now
debug-level to avoid flooding production logs while still remaining available
when verbose logging is enabled.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR #22890 used cast(str, ...) / cast(Optional[str], ...) for the return
statements; this PR's approach uses str() for explicit runtime coercion
(addressing Greptile's concern). Keep the str() version and drop the
now-unused cast import.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Per Sameerlite's review: warning-level logs trigger Slack alerts.
All 6 remaining .warning() calls were operational/fallback messages,
not actual errors. Changed to .info() to match the first fix at L510.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Documents exactly how every request and response field gets translated
when LiteLLM routes an Anthropic /v1/messages call through the OpenAI
Responses API path (for OpenAI/Azure targets). Covers messages content
block mapping, tools, tool_choice, thinking→reasoning, context_management,
and the reverse response translation. Wired into the /v1/messages sidebar.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- CreateBatchRequest.output_expires_after: drop Optional since total=False
already makes the key absent-or-present; Optional[T] incorrectly allowed
the key to exist with value None, which is incompatible with the OpenAI
SDK's OutputExpiresAfter | NotGiven expectation on batches.create()
- cost_tracking_settings._resolve_model_for_cost_lookup: replace implicit
object-to-str returns with explicit str() calls so the function is safe
even if the surrounding truthiness guards are later weakened
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
