fix(proxy): release budget reservation when a request is cancelled mid-flight - #30519
Conversation
[Infra] Promote internal staging to main
[Infra] Promote internal staging to main
chore(ci): promote internal staging to main
chore(ci): promote internal staging to main
chore(ci): promote internal staging to main
chore(ci): promote internal staging to main
chore(ci): promote internal staging to main
* fix(proxy): resolve managed video model ids for auth Co-authored-by: Cursor <cursoragent@cursor.com> * test(proxy): cover character_id router model resolution Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit d45e9e4)
…ams (BerriAI#29310) * fix(key_generate): allow team members to create keys on org-scoped teams When a virtual key is created for a team, enterprise logic inherits the team's organization_id onto the key (add_team_organization_id). Since the VERIA-55 org-IDOR fix, /key/generate then required the caller to be an explicit LiteLLM_OrganizationMembership member of that org, returning 403 "Caller is not a member of organization_id=<uuid>". Admins normally only add users to teams (not orgs), so self-serve key creation regressed for any user on an org-scoped team (regression since v1.84.0-rc.1). Skip the org-membership check when organization_id was inherited from the key's team (organization_id == team_table.organization_id). Team-level authorization already gates this path, so team membership is sufficient. The membership check still runs when a caller assigns an organization_id that did not come from the key's team, preserving the IDOR protection. Adds regression tests covering both the team-inherited (allowed) and foreign-org (still blocked) cases. Co-authored-by: Cursor <cursoragent@cursor.com> * test(key_generate): cover mismatched team org IDOR path on generate Add test_generate_key_foreign_org_with_mismatched_team_still_enforces_membership for the case where a team is present but request organization_id differs from team_table.organization_id. Enterprise inheritance is no-op'd in the test so the guard is exercised directly; membership validation must still run. Addresses Greptile review on BerriAI#29310. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit b11833c)
… reject it (Haiku 4.5) (BerriAI#29585) * fix(vertex): strip output_config.effort for models that reject it Haiku 4.5 on Vertex AI does not support output_config.effort and 400s with "output_config.effort: Extra inputs are not permitted". PR BerriAI#27074 emptied VERTEX_UNSUPPORTED_OUTPUT_CONFIG_KEYS so effort would forward for Opus/Sonnet 4.6+, but that made the strip unconditional across every Vertex Anthropic model, including ones that don't support it. Claude Code injects effort into its default Messages payload, so `claude --model claude-haiku-4.5` started failing. Make the sanitizer model-aware: drop output_config.effort for models that don't advertise output_config support (or any reasoning effort level) while forwarding it for those that do. The fix covers both the chat-completion and Messages pass-through transformation paths since they share the helper. * chore(vertex): log at debug when dropping unsupported output_config.effort Operators pointing an unregistered Vertex Claude alias that does support effort would otherwise see it stripped with no signal. Debug level keeps it out of normal logs since Claude Code sends effort on every request. (cherry picked from commit cc55662)
* fix duplicate cost callbacks for anthropic streaming pass-through Two bugs caused _PROXY_track_cost_callback to see stream=True + complete_streaming_response=None on every streaming pass-through request, making the dedup guard in dispatch_success_handlers permanently inactive: 1. pass_through_endpoints.py created the Logging object with stream=False for all requests. _is_assembled_stream_success short-circuits on self.stream is not True, so has_dispatched_final_stream_success was never set and any second dispatch went through unchecked. Fix: set logging_obj.stream = True after stream detection. 2. _create_anthropic_response_logging_payload set complete_streaming_response inside the try block after litellm.completion_cost(), so a pricing error caused an early return without setting it on model_call_details. Fix: set complete_streaming_response before the try block. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix stream * add stream to logging obj * test(pass_through): give mock logging object a real model_call_details dict The anthropic passthrough logging payload now records the assembled response on model_call_details before cost calculation, which requires model_call_details to support item assignment. In production it is always a dict; the existing unit test stubbed the logging object with a bare Mock whose attribute is not subscriptable, so the new assignment raised TypeError. Use a real dict to match the production logging object. * test(pass_through): cover streaming logging-obj stream flag The streaming branch of pass_through_request that marks the logging object as streaming (logging_obj.stream and model_call_details["stream"]) had no unit coverage, so the patch coverage gate flagged it. Add a regression test that drives a streaming pass-through request through pass_through_request and asserts the logging object is flagged as a stream before dispatch. * test(pass_through): cover SSE-response stream flag fallback branch The auto-detected streaming branch of pass_through_request (when a request that was not flagged as streaming returns a text/event-stream response) sets logging_obj.stream and model_call_details["stream"] but had no unit coverage, so the codecov patch gate failed at 60%. Drive a non-streaming pass-through request whose upstream response is SSE through pass_through_request and assert the logging object is flagged as a stream before dispatch. * fix(pass_through): gate complete_streaming_response on stream flag perform_redaction only scrubs complete_streaming_response when model_call_details["stream"] is True. Setting it unconditionally for non-streaming Anthropic pass-through responses left the assembled response unredacted in model_call_details, which is handed to logging callbacks as kwargs when message logging is disabled. Only record it for actual streaming responses so redaction always applies. --------- Co-authored-by: mubashir1osmani <mubashir.osmani777@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> (cherry picked from commit 2bbdbfa)
…_0_rc2 chore(release): patch v1.88.0-rc.1 with four staged fixes
…ng for team keys (BerriAI#29612) Non-admin users creating a team key through the UI were rejected with "max_budget cannot exceed the caller's own max_budget (0.25)". The request is authenticated by a UI/CLI session token whose max_budget is the per-session chat spend cap (max_ui_session_budget, default $0.25), and the delegated-authority budget ceiling (GHSA-q775-qw9r-2r4g) treated that cap as a delegation limit. Skip the ceiling only when a session token creates a team key (data.team_id set); that key's spend is bounded by the team budget at request time. Personal keys and every other non-admin caller keep the ceiling, so a session token cannot mint an arbitrary-budget personal key. (cherry picked from commit 97ba7e1)
…_0_rc3 chore(release): patch v1.88.0-rc.1 with BerriAI#29612 (session-token budget-ceiling exemption)
…efault_key_generate_params Capture _requested_team_id before the default_key_generate_params loop runs and key the UI/CLI session-token budget-ceiling exemption off it, instead of the post-defaults data.team_id. On an install that sets default_key_generate_params.team_id, a session token requesting a personal key (no explicit team_id) would otherwise have data.team_id auto-filled, flipping is_ui_session_team_key on and bypassing the delegated-authority ceiling -- the exact escalation GHSA-q775 closed. Mirrors the existing pre-defaults capture of _requested_max_budget. Adds a regression test. https://claude.ai/code/session_01RT583b1khYC3wjLrQ5hT5h (cherry picked from commit efeb101)
…lts_rc188 fix(key_generate): harden GHSA-q775 session-token exemption against default_key_generate_params (1.88 rc)
…R_ROOT_PATH After BerriAI#28547, get_request_route strips the deployment prefix while registry lookup still re-inflated stored paths via SERVER_ROOT_PATH, causing 404s under paths like /llmproxy/ml. Compare normalized bare routes in both is_registered_pass_through_route and get_registered_pass_through_route. Co-authored-by: Cursor <cursoragent@cursor.com>
After removing get_server_root_path from pass_through_endpoints, route and JWT tests must mock litellm.proxy.utils where normalization reads it. Co-authored-by: Cursor <cursoragent@cursor.com>
Backport of BerriAI#29982 to stable/1.88.x. Raises the PyJWT floor in pyproject (>=2.13.0,<3.0) and re-resolves uv.lock to 2.13.0; bumps the ws transitive override in the dashboard from 8.19.0 to 8.20.1 and regenerates package-lock, with jsdom and openai deduping onto the single 8.20.1 copy. Routine dependency maintenance to keep pinned versions current.
…ackport_1_88_x build(deps): bump pyjwt to 2.13.0 and ws override to 8.20.1 (1.88.x)
Bump the litellm version from 1.88.0 to 1.88.1 for the stable/1.88.x patch that carries the pyjwt and ws dependency bumps. Updates the project and commitizen versions in pyproject.toml and the editable litellm entry in uv.lock.
…88_1 chore(release): bump version to 1.88.1
When a deployment offloads prompts and responses to cold storage instead of
Postgres, the spend-log row holds only "{}" placeholders plus a
metadata.cold_storage_object_key pointer, so the UI logs detail drawer showed
nothing. The detail endpoint only read the placeholder columns and never
fetched the object back.
Resolve the payload per row based on actual content, not a config flag: if
Postgres has content, return it; otherwise read the exact stored object key and
fetch from the configured cold storage backend through ColdStorageHandler.
Reading the persisted key is a single GET. The key embeds a microsecond
timestamp that cannot be reconstructed from the millisecond-precision startTime
column, and listing the day's prefix to match on request_id would be too
expensive for this per-open path.
Also teach the detail drawer's pretty-view parser to accept a bare messages
array. The cold storage payload carries the prompt as a top-level messages list
with no proxy_server_request, so without this the output rendered while the
input stayed blank.
ColdStorageHandler gains an optional injected logger so the resolver can be unit
tested without monkeypatching. Postgres-stored prompts are unaffected: the fast
path returns the existing columns and the request-body object still renders the
same way.
…d-flight The pre-call budget reservation increments the cross-pod spend counter (`spend:key:*`, etc.) by the request's worst-case cost, then relies on the success cost callback or the failure hook to reconcile/release it. A client disconnect or timeout cancels the request task and surfaces as CancelledError / GeneratorExit, which neither path catches, so the reservation is never given back. Under retry storms the leaked reservations accumulate and pin the counter above the real spend, returning spurious 429 "Budget has been exceeded" to a key whose actual spend is far below its budget; the counter only recovers once its TTL lapses, so the failure is intermittent and self-healing. Release the reservation on cancellation in both request paths: the shared streaming generator (covers chat, anthropic /messages, and google streaming via async_sse_data_generator) and the non-streaming base_process_llm_request. The release runs under asyncio.shield so it completes despite the surrounding cancellation, and is guarded by the reservation's `finalized` flag so it is a no-op when success/failure handling already reconciled.
Greptile SummaryThis PR fixes intermittent spurious 429s caused by pre-call budget reservations leaking when a request is cancelled mid-flight (client disconnect or timeout), because
Confidence Score: 3/5The budget-reservation cancel fix is directionally correct, but the test suite verifies a synchronous proxy for cancellation rather than actual asyncio task cancellation, leaving the most important production scenario untested. The key management changes also touch the GHSA-q775-qw9r-2r4g security boundary. The streaming and non-streaming cancel paths look sound, but the three new cancel-release tests all raise
|
| Filename | Overview |
|---|---|
| litellm/proxy/spend_tracking/budget_reservation.py | Adds release_budget_reservation_on_cancel using asyncio.shield to release budget reservations on cancellation; also passes llm_router to get_model_from_request. Design is sound but the asyncio.shield creates a background task under real task cancellation, leaving finalized unset until the background task completes. |
| litellm/proxy/common_request_processing.py | Wires release_budget_reservation_on_cancel into both the non-streaming (base_process_llm_request) and streaming (async_streaming_data_generator) cancellation paths; streaming path also catches GeneratorExit alongside CancelledError. |
| tests/test_litellm/proxy/test_budget_reservation.py | Adds three tests for the cancel-release path; all three raise CancelledError from within the iterator rather than from outer-task cancellation, so asyncio.shield always completes synchronously in the tests — the production scenario where the shield fires asynchronously is not exercised. |
| litellm/proxy/management_endpoints/key_management_endpoints.py | Adds is_ui_session_team_key exemption from the GHSA-q775-qw9r-2r4g budget ceiling and _org_inherited_from_team exemption from org-assignment validation; exemption guard uses only _requested_team_id is not None before downstream team validation runs. |
| litellm/proxy/auth/auth_utils.py | Threads llm_router through get_model_from_request and _extract_models_from_managed_resource_id to resolve router-managed model IDs to their canonical names; new _resolve_model_id_with_router helper is well-guarded with try/except and fallback. |
| litellm/proxy/pass_through_endpoints/pass_through_endpoints.py | Replaces _build_full_path_with_root with _route_for_registry_lookup which strips the root from the incoming route, fixing registry lookups when a non-root server root path is configured; also sets logging_obj.stream = True for streaming pass-through responses. |
| litellm/proxy/spend_tracking/spend_management_endpoints.py | Adds _resolve_request_response_payload to transparently serve prompt/response content from cold storage when the DB row holds only placeholders; correctly falls back to PG content when cold storage returns nothing. |
| litellm/llms/vertex_ai/vertex_ai_partner_models/anthropic/output_params_utils.py | Makes sanitize_vertex_anthropic_output_params model-aware: output_config.effort is dropped for models that don't advertise support and forwarded for those that do, preventing upstream 400s. |
| litellm/proxy/spend_tracking/cold_storage_handler.py | Refactors ColdStorageHandler to support constructor-injected logger for testing; no behavior change in production. |
Comments Outside Diff (1)
-
litellm/proxy/management_endpoints/key_management_endpoints.py, line 734-753 (link)Session-token budget-ceiling exemption relies entirely on downstream team validation
is_ui_session_team_keyevaluates toTruewhenever the calling token is a UI session token (team_id == UI_SESSION_TOKEN_TEAM_ID) and the request includes any non-Noneteam_id. At this point the suppliedteam_idhas not been validated (team existence and caller membership are checked later in_common_key_generation_helper). This means the GHSA-q775-qw9r-2r4g budget ceiling is bypassed for any key creation that includes ateam_idfield, before the system confirms the team is real or the caller belongs to it.If downstream team validation is strict and always rejects unknown/inaccessible teams, the exemption is safe. But if there is any path that allows a key to be stored with an unvalidated
team_id(e.g., a team that has nomax_budget), a session token holder could create a key with an unbounded budget by providing a bogus or unlimited-budgetteam_id. A guard liketeam_table is not None(which is only set after a successful DB lookup) would make the exemption self-evidently safe without relying on ordering.
Reviews (1): Last reviewed commit: "fix(proxy): release budget reservation w..." | Re-trigger Greptile
| try: | ||
| await asyncio.shield( | ||
| release_budget_reservation(budget_reservation=budget_reservation) | ||
| ) | ||
| except asyncio.CancelledError: | ||
| pass |
There was a problem hiding this comment.
asyncio.shield fires asynchronously in the production scenario, but tests verify only the synchronous path
When the outer asyncio Task is genuinely cancelled (e.g. client disconnect), await asyncio.shield(release_budget_reservation(...)) immediately raises CancelledError in the outer coroutine; the inner release_budget_reservation Task is still running in the background. The except asyncio.CancelledError: pass silences the raise and the function returns while finalized is still False — the budget release and flag update happen later on the event loop.
The three tests in test_budget_reservation.py trigger CancelledError from inside the iterator, not from actual task cancellation. Without outer-task cancellation asyncio.shield completes synchronously (no outer CancelledError to surface), so the tests always see finalized = True immediately after the call. The production case (outer task cancelled) is not covered and would leave finalized = False until the background task finishes.
The background task still runs the release correctly on a live event loop, so no budget is permanently leaked; but the finalized guard being unset between the function returning and the background task completing means a hypothetical concurrent reconcile call (e.g. a late streaming success callback) could double-reconcile before the flag is written.
| release_budget_reservation_on_cancel, | ||
| ) | ||
|
|
||
| await release_budget_reservation_on_cancel( |
There was a problem hiding this comment.
High: Budget bypass on cancelled streams
A caller can start an expensive streaming request, receive generated chunks, then disconnect before the stream completes; this branch releases the reservation with actual_cost=0.0, and the success cost callback never runs. Reconcile the generated partial stream cost on cancellation, or otherwise keep a bounded reservation/charge path for streams that have already produced output instead of unconditionally releasing it.
PR overviewThis pull request changes proxy request handling so budget reservations are released when an in-flight request is cancelled. The touched logic is in the common request-processing path for proxied requests, including streaming flows. There is still an open issue in the cancellation path for streaming requests: a caller can receive generated output and then disconnect, causing the reservation to be released without charging for the partial work. That leaves the PR vulnerable to budget enforcement bypass and potential cost/resource abuse for streaming calls. No issues have been fixed or addressed yet, so the current posture still depends on correcting that accounting behavior before merge. Open issues (1)
Fixed/addressed: 0 · PR risk: 6/10 |
|
Superseded by #30522 (rebased onto the latest litellm_internal_staging so the diff is a single isolated commit). |
Relevant issues
Intermittent
429 Budget has been exceeded! Key=... Current cost: 805.8, Max budget: 800.0on keys whose real spend is far below budget (~$280 of an $800 budget), self-healing after a pause and recurring under load.Pre-Submission checklist
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewType
🐛 Bug Fix
Changes
The pre-call budget reservation (
reserve_budget_for_request) increments the cross-pod spend counter by a request's worst-case cost up front, then gives it back by reconciling to the actual cost on success (cost callback) or on error (async_post_call_failure_hook). Neither runs when a request is cancelled mid-flight: a client disconnect or timeout surfaces asasyncio.CancelledError(aBaseException, soexcept Exceptionmisses it) orGeneratorExitin the streaming generator, so the reservation leaks. Under an agent retry storm the leaked reservations accumulate, pin the counter above the key's real spend, and 429 subsequent requests; the counter only recovers when its TTL lapses, which is why it is intermittent and self-healing.This releases the reservation on the cancellation paths, mirroring the success/error paths.
release_budget_reservation_on_cancelwrapsrelease_budget_reservationinasyncio.shieldso it completes despite the surrounding cancellation, and short-circuits on the reservation'sfinalizedflag so it is a no-op once success/error handling already reconciled. It is wired intoasync_streaming_data_generator(whichasync_sse_data_generatordelegates to, covering chat, Anthropic/v1/messages, and Google streaming) and into the non-streamingbase_process_llm_request.A blanket
finallyrelease does not work for streaming, where the handler returns theStreamingResponsebefore the stream is consumed and the success reconcile happens at stream end; afinallywould release early and suppress the real reconcile. Catching only the cancellation signals targets the leak without touching the success or error paths. The reservation aggressiveness (worst-casemax_tokens) is a separate policy question and is intentionally left unchanged.Tests in
tests/test_litellm/proxy/test_budget_reservation.py: cancel-release gives the counter back and is idempotent; a finalized reservation is left untouched; and drivingasync_streaming_data_generatorto cancel mid-stream hands the reservation back (neutering the fix flips this test red).Screenshots / Proof of Fix
Live proxy run to be added.