From edbeb8ea0744da0a3c785a7cf7a60aecf19469b5 Mon Sep 17 00:00:00 2001 From: Adam Kirstein Date: Wed, 13 May 2026 13:20:12 -0700 Subject: [PATCH] fix(proxy): harden /v1/models against privilege escalation, DB faults, and silent skips MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-up fixes from PR review: 1. P0 / Medium: ``models=["no-default-models"]`` team could see every proxy model. Stripping the ``no-default-models`` sentinel left ``team_models=[]``. ``get_complete_model_list`` interprets an empty ``team_models`` as "no preference" and falls back to ``proxy_model_list``, so a team that explicitly opted out via the sentinel would have silently received the entire proxy catalog through /v1/models — strictly worse than the pre-fix behavior (which leaked only the sentinel string itself). Capture sentinel presence before the strip and return ``[]`` directly when the sentinel was the only thing the team had and access-group resolution didn't add anything real. The regression-guard test now configures a populated ``llm_router`` so the fallback path is exercised; without this fix, that test would fail. 2. P1: ``litellm_accessgrouptable.find_many`` failures could 500 /v1/models. The new resolver call had no exception guard, so any transient Prisma error (timeout, connection drop, etc.) propagated out of ``get_available_models_for_user``. Wrap the resolver in try/except, log via ``verbose_proxy_logger.debug``, and degrade to "no access-group contribution" — same behavior as the pre-fix state. 3. P2: Silent ``except Exception`` on ``get_team_object`` hid transient DB failures with no log output. Add a ``verbose_proxy_logger.debug`` so operators can diagnose unexpected access-group-resolution skips. Tests: 2 new tests (DB failure does not crash; direct models survive resolver failure), 1 strengthened test (sentinel-only team with populated router can no longer fall through to proxy_model_list). All 13 tests pass. Co-authored-by: Cursor --- litellm/proxy/utils.py | 59 +++++++--- .../test_get_available_models_for_user.py | 103 +++++++++++++++++- 2 files changed, 147 insertions(+), 15 deletions(-) diff --git a/litellm/proxy/utils.py b/litellm/proxy/utils.py index 26a823c5880..eb807159f8d 100644 --- a/litellm/proxy/utils.py +++ b/litellm/proxy/utils.py @@ -5971,26 +5971,59 @@ async def get_available_models_for_user( user_api_key_cache=user_api_key_cache, proxy_logging_obj=proxy_logging_obj, ) - except Exception: + except Exception as e: + # Resolution is best-effort: a DB hiccup here must never take + # down /v1/models. Log so operators can diagnose unexpected + # skips (otherwise this silently degrades to the pre-fix + # behavior). + verbose_proxy_logger.debug( + "get_available_models_for_user: failed to fetch team object " + "for team_id=%s, skipping access-group resolution. Error: %s", + user_api_key_dict.team_id, + str(e), + ) team_object = None if team_object is not None and prisma_client is not None: - access_group_model_names = await _resolve_team_access_group_model_names( - team_objects=[team_object], - prisma_client=prisma_client, - ) + try: + access_group_model_names = await _resolve_team_access_group_model_names( + team_objects=[team_object], + prisma_client=prisma_client, + ) + except Exception as e: + # Same best-effort contract as the get_team_object call above: + # a transient Prisma error in access-group resolution must not + # 500 the /v1/models endpoint. Degrade to "no access-group + # contribution" and let the sentinel-aware logic below decide + # what to return. + verbose_proxy_logger.debug( + "get_available_models_for_user: access-group resolution " + "failed for team_id=%s, returning team.models[] only. " + "Error: %s", + getattr(team_object, "team_id", None), + str(e), + ) + access_group_model_names = [] for model_name in access_group_model_names: if model_name not in team_models: team_models.append(model_name) - # Strip the ``no-default-models`` sentinel before passing to - # get_complete_model_list. The sentinel is an internal marker meaning - # "this team has no direct models"; it must never appear in the - # OpenAI-format response (otherwise OpenWebUI shows it as a selectable - # model id). Matches the user-path treatment in auth_checks.py. - team_models = [ - m for m in team_models if m != SpecialModelNames.no_default_models.value - ] + # ``no-default-models`` is an internal "this team has no direct models" + # marker. It must never appear in the OpenAI-format response (otherwise + # OpenWebUI renders it as a selectable model id) and must NOT be + # silently dropped — ``get_complete_model_list`` interprets an empty + # ``team_models`` as "no preference" and falls back to the full + # ``proxy_model_list``, which would grant every proxy model to a team + # that explicitly opted out. Match the user-path treatment in + # ``auth_checks.py``: if the sentinel was the only thing the team had + # and access-group resolution didn't add anything real, return ``[]`` + # immediately rather than letting the fallback path widen access. + sentinel_value = SpecialModelNames.no_default_models.value + had_no_default_models_sentinel = sentinel_value in team_models + team_models = [m for m in team_models if m != sentinel_value] + + if had_no_default_models_sentinel and not team_models and not key_models: + return [] # Get complete model list all_models = get_complete_model_list( diff --git a/tests/test_litellm/proxy/test_get_available_models_for_user.py b/tests/test_litellm/proxy/test_get_available_models_for_user.py index 8f74898d716..bf253cdc8aa 100644 --- a/tests/test_litellm/proxy/test_get_available_models_for_user.py +++ b/tests/test_litellm/proxy/test_get_available_models_for_user.py @@ -256,6 +256,13 @@ async def test_sentinel_only_team_without_access_groups_returns_empty(): A team with ``models=["no-default-models"]`` and no access groups must return an empty list — never the literal sentinel string, which OpenWebUI would otherwise render as a selectable model id. + + Regression guard: this test now configures a populated ``llm_router``. + Without the sentinel-aware short-circuit in + ``get_available_models_for_user``, stripping the sentinel would leave + ``team_models=[]`` and ``get_complete_model_list`` would fall through to + ``proxy_model_list``, silently granting every proxy model to a team + that explicitly opted out. """ team = _make_team_cached_obj(models=["no-default-models"], access_group_ids=None) prisma = _make_prisma_client_with_ag_rows([]) @@ -264,6 +271,99 @@ async def test_sentinel_only_team_without_access_groups_returns_empty(): team_id=team.team_id, team_models=["no-default-models"] ) + mock_router = MagicMock() + mock_router.get_model_names.return_value = [ + "gpt-4o", + "claude-opus-4-5", + "mistral-large", + ] + mock_router.get_model_access_groups.return_value = {} + + with patch( + "litellm.proxy.auth.auth_checks.get_team_object", + new=_patched_get_team_object(team), + ): + result = await get_available_models_for_user( + user_api_key_dict=user_api_key_dict, + llm_router=mock_router, + general_settings={}, + user_model=None, + prisma_client=prisma, + proxy_logging_obj=MagicMock(), + team_id=None, + user_api_key_cache=MagicMock(), + ) + + assert SpecialModelNames.no_default_models.value not in result + assert result == [] + assert ( + "gpt-4o" not in result + ), "A no-default-models team must not silently see every proxy model" + + +@pytest.mark.asyncio +async def test_db_failure_in_access_group_resolution_does_not_crash(): + """ + ``/v1/models`` must not 500 on a transient Prisma failure. If + ``litellm_accessgrouptable.find_many`` raises, we degrade to "no + access-group contribution" — same behavior as the pre-fix state — and + still honor the sentinel short-circuit so a no-default-models team + doesn't accidentally get full access. + """ + team = _make_team_cached_obj( + models=["no-default-models"], access_group_ids=["ag-1"] + ) + + prisma = MagicMock() + prisma.db.litellm_accessgrouptable.find_many = AsyncMock( + side_effect=RuntimeError("simulated transient DB error") + ) + + user_api_key_dict = _make_user_api_key_dict( + team_id=team.team_id, team_models=["no-default-models"] + ) + + mock_router = MagicMock() + mock_router.get_model_names.return_value = ["gpt-4o", "claude-opus-4-5"] + mock_router.get_model_access_groups.return_value = {} + + with patch( + "litellm.proxy.auth.auth_checks.get_team_object", + new=_patched_get_team_object(team), + ): + result = await get_available_models_for_user( + user_api_key_dict=user_api_key_dict, + llm_router=mock_router, + general_settings={}, + user_model=None, + prisma_client=prisma, + proxy_logging_obj=MagicMock(), + team_id=None, + user_api_key_cache=MagicMock(), + ) + + assert result == [] + assert "gpt-4o" not in result + + +@pytest.mark.asyncio +async def test_team_with_direct_models_unaffected_by_resolver_failure(): + """ + If ``team.models[]`` contains real model names and access-group + resolution fails, the team's direct models still come through. Only the + access-group contribution is dropped on resolver failure. + """ + team = _make_team_cached_obj(models=["gpt-4o"], access_group_ids=["ag-1"]) + + prisma = MagicMock() + prisma.db.litellm_accessgrouptable.find_many = AsyncMock( + side_effect=RuntimeError("simulated transient DB error") + ) + + user_api_key_dict = _make_user_api_key_dict( + team_id=team.team_id, team_models=["gpt-4o"] + ) + with patch( "litellm.proxy.auth.auth_checks.get_team_object", new=_patched_get_team_object(team), @@ -279,8 +379,7 @@ async def test_sentinel_only_team_without_access_groups_returns_empty(): user_api_key_cache=MagicMock(), ) - assert SpecialModelNames.no_default_models.value not in result - assert result == [] + assert result == ["gpt-4o"] @pytest.mark.asyncio