Repository navigation
fix: tighten budget field validation and authorization checks #27897
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
d0368bc
a0724d5
09bff75
e154a36
62b1c47
c3b33b1
e4010a8
bf9c279
e66e369
cc0751a
b89f1ae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |
| """ | ||
|
|
||
| import asyncio | ||
| import math | ||
| import re | ||
| import time | ||
| from typing import TYPE_CHECKING, Any, Dict, List, Literal, Optional, Type, Union, cast | ||
|
|
@@ -328,7 +329,10 @@ def _global_proxy_budget_check( | |
| and route != "/v1/models" | ||
| and route != "/models" | ||
| ): | ||
| if global_proxy_spend > litellm.max_budget: | ||
| if ( | ||
| math.isfinite(litellm.max_budget) | ||
| and global_proxy_spend > litellm.max_budget | ||
| ): | ||
| raise litellm.BudgetExceededError( | ||
| current_cost=global_proxy_spend, max_budget=litellm.max_budget | ||
| ) | ||
|
|
@@ -645,7 +649,7 @@ async def common_checks( # noqa: PLR0915 | |
| counter_key=f"spend:user:{user_object.user_id}", | ||
| fallback_spend=user_object.spend or 0.0, | ||
| ) | ||
| if user_spend >= user_budget: | ||
| if math.isfinite(user_budget) and user_spend >= user_budget: | ||
| raise litellm.BudgetExceededError( | ||
| current_cost=user_spend, | ||
| max_budget=user_budget, | ||
|
|
@@ -3280,7 +3284,10 @@ async def _virtual_key_max_budget_check( | |
| # collect information for alerting # | ||
| #################################### | ||
|
|
||
| if spend >= valid_token.max_budget: | ||
| # Defense-in-depth (GHSA-2rv4-xv66-fpjg): spend >= NaN is always False, | ||
| # so a NaN max_budget would silently disable enforcement. Treat a | ||
| # non-finite max_budget as "no configured limit" rather than as a bypass. | ||
| if math.isfinite(valid_token.max_budget) and spend >= valid_token.max_budget: | ||
| raise litellm.BudgetExceededError( | ||
| current_cost=spend, | ||
| max_budget=valid_token.max_budget, | ||
|
|
@@ -3313,7 +3320,7 @@ async def _virtual_key_multi_budget_check( | |
| counter_key=counter_key, | ||
| fallback_spend=0.0, | ||
| ) | ||
| if window_spend >= w["max_budget"]: | ||
| if math.isfinite(w["max_budget"]) and window_spend >= w["max_budget"]: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. High: Non-finite window budgets are treated as unlimited
|
||
| raise litellm.BudgetExceededError( | ||
| current_cost=window_spend, | ||
| max_budget=w["max_budget"], | ||
|
|
@@ -3568,7 +3575,10 @@ async def _check_team_member_budget( | |
| fallback_spend=team_member_spend, | ||
| ) | ||
|
|
||
| if team_member_spend >= team_member_budget: | ||
| if ( | ||
| math.isfinite(team_member_budget) | ||
| and team_member_spend >= team_member_budget | ||
| ): | ||
| raise litellm.BudgetExceededError( | ||
| current_cost=team_member_spend, | ||
| max_budget=team_member_budget, | ||
|
|
@@ -3650,7 +3660,7 @@ async def _team_max_budget_check( | |
| fallback_spend=team_object.spend or 0.0, | ||
| ) | ||
|
|
||
| if spend > team_object.max_budget: | ||
| if math.isfinite(team_object.max_budget) and spend > team_object.max_budget: | ||
| if valid_token: | ||
| call_info = CallInfo( | ||
| token=valid_token.token, | ||
|
|
@@ -3698,7 +3708,7 @@ async def _team_multi_budget_check( | |
| counter_key=counter_key, | ||
| fallback_spend=0.0, | ||
| ) | ||
| if window_spend >= w["max_budget"]: | ||
| if math.isfinite(w["max_budget"]) and window_spend >= w["max_budget"]: | ||
| raise litellm.BudgetExceededError( | ||
| current_cost=window_spend, | ||
| max_budget=w["max_budget"], | ||
|
|
@@ -3812,6 +3822,7 @@ async def _project_max_budget_check( | |
| if ( | ||
| max_budget is not None | ||
| and project_object.spend is not None | ||
| and math.isfinite(max_budget) | ||
| and project_object.spend > max_budget | ||
| ): | ||
| if valid_token: | ||
|
|
@@ -4004,7 +4015,7 @@ async def _organization_max_budget_check( | |
| ) | ||
|
|
||
| # Check if organization spend exceeds max budget | ||
| if org_spend >= org_max_budget: | ||
| if math.isfinite(org_max_budget) and org_spend >= org_max_budget: | ||
| # Trigger budget alert | ||
| call_info = CallInfo( | ||
| token=valid_token.token, | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -1235,6 +1235,19 @@ async def _update_single_user_helper( | |||||
| }, | ||||||
| ) | ||||||
|
|
||||||
| # Non-admins cannot modify budget-sensitive fields even on their own record | ||||||
| # (GHSA-wvg4-6222-3q4r). | ||||||
| if user_api_key_dict.user_role != LitellmUserRoles.PROXY_ADMIN.value: | ||||||
| _protected_fields = ("max_budget", "soft_budget", "spend") | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. High: User budget reset bypass
Suggested change
|
||||||
| for _field in _protected_fields: | ||||||
| if _field in non_default_values: | ||||||
| raise HTTPException( | ||||||
| status_code=403, | ||||||
| detail={ | ||||||
| "error": f"Non-admin users cannot modify '{_field}'. Contact your proxy admin." | ||||||
| }, | ||||||
| ) | ||||||
|
krrish-berri-2 marked this conversation as resolved.
Resolved
|
||||||
|
|
||||||
| existing_metadata = ( | ||||||
| cast(Dict, getattr(existing_user_row, "metadata", {}) or {}) | ||||||
| if existing_user_row is not None | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| import copy | ||
| import inspect | ||
| import json | ||
| import math | ||
| import os | ||
| import re | ||
| import secrets | ||
|
|
@@ -608,6 +609,11 @@ async def validate_team_id_used_in_service_account_request( | |
| return True | ||
|
|
||
|
|
||
| _BUDGET_NUMERIC_KEYS = frozenset( | ||
| ["max_budget", "soft_budget", "max_parallel_requests", "tpm_limit", "rpm_limit"] | ||
| ) | ||
|
|
||
|
|
||
| def _enforce_upperbound_key_params( | ||
| data: Union[GenerateKeyRequest, UpdateKeyRequest], | ||
| fill_defaults: bool = True, | ||
|
|
@@ -618,6 +624,21 @@ def _enforce_upperbound_key_params( | |
| For key generation (fill_defaults=True): fills None values with upperbound defaults. | ||
| For key update (fill_defaults=False): only validates explicitly provided values. | ||
| """ | ||
| # Always reject NaN / Inf regardless of whether an upperbound config is set | ||
| # (GHSA-2rv4-xv66-fpjg): float('nan') passes every `< 0` check because | ||
| # nan < 0 is False, and spend >= nan is always False, permanently disabling | ||
| # budget enforcement for any key that carries it. | ||
| for elem in data: | ||
| key, value = elem | ||
| if key in _BUDGET_NUMERIC_KEYS and value is not None: | ||
| if not math.isfinite(value): | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail={ | ||
| "error": f"{key} must be a finite number. Received: {value}" | ||
| }, | ||
| ) | ||
|
|
||
| if litellm.upperbound_key_generate_params is None: | ||
| return | ||
|
|
||
|
|
@@ -687,6 +708,11 @@ async def _common_key_generation_helper( # noqa: PLR0915 | |
| prisma_client=prisma_client, | ||
| ) | ||
|
|
||
| # Capture the caller-supplied max_budget before any defaults or upperbound | ||
| # params can fill it, so the ceiling check only fires when the caller | ||
| # explicitly requested a budget. | ||
| _requested_max_budget = data.max_budget | ||
|
|
||
| # check if user set default key/generate params on config.yaml | ||
| if litellm.default_key_generate_params is not None: | ||
| for elem in data: | ||
|
|
@@ -710,6 +736,35 @@ async def _common_key_generation_helper( # noqa: PLR0915 | |
| # check if user set upperbound key/generate params on config.yaml | ||
| _enforce_upperbound_key_params(data, fill_defaults=True) | ||
|
|
||
| # Delegated-authority ceiling (GHSA-q775-qw9r-2r4g): a non-admin caller | ||
| # cannot grant a key more budget than the caller's own key carries. This | ||
| # prevents budget escalation when no upperbound_key_generate_params config | ||
| # is present. | ||
| if ( | ||
| user_api_key_dict.user_role != LitellmUserRoles.PROXY_ADMIN.value | ||
| and _requested_max_budget is not None | ||
| ): | ||
| if user_api_key_dict.max_budget is None: | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail={ | ||
| "error": ( | ||
| f"Cannot set max_budget ({_requested_max_budget}) on a generated " | ||
| "key because the caller's own key has no budget configured." | ||
| ) | ||
| }, | ||
| ) | ||
|
krrish-berri-2 marked this conversation as resolved.
Resolved
|
||
| if _requested_max_budget > user_api_key_dict.max_budget: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. High: Budget multiplication through generated keys This compares each requested child key budget to the caller's key budget independently. A non-admin caller with a |
||
| raise HTTPException( | ||
| status_code=400, | ||
| detail={ | ||
| "error": ( | ||
| f"max_budget ({_requested_max_budget}) cannot exceed the caller's " | ||
| f"own max_budget ({user_api_key_dict.max_budget})." | ||
| ) | ||
| }, | ||
| ) | ||
|
|
||
| # APPLY ENTERPRISE KEY MANAGEMENT PARAMS | ||
| try: | ||
| from litellm_enterprise.proxy.management_endpoints.key_management_endpoints import ( | ||
|
|
@@ -1416,19 +1471,24 @@ async def generate_key_fn( | |
|
|
||
| await check_org_admin_can_generate_keys(user_api_key_dict=user_api_key_dict) | ||
|
|
||
| # Validate budget values are not negative | ||
| if data.max_budget is not None and data.max_budget < 0: | ||
| # Validate budget values are not negative and are finite numbers | ||
| # (GHSA-2rv4-xv66-fpjg): float('nan') passes `< 0` because nan < 0 is False. | ||
| if data.max_budget is not None and ( | ||
| not math.isfinite(data.max_budget) or data.max_budget < 0 | ||
| ): | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail={ | ||
| "error": f"max_budget cannot be negative. Received: {data.max_budget}" | ||
| "error": f"max_budget must be a non-negative finite number. Received: {data.max_budget}" | ||
| }, | ||
| ) | ||
| if data.soft_budget is not None and data.soft_budget < 0: | ||
| if data.soft_budget is not None and ( | ||
| not math.isfinite(data.soft_budget) or data.soft_budget < 0 | ||
| ): | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail={ | ||
| "error": f"soft_budget cannot be negative. Received: {data.soft_budget}" | ||
| "error": f"soft_budget must be a non-negative finite number. Received: {data.soft_budget}" | ||
| }, | ||
| ) | ||
|
|
||
|
|
@@ -1880,10 +1940,12 @@ def _validate_max_budget(max_budget: Optional[float]) -> None: | |
| Raises: | ||
| HTTPException: If max_budget is negative | ||
| """ | ||
| if max_budget is not None and max_budget < 0: | ||
| if max_budget is not None and (not math.isfinite(max_budget) or max_budget < 0): | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail={"error": f"max_budget cannot be negative. Received: {max_budget}"}, | ||
| detail={ | ||
| "error": f"max_budget must be a non-negative finite number. Received: {max_budget}" | ||
| }, | ||
| ) | ||
|
|
||
|
|
||
|
|
@@ -2413,12 +2475,14 @@ async def update_key_fn( # noqa: PLR0915 | |
| ) | ||
|
|
||
| try: | ||
| # Validate budget values are not negative | ||
| if data.max_budget is not None and data.max_budget < 0: | ||
| # Validate budget values are not negative and are finite numbers | ||
| if data.max_budget is not None and ( | ||
| not math.isfinite(data.max_budget) or data.max_budget < 0 | ||
| ): | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail={ | ||
| "error": f"max_budget cannot be negative. Received: {data.max_budget}" | ||
| "error": f"max_budget must be a non-negative finite number. Received: {data.max_budget}" | ||
| }, | ||
| ) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Medium: Non-finite user budget bypass
The runtime budget check skips enforcement for non-finite
user_budgetvalues. If/user/newacceptsmax_budgetwithout finite-number validation, a caller allowed to create users can sendNaNorInfinityand receive a user key whose user-level spend cap is skipped here. Fail closed at enforcement or add equivalent validation on user creation.