[Fix] Prevent Internal Users from Creating Invalid Keys

[yuneng-jiang]

Relevant issues

Closes LIT-1884

Summary

Failure Path (Before Fix)

Internal users with key generation permissions could exploit /key/generate and /key/update to:

1. Create keys with only an alias (no user_id or team), resulting in unbound keys with no budget controls
2. Set non-existent team_id values on keys (the get_team_object failure was silently swallowed)
3. Update keys to remove their user_id (by setting it to empty string) or change to an invalid team_id

Fix

• key/generate: Auto-assign caller's user_id when not provided by non-admin callers. Raise 400 when get_team_object() fails for non-admins instead of swallowing the error. Close the bypass in key_generation_check where team_table=None with no key_generation_settings returned True for non-admins.
• key/update: Reject empty-string user_id (nullification attempt) with 403 for non-admins. Validate team_id exists in DB when changed by non-admins.
• Admin callers are unaffected — all new checks are bypassed for PROXY_ADMIN.

Testing

9 unit tests added covering:

• Internal user generate key without user_id → auto-assigned
• Internal user generate key with invalid team_id → 400
• Admin generate key with invalid team_id → allowed (regression)
• Admin generate key without user_id → not auto-assigned (regression)
• key_generation_check non-admin with no team table → 400
• key_generation_check admin with no team table → allowed
• Internal user update key removing user_id → 403
• Internal user update key with invalid team_id → 400
• Admin update key removing user_id → allowed (regression)

All 9 new tests pass. No regressions in existing 121 key management tests.

Type

🐛 Bug Fix
✅ Test

[vercel]

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

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

[greptile-apps]

Greptile Summary

This PR fixes LIT-1884 by closing several security gaps that allowed internal users to create unbound or team-less keys through /key/generate and /key/update. The fixes include auto-assigning the caller's user_id for non-admin key generation, raising 400 when a non-existent team_id is used, blocking empty-string user_id nullification attempts on update, and tightening key_generation_check to reject non-admins when team_table is None.

Key observations:

• Logic issue in _validate_update_key_data: The admin bypass check if team_obj is None and data.team_id is not None and not _is_proxy_admin (line 1908) is dead code. get_team_object raises HTTPException(404) for non-existent teams rather than returning None — confirmed by the test mock which uses side_effect=HTTPException(404). As a result, when an invalid team_id is set, execution leaves the block at the raise site before reaching line 1908, meaning the not _is_proxy_admin guard is never evaluated. Admin callers updating a key to a non-existent team would be unexpectedly blocked by the raw 404, contradicting the PR's stated guarantee of "Admin callers are unaffected." The fix in generate_key_fn (wrapping get_team_object in try/except and only re-raising for non-admins) needs to be applied to the update path as well.
• Missing regression test: There is no test verifying that an admin can update a key to a non-existent team_id without error, which would have caught the above gap immediately.
• The auto-assign of user_id for non-admin callers and the key_generation_check tightening are well-implemented and are correctly gated by the _is_proxy_admin check.

Confidence Score: 3/5

• The security fixes for non-admins are sound, but the admin bypass for the key/update team validation path is dead code and may inadvertently regress admin behavior.
• The non-admin security fixes are correctly implemented and tested. However, the admin bypass check in _validate_update_key_data is unreachable because get_team_object raises rather than returning None for missing teams, and there is no regression test for the admin update-with-invalid-team-id case. This constitutes a logic gap in one of the stated invariants ("Admin callers are unaffected").
• litellm/proxy/management_endpoints/key_management_endpoints.py — specifically the _validate_update_key_data function around lines 1899-1912.

Important Files Changed

