Cherry-pick #29311, #29343, #29358, #27913, and #29447 onto stable/1.85.x#29460
Conversation
…9343) * refactor(proxy/auth): normalize Bearer prefix in safe-hash helper UserAPIKeyAuth._safe_hash_litellm_api_key now strips a leading "Bearer "/"bearer " prefix before its existing sk-/JWT classification, so the helper produces the same hashed output regardless of whether the caller stripped the Authorization header prefix or passed the header value through unchanged. * refactor(proxy/auth): make Bearer-prefix strip case-insensitive Per RFC 7235 the HTTP authorization scheme token is case-insensitive. Replace the two-prefix loop with a single case-insensitive check so the helper normalizes "Bearer ", "bearer ", "BEARER ", and any mixed-case variant before classifying the remainder as sk- or JWT. The contract test gains coverage of "BEARER " and "BeArEr ". * test(mcp): align auth-handler test expectations with safe-hash helper The two MCP auth tests asserted that UserAPIKeyAuth(api_key="Bearer ...") retained the raw header bytes on the api_key field. _safe_hash_litellm_api_key now normalizes that input — stripping the Bearer prefix and hashing the resulting sk- key — so the expectations move to the normalized form: the bare token in the parametrize case, and hash_token("sk-...") in the backward-compat assertion. This matches what the real auth flow produces (the builder strips Bearer and the DB stores the hashed token), so the mocks now line up with production rather than with the un-normalized validator output.
…eroing counter (#29358) * fix(reset_budget): write only {spend, budget_reset_at} and stop pre-zeroing counter ResetBudgetJob's batched update_data path shipped the full key/user/team model on each reset. Prisma rejects object_permission_id and budget_limits on the update input type, so any row carrying those fields detonated the entire batch -- spend never reset, budget_reset_at never advanced. After v1.84.0 started populating object_permission_id on UI-created keys, this fires routinely. _reset_budget_common also zeroed the cross-pod spend counter before the DB write, so failed resets left enforcement reading 0 from the counter while the DB still held the over-budget spend, admitting requests past the cap until the counter naturally re-saturated from new reservations. Switch the write to per-row narrow updates ({spend, budget_reset_at}) via db.batch_, and move the counter invalidation out of _reset_budget_common so it only fires after the DB write commits. On DB-write failure the counter is left untouched, enforcement continues to block, and the next scheduler tick can retry without leaving a bypass window. Fixes #27730. * fix(reset_budget): address Greptile review on #29358 - Strengthen the bypass-half regression test: replace the for-loop over call_args_list (vacuously true when empty) with assert_not_called(), so the test would actually flag a re-introduction of counter-zeroing via any code path. - Add the same explanatory docstring on _write_user_reset_updates and _write_team_reset_updates that _write_key_reset_updates already has, so all three helpers point future maintainers at #27730. * test(reset_budget): update test_proxy_budget_reset for new batch-write path Same shape as the previous test_reset_budget_job.py update: keys/users/teams now write through prisma.db.batch_().<table>.update, not update_data, so the tests need a batcher mock and updated assertions. Adds: - _wire_batcher_for_test helper that returns a list which accumulates per-row batch updates captured from prisma_client.db.batch_(). - _attrify helper that wraps dict fixtures so getattr(item, "token") works alongside the dict item-access the fake_reset_* mocks rely on. The new narrow-write helpers use getattr to pull out the row's id, and would silently skip plain dicts otherwise. - Updates 3 partial_failure tests to assert against the batch-call list (rows by id, payload contains only {spend, budget_reset_at}) instead of update_data.assert_awaited_once + data_list inspection. - Updates test_reset_budget_continues_other_categories_on_failure: only budget + enduser still flow through update_data; key/user/team go through the batch path now. - Wires the batcher mock into 3 service_logger_*_success tests so commit() is actually awaitable and the success hook fires. These tests were silently passing locally only because the editable install in .venv pointed at the main repo, not the worktree — running pytest with PYTHONPATH overridden to the worktree (matching CI) reproduces the failures.
…der body (#27913) * fix(rate-limit): stop v3 limiter from leaking internal stash to provider body PR #27001 (atomic TPM rate limit) introduced a reservation flow that writes four LiteLLM-internal keys onto the request data dict: _litellm_rate_limit_descriptors _litellm_tpm_reserved_tokens _litellm_tpm_reserved_model _litellm_tpm_reserved_scopes _litellm_tpm_reservation_released These keys are forwarded as request body params to the upstream provider, which rejects them as unknown fields: OpenAI -> 400 'Unknown parameter: _litellm_rate_limit_descriptors' (mapped by litellm to RateLimitError / 429, hiding the bug behind a misleading 'throttling_error' code) Anthropic -> 400 '_litellm_rate_limit_descriptors: Extra inputs are not permitted' Net effect: every chat completion against any real provider fails the moment a virtual key has any tpm_limit / rpm_limit set — i.e. v3-enforced key-level TPM/RPM limits are broken end-to-end. The v3 RPM/TPM check itself still runs (raises 429 on over-limit), but the success path poisons the upstream body. Reproduced on litellm_internal_staging HEAD (410ce76) against gpt-4o-mini and claude-haiku-4-5 with a 1-RPM/1-TPM key — first request fails with the provider's unknown-field error. Fix: the stash is metadata only. - Add RATE_LIMIT_DESCRIPTORS_KEY constant and a _LITELLM_STASH_KEYS registry so we have a single source of truth for stash keys. - New helper _stash_value_in_metadata_channels writes to data['metadata'] / data['litellm_metadata'] without touching the top level. - _stash_reservation_in_data and the descriptor stash now route through that helper. _mark_reservation_released stops writing top-level. - _lookup_stashed_value also checks kwargs['metadata'] / kwargs['litellm_metadata'] (raw request_data shape) in addition to kwargs['litellm_params']['metadata'] (completion kwargs shape). - async_post_call_failure_hook now reads descriptors via the unified metadata lookup instead of request_data.get(top-level). - Defense in depth: async_pre_call_hook strips any stash key that somehow surfaced at the top level (stale cache, future refactor, test fixture) before returning. Tests: - New regression test asserts no _litellm_* stash key is present at the top level of data after async_pre_call_hook, and that the metadata channel still carries the reservation + descriptors so success / failure reconciliation works. - Existing test_tpm_concurrent.py tests that asserted top-level presence are updated to read from data['metadata'] — the location is an implementation detail; the spec is that post-call callbacks can resolve the stash. Verified end-to-end against OpenAI gpt-4o-mini and Anthropic claude-haiku-4-5 via /v1/chat/completions on a low-rpm key: - With limits not exceeded: HTTP 200, valid completion response, no leaked fields in body. - With RPM exceeded: HTTP 429 from v3 enforcement ('Rate limit exceeded ... Limit type: requests'). - With TPM exceeded: HTTP 429 from v3 enforcement ('Rate limit exceeded ... Limit type: tokens'). Full v3 hook test suite passes (171 tests). Co-authored-by: Mateo Wang <[email protected]> * chore(rate-limit): use RATE_LIMIT_DESCRIPTORS_KEY constant in test, trim noisy comments Address greptile P2: test fixture now uses the imported constant. Drop comments that re-explain what well-named identifiers already convey. * fix(rate-limit): reject caller-supplied stash values to prevent TPM-refund abuse Strip _LITELLM_STASH_KEYS from data top-level and both metadata channels at the start of async_pre_call_hook. Without this, an authenticated caller can inject _litellm_rate_limit_descriptors plus _litellm_tpm_reserved_tokens in body metadata, trigger a proxy-side rejection, and cause async_post_call_failure_hook to refund TPM counters against attacker-named scopes (e.g. another tenant's api_key). --------- Co-authored-by: Cursor Agent <[email protected]> Co-authored-by: Mateo Wang <[email protected]>
…quest body (#29447) * fix: stop use_chat_completions_api flag from leaking into provider request body use_chat_completions_api is a LiteLLM control flag that forces the /responses -> /chat/completions bridge. It was missing from all_litellm_params, so get_non_default_completion_params treated it as a model-specific param and forwarded it to the upstream provider. A model-level "use_chat_completions_api: true" in the proxy config therefore reached the chat-completions path and was rejected by strict providers (OpenAI/Anthropic) with HTTP 400 for an unknown body field. Register it as a known internal param so it is stripped on every path (completion, the responses bridge that calls litellm.completion, and filter_out_litellm_params). Adds a regression test driving litellm.completion() with a mocked OpenAI client that asserts the flag never reaches the request body. * test: clarify extra_body assertion in use_chat_completions_api leak test Replace the misleading 'not in ... or {}' precedence idiom with an explicit parenthesized guard that also handles extra_body being None.
|
Replaces #29458. Branch rename from |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis cherry-pick PR backports five bug fixes onto
Confidence Score: 4/5Safe to merge; all five fixes are narrow and well-tested. The deferred stream logging refactor is the most complex change but is covered by new unit tests. The budget reset fix correctly moves Redis counter zeroing to after the DB commit and adds previously-missing user-counter invalidation. The rate-limiter stash-key fix closes a real injection path and is backed by two new regression tests. The dispatch_success_handlers refactor is internally consistent but relaxes the should_run_logging double-log guard for assembled stream responses, creating a latent risk if async_success_handler is called directly with such a result in future code. The budget_reset_at field is written back unchanged for keys with no budget_duration, causing harmless redundant reset cycles but not data loss. litellm/litellm_core_utils/litellm_logging.py and litellm/proxy/common_utils/reset_budget_job.py
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/litellm_logging.py | Introduces dispatch_success_handlers, _is_assembled_stream_success, and _is_sync_litellm_request; adds has_dispatched_final_stream_success dedup guard for streaming; modifies async_success_handler to bypass the should_run_logging guard for assembled stream responses. |
| litellm/litellm_core_utils/streaming_handler.py | Per-chunk sync logging is now conditional on _is_sync_litellm_request; assembled-stream dispatch switches from async_success_handler + executor.submit to dispatch_success_handlers(prefer_async_handlers=True). |
| litellm/proxy/_types.py | Adds case-insensitive Bearer prefix stripping in _safe_hash_litellm_api_key before sk-/JWT classification; the normalized (stripped) key is hashed or returned. |
| litellm/proxy/common_utils/reset_budget_job.py | Replaces bulk update_data(update_many) with narrow per-row prisma.db.batch_().update({spend, budget_reset_at}); removes pre-zeroing of Redis spend counters from _reset_budget_common and adds post-commit _invalidate_spend_counter calls in each caller; also adds previously-missing user spend counter invalidation. |
| litellm/proxy/hooks/parallel_request_limiter_v3.py | Stops stash keys from living at the top-level of data; introduces _stash_value_in_metadata_channels, _strip_stash_keys_from_top_level/all_channels; _lookup_stashed_value now only reads metadata channels (not top-level kwargs); RATE_LIMIT_DESCRIPTORS_KEY is stored and read from metadata. |
| litellm/types/utils.py | Adds use_chat_completions_api to all_litellm_params so it is filtered from provider request bodies. |
Comments Outside Diff (1)
-
litellm/proxy/common_utils/reset_budget_job.py, line 486-492 (link)Empty batch committed when all IDs are None
_write_key_reset_updates(and its user/team siblings) callsbatcher.commit()unconditionally after iteratingupdated_keys. If every item in the list hastoken=None(all skipped by thecontinue), the commit fires against an empty batch. Prisma's batch API handles this gracefully in most drivers, but the behaviour is driver-version-dependent and will silently skip all writes with no error. The outerif updated_keys:guard only ensures the list is non-empty — it doesn't guarantee any items have a non-None identifier.
Reviews (1): Last reviewed commit: "fix: stop use_chat_completions_api flag ..." | Re-trigger Greptile
| batcher.litellm_verificationtoken.update( | ||
| where={"token": token}, | ||
| data={"spend": 0, "budget_reset_at": k.budget_reset_at}, | ||
| ) |
There was a problem hiding this comment.
budget_reset_at written back as-is when budget_duration is None
When a key has no budget_duration, _reset_budget_common does not update budget_reset_at, so k.budget_reset_at still holds the old (expired) timestamp. _write_key_reset_updates then writes {"spend": 0, "budget_reset_at": <old-expired-value>} back to the DB. On the next job run the key will appear in keys_to_reset again (because budget_reset_at <= now), triggering a harmless but redundant reset cycle. Guarding the field with a non-None check prevents writing a stale timestamp.
| batcher.litellm_verificationtoken.update( | |
| where={"token": token}, | |
| data={"spend": 0, "budget_reset_at": k.budget_reset_at}, | |
| ) | |
| batcher.litellm_verificationtoken.update( | |
| where={"token": token}, | |
| data={ | |
| "spend": 0, | |
| **( | |
| {"budget_reset_at": k.budget_reset_at} | |
| if k.budget_reset_at is not None | |
| else {} | |
| ), | |
| }, | |
| ) |
| if not self._is_assembled_stream_success( | ||
| result | ||
| ) and not self.should_run_logging( | ||
| event_type="async_success" | ||
| ): # prevent double logging | ||
| ): # prevent double logging (non-streaming) | ||
| return |
There was a problem hiding this comment.
should_run_logging guard bypassed for all assembled stream calls to async_success_handler
async_success_handler previously checked should_run_logging unconditionally to prevent double logging. The new condition skips that guard whenever _is_assembled_stream_success returns True, relying on the has_dispatched_final_stream_success flag set by dispatch_success_handlers. Any call site that invokes async_success_handler directly with an assembled stream result (bypassing dispatch_success_handlers) will now skip the double-log guard with no fallback. The comment should at minimum make this assumption explicit so future callers don't inadvertently create double-logging regressions.
| if not self._is_assembled_stream_success( | |
| result | |
| ) and not self.should_run_logging( | |
| event_type="async_success" | |
| ): # prevent double logging | |
| ): # prevent double logging (non-streaming) | |
| return | |
| # For assembled stream results, dedup is enforced by the | |
| # ``has_dispatched_final_stream_success`` guard in | |
| # ``dispatch_success_handlers``. Always route assembled stream | |
| # responses through ``dispatch_success_handlers`` — calling | |
| # ``async_success_handler`` directly with an assembled result | |
| # bypasses that guard and may produce duplicate log events. | |
| if not self._is_assembled_stream_success( | |
| result | |
| ) and not self.should_run_logging( | |
| event_type="async_success" | |
| ): # prevent double logging (non-streaming) | |
| return |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
The #29311 cherry-pick onto stable/1.85.x carried the test test_route_streaming_logging_runs_async_handler_for_sdk_passthrough, which patches PassThroughStreamingHandler._build_passthrough_logging_result to verify the SDK-passthrough dispatch contract. The manual conflict resolution kept the per-endpoint if/elif/elif chain inline in _route_streaming_logging_to_handler (matching v1.84.4's resolution), so the patched attribute did not exist and the test errored at collection with AttributeError. Extract the chain into the static _build_passthrough_logging_result helper as #29089 originally designed it. _route_streaming_logging_to_handler now resolves (standard_logging_response_object, kwargs) through the helper and dispatches via dispatch_success_handlers; the helper itself is synchronous and CPU-bound, suitable for the unit test's patch target. Verified locally: tests/pass_through_unit_tests/test_unit_test_streaming.py passes (5/5) and tests/test_litellm/litellm_core_utils/test_litellm_logging.py passes (83/83). v1.84.4 ships with the same broken test; this strictly improves on that resolution.
Relevant issues
Backport of three PRs that landed in v1.84.4 via #29352 and #29363, plus two more (#27913 and #29447) that don't apply to the 1.84.x line but do apply to 1.85.x. All five carry into the next 1.85 patch.
Linear ticket
N/A
Pre-Submission checklist
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewCI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
Five cherry-picks from
litellm_internal_stagingontostable/1.85.x, plus one follow-on fix to the manual conflict resolution:fcdf0231d0581c30f1e887b0e4748594a043efb2a06ec43b367d1bd9d9f40e2510dee27a462a4220acbbfe9cae65b6e04da68824745c4e_build_passthrough_logging_result)One conflict during the first cherry-pick in
litellm/proxy/pass_through_endpoints/streaming_handler.py:stable/1.85.xcarried the olderawait litellm_logging_obj.async_success_handler(...)call that the PR rewrites. Initial resolution kept the surrounding endpoint-dispatchif/elif/elifchain inline and replaced only the trailing call withdispatch_success_handlers, which matched v1.84.4's resolution exactly. That turned out to leave the regression testtest_route_streaming_logging_runs_async_handler_for_sdk_passthroughbroken because it patchesPassThroughStreamingHandler._build_passthrough_logging_result, a helper that #29089's design extracts the if/elif/elif chain into but my initial resolution kept inline. v1.84.4 ships with the same broken test.The follow-on commit
8824745c4eextracts the helper exactly as #29089 designed it, restoring the testable shape._route_streaming_logging_to_handlernow resolves(standard_logging_response_object, kwargs)through the helper and dispatches viadispatch_success_handlers. Verified locally:tests/pass_through_unit_tests/test_unit_test_streaming.pypasses 5/5 andtests/test_litellm/litellm_core_utils/test_litellm_logging.pypasses 83/83.The remaining four cherry-picks applied with auto-merges only, no manual conflict resolution.
Type
🐛 Bug Fix
Changes
Three fixes already shipped in v1.84.4 carried into the 1.85.x line, plus two that only apply to the 1.85 line, plus one follow-on fix to the manual conflict resolution that strictly improves on what v1.84.4 shipped.
#29089 / #29311 removes a duplicate Claude Code trace emission caused by the pass-through streaming handler invoking the success-handler path twice. #29343 normalizes a leading
Bearerprefix (case-insensitive, per RFC 7235) before classifying an API key as sk- or JWT inUserAPIKeyAuth._safe_hash_litellm_api_key, so the helper produces consistent hashed output regardless of whether the caller stripped the prefix. #29358 switchesResetBudgetJob's batched key/user/team update to a per-row narrow{spend, budget_reset_at}write and removes the pre-zeroing of the cross-pod spend counter, so a single bad row no longer detonates the batch and a failed reset no longer leaves a budget-bypass window.#27913 stops the v3 rate-limiter from leaking its internal TPM/RPM-reservation stash (
_litellm_rate_limit_descriptors,_litellm_tpm_reserved_*) onto the provider request body. #29447 stops theuse_chat_completions_apicontrol flag from leaking onto the provider request body via the OpenAI bridge. Both surface as upstream HTTP 400s for unknown fields; both were absent from v1.84.x (different code paths) so the v1.84.4 cherry-pick set didn't include them.The follow-on commit completes the #29089 cherry-pick by extracting the per-endpoint dispatch chain into
_build_passthrough_logging_result, matching the source PR's design and unbreaking its regression test.First cherry-pick PR against the new
stable/1.85.xline branch (created from v1.85.2 earlier today).