fix(proxy): admin-gate permissions on /key/update and /key/regenerate (LIT-4092)

[yucheng-berri]

Relevant issues

Linear ticket

Resolves LIT-4092

Pre-Submission checklist

• I have added meaningful tests
• My PR passes all CI/CD checks (e.g., lint, format, unit tests)
• 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).

Type

Bug Fix

Changes

_check_permissions_caller_permission was only wired into _common_key_generation_helper. It is now also invoked from _validate_update_key_data and regenerate_key_fn so the three write paths share the same admin-only rule for the permissions field on the key model

The helper is refactored to accept the full request model and key its check on "permissions" in data.model_fields_set rather than the value's truthiness. The model default (permissions = {} on GenerateRequestBase) is not in model_fields_set when the field is omitted, so the omit case continues to pass for non-admin callers on all three endpoints. Any explicit value in the request body, including {} or null, is treated the same as any other write and requires PROXY_ADMIN

In regenerate_key_fn the gate runs before the premium_user license check. Ordering is pinned by test_regenerate_key_non_admin_permissions_rejected_before_enterprise_gate, which fails on a mutant that swaps the two gates (the test observes a 500 enterprise error instead of the 403 permissions rejection)

Nine tests in tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py cover the matrix: non-admin explicit-non-empty, explicit-empty, explicit-null (rejected 403), non-admin omit-field (allowed), and PROXY_ADMIN across every shape (allowed). Seven of the nine fail on the pre-fix HEAD; all nine pass on this commit. Full mapped test file (339 tests) green

Screenshots / Proof of Fix

Live proxy on localhost:4010 against Postgres, LITELLM_LICENSE set to make /key/regenerate reachable. A non-admin key $USER_KEY owned by an internal_user role sends permissions in the request body

/key/update explicit non-empty

$ curl -sS -X POST http://localhost:4010/key/update \
    -H "Authorization: Bearer $USER_KEY" -H "Content-Type: application/json" \
    -d "{\"key\":\"$OWN_KEY\",\"permissions\":{\"get_spend_routes\":true}}"
{"error":{"message":"{'error': 'Only proxy admins can set `permissions` on a key.'}","code":"403"}}

/key/update explicit empty

$ curl -sS -X POST http://localhost:4010/key/update \
    -H "Authorization: Bearer $USER_KEY" -H "Content-Type: application/json" \
    -d "{\"key\":\"$OWN_KEY\",\"permissions\":{}}"
{"error":{"message":"{'error': 'Only proxy admins can set `permissions` on a key.'}","code":"403"}}

/key/regenerate explicit non-empty

$ curl -sS -X POST http://localhost:4010/key/regenerate \
    -H "Authorization: Bearer $USER_KEY" -H "Content-Type: application/json" \
    -d "{\"key\":\"$OWN_KEY\",\"permissions\":{\"get_spend_routes\":true}}"
{"error":{"message":"{'error': 'Only proxy admins can set `permissions` on a key.'}","code":"403"}}

/key/regenerate explicit empty

$ curl -sS -X POST http://localhost:4010/key/regenerate \
    -H "Authorization: Bearer $USER_KEY" -H "Content-Type: application/json" \
    -d "{\"key\":\"$OWN_KEY\",\"permissions\":{}}"
{"error":{"message":"{'error': 'Only proxy admins can set `permissions` on a key.'}","code":"403"}}

Ordering: same proxy launched with LITELLM_LICENSE unset (premium_user=False). Non-admin with permissions in the request body gets the 403 rather than the 500 enterprise error, confirming the gate runs before the license check

$ curl -sS -X POST http://localhost:4010/key/regenerate \
    -H "Authorization: Bearer $USER_KEY" -H "Content-Type: application/json" \
    -d "{\"key\":\"$OWN_KEY\",\"permissions\":{\"get_spend_routes\":true}}"
{"error":{"message":"{'error': 'Only proxy admins can set `permissions` on a key.'}","code":"403"}}

