[Infra] Merge daily dev branch with main#23827
Conversation
Tests added for: UiLoadingSpinner, HashicorpVaultEmptyPlaceholder, PageVisibilitySettings, errorUtils, and mcpToolCrudClassification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
[Test] UI Dashboard - Add unit tests for 5 untested files
…x_budget updates to admins Non-admin users (INTERNAL_USER) could call /key/block and /key/unblock on arbitrary keys, and modify max_budget on their own keys via /key/update. These endpoints are now restricted to proxy admins, team admins, or org admins. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…calation_fix [Fix] Privilege Escalation on /key/block, /key/unblock, and /key/update max_budget
fix: set oauth2_flow when building MCPServer in _execute_with_mcp_client
Remove `.length > 0` check so that when a backend filter returns an empty result set the table correctly shows no data instead of falling back to the previous unfiltered logs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
[Fix] UI - Logs: Empty Filter Results Show Stale Data
…ate and key/update Internal users could exploit key/generate and key/update to create unbound keys (no user_id, no budget) or attach keys to non-existent teams. This adds validation for non-admin callers: auto-assign user_id on generate, reject invalid team_ids, and prevent removing user_id on update. Closes LIT-1884 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move the non-admin team validation into the existing get_team_object call site to avoid an extra DB round-trip. The existing call already fetches the team for limits checking — we now add the LIT-1884 guard there when team_obj is None for non-admin callers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lid_keys [Fix] Prevent Internal Users from Creating Invalid Keys
…changed When updating or regenerating a key without changing its key_alias, the existing alias was being re-validated against current format rules. This caused keys with legacy aliases (created before stricter validation) to become uneditable. Now validation only runs when the alias actually changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…n_on_update [Fix] Key Alias Re-validation on Update Blocks Legacy Aliases
The test expected fallback to all logs when backend filters return empty, but the source was intentionally changed to show empty results instead of stale data. Updated test to match. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add disable_custom_api_keys UI setting that prevents users from specifying custom key values during key generation and regeneration. When enabled, all keys must be auto-generated, eliminating the risk of key hash collisions in multi-tenant environments. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Without this field on the model, GET /get/ui_settings omits the setting from the response and field_schema, preventing the UI from reading it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a toggle switch to the admin UI Settings page so administrators can enable/disable custom API key values without making direct API calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
[Feature] Disable Custom Virtual Key Values via UI Setting
fix: langfuse trace leak key on model params
[Infra] Merge personal dev branch with daily dev branch
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| data.user_id = user_api_key_dict.user_id | ||
| verbose_proxy_logger.warning( | ||
| "key/generate: auto-assigning user_id=%s for non-admin caller", | ||
| user_api_key_dict.user_id, |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 13 days ago
To fix log injection, sanitize the potentially user-controlled value before logging it. For plain-text logs, this typically means removing newline and carriage-return characters (and optionally other control characters) from user input before it is passed to the logger. This preserves the value for identification while preventing a malicious user from injecting extra log lines or manipulating the log structure.
In this file, the only problematic sink identified is the warning at lines 1259–1262:
verbose_proxy_logger.warning(
"key/generate: auto-assigning user_id=%s for non-admin caller",
user_api_key_dict.user_id,
)We can fix it locally by creating a sanitized version of user_api_key_dict.user_id just for logging. A simple and non-breaking approach is to convert to str and remove \r and \n. This does not change any functional behavior of the endpoint, only how the value is represented in the log line. No new imports are required.
Concretely:
- Immediately before the
verbose_proxy_logger.warningcall, define a new variable, e.g.safe_user_id = str(user_api_key_dict.user_id).replace("\r", "").replace("\n", ""). - Pass
safe_user_idto the logger instead ofuser_api_key_dict.user_id.
No other parts of the function or file need to be changed to address this specific alert.
| @@ -1256,9 +1256,10 @@ | ||
| ) | ||
| if not _is_proxy_admin and data.user_id is None: | ||
| data.user_id = user_api_key_dict.user_id | ||
| safe_user_id = str(user_api_key_dict.user_id).replace("\r", "").replace("\n", "") | ||
| verbose_proxy_logger.warning( | ||
| "key/generate: auto-assigning user_id=%s for non-admin caller", | ||
| user_api_key_dict.user_id, | ||
| safe_user_id, | ||
| ) | ||
|
|
||
| team_table: Optional[LiteLLM_TeamTableCachedObj] = None |
…key_leakage Revert "fix: langfuse trace leak key on model params"
Greptile SummaryThis is a daily Key concerns:
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| litellm/proxy/management_endpoints/key_management_endpoints.py | Largest changed file. Adds multiple LIT-1884 security fixes (auto-assign user_id, team validation, block/unblock admin guards, max_budget admin guard, disable_custom_api_keys) and makes get_new_token async. Contains a direct DB query in _check_key_admin_access that bypasses helper functions, a redundant key re-fetch when used from _validate_update_key_data, and backwards-incompatible behavior for non-admin team key creation. |
| litellm/proxy/_experimental/mcp_server/rest_endpoints.py | Adds oauth2_flow explicit override support to MCP server; adds guard to clear client_credentials flow when token_url is absent. Clean fix, well-commented. |
| litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py | Adds disable_custom_api_keys field to UISettings model and ALLOWED_UI_SETTINGS_FIELDS. Field intentionally not added to _RUNTIME_GENERAL_SETTINGS_FLAGS since it's read via get_ui_settings_cached() directly. |
| ui/litellm-dashboard/src/components/view_logs/log_filter_logic.tsx | Removes .length > 0 check so empty backend filter results are returned directly instead of falling back to cached logs — intentional UX fix, test updated to match. |
| tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py | Updates existing async tests and adds thorough mock-only tests for all LIT-1884 and disable_custom_api_keys changes. All tests mock external dependencies correctly. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[POST /key/generate] --> B{caller is proxy admin?}
B -- yes --> C[Skip user_id auto-assign]
B -- no --> D[Auto-assign caller's user_id]
D --> E{team_id provided?}
C --> E
E -- no --> F[_common_key_generation_helper]
E -- yes --> G{get_team_object succeeds?}
G -- yes --> H[key_generation_check]
G -- no, admin --> F
G -- no, non-admin --> ERR400[400 Team not found]
H --> F
F --> CHK[_check_custom_key_allowed]
CHK -- disabled & custom key --> ERR403A[403 Custom keys disabled]
CHK -- ok --> DONE[Key created]
subgraph /key/update max_budget path
U[PATCH /key/update] --> V{max_budget changed?}
V -- no --> W[normal update]
V -- yes --> X[_check_key_admin_access]
X --> Y{is proxy/team/org admin?}
Y -- yes --> W
Y -- no --> ERR403B[403 Admin only]
X -->|DB: find_unique verificationtoken| DB[(Database)]
end
subgraph /key/block + /key/unblock
BLK[POST /key/block or /key/unblock] --> CA[_check_key_admin_access]
CA -->|DB: find_unique verificationtoken| DB
CA --> Z{is proxy/team/org admin?}
Z -- yes --> BLK2[proceed with block/unblock]
Z -- no --> ERR403C[403 Admin only]
end
Last reviewed commit: b4c9c8a
| elif team_table is None: | ||
| return True # assume user is assigning team_id without using the team table | ||
| if _is_admin: | ||
| return True # admins can assign team_id without team table | ||
| # Non-admin callers must have a valid team (LIT-1884) | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail=f"Unable to find team object in database. Team ID: {data.team_id}", | ||
| ) |
There was a problem hiding this comment.
Backwards-incompatible breaking change for non-admin key creation
Before this PR, when team_table is None (i.e., the team doesn't exist in DB) and key_generation_settings is not configured, the code returned True for all callers — meaning non-admin users could create keys with arbitrary team_id values that didn't correspond to a real team. This PR now raises a 400 for non-admins in that case.
This is a security hardening fix, but it is a backwards-incompatible change for any non-admin users currently relying on the old permissive behaviour (e.g. creating keys tagged with a team_id that hasn't been registered in the team table). Per the project's policy on backwards-incompatible changes, new restrictions like this should be gated behind a user-controlled flag so that existing deployments aren't silently broken on upgrade.
Consider introducing a feature flag (e.g. litellm.require_valid_team_for_key_generation) and defaulting it to False so the behaviour is opt-in, with plans to flip the default in a future major release.
Rule Used: What: avoid backwards-incompatible changes without... (source)
| # Admin-only: only proxy admins, team admins, or org admins can modify max_budget | ||
| if data.max_budget is not None and data.max_budget != existing_key_row.max_budget: | ||
| if prisma_client is not None: | ||
| hashed_key = existing_key_row.token | ||
| await _check_key_admin_access( | ||
| user_api_key_dict=user_api_key_dict, | ||
| hashed_token=hashed_key, | ||
| prisma_client=prisma_client, | ||
| user_api_key_cache=user_api_key_cache, | ||
| route="/key/update (max_budget)", | ||
| ) |
There was a problem hiding this comment.
Backwards-incompatible restriction on
max_budget updates for non-admins
Prior to this PR, any user who had the key ownership permissions could update max_budget on their own key. Now only proxy admins, team admins, or org admins can do so (enforced via _check_key_admin_access). This is a new restriction on previously-allowed behaviour.
For users who currently auto-provision keys with budgets via non-admin service accounts, this will silently start returning 403 after upgrade. Per the project policy on backwards-incompatible changes, this restriction should be gated behind a feature flag (e.g. litellm.restrict_max_budget_updates_to_admins) so existing deployments are not broken on upgrade.
Rule Used: What: avoid backwards-incompatible changes without... (source)
| target_key_row = await prisma_client.db.litellm_verificationtoken.find_unique( | ||
| where={"token": hashed_token} | ||
| ) |
There was a problem hiding this comment.
Direct DB query bypasses helper function; causes redundant round-trip
prisma_client.db.litellm_verificationtoken.find_unique is a direct Prisma query rather than going through get_key_object, which is the designated helper per the project's DB access guidelines.
Additionally, when _check_key_admin_access is called from _validate_update_key_data for max_budget changes (line ~1912), the caller already holds existing_key_row — which contains team_id — so passing hashed_token and re-fetching the same row is an unnecessary extra DB round-trip. Consider accepting an optional team_id parameter so that callers who already have the row in memory can skip the lookup entirely.
Rule Used: What: In critical path of request, there should be... (source)
Relevant issues
Failing only 1 flaky test: https://app.circleci.com/pipelines/github/BerriAI/litellm?branch=litellm_internal_dev_03_16_2026
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Type
🚄 Infrastructure
Changes