From e3806e4c85d459db896eb6d5d2076ac64fefb9a8 Mon Sep 17 00:00:00 2001 From: songkuan-zheng <252822057+songkuan-zheng@users.noreply.github.com> Date: Tue, 16 Jun 2026 12:41:25 +0000 Subject: [PATCH] refactor(proxy): switch by-id /v1/model/info from 403 to deployment-level filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- litellm/proxy/proxy_server.py | 51 ++++++++----------- .../test_v1_model_info_user_filter.py | 11 ++-- 2 files changed, 27 insertions(+), 35 deletions(-) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index d710144f904..0f4bc6b8d50 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -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- diff --git a/tests/test_litellm/proxy/discovery_endpoints/test_v1_model_info_user_filter.py b/tests/test_litellm/proxy/discovery_endpoints/test_v1_model_info_user_filter.py index 47ff8231362..d4bc080a226 100644 --- a/tests/test_litellm/proxy/discovery_endpoints/test_v1_model_info_user_filter.py +++ b/tests/test_litellm/proxy/discovery_endpoints/test_v1_model_info_user_filter.py @@ -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(