Filename	Overview
litellm/proxy/management_endpoints/key_management_endpoints.py	Core security fix for LIT-1884. Logic issue: the admin bypass for invalid team_id in _validate_update_key_data is dead code because get_team_object raises instead of returning None for non-existent teams, making the team_obj is None guard unreachable. Other changes (auto-assign user_id, key_generation_check non-admin guard) are well-implemented.
tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py	9 new unit tests covering the main fix scenarios. All tests use mocks and no real network calls. Missing a regression test for admin updating a key to a non-existent team_id, which would have caught the dead-code admin bypass.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["key/generate or key/update called"] --> B{"Is PROXY_ADMIN?"}

    B -- Yes --> C["Skip user_id auto-assign\nSkip team validation\nProceed normally"]
    B -- No --> D{"user_id provided?"}

    D -- No --> E["Auto-assign caller user_id"]
    D -- Yes --> F{"team_id in request?"}
    E --> F

    F -- No --> G["Proceed to key_generation_check"]
    F -- Yes --> H["call get_team_object"]

    H -- Success --> I{"team_obj returned?"}
    H -- Raises exception --> J{"Is generate path?"}

    J -- "Yes - caught in try/except" --> K["Raise HTTP 400 for non-admin\nAdmin continues with team_table=None"]
    J -- "No - update path has no try/except" --> L["404 propagates to ALL callers\nincluding admins - BUG"]

    I -- None --> M{"key_generation_check:\nteam_table=None and non-admin?"}
    I -- Found --> N["Check team key limits"]

    M -- Yes --> O["Raise HTTP 400"]
    M -- "No - admin" --> P["Return True"]
    N --> G
    G --> Q["key_generation_check passes"]
    Q --> R["Generate or Update key"]

    style L fill:#f96,stroke:#c00
    style K fill:#fc6,stroke:#960

Loading

_{Last reviewed commit: 208740a}

[greptile-apps]

Backwards-incompatible behavior change without a feature flag

Auto-assigning the caller's user_id for every non-admin /key/generate call is a breaking change for existing users who intentionally generate keys without a user_id. Per the codebase convention, behavioral changes that affect existing users should be guarded by a user-controlled flag (e.g., litellm.enforce_user_id_on_key_generate) rather than silently changing behavior for all non-admin callers.

A user relying on the old behavior (generating unbound keys through the API) will silently find their keys now have their user_id attached — changing budget scoping, key list visibility, and any downstream logic that checks key.user_id is None.

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

[greptile-apps]

Redundant DB call — result discarded before second fetch

When data.team_id is set and the caller is a non-admin, this new validation block calls get_team_object(..., check_db_only=True) to verify the team exists. Because check_db_only=True bypasses the cache, this goes straight to the database. However, the resolved _team_obj is never reused — a second, identical get_team_object call is made at line 1925 (the existing team-limits check) also with check_db_only=True, hitting the DB again for the same team.

The result from the first call should be threaded forward to replace the second fetch, avoiding the redundant round-trip.

Rule Used: What: Avoid creating new database requests or Rout... (source)

[greptile-apps]

Inconsistent _is_proxy_admin null guard

The _is_proxy_admin check here omits the user_role is not None guard used in every other occurrence in the same file (e.g., generate_key_fn at line 1229 and key_generation_check at line 357). While None == LitellmUserRoles.PROXY_ADMIN.value evaluates to False in Python, making the behavior identical in practice, the inconsistency is surprising and could mask a future refactor where None might be treated differently.

Suggested change

	_is_proxy_admin = (
	user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN.value
	)
	_is_proxy_admin = (
	user_api_key_dict.user_role is not None
	and user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN.value
	)

[greptile-apps]

Admin bypass is unreachable dead code

The guard if team_obj is None at line 1908 is never triggered in practice because get_team_object raises HTTPException(404) when a team doesn't exist, rather than returning None. The companion test test_internal_user_cannot_set_invalid_team_id confirms this explicitly — it mocks get_team_object with side_effect=HTTPException(404), and the test asserts the 404 propagates directly.

This means:

1. When get_team_object raises at line 1900, execution immediately leaves the block — lines 1908-1912 are never reached.
2. The and not _is_proxy_admin guard is therefore never evaluated.
3. An admin user updating a key to a non-existent team_id would also be blocked by the raw 404, contradicting the PR's stated goal that "Admin callers are unaffected."

Compare with the equivalent logic in generate_key_fn around line 1250, where the exception from get_team_object is explicitly caught and only re-raised for non-admins (admins fall through with team_table=None). The same pattern is needed here: wrap the get_team_object call in a try/except and only re-raise for non-admin callers.

There is also no regression test covering the admin-update-with-invalid-team_id scenario; adding one would have surfaced this gap.