From ba876c98e6727fca5c2f2740c4e6912d86ec2d49 Mon Sep 17 00:00:00 2001 From: Yassin Kortam Date: Sat, 22 Aug 2026 14:25:29 -0700 Subject: [PATCH] fix(auth): stop the team fallback from widening model access (#37962) When get_team_object fails, the centralized auth gate rebuilds the team from the token's own fields. A token whose team row was missing when the key was read carries team_models=[] and team_blocked=False, and the model-access check reads an empty model list as every model, so the rebuilt team grants more than the real team ever did. get_team_object reported a deleted team and a database that would not answer as the same 404, so the fallback could not tell a definitive answer from a degraded read. Raise a TeamNotFoundError subclass, still a 404 with the same detail so every other caller is unaffected, only when the database answers and the row is absent. A team that is provably gone now refuses, and no setting overrides that. Otherwise the grant is merely unknown: a token carrying one may vouch, since replaying a recorded grant cannot widen it, and a token carrying none may not. allow_requests_on_db_unavailable still opts back out there, and is only consulted once the failure is known to be a degraded read. The Admin UI mints every session key against the UI_TEAM_ID sentinel, which by design never has a team row, so every UI request hit the new refusal with no override. Exempt UI_TEAM_ID explicitly so it keeps reconstructing from the token unconditionally, matching how the MCP handler and agent_permission_handler already special-case it. Resolves LIT-5522 --- litellm/proxy/auth/auth_checks.py | 28 ++ litellm/proxy/auth/user_api_key_auth.py | 39 ++- .../test_user_api_key_auth.py | 2 + .../proxy/auth/test_auth_checks.py | 47 +++ .../proxy/auth/test_user_api_key_auth.py | 277 ++++++++++++++++++ 5 files changed, 392 insertions(+), 1 deletion(-) diff --git a/litellm/proxy/auth/auth_checks.py b/litellm/proxy/auth/auth_checks.py index 12d6b44a648..e7b98b3cc7f 100644 --- a/litellm/proxy/auth/auth_checks.py +++ b/litellm/proxy/auth/auth_checks.py @@ -19,6 +19,7 @@ from typing import TYPE_CHECKING, Any, Final, Literal, Optional, Protocol, cast from fastapi import HTTPException, Request, status from pydantic import BaseModel +from typing_extensions import ReadOnly, TypedDict import litellm from litellm._logging import verbose_proxy_logger @@ -2512,6 +2513,27 @@ async def delete_cache_key_objects( await publish_auth_cache_invalidation(cache_key=hashed_token) +class _TeamNotFoundDetail(TypedDict): + error: ReadOnly[str] + + +class TeamNotFoundError(HTTPException): + """The team row is provably absent, as opposed to merely unreadable. + + ``get_team_object`` reports every failure as a 404, so a deleted team and a + database that would not answer are indistinguishable to its callers. Callers + that must not treat a degraded read as a definitive answer, such as the + authorization fallback in ``user_api_key_auth``, key on this subclass. It + stays a 404 carrying the same detail, so every other caller is unaffected. + """ + + def __init__(self, team_id: str) -> None: + detail: Final[_TeamNotFoundDetail] = { + "error": f"Team doesn't exist in db. Team={team_id}. Create team via `/team/new` call." + } + super().__init__(status_code=404, detail=detail) + + @log_db_metrics async def _get_team_db_check( team_id: str, prisma_client: PrismaClient, team_id_upsert: bool | None = None @@ -2557,6 +2579,10 @@ async def _get_team_object_from_user_api_key_cache( ) if should_check_db: response = await _get_team_db_check(team_id=team_id, prisma_client=prisma_client, team_id_upsert=team_id_upsert) + # The database answered and the row is not there. Distinct from every + # other failure here, which leaves the team's grant unknown. + if response is None: + raise TeamNotFoundError(team_id=team_id) else: response = None @@ -2678,6 +2704,8 @@ async def get_team_object( key=key, team_id_upsert=team_id_upsert, ) + except TeamNotFoundError: + raise except Exception: raise HTTPException( status_code=404, diff --git a/litellm/proxy/auth/user_api_key_auth.py b/litellm/proxy/auth/user_api_key_auth.py index e04cd19ffcc..84e60eb0dd8 100644 --- a/litellm/proxy/auth/user_api_key_auth.py +++ b/litellm/proxy/auth/user_api_key_auth.py @@ -35,6 +35,7 @@ from litellm.litellm_core_utils.dot_notation_indexing import get_nested_value from litellm.proxy._types import * from litellm.proxy.auth.auth_checks import ( ExperimentalUIJWTToken, + TeamNotFoundError, _cache_key_object, _can_object_call_model, _check_end_user_budget, @@ -87,6 +88,7 @@ from litellm.proxy.common_utils.http_parsing_utils import ( ) from litellm.proxy.common_utils.realtime_utils import _realtime_request_body from litellm.proxy.common_utils.user_api_key_cache import UserApiKeyCache +from litellm.proxy.db.exception_handler import PrismaDBExceptionHandler from litellm.proxy.litellm_pre_call_utils import LiteLLMProxyRequestSetup from litellm.proxy.utils import ( PrismaClient, @@ -2290,6 +2292,36 @@ def _team_obj_from_token(valid_token: UserAPIKeyAuth) -> LiteLLM_TeamTableCached ) +def _token_can_vouch_for_team(valid_token: UserAPIKeyAuth, lookup_error: BaseException) -> bool: + """Whether the token's own team fields may stand in for a team that failed to + resolve, without widening access. + + The UI dashboard mints every session key against the ``UI_TEAM_ID`` sentinel, + which by design never has a team row, so a failed lookup for it is not a + degraded read to be treated with suspicion; it always vouches, exactly as it + always safely has (these keys are restricted elsewhere to UI-only routes). + + For every other team, a team that is provably gone is a definitive answer, + not a degraded read, so nothing may stand in for it and no setting may + override that. + + Otherwise the team's grant is merely unknown. A token carrying one may vouch, + since replaying a recorded grant cannot widen it and denying every team key + while the row is briefly unreadable would trade the widening for an outage. A + token carrying none may not: ``team_models=[]`` reads as every model and + ``team_blocked=False`` as unblocked. ``allow_requests_on_db_unavailable`` opts + back out, and is only consulted here because the failure is known by this + point to be a degraded read. + """ + if valid_token.team_id == UI_TEAM_ID: + return True + if isinstance(lookup_error, TeamNotFoundError): + return False + if valid_token.team_models: + return True + return PrismaDBExceptionHandler.should_allow_request_on_db_unavailable() + + @tracer.wrap() async def _run_centralized_common_checks( user_api_key_auth_obj: UserAPIKeyAuth, @@ -2494,7 +2526,12 @@ async def _run_centralized_common_checks( if isinstance(team_result, BaseException): # Token-derived fallback only valid when a team_id is set; # _team_obj_from_token asserts that precondition. - team_object = _team_obj_from_token(user_api_key_auth_obj) if user_api_key_auth_obj.team_id is not None else None + if user_api_key_auth_obj.team_id is None: + team_object = None + elif _token_can_vouch_for_team(user_api_key_auth_obj, team_result): + team_object = _team_obj_from_token(user_api_key_auth_obj) + else: + raise team_result else: team_object = team_result diff --git a/tests/proxy_unit_tests/test_user_api_key_auth.py b/tests/proxy_unit_tests/test_user_api_key_auth.py index cc7de71aa56..0cdf3500d50 100644 --- a/tests/proxy_unit_tests/test_user_api_key_auth.py +++ b/tests/proxy_unit_tests/test_user_api_key_auth.py @@ -154,6 +154,7 @@ async def test_team_object_has_object_permission_id(): token=hashed_key, last_refreshed_at=time.time(), team_object_permission_id=permission_id, + team_models=["gpt-4o"], ) user_api_key_cache.set_cache(key=hashed_key, value=valid_token) @@ -242,6 +243,7 @@ async def test_aaauser_personal_budgets(key_ownership): user_id=_user_id, team_id="my-special-team", team_max_budget=100, + team_models=["gpt-4o"], spend=20, ) diff --git a/tests/test_litellm/proxy/auth/test_auth_checks.py b/tests/test_litellm/proxy/auth/test_auth_checks.py index a34df54adfa..04f38b5e2ed 100644 --- a/tests/test_litellm/proxy/auth/test_auth_checks.py +++ b/tests/test_litellm/proxy/auth/test_auth_checks.py @@ -2455,6 +2455,53 @@ async def test_get_team_object_raises_404_when_not_found(): assert "Team doesn't exist in db" in str(exc_info.value.detail) +def _mock_prisma_for_team_lookup(find_unique): + from unittest.mock import MagicMock + + mock_prisma_client = MagicMock() + mock_prisma_client.db.litellm_teamtable.find_unique = find_unique + return mock_prisma_client + + +@pytest.mark.asyncio +async def test_get_team_object_distinguishes_absent_team_from_unreadable_row(): + """A deleted team and a database that would not answer both surface as a 404, + which leaves callers unable to tell a definitive answer from a degraded read. + Only the row being positively absent raises the subclass; anything else keeps + the plain 404 so every existing caller is unaffected.""" + from unittest.mock import AsyncMock, MagicMock + + from fastapi import HTTPException + + from litellm.proxy.auth.auth_checks import TeamNotFoundError, get_team_object + + mock_cache = MagicMock() + mock_cache.async_get_cache = AsyncMock(return_value=None) + + # The database answered, and the row is not there. + with pytest.raises(TeamNotFoundError) as absent_info: + await get_team_object( + team_id="absent-team-lit5522", + prisma_client=_mock_prisma_for_team_lookup(AsyncMock(return_value=None)), + user_api_key_cache=mock_cache, + check_db_only=True, + ) + assert absent_info.value.status_code == 404 + assert "Team doesn't exist in db" in str(absent_info.value.detail) + + # The database did not answer. Same status and detail, but not the subclass, + # so a caller keying on it does not read this as proof the team is gone. + with pytest.raises(HTTPException) as unreadable_info: + await get_team_object( + team_id="unreadable-team-lit5522", + prisma_client=_mock_prisma_for_team_lookup(AsyncMock(side_effect=ConnectionError("db unreachable"))), + user_api_key_cache=mock_cache, + check_db_only=True, + ) + assert unreadable_info.value.status_code == 404 + assert not isinstance(unreadable_info.value, TeamNotFoundError) + + # Reject Client-Side Metadata Tags Tests 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 264f43c7259..470a01cfaa3 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 @@ -4693,6 +4693,283 @@ async def test_centralized_common_checks_team_404_does_not_zero_other_contexts() setattr(_proxy_server_mod, k, v) +@pytest.mark.asyncio +async def test_centralized_common_checks_unresolvable_team_without_grant_is_refused(): + """The store restricts the team to gpt-4o-mini and the read of it fails, so the + only surviving team record is the token's own, which carries ``team_models=[]`` + and reads as every model. The request must be refused with the original lookup + error. Pre-fix it was served.""" + import litellm.proxy.proxy_server as _proxy_server_mod + from fastapi import HTTPException, Request + from starlette.datastructures import URL + + token = UserAPIKeyAuth( + api_key="sk-test", + team_id="restricted-team", + models=[], + team_models=[], + ) + request = Request(scope={"type": "http"}) + request._url = URL(url="/chat/completions") + request._body = json.dumps({"model": "gpt-4.1"}).encode() + + team_read_failure = HTTPException( + status_code=404, + detail={"error": "Team doesn't exist in db. Team=restricted-team."}, + ) + + 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_team_object", + new_callable=AsyncMock, + side_effect=team_read_failure, + ): + with pytest.raises(HTTPException) as exc_info: + await _run_centralized_common_checks( + user_api_key_auth_obj=token, + request=request, + request_data={"model": "gpt-4.1"}, + route="/chat/completions", + ) + assert exc_info.value is team_read_failure + finally: + for k, v in originals.items(): + setattr(_proxy_server_mod, k, v) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("token_team_models", [[], ["gpt-4.1"]]) +async def test_centralized_common_checks_absent_team_refused_despite_db_unavailable_optout(token_team_models): + """A team that is provably gone is a definitive answer, not a degraded read. + ``allow_requests_on_db_unavailable`` is a static settings read, so without the + absent-versus-unreadable distinction it would hand a deleted team's key the + old permissive fallback while the database is perfectly healthy. Refused in + both token shapes, including the one whose grant would otherwise vouch. + + Imported from the module under test rather than from ``auth_checks``: other + tests in this suite ``importlib.reload`` that module, which rebinds the class + and would leave this raising a type the guard has never seen.""" + import litellm.proxy.proxy_server as _proxy_server_mod + from fastapi import HTTPException, Request + from starlette.datastructures import URL + + from litellm.proxy.auth.user_api_key_auth import TeamNotFoundError + + token = UserAPIKeyAuth( + api_key="sk-test", + team_id="deleted-team", + models=[], + team_models=token_team_models, + ) + request = Request(scope={"type": "http"}) + request._url = URL(url="/chat/completions") + request._body = json.dumps({"model": "gpt-4.1"}).encode() + + team_absent = TeamNotFoundError(team_id="deleted-team") + + attrs = _proxy_attrs_for_centralized_checks(user_custom_auth=None) + attrs["general_settings"] = {"allow_requests_on_db_unavailable": True} + 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_team_object", + new_callable=AsyncMock, + side_effect=team_absent, + ): + with pytest.raises(HTTPException) as exc_info: + await _run_centralized_common_checks( + user_api_key_auth_obj=token, + request=request, + request_data={"model": "gpt-4.1"}, + route="/chat/completions", + ) + assert exc_info.value is team_absent + finally: + for k, v in originals.items(): + setattr(_proxy_server_mod, k, v) + + +@pytest.mark.asyncio +async def test_centralized_common_checks_unreadable_team_keeps_db_unavailable_optout(): + """The counterpart: an unreadable team leaves the grant unknown rather than + answered, so an operator who has accepted degraded authorization during a + database fault still gets the fallback. Without this the fix would trade the + widening for a lockout with no way out.""" + import litellm.proxy.proxy_server as _proxy_server_mod + from fastapi import HTTPException as _HTTPException + from fastapi import Request + from starlette.datastructures import URL + + from litellm.proxy._types import LiteLLM_TeamTableCachedObj + + token = UserAPIKeyAuth(api_key="sk-test", team_id="unreadable-team", models=[], team_models=[]) + request = Request(scope={"type": "http"}) + request._url = URL(url="/chat/completions") + request._body = json.dumps({"model": "gpt-4.1"}).encode() + + received_team_objects: list[LiteLLM_TeamTableCachedObj | None] = [] + + async def _capturing_common_checks(*_args, **kwargs) -> bool: + received_team_objects.append(kwargs.get("team_object")) + return True + + attrs = _proxy_attrs_for_centralized_checks(user_custom_auth=None) + attrs["general_settings"] = {"allow_requests_on_db_unavailable": True} + 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_team_object", + new_callable=AsyncMock, + side_effect=_HTTPException(status_code=404, detail={"error": "team unreadable"}), + ), + patch( + "litellm.proxy.auth.user_api_key_auth.common_checks", + _capturing_common_checks, + ), + ): + await _run_centralized_common_checks( + user_api_key_auth_obj=token, + request=request, + request_data={"model": "gpt-4.1"}, + route="/chat/completions", + ) + assert len(received_team_objects) == 1 + received_team_object = received_team_objects[0] + assert received_team_object is not None + assert received_team_object.team_id == "unreadable-team" + finally: + for k, v in originals.items(): + setattr(_proxy_server_mod, k, v) + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "requested_model, is_granted", + [("gpt-4o-mini", True), ("gpt-4.1", False)], +) +async def test_centralized_common_checks_unresolvable_team_with_grant_enforces_it(requested_model, is_granted): + """Mirror of the refusal above: a token that does carry a team model grant keeps + the fallback, and the reconstructed team must still enforce that grant rather + than wave the request through.""" + import litellm.proxy.proxy_server as _proxy_server_mod + from fastapi import HTTPException, Request + from starlette.datastructures import URL + + from litellm.proxy._types import ProxyErrorTypes, ProxyException + + token = UserAPIKeyAuth( + api_key="sk-test", + team_id="restricted-team", + models=[], + team_models=["gpt-4o-mini"], + ) + request = Request(scope={"type": "http"}) + request._url = URL(url="/chat/completions") + request._body = json.dumps({"model": requested_model}).encode() + + 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_team_object", + new_callable=AsyncMock, + side_effect=HTTPException(status_code=404, detail={"error": "team unreadable"}), + ): + if is_granted: + await _run_centralized_common_checks( + user_api_key_auth_obj=token, + request=request, + request_data={"model": requested_model}, + route="/chat/completions", + ) + else: + with pytest.raises(ProxyException) as exc_info: + await _run_centralized_common_checks( + user_api_key_auth_obj=token, + request=request, + request_data={"model": requested_model}, + route="/chat/completions", + ) + assert exc_info.value.type == ProxyErrorTypes.team_model_access_denied + finally: + for k, v in originals.items(): + setattr(_proxy_server_mod, k, v) + + +@pytest.mark.asyncio +async def test_centralized_common_checks_ui_sentinel_team_vouches_despite_absent_row(): + """The Admin UI mints every session key against the ``UI_TEAM_ID`` sentinel, + which by design never has a ``LiteLLM_TeamTable`` row, so ``get_team_object`` + always raises ``TeamNotFoundError`` for it. That must NOT be read as "team + provably gone, refuse" the way it is for a real team_id: PR #36837 made that + exact mistake and PR #36982 reverted it because every dashboard request + 404'd. The sentinel must keep vouching from the token unconditionally.""" + import litellm.proxy.proxy_server as _proxy_server_mod + from fastapi import Request + from starlette.datastructures import URL + + from litellm.proxy._types import UI_TEAM_ID, LiteLLM_TeamTableCachedObj + from litellm.proxy.auth.user_api_key_auth import TeamNotFoundError + + token = UserAPIKeyAuth( + api_key="sk-test", + user_id="ui-session-user", + team_id=UI_TEAM_ID, + models=[], + team_models=[], + ) + request = Request(scope={"type": "http"}) + request._url = URL(url="/user/info") + request._body = b"{}" + + received_team_objects: list[LiteLLM_TeamTableCachedObj | None] = [] + + async def _capturing_common_checks(*_args, **kwargs) -> bool: + received_team_objects.append(kwargs.get("team_object")) + return True + + 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_team_object", + new_callable=AsyncMock, + side_effect=TeamNotFoundError(team_id=UI_TEAM_ID), + ), + patch( + "litellm.proxy.auth.user_api_key_auth.common_checks", + _capturing_common_checks, + ), + ): + await _run_centralized_common_checks( + user_api_key_auth_obj=token, + request=request, + request_data={}, + route="/user/info", + ) + assert len(received_team_objects) == 1 + received_team_object = received_team_objects[0] + assert received_team_object is not None + assert received_team_object.team_id == UI_TEAM_ID + finally: + for k, v in originals.items(): + setattr(_proxy_server_mod, k, v) + + @pytest.mark.asyncio async def test_centralized_common_checks_user_http_exception_isolates_to_user_only(): """Per-fetch isolation, mirror of the team case: an HTTPException