Same non-premium proxy, same caller, no permissions in the body: reaches the license check as expected

$ curl -sS -X POST http://localhost:4010/key/regenerate \
    -H "Authorization: Bearer $USER_KEY" -H "Content-Type: application/json" \
    -d "{\"key\":\"$OWN_KEY\"}"
{"error":{"message":"Regenerating Virtual Keys is an Enterprise feature...","code":"500"}}

Controls unchanged: PROXY_ADMIN can still set permissions to any value on all three endpoints; a non-admin caller who omits permissions on /key/update still uses the personal-key fast path for unrelated fields; a non-admin caller who omits permissions on /key/generate still creates a key with the model default

Out of scope, tracked separately

• LIT-4137: bulk-update handlers go through _process_single_key_update which does not invoke the helper. /key/bulk_update is admin-only. /team/key/bulk_update broadcasts via KeyUpdateFields, an extra="forbid" allowlist that omits permissions, so the field is currently unreachable through that route; adding it to the allowlist without also invoking the helper would regress
• LIT-4138: NewUserRequest and UpdateUserRequest inherit the permissions field. /user/new with auto_create_key=True (default) can carry the field into generate_key_helper_fn; the current helper is invoked from _common_key_generation_helper but not from generate_key_helper_fn directly, so the /user/new path is not covered
• LIT-4139: _check_allowed_routes_caller_permission uses the truthiness pattern this PR replaced for permissions. The same model_fields_set treatment applies

---

Note

High Risk
Authorization on virtual-key permissions is security-sensitive; the change tightens gates on update/regenerate paths where the bug allowed self-granted capabilities.

Overview
Closes a privilege-escalation gap where non-admins could set key permissions (e.g. get_spend_routes) via /key/update and /key/regenerate because _check_permissions_caller_permission only ran on key generation.

The helper now takes the full request model and treats permissions as “in the body” when it appears in model_fields_set, so explicit {}, null, or non-empty dicts all require PROXY_ADMIN; omitting the field still allows normal non-admin flows. The same check is wired into _validate_update_key_data and regenerate_key_fn (before the enterprise license gate on regenerate). Tests cover generate/update/regenerate for omit vs explicit values and admin vs non-admin.

^{Reviewed by Cursor Bugbot for commit 79a6301. Bugbot is set up for automated code reviews on this repo. Configure here.}

[yucheng-berri]

@greptileai

[greptile-apps]

Greptile Summary

This PR extends the permissions-field admin gate to the /key/update and /key/regenerate write paths, which previously had no guard. The helper is also refactored to use model_fields_set for presence detection, so omitting the field continues to pass for non-admin callers while any explicit value in the request body requires PROXY_ADMIN.

• _check_permissions_caller_permission now accepts the full request model and detects field presence via model_fields_set; it is wired into _validate_update_key_data and regenerate_key_fn in addition to its existing call site in _common_key_generation_helper.
• In regenerate_key_fn the gate is placed before the premium_user enterprise check, ensuring a non-premium proxy returns a 403 rather than a 500 when a non-admin sends permissions; the ordering is pinned by a dedicated test.
• Nine new unit tests cover the full non-admin rejection matrix (explicit non-empty, explicit empty, explicit null, omit-field) for all three write paths, plus admin-allowed cases for generate and update.

Confidence Score: 5/5

Safe to merge — the change adds missing guards on two write endpoints without altering any existing behavior for callers who omit the field.

The refactor is narrow and mechanically correct: model_fields_set reliably distinguishes an omitted field from an explicitly sent one in Pydantic v2, all three write paths now share the same gate, and the enterprise-gate ordering is pinned by a dedicated test. The type hierarchy (UpdateKeyRequest and RegenerateKeyRequest both extend GenerateRequestBase) is consistent with the updated function signature. No existing behavior for non-admin callers who omit permissions changes.

No files require special attention; both changed files are straightforward and well-tested.

Important Files Changed

