Skip to content

[Infra] Merge personal dev branch with daily dev branch#23826

Merged
yuneng-jiang merged 20 commits intolitellm_internal_dev_03_16_2026from
litellm_yj_march_16_2026
Mar 17, 2026
Merged

[Infra] Merge personal dev branch with daily dev branch#23826
yuneng-jiang merged 20 commits intolitellm_internal_dev_03_16_2026from
litellm_yj_march_16_2026

Conversation

@yuneng-jiang
Copy link
Copy Markdown
Contributor

Relevant issues

CI/CD: only failing known flaky tests: https://app.circleci.com/pipelines/github/BerriAI/litellm?branch=litellm_yj_march_16_2026

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have Added testing in the tests/test_litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Delays 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)

CI status guideline:

  • 50-55 passing tests: main is stable with minor issues.
  • 45-49 passing tests: acceptable but needs attention
  • <= 40 passing tests: unstable; be careful with your merges and assess the risk.
  • Branch creation CI run
    Link:

  • CI run for the last commit
    Link:

  • Merge / cherry-pick CI run
    Links:

Type

🚄 Infrastructure

Changes

yuneng-jiang and others added 20 commits March 16, 2026 12:53
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
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>
[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
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Mar 17, 2026 6:47am

Request Review

@yuneng-jiang yuneng-jiang merged commit dcbaa05 into litellm_internal_dev_03_16_2026 Mar 17, 2026
64 of 66 checks passed
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 17, 2026

Greptile Summary

This PR merges a personal dev branch into the daily dev branch, delivering security hardening for key management endpoints (LIT-1884), a new admin UI toggle to disable custom virtual key values, a bug fix for log-filter fallback behaviour, and a batch of new unit tests across Python and TypeScript.

Key changes:

  • disable_custom_api_keys setting — new UISettings field + _check_custom_key_allowed guard enforces at the API level; the create-key UI hides the custom key field when enabled. Well-implemented end-to-end.
  • LIT-1884 key security hardening — non-admin users can no longer create keys with non-existent team_ids, are prevented from removing user_id from a key, have user_id auto-assigned if they omit it, and cannot block/unblock/modify max_budget on arbitrary keys. These are security improvements, but several are backwards-incompatible behaviour changes applied without a user-controlled feature flag (see inline comments).
  • _check_key_admin_access helper — new function performs a direct prisma_client.db.litellm_verificationtoken.find_unique call instead of using the existing get_key_object helper, bypassing DualCache and diverging from the established helper-function pattern.
  • log_filter_logic.tsx — removes the .length > 0 guard so an empty backend-filtered response is returned directly, rather than silently falling back to unfiltered client-side data. The corresponding test is correctly updated to reflect the new expected behaviour.
  • New unit tests — comprehensive mock-based tests for all the new Python behaviours, plus new Vitest tests for several UI components and utility functions.

Confidence Score: 2/5

  • This PR contains multiple backwards-incompatible behaviour changes in the key management critical path applied without feature flags, and a direct DB query that bypasses the established cache/helper pattern.
  • Score is low due to: (1) generate_key_fn silently auto-assigns user_id for non-admin callers — any existing automation creating shared/service keys without a user_id will have unintended user associations injected; (2) non-admins can no longer create keys with a non-existent team_id, which was previously allowed and may break integrations that pre-create keys before teams exist; (3) _check_key_admin_access uses a raw Prisma query instead of get_key_object, bypassing DualCache. The UI changes and new tests are clean and well-structured.
  • Pay close attention to litellm/proxy/management_endpoints/key_management_endpoints.py — specifically the generate_key_fn auto-assignment block (~line 1257), the key_generation_check non-admin team-validation change (~line 63), and the _check_key_admin_access direct DB query (~line 4854).

Important Files Changed

Filename Overview
litellm/proxy/management_endpoints/key_management_endpoints.py Core key management endpoint — adds custom-key-allow check, auto user_id assignment for non-admins (breaking), team validation (breaking), block/unblock admin enforcement, and key-alias skip-validation on unchanged alias. Contains backwards-incompatible changes and a direct DB query bypassing the helper function pattern.
litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py Adds disable_custom_api_keys field to UISettings model and ALLOWED_UI_SETTINGS_FIELDS; exposes get_ui_settings_cached helper (10-min cache). Clean and consistent with existing patterns.
tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py Adds comprehensive mock-only tests for the new LIT-1884 behaviours: custom-key-disable enforcement, admin/non-admin key generation checks, block/unblock access control, and key-alias unchanged-skip logic. Tests are well-structured and appropriately mocked. Tests are properly located in the mock-only test folder.
ui/litellm-dashboard/src/components/Settings/AdminSettings/UISettings/UISettings.tsx Adds a new "Disable custom Virtual key values" toggle backed by disable_custom_api_keys. Clean addition that follows the existing pattern for other UI settings toggles.
ui/litellm-dashboard/src/components/organisms/create_key_button.tsx Reads disable_custom_api_keys from UI settings and conditionally hides the custom key field in the create-key form. Small, correct change.
ui/litellm-dashboard/src/components/view_logs/log_filter_logic.tsx Removes the .length > 0 guard so an empty backend-filtered result is returned directly rather than falling back to unfiltered client-side logs. Behaviour change is intentional and matched by the updated test.

Sequence Diagram

sequenceDiagram
    participant Client
    participant generate_key_fn
    participant _check_custom_key_allowed
    participant get_ui_settings_cached
    participant key_generation_check
    participant _common_key_generation_helper

    Client->>generate_key_fn: POST /key/generate (data, user_api_key_dict)
    generate_key_fn->>generate_key_fn: [NEW] auto-assign user_id if non-admin & user_id is None
    generate_key_fn->>generate_key_fn: [NEW] reject non-existent team_id for non-admins
    generate_key_fn->>key_generation_check: validate team permissions
    generate_key_fn->>_common_key_generation_helper: proceed with key creation
    _common_key_generation_helper->>_check_custom_key_allowed: check data.key (custom value)
    _check_custom_key_allowed->>get_ui_settings_cached: read disable_custom_api_keys (10-min cache)
    get_ui_settings_cached-->>_check_custom_key_allowed: settings dict
    alt disable_custom_api_keys == true AND custom key provided
        _check_custom_key_allowed-->>Client: 403 Forbidden
    else
        _check_custom_key_allowed-->>_common_key_generation_helper: OK
        _common_key_generation_helper-->>Client: Generated key response
    end

    participant block_key_fn as block_key / unblock_key
    participant _check_key_admin_access
    participant prisma_direct as prisma_client.db (direct)
    participant get_team_object

    Client->>block_key_fn: POST /key/block (data, user_api_key_dict)
    block_key_fn->>_check_key_admin_access: [NEW] enforce admin access
    alt user is PROXY_ADMIN
        _check_key_admin_access-->>block_key_fn: OK
    else
        _check_key_admin_access->>prisma_direct: find_unique(token) — bypasses cache
        prisma_direct-->>_check_key_admin_access: key row (team_id)
        _check_key_admin_access->>get_team_object: fetch team (with cache)
        get_team_object-->>_check_key_admin_access: team obj
        alt caller is team admin or org admin
            _check_key_admin_access-->>block_key_fn: OK
        else
            _check_key_admin_access-->>Client: 403 Forbidden
        end
    end
    block_key_fn-->>Client: blocked key response
Loading

Comments Outside Diff (2)

  1. litellm/proxy/management_endpoints/key_management_endpoints.py, line 63-70 (link)

    P1 Backwards-incompatible rejection of keys with non-existent team_id without feature flag

    Previously, when team_table was None (team not found in DB), key_generation_check returned True unconditionally — allowing any caller to assign an arbitrary team_id. Now non-admins receive a 400 error.

    Any existing non-admin workflow that creates keys with a team_id where the team doesn't yet exist in the DB (or was deleted) will now break without warning. This behaviour change should be controlled by an opt-in flag rather than applied globally, consistent with the backwards-compatibility policy in this codebase.

    The same concern applies to the non-admin team validation added in _validate_update_key_data (line ~1928) and in generate_key_fn (~line 1279).

    Rule Used: What: avoid backwards-incompatible changes without... (source)

  2. ui/litellm-dashboard/src/components/view_logs/log_filter_logic.test.tsx, line 1576-1587 (link)

    P2 Test expectation changed to match new behaviour — verify intent is correct

    The old test asserted that when backend filters are active but the API returns an empty array, the hook falls back to client-side logs (1 result). The updated test now expects 0 results, matching the updated log_filter_logic.tsx change that removes the length > 0 guard.

    The code change makes sense: showing unfiltered client-side data when an active backend filter returns no matches is misleading. Just confirm this is the intended UX change (i.e. empty backend response = "no results", not a trigger to show all local logs).

Last reviewed commit: a087c44

Comment on lines +1257 to +1262
if not _is_proxy_admin and data.user_id is None:
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,
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Backwards-incompatible auto-assignment of user_id without feature flag

Non-admin internal users who previously called /key/generate without a user_id (e.g. to create shared/service keys) will now have the caller's user_id silently auto-assigned to the key. This is a breaking behavior change — existing automation scripts or integrations relying on creating unbound keys will produce different results without any opt-in.

Per the repo's backwards-compatibility policy, this should be gated behind a feature flag (e.g. litellm.enforce_key_user_id_binding = True) so existing deployments are not broken.

# Safer approach — opt-in flag
if not _is_proxy_admin and data.user_id is None and litellm.enforce_key_user_id_binding:
    data.user_id = user_api_key_dict.user_id
    verbose_proxy_logger.warning(...)

Rule Used: What: avoid backwards-incompatible changes without... (source)

Comment on lines +4854 to +4856
target_key_row = await prisma_client.db.litellm_verificationtoken.find_unique(
where={"token": hashed_token}
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Direct DB query bypasses established helper-function pattern

_check_key_admin_access issues a raw prisma_client.db.litellm_verificationtoken.find_unique call instead of going through the existing get_key_object helper. This bypasses DualCache (so the key record is fetched from DB on every block/unblock/update call even if it was recently read) and is inconsistent with the pattern used elsewhere in this file, where all key look-ups go through get_key_object / get_team_object. Any caching logic, telemetry spans, or future changes to key lookup behaviour won't automatically apply here.

Consider using the get_key_object helper instead, which handles cache read/write and is consistent with the rest of the codebase.

Rule Used: What: In critical path of request, there should be... (source)

@ishaan-berri ishaan-berri deleted the litellm_yj_march_16_2026 branch March 26, 2026 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant