mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-31 06:56:59 +00:00
fix(proxy): harden /v1/models against privilege escalation, DB faults, and silent skips
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 <cursoragent@cursor.com>
This commit is contained in:
parent
295bfe072f
commit
edbeb8ea07
2 changed files with 147 additions and 15 deletions
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue