From 06e3691bc3173127902252512946f427190ede86 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Fri, 24 Apr 2026 16:50:29 -0700 Subject: [PATCH] fix(auth): exempt public routes and trust admin token role in centralized authz gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two regressions introduced by 3737d6a1f3 (centralized common_checks): 1. Public routes (e.g. /health/readiness, /metrics) are exempted by the builder fast-path but the wrapper then ran common_checks on the synthetic INTERNAL_USER_VIEW_ONLY token, which has no user_id, no team, no scopes — so common_checks rejected the request as admin- only. This broke every k8s readiness probe when master_key is set (helm chart job confirmed: pod never goes Ready, service has no endpoints). 2. The admin user_object synthesis only triggered when user_object is None. After any team-creation flow runs, the row for litellm_proxy_admin_name (default "default_user_id") exists in litellm_usertable with the default user_role=internal_user. get_user_object then returned that row, the synthesis was skipped, and master_key requests were demoted to internal_user — failing /team/update, /team/block, etc. The token's user_role is the source of truth for these paths (set inside the authenticated master_key / JWT-admin builders); a stale DB row must not override it. Fix: - Short-circuit _run_centralized_common_checks for routes already in LiteLLMRoutes.public_routes (or general_settings.public_routes). Same exemption surface the builder already trusts. - When the token's user_role is PROXY_ADMIN, force the synthesized admin user_object regardless of what get_user_object returned. Preserves the spend value from the DB row. Neither change reopens any of the seven bypasses the original commit closed: OAuth2, JWT non-admin, DB-fallback, /user/auth, pass-through headers, etc., still go through the gate. Only paths that were already admin or already public skip it. Adds two regression tests: - test_centralized_common_checks_skips_public_routes - test_centralized_common_checks_master_key_admin_overrides_db_user_role --- litellm/proxy/auth/user_api_key_auth.py | 25 +++-- .../proxy/auth/test_user_api_key_auth.py | 101 ++++++++++++++++++ 2 files changed, 119 insertions(+), 7 deletions(-) diff --git a/litellm/proxy/auth/user_api_key_auth.py b/litellm/proxy/auth/user_api_key_auth.py index 47b167273dd..ccb8fcddd26 100644 --- a/litellm/proxy/auth/user_api_key_auth.py +++ b/litellm/proxy/auth/user_api_key_auth.py @@ -1628,6 +1628,16 @@ async def _run_centralized_common_checks( user_custom_auth, ) + # Public routes (e.g. /health/readiness, /metrics) are exempt from + # auth in the builder — the wrapper must not retroactively apply + # authz on top, or k8s readiness probes and other unauthenticated + # callers get 401. + if ( + route in LiteLLMRoutes.public_routes.value # type: ignore[attr-defined] + or route_in_additonal_public_routes(current_route=route) + ): + return + # No-auth dev mode: master_key unset AND no JWT/OAuth2 auth # configured. The builder returns an INTERNAL_USER token for any # api_key; the proxy is unauthenticated by configuration. @@ -1754,16 +1764,17 @@ async def _run_centralized_common_checks( # common_checks identifies admin via user_object, not the token # (non_proxy_admin_allowed_routes_check). JWT admin shortcut and - # master_key tokens have no DB user row — synthesize one so admin - # route access is granted after centralization. - if ( - user_object is None - and user_api_key_auth_obj.user_role == LitellmUserRoles.PROXY_ADMIN - ): + # master_key tokens get admin from the token; the DB row for the + # same user_id (e.g. litellm_proxy_admin_name = "default_user_id") + # may have a non-admin user_role and would otherwise demote the + # caller. The token is the source of truth for these paths — force + # the admin user_object whenever the token says PROXY_ADMIN, even + # if a DB row was fetched. + if user_api_key_auth_obj.user_role == LitellmUserRoles.PROXY_ADMIN: user_object = LiteLLM_UserTable( user_id=user_api_key_auth_obj.user_id or litellm_proxy_admin_name, user_role=LitellmUserRoles.PROXY_ADMIN, - spend=0.0, + spend=user_object.spend if user_object is not None else 0.0, ) if project_object is not None: diff --git a/tests/test_litellm/proxy/auth/test_user_api_key_auth.py b/tests/test_litellm/proxy/auth/test_user_api_key_auth.py index e52876ce066..0105432d071 100644 --- a/tests/test_litellm/proxy/auth/test_user_api_key_auth.py +++ b/tests/test_litellm/proxy/auth/test_user_api_key_auth.py @@ -15,6 +15,8 @@ import litellm.proxy.proxy_server from litellm.caching.dual_cache import DualCache from litellm.proxy._types import ( LiteLLM_JWTAuth, + LiteLLM_UserTable, + LitellmUserRoles, ProxyErrorTypes, ProxyException, UserAPIKeyAuth, @@ -2052,6 +2054,105 @@ async def test_centralized_common_checks_short_circuits_when_master_key_unset(): setattr(_proxy_server_mod, k, v) +@pytest.mark.asyncio +async def test_centralized_common_checks_skips_public_routes(): + """Regression: public routes (e.g. /health/readiness) are exempted + by the builder fast-path. The wrapper must not retroactively run + common_checks on top — the synthetic INTERNAL_USER_VIEW_ONLY token + has no user_id, so common_checks would reject the request as + admin-only. Breaks k8s readiness probes when master_key is set.""" + import litellm.proxy.proxy_server as _proxy_server_mod + from fastapi import Request + from starlette.datastructures import URL + + token = UserAPIKeyAuth(user_role=LitellmUserRoles.INTERNAL_USER_VIEW_ONLY) + request = Request(scope={"type": "http"}) + request._url = URL(url="/health/readiness") + + attrs = _proxy_attrs_for_centralized_checks(user_custom_auth=None) + originals = {a: getattr(_proxy_server_mod, a, None) for a in attrs} + try: + for k, v in attrs.items(): + setattr(_proxy_server_mod, k, v) + with patch( + "litellm.proxy.auth.user_api_key_auth.common_checks", + new_callable=AsyncMock, + ) as mock_checks: + await _run_centralized_common_checks( + user_api_key_auth_obj=token, + request=request, + request_data={}, + route="/health/readiness", + ) + mock_checks.assert_not_awaited() + finally: + for k, v in originals.items(): + setattr(_proxy_server_mod, k, v) + + +@pytest.mark.asyncio +async def test_centralized_common_checks_master_key_admin_overrides_db_user_role(): + """Regression: master_key tokens have user_id=litellm_proxy_admin_name + (default 'default_user_id') and user_role=PROXY_ADMIN. If a row with + that user_id exists in litellm_usertable with a non-admin user_role + (created as a side effect of team membership), get_user_object + returns it and the synthesized admin user_object is skipped — so + common_checks demotes the master_key request to internal_user and + blocks /team/update. The token is the source of truth for admin + status; the DB row must not override it.""" + import litellm.proxy.proxy_server as _proxy_server_mod + from fastapi import Request + from starlette.datastructures import URL + + token = UserAPIKeyAuth( + api_key="sk-master", + user_id="default_user_id", + user_role=LitellmUserRoles.PROXY_ADMIN, + ) + request = Request(scope={"type": "http"}) + request._url = URL(url="/team/update") + + # Simulate the DB row that team-creation created: same user_id, but + # with user_role=internal_user (the default for new user rows). + db_user = LiteLLM_UserTable( + user_id="default_user_id", + user_role=LitellmUserRoles.INTERNAL_USER.value, + spend=1.5, + max_budget=None, + ) + + attrs = _proxy_attrs_for_centralized_checks(user_custom_auth=None) + originals = {a: getattr(_proxy_server_mod, a, None) for a in attrs} + try: + for k, v in attrs.items(): + setattr(_proxy_server_mod, k, v) + with ( + patch( + "litellm.proxy.auth.user_api_key_auth.get_user_object", + new_callable=AsyncMock, + return_value=db_user, + ), + patch( + "litellm.proxy.auth.user_api_key_auth.common_checks", + new_callable=AsyncMock, + ) as mock_checks, + ): + await _run_centralized_common_checks( + user_api_key_auth_obj=token, + request=request, + request_data={"team_id": "t1", "max_budget": 10}, + route="/team/update", + ) + mock_checks.assert_awaited_once() + forwarded = mock_checks.call_args.kwargs["user_object"] + assert forwarded is not None + assert forwarded.user_role == LitellmUserRoles.PROXY_ADMIN + assert forwarded.user_id == "default_user_id" + finally: + for k, v in originals.items(): + setattr(_proxy_server_mod, k, v) + + @pytest.mark.asyncio async def test_centralized_common_checks_http_exception_without_team_id(): """Regression: an HTTPException raised by any of the five parallel