From c27951b53b6fe6f0581e4309d545832c3cea5e0b Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Fri, 1 May 2026 21:06:16 +0000 Subject: [PATCH 1/2] fix(auth): block missing write routes for proxy admin viewers `_check_proxy_admin_viewer_access` enumerates write routes a PROXY_ADMIN_VIEW_ONLY caller may not invoke, then falls through to "allow" for any management route not listed. Several write endpoints were never added to the blocklist, so a viewer could: - block or unblock any team via `/team/block` / `/team/unblock` - mutate team permissions via `/team/permissions_update` and `/team/permissions_bulk_update` - create, update, or delete JWT key mappings via `/jwt/key/mapping/{new,update,delete}` - bulk-edit keys via `/key/bulk_update` - reset key spend via the path-parameterized `/key/{id}/reset_spend` Hoist the blocklist into a module-level frozenset and a tuple of suffix patterns so it's clear what to extend when a new write route is added, and pull the existing key write routes from the `KeyManagementRoutes` enum so the two stay in sync. Adds parametrized tests over the newly-blocked routes plus baseline coverage for routes that should remain allowed (info / list / daily-activity reads). Co-Authored-By: Claude Opus 4.7 (1M context) --- litellm/proxy/auth/route_checks.py | 69 +++++++++++++------ .../proxy/auth/test_route_checks.py | 67 ++++++++++++++++++ 2 files changed, 114 insertions(+), 22 deletions(-) diff --git a/litellm/proxy/auth/route_checks.py b/litellm/proxy/auth/route_checks.py index 6417307f691..5f49b5eb354 100644 --- a/litellm/proxy/auth/route_checks.py +++ b/litellm/proxy/auth/route_checks.py @@ -6,6 +6,7 @@ from fastapi import HTTPException, Request, status from litellm._logging import verbose_proxy_logger from litellm.proxy._types import ( CommonProxyErrors, + KeyManagementRoutes, LiteLLM_UserTable, LiteLLMRoutes, LitellmUserRoles, @@ -14,6 +15,49 @@ from litellm.proxy._types import ( from .auth_checks_organization import _user_is_org_admin +# Management write routes denied to PROXY_ADMIN_VIEW_ONLY. Adding a new write +# endpoint to a management router REQUIRES adding it here too — the surrounding +# check falls through to "allow" if the route is not matched, which previously +# let view-only admins call /team/block, /team/unblock, /key/bulk_update, etc. +_PROXY_ADMIN_VIEW_ONLY_BLOCKED_ROUTES = frozenset( + [ + # user + "/user/new", + "/user/delete", + "/user/bulk_update", + # team + "/team/new", + "/team/update", + "/team/delete", + "/team/block", + "/team/unblock", + "/team/permissions_update", + "/team/permissions_bulk_update", + # model + "/model/new", + "/model/update", + "/model/delete", + # JWT key mapping + "/jwt/key/mapping/new", + "/jwt/key/mapping/update", + "/jwt/key/mapping/delete", + # key management — keep in sync with KeyManagementRoutes write entries + KeyManagementRoutes.KEY_GENERATE.value, + KeyManagementRoutes.KEY_UPDATE.value, + KeyManagementRoutes.KEY_DELETE.value, + KeyManagementRoutes.KEY_REGENERATE.value, + KeyManagementRoutes.KEY_GENERATE_SERVICE_ACCOUNT.value, + KeyManagementRoutes.KEY_BLOCK.value, + KeyManagementRoutes.KEY_UNBLOCK.value, + KeyManagementRoutes.KEY_BULK_UPDATE.value, + ] +) + +# Suffixes for `/key/{key_id}/...` path-parameterized write routes that the +# enum templates with `{key_id}`. The blocklist above can't match templated +# paths directly because the request route carries the resolved key id. +_PROXY_ADMIN_VIEW_ONLY_BLOCKED_KEY_SUFFIXES = ("/regenerate", "/reset_spend") + class RouteChecks: @staticmethod @@ -626,28 +670,9 @@ class RouteChecks: status_code=status.HTTP_403_FORBIDDEN, detail=f"user not allowed to access this route, role= {_user_role}. Trying to access: {route} and updating invalid param: {param}. only user_email and password can be updated", ) - elif ( - route - in [ - "/user/new", - "/user/delete", - "/user/bulk_update", - "/team/new", - "/team/update", - "/team/delete", - "/model/new", - "/model/update", - "/model/delete", - "/key/generate", - "/key/delete", - "/key/update", - "/key/regenerate", - "/key/service-account/generate", - "/key/block", - "/key/unblock", - ] - or route.startswith("/key/") - and route.endswith("/regenerate") + elif route in _PROXY_ADMIN_VIEW_ONLY_BLOCKED_ROUTES or ( + route.startswith("/key/") + and route.endswith(_PROXY_ADMIN_VIEW_ONLY_BLOCKED_KEY_SUFFIXES) ): # Block write operations for PROXY_ADMIN_VIEW_ONLY raise HTTPException( diff --git a/tests/test_litellm/proxy/auth/test_route_checks.py b/tests/test_litellm/proxy/auth/test_route_checks.py index a5d405cfc2d..ac2efb9ab69 100644 --- a/tests/test_litellm/proxy/auth/test_route_checks.py +++ b/tests/test_litellm/proxy/auth/test_route_checks.py @@ -90,6 +90,73 @@ def test_proxy_admin_viewer_config_update_route_rejected(): assert "role= proxy_admin_viewer" in str(exc_info.value.detail) +@pytest.mark.parametrize( + "blocked_route", + [ + # team write routes that previously fell through the blocklist + "/team/block", + "/team/unblock", + "/team/permissions_update", + "/team/permissions_bulk_update", + # JWT key mapping write routes + "/jwt/key/mapping/new", + "/jwt/key/mapping/update", + "/jwt/key/mapping/delete", + # key write routes + "/key/bulk_update", + # path-parameterized key write routes (suffix match) + "/key/abc123/regenerate", + "/key/abc123/reset_spend", + # baseline coverage of routes that were already blocked + "/team/new", + "/team/delete", + "/key/generate", + "/key/delete", + "/model/new", + "/model/delete", + ], +) +def test_proxy_admin_viewer_blocked_management_writes(blocked_route): + """View-only admins must be denied on every management write route — the + fall-through path previously allowed /team/block, /team/unblock, + /key/bulk_update, /key/{id}/reset_spend, and the JWT key-mapping routes.""" + with pytest.raises(HTTPException) as exc_info: + RouteChecks._check_proxy_admin_viewer_access( + route=blocked_route, + _user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY.value, + request_data={}, + ) + assert exc_info.value.status_code == 403 + assert blocked_route in str(exc_info.value.detail) + + +@pytest.mark.parametrize( + "allowed_read_route", + [ + "/team/info", + "/team/list", + "/v2/team/list", + "/team/permissions_list", + "/team/daily/activity", + "/user/info", + "/user/list", + "/key/info", + "/key/list", + "/model/info", + "/jwt/key/mapping/list", + "/jwt/key/mapping/info", + ], +) +def test_proxy_admin_viewer_allowed_management_reads(allowed_read_route): + """View-only admins must still be allowed to read management routes.""" + # Should not raise + RouteChecks._check_proxy_admin_viewer_access( + route=allowed_read_route, + _user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY.value, + request_data={}, + ) + + def test_virtual_key_allowed_routes_with_litellm_routes_member_name_allowed(): """Test that virtual key is allowed to call routes when allowed_routes contains LiteLLMRoutes member name""" From 2a8fe32850a7fef65f305bfaa08923dd6e513d97 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Sat, 2 May 2026 02:32:01 +0000 Subject: [PATCH 2/2] fix(types): add /team/permissions_bulk_update to management_routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The blocklist check in _check_proxy_admin_viewer_access only fires for routes that match LiteLLMRoutes.management_routes — the bulk-update endpoint was missing from that list, so the test for view-only admins on /team/permissions_bulk_update fell through to "allow." Co-Authored-By: Claude Opus 4.7 (1M context) --- litellm/proxy/_types.py | 1 + 1 file changed, 1 insertion(+) diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 3cca23f07ab..3c16f6fdda7 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -565,6 +565,7 @@ class LiteLLMRoutes(enum.Enum): "/team/available", "/team/permissions_list", "/team/permissions_update", + "/team/permissions_bulk_update", "/team/daily/activity", # model "/model/new",