Filename	Overview
litellm/proxy/management_endpoints/key_management_endpoints.py	Refactors _check_permissions_caller_permission to accept the full request model and use model_fields_set for presence detection; wires the gate into _validate_update_key_data and regenerate_key_fn
tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py	Adds nine new tests covering the non-admin rejection matrix (explicit non-empty, explicit empty, explicit null, omit-field) for generate, update, and regenerate paths, plus the enterprise-gate ordering invariant

_{Reviews (4): Last reviewed commit: "fix(proxy): admin-gate `permissions` on ..." | Re-trigger Greptile}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[greptile-apps]

Greptile Summary

This PR adds the permissions field admin-only guard to the /key/update and /key/regenerate endpoints, closing two gaps left open after the original /key/generate gate was introduced. The core change moves the guard function from a permissions value argument to a data: GenerateRequestBase argument so it can check model_fields_set, distinguishing an omitted field (which carries the model default {}) from an explicit permissions payload (which triggers the admin check regardless of truthiness).

• Guard function refactored (_check_permissions_caller_permission): now uses \"permissions\" in data.model_fields_set instead of truthiness, so an explicit permissions: {} or permissions: null is correctly rejected for non-admins, while an omitted field silently passes.
• New call sites added: _validate_update_key_data (covers /key/update) and regenerate_key_fn (covers /key/regenerate), both placed before any DB work so the rejection is fast and enterprise-license-independent.
• Test suite: eight new mock-only tests cover non-empty self-grant, explicit-empty clear, null clear, omit-field control, admin control, and the create-path tightening; no real network calls are made.

Confidence Score: 4/5

Safe to merge for the targeted update and regenerate endpoints; the one non-admin client-visible change (rejecting explicit permissions: {} on key creation) is documented and low-risk in practice.

The guard logic is correct across all explicit/omit/null combinations, the new call sites are placed before any DB work, and the test suite covers every documented attack scenario plus controls. The only concern is a deliberate behavior shift on the create path for non-admins who send an explicit empty permissions object — the PR author acknowledges it and explains why legitimate callers are unaffected, but it is still a breaking change without a flag.

Both changed files look solid; key_management_endpoints.py deserves a second read around the _check_permissions_caller_permission early-return condition to confirm the model_fields_set logic reads correctly to future maintainers.

Important Files Changed

Filename	Overview
litellm/proxy/management_endpoints/key_management_endpoints.py	Guard function signature changed to accept full data object for model_fields_set inspection; new call sites added to _validate_update_key_data and regenerate_key_fn in the correct pre-DB position; logic is sound for all explicit/omit combinations.
tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py	Eight new mock-only tests covering all attack vectors (non-empty grant, explicit-empty clear, null clear) plus controls (omit-field non-admin pass, admin unrestricted); no real network calls; no existing tests weakened.

_{Reviews (2): Last reviewed commit: "fix(proxy): gate `permissions` admin-onl..." | Re-trigger Greptile}

[yucheng-berri]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit e1afc85. Configure here.}

[yucheng-berri]

@greptileai please re-review. The amended commit (e4076a3) adds test_regenerate_key_non_admin_permissions_rejected_before_enterprise_gate, which pins your Review 1 concern about the untested ordering guarantee between the permissions gate and the enterprise-license gate inside regenerate_key_fn. Mutation-killed against a reorder: I flipped the two gates in source, the test failed with the expected enterprise error instead of 403, then restored. Also verified live on a non-premium proxy (no LITELLM_LICENSE): attack payload returns 403, no-attack payload returns the 500 enterprise error, from the same caller against the same proxy. PR body now includes both live curls under a new "Enterprise-gate ordering" section.

[yucheng-berri]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit e4076a3. Configure here.}

[yucheng-berri]

@greptileai re-review please, force-pushed with reworked docstrings, commit message, and PR body. Same code change, less prose describing the pre-fix behavior. New commit is 79a6301.

[yucheng-berri]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 79a6301. Configure here.}

[mateo-berri]

LGTM; thanks!