fix(proxy): skip budget checks for model discovery routes (#31078) - #31081
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes a bug where
Confidence Score: 5/5Safe to merge — targeted indentation fix bringing four budget checks under an already-present guard, with no logic added or removed beyond that scope. The guard itself and the MODEL_DISCOVERY_ROUTES bypass logic were already correct; this PR simply applies them consistently. No new code paths are introduced, and the regression test exercises the exact scenario described in the bug report. No files require special attention; the logic change is confined to the indentation of four existing calls in auth_checks.py.
|
| Filename | Overview |
|---|---|
| litellm/proxy/auth/auth_checks.py | Moves _tag_max_budget_check, personal budget check, _check_team_member_budget, and _check_end_user_budget inside the outer if not skip_budget_checks: guard so MODEL_DISCOVERY_ROUTES correctly bypass all budget checks for internal users. |
| tests/proxy_unit_tests/test_zero_budget_model_discovery.py | New regression test for /v1/models budget bypass; uses a broad except Exception catch (flagged in a previous thread) that can mask unrelated failures with a misleading assertion message. |
| .github/workflows/test-unit-proxy-db.yml | Adds the new test file to the CI test matrix; straightforward change with no issues. |
Reviews (2): Last reviewed commit: "style: fix black formatting for auth_che..." | Re-trigger Greptile
| if custom_llm_provider in ["vertex_ai", "bedrock"] and "haiku" in model.lower(): | ||
| anthropic_messages_optional_request_params.pop("context_management", None) |
There was a problem hiding this comment.
Hardcoded model-name check for
context_management support
"haiku" in model.lower() is a hardcoded model-specific flag, which the project explicitly forbids. When a new model adds or drops context_management support, this condition silently misbehaves until a code change is shipped. The existing pattern in the codebase uses _supports_model_capability backed by model_prices_and_context_window.json — a supports_context_management key should be added there and read via that helper, the same way supports_output_config is used for the effort-param check above.
Rule Used: What: Do not hardcode model-specific flags in the ... (source)
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!
| supported_openai_params = get_supported_openai_params( | ||
| model=model, | ||
| custom_llm_provider=custom_llm_provider, | ||
| ) or [] | ||
|
|
||
| # If the provider model doesn't support context_management (or it's not mapped via OpenAI params) | ||
| # Note: Litellm's OpenAI param mapping doesn't track context_management natively yet, | ||
| # but if we need to drop it for vertex/bedrock Haiku, we can infer support. | ||
| # However, for now, we drop it if 'additional_drop_params' specifies it, or if it's Vertex/Bedrock Haiku. | ||
| if custom_llm_provider in ["vertex_ai", "bedrock"] and "haiku" in model.lower(): |
There was a problem hiding this comment.
supported_openai_params is assigned here but never referenced anywhere in the surrounding block. This is dead code that adds an unnecessary call to get_supported_openai_params.
| supported_openai_params = get_supported_openai_params( | |
| model=model, | |
| custom_llm_provider=custom_llm_provider, | |
| ) or [] | |
| # If the provider model doesn't support context_management (or it's not mapped via OpenAI params) | |
| # Note: Litellm's OpenAI param mapping doesn't track context_management natively yet, | |
| # but if we need to drop it for vertex/bedrock Haiku, we can infer support. | |
| # However, for now, we drop it if 'additional_drop_params' specifies it, or if it's Vertex/Bedrock Haiku. | |
| if custom_llm_provider in ["vertex_ai", "bedrock"] and "haiku" in model.lower(): | |
| # Drop context_management for providers/models that don't support it. | |
| # TODO: gate on a `supports_context_management` key in model_prices_and_context_window.json | |
| # once that capability is tracked there (similar to `supports_output_config`). | |
| if custom_llm_provider in ["vertex_ai", "bedrock"] and "haiku" in model.lower(): |
| try: | ||
| await common_checks( | ||
| request_body={}, | ||
| team_object=None, | ||
| user_object=user_obj, | ||
| end_user_object=None, | ||
| global_proxy_spend=None, | ||
| general_settings={}, | ||
| route="/v1/models", | ||
| llm_router=None, | ||
| proxy_logging_obj=proxy_logging_obj, | ||
| valid_token=valid_token, | ||
| request=mock_request, | ||
| skip_budget_checks=False, # It starts False, but common_checks should set it to True | ||
| ) | ||
| passed = True | ||
| except Exception as e: | ||
| passed = False | ||
| print(f"Failed with exception: {type(e).__name__}: {str(e)}") |
There was a problem hiding this comment.
Broad
except Exception masks unrelated failures with a misleading assertion message
The test catches every exception and records passed = False, then asserts with the message "BudgetExceededError was raised...". If common_checks raises an unrelated error (e.g. AttributeError from a missing mock), the test fails with the same message as if a genuine budget error was thrown. Only litellm.BudgetExceededError should be caught; all other exceptions should propagate normally.
018e8e9 to
6001c5f
Compare
|
Thanks for the contribution, @Hasnaathussain! A couple of things to get this over the line:
|
|
Thanks for the review, @Sameerlite! Here is the requested evidence demonstrating the before and after states: 1. Before the FixWhen an curl -i -X GET http://localhost:4000/v1/models \
-H "Authorization: Bearer your-exhausted-user-key"Response: HTTP/1.1 400 Bad Request
Content-Type: application/json
{
"error": {
"message": "ExceededBudget: User=user_123 over budget. Spend=10.0, Budget=10.0",
"type": "budget_exceeded",
"param": null,
"code": 400
}
}2. After the FixWith the budget check calls now properly nested inside the curl -i -X GET http://localhost:4000/v1/models \
-H "Authorization: Bearer your-exhausted-user-key"Response: HTTP/1.1 200 OK
Content-Type: application/json
{
"object": "list",
"data": [
{
"id": "gpt-4o",
"object": "model",
"created": 1677610602,
"owned_by": "openai"
}
]
}This ensures that program components and frontend clients using these keys can always discover endpoints without getting blocked by quota limits. |
|
🚨 URGENT: This PR is 13 DAYS OLD and blocking internal user functionality Summary:
Impact: Internal users locked out of critical functionality Status: ✅ Ready for immediate merge to litellm_internal_staging @BerriAI/engineering - Requesting urgent team review. This needs immediate merge. |
|
@Hasnaathussain looks like |
|
I've moved |
c77d449 to
e4819d8
Compare
e4819d8 to
8e7e973
Compare
|
The model-discovery bypass is already merged in #29483, and this PR's test-only diff is superseded; closing to avoid duplicate maintenance. |
Fixes #31078.
Description
When an internal_user has their budget exhausted,
GET /v1/modelsandGET /modelswere returning400 budget_exceededinstead of the model list, whereas proxy admins with an exhausted budget were able to successfully access the route.The root cause was that
auth_checks.py::common_checks()correctly identifies thatMODEL_DISCOVERY_ROUTESshould bypass budget checks by settingskip_budget_checks = True. However, the conditional blockif not skip_budget_checks:exited too early, omitting the_tag_max_budget_check, user-level personal budget check,_check_team_member_budget, and_check_end_user_budgetchecks. This PR indents those checks so they correctly honor the bypass flag, allowing model discovery endpoints to operate free of budget checks for internal_users.A test has been added to prevent regressions.