fix(proxy): apply temp_budget_increase to multi-window budget_limits - #35265
fix(proxy): apply temp_budget_increase to multi-window budget_limits#35265a-korath wants to merge 2 commits into
Conversation
temp_budget_increase / temp_budget_expiry were silently ignored for keys whose budget is configured via budget_limits (multi-window) instead of a top-level max_budget. _update_key_budget_with_temp_budget_increase returns early when max_budget is None, and _virtual_key_multi_budget_check never read the boost, so the feature was a complete no-op for such keys. _virtual_key_multi_budget_check now reads the active boost via _get_temp_budget_increase and raises each finite window's effective limit by that amount, mirroring the single-max_budget behaviour. Expired boosts are ignored. Fixes BerriAI#35247
|
|
| if math.isfinite(effective_max_budget): | ||
| effective_max_budget += temp_budget_increase |
There was a problem hiding this comment.
Non-finite boosts disable enforcement
When a key update supplies Infinity or NaN with a future expiry, this addition makes the effective limit non-finite, causing the subsequent math.isfinite condition to skip enforcement of the otherwise finite budget window until expiry. Reject non-finite increases at ingestion or fail closed here. How this was verified: The accepted update value flows unchanged from persisted token metadata through _get_temp_budget_increase into this addition and the subsequent finiteness guard.
Rule Used: What: Fail any PR which may contains a security in... (source)
Knowledge Base Used: Proxy Authentication and Authorization
Greptile SummaryThis PR applies active temporary budget increases to finite multi-window key budgets.
Confidence Score: 4/5The PR should not merge until non-finite temporary increases are prevented from disabling multi-window budget enforcement. The new arithmetic correctly handles ordinary finite boosts, but an accepted non-finite value makes the effective limit fail the enforcement guard and leaves the affected key window uncapped until expiry. Files Needing Attention: litellm/proxy/auth/auth_checks.py
|
| Filename | Overview |
|---|---|
| litellm/proxy/auth/auth_checks.py | Applies temporary increases consistently to multi-window checks, but does not fail closed when the resulting limit is non-finite. |
| tests/test_litellm/proxy/auth/test_multi_budget_windows.py | Adds focused coverage for ordinary active and expired boosts, but does not cover non-finite boost values. |
Reviews (1): Last reviewed commit: "fix(proxy): apply temp_budget_increase t..." | Re-trigger Greptile
| from litellm.proxy.auth.user_api_key_auth import _get_temp_budget_increase | ||
| from litellm.proxy.proxy_server import get_current_spend | ||
|
|
||
| temp_budget_increase = _get_temp_budget_increase(valid_token) or 0.0 |
There was a problem hiding this comment.
Medium: Unprivileged users can raise their own budget
temp_budget_increase is trusted here, but /key/update does not classify either the dedicated temporary-grant fields or the equivalent raw metadata keys as a budget change. A key owner or team member with /key/update can therefore set a future expiry and an arbitrarily large increase, which this change now applies to every multi-window limit. Require the same admin authorization as max_budget and budget_limits, validate that the increase is finite and non-negative, and reject these reserved keys when supplied through raw metadata.
PR overviewThis pull request updates proxy authorization checks so temporary budget increases are applied to multi-window budget limits. One security issue remains open: users with key or team update access can grant themselves an arbitrarily large temporary budget increase, bypassing the intended budget controls across all configured windows. No reported issues have yet been addressed, so authorization and input validation for these temporary grant fields still need to be enforced. Open issues (1)
Fixed/addressed: 0 · PR risk: 6/10 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Issue BerriAI#35247 has two enforcement paths for multi-window key budgets: the auth-time check and the pre-call reservation. The prior commit only patched the auth check, so a boosted key still blocked at the reservation gate. Thread the boost through _get_budget_limit_counters and route both paths through a shared apply_temp_budget_increase helper so they cannot diverge.
TLDR
Problem this solves:
temp_budget_increaseis silently ignored onbudget_limitskeysHow it solves it:
Relevant issues
Fixes #35247
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Reproduction below uses tiny cent-scale windows so a real over-budget state is reached in a couple of live chat calls rather than needing to burn $500. Captured at commit c8b55a0 against a live proxy hitting a real provider
budget_limits(no top-levelmax_budget), a24hwindow capped at $0.01BudgetExceededError: Key over 24h budget/key/updateBudgetExceededError, after this PR it succeeds until the boost expires or the boosted ceiling ($0.06) is hit. Before the reservation-path part of this fix the boosted key cleared the auth check but still blocked at the reservation gate with a distinctBudget has been exceeded! Key=...:24herror, so the boost was only fully honored once both paths were patchedType
🐛 Bug Fix
Changes
temp_budget_increase/temp_budget_expiry(set via/key/update) were only ever applied to a key's top-levelmax_budget._update_key_budget_with_temp_budget_increasereturns early whenmax_budget is None, which is exactly the case for a key configured purely throughbudget_limits. Multi-window key budgets are enforced by two independent paths and neither read the boost: the auth-time check (_virtual_key_multi_budget_check) and the pre-call reservation (_get_budget_countersinbudget_reservation.py). So for any multi-window key the boost saved to metadata, showed up in/key/info, and did nothing to request authorization; patching only the auth check still left the reservation gate blocking a boosted keyBoth paths now raise each finite window's effective limit by the active boost, read through the existing
_get_temp_budget_increasehelper, and route through a singleapply_temp_budget_increasehelper so the two enforcement sites cannot drift. Infinite windows are left untouched since a boost oninfis stillinf, and expired boosts returnNoneso they change nothing. The boost is key-level metadata, so team, user, and org window builders intentionally do not receive it. On the auth path the boosted ceiling also flows intoget_current_spendso its stale-counter re-check uses the same threshold, and the raisedBudgetExceededErrorreports the effective limitFinal Attestation