Skip to content

fix(proxy): allow non-admin key_type preset transitions on /key/update #35006

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yucheng-berri wants to merge 2 commits into
base: litellm_internal_staging
Choose a base branch
from
+374 −9
Open

fix(proxy): allow non-admin key_type preset transitions on /key/update #35006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf61a33
fix(proxy): allow non-admin key_type preset transitions on /key/updat…
yucheng-berri Jul 28, 2026
696f6ee
docs(proxy): state the preset-provenance decision in the allowed_rout…
yucheng-berri Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions litellm/proxy/management_endpoints/key_management_endpoints.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import re
import secrets
import traceback
from collections.abc import Mapping
from collections.abc import Mapping, Sequence
from datetime import datetime, timedelta, timezone
from typing import Any, Callable, Dict, List, Literal, Optional, Tuple, cast

Expand Down Expand Up @@ -540,10 +540,12 @@ def _check_allowed_routes_caller_permission(
*,
allowed_routes_was_provided: bool = False,
allow_safe_presets: bool = False,
existing_allowed_routes: Sequence[str] | None = None,
) -> None:
"""
Require PROXY_ADMIN when `allowed_routes` is present in the request body,
unless the caller went through the `key_type` preset flow.
unless the caller went through the `key_type` preset flow or is moving the
key between safe preset buckets.

Raw-body call sites pass
`allowed_routes_was_provided="allowed_routes" in data.model_fields_set` so a
Expand All @@ -555,6 +557,27 @@ def _check_allowed_routes_caller_permission(
body, so `allowed_routes_was_provided` stays False and the safe-preset
carve-out below accepts any list of tokens in
`_NON_ADMIN_SAFE_ALLOWED_ROUTES_PRESETS`.

`/key/update` additionally passes `existing_allowed_routes` (the key's
current DB value): when both the requested value and the existing value
consist solely of safe preset tokens (either side may also be empty,
meaning unrestricted), the write is a key_type preset transition — e.g.
the Admin UI switching a key from "AI APIs" to "Full access" sends
`allowed_routes: []` — and this field-level gate lets it through. WHO may
perform the transition is enforced by the caller's downstream checks
(`can_team_member_execute_key_management_endpoint` and the
creator/ownership rules in `_validate_update_key_data`): the key's
creator-owner, a team admin, or a team member holding the `/key/update`
grant. A key whose existing value contains anything outside the safe
presets was route-restricted by an admin, so every non-admin write on it,
including clearing, stays rejected regardless of ownership.

Provenance of a safe preset is deliberately not tracked: the preset tier
is the self-service vocabulary (the 403 below directs non-admins to
`key_type`), and preset values were owner-adjustable on every release
before the LIT-4139 hardening. An admin who needs a restriction the
key's owner cannot lift must use a custom route list, which non-admins
can never write or clear.
"""
if not allowed_routes_was_provided and not allowed_routes:
return
Expand All @@ -566,6 +589,12 @@ def _check_allowed_routes_caller_permission(
and all(r in _NON_ADMIN_SAFE_ALLOWED_ROUTES_PRESETS for r in allowed_routes)
):
return
if (
existing_allowed_routes is not None

@veria-ai veria-ai Bot Jul 28, 2026

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.

High: Administrator route restrictions can be removed

The stored value does not indicate who selected the preset, so this branch also accepts transitions away from an administrator-enforced info_routes or llm_api_routes restriction. A regular team member holding the /key/update grant can change an admin-created team key from info_routes to llm_api_routes, enabling billable inference, or clear llm_api_routes to [], which disables the virtual-key route gate and exposes management routes allowed by the key holder's role. Keep widening transitions proxy-admin-only unless the database records that the restriction is explicitly user-managed; non-admins can safely be permitted to narrow an unrestricted key.

and all(isinstance(r, str) and r in _NON_ADMIN_SAFE_ALLOWED_ROUTES_PRESETS for r in (allowed_routes or []))
and all(isinstance(r, str) and r in _NON_ADMIN_SAFE_ALLOWED_ROUTES_PRESETS for r in existing_allowed_routes)
):
return
raise HTTPException(
status_code=403,
Comment thread
greptile-apps[bot] marked this conversation as resolved.
detail={
Expand Down Expand Up @@ -1888,6 +1917,9 @@ async def prepare_key_update_data(
data_json.pop("key", None)
data_json.pop("new_key", None)
data_json.pop("grace_period", None) # Request-only param, not a DB column
if "allowed_routes" in data_json and data_json["allowed_routes"] is None:
# The allowed_routes DB column is a non-nullable String[].
data_json["allowed_routes"] = []
Comment on lines +1920 to +1922

@devin-ai-integration devin-ai-integration Bot Jul 28, 2026

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.

🟡 New code comment added despite repository rule forbidding comments

A new inline code comment is introduced (key_management_endpoints.py:1914) even though the repository's mandatory coding guidelines forbid writing any new comments unless explicitly requested in a user prompt.
Impact: The change does not comply with the repository's stated contribution rules.

Rule source in CLAUDE.md

AGENTS.md points to CLAUDE.md, whose first rule states: "Do not write any comments (existing comments can stay) unless explicitly asked to in a user (not system) prompt". The added line # The allowed_routes DB column is a non-nullable String[]. is a newly introduced comment. The behavior it documents can be conveyed via the commit message or left uncommented per the rule.

Suggested change
if "allowed_routes" in data_json and data_json["allowed_routes"] is None:
# The allowed_routes DB column is a non-nullable String[].
data_json["allowed_routes"] = []
if "allowed_routes" in data_json and data_json["allowed_routes"] is None:
data_json["allowed_routes"] = []
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

if (
data.metadata is not None
and data.metadata.get("service_account_id") is not None
Expand Down Expand Up @@ -2268,10 +2300,12 @@ async def _validate_update_key_data(

_is_proxy_admin = user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN.value

_existing_allowed_routes = getattr(existing_key_row, "allowed_routes", None)
_check_allowed_routes_caller_permission(
allowed_routes=data.allowed_routes,
user_api_key_dict=user_api_key_dict,
allowed_routes_was_provided="allowed_routes" in data.model_fields_set,
existing_allowed_routes=(_existing_allowed_routes if isinstance(_existing_allowed_routes, list) else None),
)
_check_passthrough_routes_caller_permission(
data=data,
Expand Down
Loading
Loading