From 5feb6008d89043cc555da9cb844e84497be83597 Mon Sep 17 00:00:00 2001 From: Sameer Kankute Date: Fri, 1 May 2026 17:56:37 +0530 Subject: [PATCH] Fix greptile review --- litellm/router.py | 6 ++- tests/test_litellm/test_router.py | 72 +++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/litellm/router.py b/litellm/router.py index 7a2e4fc996b..0020f2fbe18 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -9400,6 +9400,7 @@ class Router: healthy_deployments = self._get_all_deployments( model_name=model, team_id=request_team_id ) + _pre_model_access_group_filter_len = len(healthy_deployments) healthy_deployments = self._filter_deployments_by_model_access_groups( model=model, healthy_deployments=healthy_deployments, @@ -9409,7 +9410,10 @@ class Router: if len(healthy_deployments) == 0: # check if the user sent in a deployment name instead - healthy_deployments = self._get_deployment_by_litellm_model(model=model) + # Do not fall back when access-group filtering removed every candidate; + # _get_deployment_by_litellm_model does not re-apply that filter. + if _pre_model_access_group_filter_len == 0: + healthy_deployments = self._get_deployment_by_litellm_model(model=model) if verbose_router_logger.isEnabledFor(logging.DEBUG): verbose_router_logger.debug( diff --git a/tests/test_litellm/test_router.py b/tests/test_litellm/test_router.py index af8633361ac..e62a650d999 100644 --- a/tests/test_litellm/test_router.py +++ b/tests/test_litellm/test_router.py @@ -3319,3 +3319,75 @@ def test_explicit_model_access_does_not_force_access_group_filtering(): deployment_groups = [d.get("model_info", {}).get("access_groups") for d in deployments] assert ["AG1"] in deployment_groups assert ["AG2"] in deployment_groups + + +def test_access_group_filter_empty_does_not_bypass_via_litellm_model_fallback( + monkeypatch: pytest.MonkeyPatch, +): + """ + When access-group filtering removes all candidates, _get_deployment_by_litellm_model + must not run: it does not re-apply access groups and could return blocked deployments + that share the same litellm_params.model as the request model string. + + ``get_model_access_groups`` is patched to expose AG1 for the public model (so the + access-group filter runs with a non-empty allowed set) while every deployment + returned for that name is AG2-only — filtered to empty. Without the guard, the + litellm-model fallback would return both rows because ``litellm_params.model`` matches. + """ + from litellm.proxy._types import UserAPIKeyAuth + + router = litellm.Router( + model_list=[ + { + "model_name": "gpt-5", + "litellm_params": { + "model": "gpt-5", + "api_key": "key1", + "mock_response": "blocked-dep-1", + }, + "model_info": {"access_groups": ["AG2"]}, + }, + { + "model_name": "gpt-5", + "litellm_params": { + "model": "gpt-5", + "api_key": "key2", + "mock_response": "blocked-dep-2", + }, + "model_info": {"access_groups": ["AG2"]}, + }, + ] + ) + + orig_groups = router.get_model_access_groups + + def fake_get_model_access_groups( + model_name=None, model_access_group=None, team_id=None + ): + if model_name == "gpt-5" and model_access_group is None: + return {"AG1": ["gpt-5"], "AG2": ["gpt-5"]} + return orig_groups( + model_name=model_name, + model_access_group=model_access_group, + team_id=team_id, + ) + + monkeypatch.setattr(router, "get_model_access_groups", fake_get_model_access_groups) + + scoped_key = UserAPIKeyAuth( + api_key="hashed-key", + team_id="team2", + models=["AG1"], + team_models=["AG1"], + ) + + with pytest.raises(litellm.BadRequestError): + router._common_checks_available_deployment( + model="gpt-5", + request_kwargs={ + "metadata": { + "user_api_key_team_id": "team2", + "user_api_key_auth": scoped_key, + } + }, + )