mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-11 03:38:38 +00:00
refactor(proxy): switch by-id /v1/model/info from 403 to deployment-level filter
Upstream's new tests (test_routes_model_info.py + test_team_model_name_translation.py
in litellm_internal_staging) pin the contract: /v1/model/info?litellm_model_id=...
always returns 200, with the deployment dropped from the response when the
caller can't see it. The pre-existing spec comment at
litellm/proxy/auth/route_checks.py:189 ("/model/info just shows models user
has access to") backs the same contract.
Our earlier commits (56c8e35be7, bb875546cc, c3e942ff74) closed the
correct security hole (anyone with a valid key could pull any deployment's
litellm_params via by-id lookup) but with the wrong mechanism — 403 with
"Model id = X not authorized". That actually introduced a new
enumeration leak (403 = exists but not yours vs 404 = does not exist)
and conflicts with upstream's downstream filter chain.
Switch to the filter approach already in place for /v2/model/info: run
the by-id result through the same _populate_team_access_on_models +
include_team_models + teamId + apply_user_models_filter_to_deployments
chain as the listing branch. ACL violations now return 200 + {data: []},
matching upstream's contract and the spec comment.
Tests updated:
- test_v1_model_info_by_id_denied_when_model_not_in_user_models →
test_v1_model_info_by_id_filtered_when_model_not_in_user_models:
asserts 200 + {data: []} instead of 403.
- test_v1_model_info_by_id_master_key_bypass + _allowed_when_in_user_models
unchanged (still 200 + deployment).
Verified locally:
- 4 upstream tests (test_v1_model_info_specific_id_happy[/v1/model/info|/model/info],
test_model_info_v1_litellm_model_id_include_team_models_filters_inaccessible,
test_model_info_v1_litellm_model_id_team_id_applies_team_filter): all PASS
- Our 59 tests/test_litellm/proxy/discovery_endpoints/: all PASS
- 28 tests/test_litellm/proxy/proxy_server/{test_routes_model_info,test_team_model_name_translation}.py: all PASS
This commit is contained in:
parent
587ae66d33
commit
e3806e4c85
2 changed files with 27 additions and 35 deletions
|
|
@ -13453,37 +13453,6 @@ async def model_info_v1(
|
|||
"error": f"Model id = {litellm_model_id} not found on litellm proxy"
|
||||
},
|
||||
)
|
||||
# Authorize against key/team/user models — by-id was previously
|
||||
# short-circuiting before the listing-path filter, letting a
|
||||
# restricted virtual key (including service accounts with no
|
||||
# user_id but with key.models or team_models constraints) pull
|
||||
# metadata for any deployment via /v1/model/info?litellm_model_id=.
|
||||
# Reuse `get_available_models_for_user` so by-id authorization
|
||||
# tracks the listing-path filter exactly: key.models, team_models,
|
||||
# and user.models all narrow the allowlist; `_apply_user_models_filter`
|
||||
# bypasses the user.models step when user_id is absent.
|
||||
from litellm.proxy.utils import get_available_models_for_user
|
||||
|
||||
authorized_models = await get_available_models_for_user(
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
llm_router=llm_router,
|
||||
general_settings=general_settings,
|
||||
user_model=user_model,
|
||||
prisma_client=prisma_client,
|
||||
proxy_logging_obj=proxy_logging_obj,
|
||||
team_id=None,
|
||||
include_model_access_groups=False,
|
||||
only_model_access_groups=False,
|
||||
return_wildcard_routes=False,
|
||||
user_api_key_cache=user_api_key_cache,
|
||||
)
|
||||
if deployment_info.model_name not in authorized_models:
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail={
|
||||
"error": f"Model id = {litellm_model_id} not authorized for this key/user"
|
||||
},
|
||||
)
|
||||
_deployment_info_dict = _get_proxy_model_info(
|
||||
model=deployment_info.model_dump(exclude_none=True)
|
||||
)
|
||||
|
|
@ -13505,6 +13474,26 @@ async def model_info_v1(
|
|||
llm_router=llm_router,
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
)
|
||||
# Bound by LiteLLM_UserTable.models (Personal Models) using the
|
||||
# same deployment-level filter as the listing branch. by-id used
|
||||
# to short-circuit before this filter, letting a restricted user
|
||||
# pull any deployment's litellm_params via /v1/model/info?litellm_model_id=
|
||||
# — same leak BerriAI/litellm#26420 fixed for /v1/models.
|
||||
#
|
||||
# Filter (drop unauthorized deployments → empty list) instead of
|
||||
# 403: matches the route_checks.py:189 spec comment "/model/info
|
||||
# just shows models user has access to" and avoids leaking model
|
||||
# existence via 403-vs-404 enumeration.
|
||||
from litellm.proxy.utils import apply_user_models_filter_to_deployments
|
||||
|
||||
single_model_list = await apply_user_models_filter_to_deployments(
|
||||
deployments=single_model_list,
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
llm_router=llm_router,
|
||||
prisma_client=prisma_client,
|
||||
proxy_logging_obj=proxy_logging_obj,
|
||||
user_api_key_cache=user_api_key_cache,
|
||||
)
|
||||
return {"data": single_model_list}
|
||||
|
||||
# Return router deployments (same source as /v2/model/info), not wildcard-
|
||||
|
|
|
|||
|
|
@ -261,10 +261,13 @@ def _deployment_id(router, model_name):
|
|||
raise AssertionError(f"model {model_name!r} not in router")
|
||||
|
||||
|
||||
def test_v1_model_info_by_id_denied_when_model_not_in_user_models(
|
||||
def test_v1_model_info_by_id_filtered_when_model_not_in_user_models(
|
||||
client, configure_router, monkeypatch
|
||||
):
|
||||
"""Regression: by-id lookup must respect user.models (veria-ai #29748)."""
|
||||
"""Regression: by-id lookup must respect user.models. Returns 200 + empty
|
||||
data (not 403) so existence isn't leaked via 403-vs-404 enumeration —
|
||||
matches the route_checks.py:189 spec '/model/info just shows models
|
||||
user has access to' and the /v2/model/info pattern. See #26420 / #29748."""
|
||||
_override_auth(user_id="u-test")
|
||||
_patch_user(monkeypatch, models=["gpt-4"])
|
||||
disallowed_id = _deployment_id(configure_router, "claude-3-opus")
|
||||
|
|
@ -273,8 +276,8 @@ def test_v1_model_info_by_id_denied_when_model_not_in_user_models(
|
|||
f"/v1/model/info?litellm_model_id={disallowed_id}",
|
||||
headers={"Authorization": "Bearer sk-test"},
|
||||
)
|
||||
assert resp.status_code == 403
|
||||
assert "not authorized" in resp.text.lower()
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {"data": []}
|
||||
|
||||
|
||||
def test_v1_model_info_by_id_allowed_when_model_in_user_models(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue