fix(proxy): admin-gate permissions on _process_single_key_update (LIT-4137)

[yucheng-berri]

Relevant issues

Linear ticket

Resolves LIT-4137

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

The bulk-update entrypoints /key/bulk_update and /team/key/bulk_update route through _process_single_key_update, not through _validate_update_key_data, so the _check_permissions_caller_permission gate LIT-4092 wired into the single-key path never fires on bulk

The bulk paths are currently safe by two independent structural barriers: BulkUpdateKeyRequestItem (per-item request model for /key/bulk_update) doesn't declare permissions, so Pydantic silently drops the field before it can reach persistence. KeyUpdateFields (broadcast payload for /team/key/bulk_update) uses model_config = ConfigDict(extra="forbid"), so Pydantic 422s at parse time. Neither structural barrier is pinned by a test that would fail if the barrier were ever loosened

This wires _check_permissions_caller_permission(data=update_key_request, user_api_key_dict=user_api_key_dict) into _process_single_key_update right after _validate_max_budget and before prepare_key_update_data. Zero behavior change today for callers routing through the current bulk request models. Defense-in-depth for the class: a future widening of either allowlist to include permissions cannot reopen the vulnerability

Two regression tests in tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py

• test_process_single_key_update_non_admin_permissions_rejected — non-admin UpdateKeyRequest with permissions={"get_spend_routes": True} returns 403
• test_process_single_key_update_non_admin_permissions_explicit_empty_rejected — same for permissions={} via model_fields_set

Both mutation-killed against removing the gate. Full mapped test file (341 tests) green

Screenshots / Proof of Fix

Live proxy on localhost:4010 against Postgres. Admin mints two keys, then exercises the current bulk-update surface

Existing bulk behavior on non-permissions field (unchanged control)

$ curl -sS -X POST http://localhost:4010/key/bulk_update \
    -H "Authorization: Bearer sk-1234" -H "Content-Type: application/json" \
    -d "{\"keys\":[{\"key\":\"$K1\",\"max_budget\":42},{\"key\":\"$K2\",\"max_budget\":99}]}"
total_requested: 2
successful_updates: 2
failed_updates: 0

/key/bulk_update with permissions in the request body. BulkUpdateKeyRequestItem silently drops the unknown field, the update proceeds without persisting permissions

$ curl -sS -X POST http://localhost:4010/key/bulk_update \
    -H "Authorization: Bearer sk-1234" -H "Content-Type: application/json" \
    -d "{\"keys\":[{\"key\":\"$K1\",\"permissions\":{\"get_spend_routes\":true}}]}"
$ curl -sS "http://localhost:4010/key/info?key=$K1" -H "Authorization: Bearer sk-1234"
permissions: {}

/team/key/bulk_update with permissions in update_fields. KeyUpdateFields is extra="forbid", so Pydantic 422s at parse time

$ curl -sS -X POST http://localhost:4010/team/key/bulk_update \
    -H "Authorization: Bearer sk-1234" -H "Content-Type: application/json" \
    -d "{\"team_id\":\"litellm-dashboard\",\"key_ids\":[\"$K1\"],\"update_fields\":{\"permissions\":{\"get_spend_routes\":true}}}"
{"detail":[{"type":"extra_forbidden","loc":["body","update_fields","permissions"],"msg":"Extra inputs are not permitted", ...}]}

Non-admin cannot reach /key/bulk_update at all (existing admin-only gate at handler layer)

$ curl -sS -X POST http://localhost:4010/key/bulk_update \
    -H "Authorization: Bearer $NA_KEY" -H "Content-Type: application/json" \
    -d '{"keys":[{"key":"sk-anything","max_budget":42}]}'
{"detail":{"error":"Only proxy admins can perform bulk key updates"}}

The new gate is a defense-in-depth layer on the inner perimeter. Every write path through _process_single_key_update that would ever carry permissions from a non-admin now hits the 403 before the DB write

Related

Closes the third item in the LIT-4092 / LIT-4139 / LIT-4137 family. LIT-4092 gated permissions on the single-key write paths; LIT-4139 did the same for allowed_routes; this PR extends the permissions gate to the bulk write path helper so all four write paths (/key/generate, /key/update, /key/regenerate, and both bulk endpoints via the helper) share the same admin-only rule

---

Note

Low Risk
Small authorization hardening on key management with no intended behavior change for current bulk APIs; tests cover the new gate.

Overview
Adds defense-in-depth so bulk key updates cannot set permissions unless the caller is a proxy admin.

_check_permissions_caller_permission (already used on single-key validation) is now invoked at the start of _process_single_key_update, which backs /key/bulk_update and /team/key/bulk_update. Any UpdateKeyRequest that explicitly includes permissions—including {} via model_fields_set—returns 403 for non-admins before DB work.

Current bulk request models still strip or forbid permissions, so day-to-day behavior is unchanged; the gate protects the shared helper if those allowlists widen later. Two async unit tests lock in the 403 behavior on _process_single_key_update.

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

[yucheng-berri]

@greptileai

[yucheng-berri]

bugbot run

[greptile-apps]

Greptile Summary

This PR adds a call to _check_permissions_caller_permission inside _process_single_key_update and backs it with two regression tests.

• The new gate is placed right after _validate_max_budget and before any DB reads, so it fires on every write path that flows through the helper — including the bulk-update entrypoints — without touching callers that never set permissions.
• Two async pytest tests cover the non-admin rejection for both a non-empty and an explicit-empty permissions value, asserting HTTP 403 and the permissions keyword in the error detail.

Confidence Score: 5/5

The change adds a synchronous guard that raises before any DB access, so the only risk is over-rejection — but default callers that never set permissions are explicitly exempted by the model_fields_set / falsy-value check, preserving existing behavior.

The inserted call is correctly typed (UpdateKeyRequest inherits from GenerateRequestBase), is placed before any await, and mirrors the identical guard already present on every other write path through this file. The two new tests are mutation-resistant and cover the explicit-empty edge case. No existing test coverage is weakened and no backwards-incompatible behavior change is introduced for current callers.

No files require special attention.

Important Files Changed

Filename	Overview
litellm/proxy/management_endpoints/key_management_endpoints.py	Adds _check_permissions_caller_permission call into _process_single_key_update at the correct position; type-compatible with GenerateRequestBase, synchronous, raises before any DB I/O.
tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py	Two new async tests validate the 403 gate for non-admin callers with permissions set; no redundant local imports (amended commit), assertions are mutation-resistant.

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

[greptile-apps]

Greptile Summary

This PR adds _check_permissions_caller_permission to _process_single_key_update, extending the existing admin-only gate on permissions to all write paths that funnel through that helper (including the bulk-update endpoints).

• Core change (5 lines in key_management_endpoints.py): the permission check is inserted immediately after _validate_max_budget and before prepare_key_update_data, mirroring its placement in _validate_update_key_data and the regenerate path.
• Tests: two new async mock-only regression tests verify that a non-admin UpdateKeyRequest carrying permissions (non-empty and explicit-empty) receives a 403 before any DB write occurs. Both tests use only AsyncMock/MagicMock and make no real network calls.

Confidence Score: 4/5

Safe to merge; the change is a small, additive permission gate with no side effects for callers that don't supply the guarded field.

The five-line production change adds an existing, well-tested helper into one more call site. Both new tests are mock-only, mutation-killable, and cover the explicit-empty edge case. The only findings are redundant local imports inside the two new test functions — no logic issues.

No files require special attention.

Important Files Changed

Filename	Overview
litellm/proxy/management_endpoints/key_management_endpoints.py	Wires _check_permissions_caller_permission into _process_single_key_update right after _validate_max_budget; five-line addition that mirrors the guard already present in _validate_update_key_data and the regenerate path
tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py	Adds two async mock-only regression tests for the new gate; redundant local re-import of _process_single_key_update inside each test body (already imported at module level)

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

[greptile-apps]

_process_single_key_update is already imported at module level (line 40), so the local re-import inside each test function is redundant. Removing it keeps the tests consistent with the rest of the file.

Suggested change

	from litellm.proxy.management_endpoints.key_management_endpoints import (
	_process_single_key_update,
	)
	update_key_request = UpdateKeyRequest(
	key="abc123",
	permissions={"get_spend_routes": True},
	)
	update_key_request = UpdateKeyRequest(
	key="abc123",
	permissions={"get_spend_routes": True},
	)

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!

[greptile-apps]

Same redundant local import in the second test — _process_single_key_update is already available from the module-level import.

Suggested change

	from litellm.proxy.management_endpoints.key_management_endpoints import (
	_process_single_key_update,
	)
	update_key_request = UpdateKeyRequest(key="abc123", permissions={})
	update_key_request = UpdateKeyRequest(key="abc123", permissions={})

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!

[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 9fa0ccd. Configure here.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[yucheng-berri]

@greptileai re-review, amended commit (57ac7b8) drops the redundant local re-imports of _process_single_key_update from both new tests per your P2 nit. The symbol is already imported at module scope (line 40). Zero behavior change.

[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 57ac7b8. Configure here.}

[mateo-berri]

LGTM; thanks!