From d8a47ebb69423b6e676ee9b6fbf8f3bd5449b1b7 Mon Sep 17 00:00:00 2001 From: Harshit28j Date: Sun, 22 Mar 2026 06:17:03 +0530 Subject: [PATCH] fix: greptile feedback --- docs/my-website/docs/proxy/model_access.md | 6 +++ litellm/proxy/auth/auth_checks.py | 6 +++ .../proxy/auth/test_team_model_overrides.py | 47 ++++++++++++------- 3 files changed, 42 insertions(+), 17 deletions(-) diff --git a/docs/my-website/docs/proxy/model_access.md b/docs/my-website/docs/proxy/model_access.md index 494ce5f9c1b..0ce33537339 100644 --- a/docs/my-website/docs/proxy/model_access.md +++ b/docs/my-website/docs/proxy/model_access.md @@ -250,6 +250,12 @@ curl -L 'http://localhost:4000/team/member_update' \ | Key `models` must be a subset of effective models | `403` on `/key/generate` | | Narrowing `team.models` auto-prunes stale `default_models` | Automatic on `/team/update` | +### Important Notes + +- **Effective models are capped by `team.models`**: If `team.models` is later narrowed, any member overrides outside the new pool are silently excluded at runtime — no access is granted beyond the team's allowed list. +- **Service/bot keys** (keys without a `user_id`) are **not affected** by this feature. They always use the full `team.models` pool, preserving backward compatibility. +- **Access groups** (`access_group_ids`) are a team-level concept and are **not** restricted by per-member overrides. Models granted via access groups remain available to all team members. + ### Backward Compatibility When the feature flag is off **or** when neither `default_models` nor member `models` is configured: diff --git a/litellm/proxy/auth/auth_checks.py b/litellm/proxy/auth/auth_checks.py index 17b905e4bb7..e657c0e0c3e 100644 --- a/litellm/proxy/auth/auth_checks.py +++ b/litellm/proxy/auth/auth_checks.py @@ -2736,6 +2736,12 @@ def get_effective_team_models( ): return team_object.models if team_object else [] + # Service/bot keys (no user_id) are not team members — skip per-member + # effective models and use the full team.models pool. This preserves + # backward compatibility for service accounts. + if valid_token and not valid_token.user_id: + return team_object.models if team_object else [] + # Get from team defaults — prefer team_object (authoritative, fresh from DB/cache) # over valid_token (snapshot from key creation time, may be stale). # Use `is not None` instead of truthiness so that an explicit empty list [] diff --git a/tests/test_litellm/proxy/auth/test_team_model_overrides.py b/tests/test_litellm/proxy/auth/test_team_model_overrides.py index 48192a3150d..1db9e9c223e 100644 --- a/tests/test_litellm/proxy/auth/test_team_model_overrides.py +++ b/tests/test_litellm/proxy/auth/test_team_model_overrides.py @@ -59,14 +59,14 @@ class TestGetEffectiveTeamModels: team = LiteLLM_TeamTable( team_id="t1", models=["m1", "d1", "mo1"], default_models=["d1"] ) - token = UserAPIKeyAuth(team_member_models=["mo1"]) + token = UserAPIKeyAuth(user_id="test-user", team_member_models=["mo1"]) result = get_effective_team_models(team, token) assert set(result) == {"d1", "mo1"} def test_overrides_only_no_defaults(self): """3. User with overrides only (no defaults) → can access override models.""" team = LiteLLM_TeamTable(team_id="t1", models=["m1", "mo1", "mo2"]) - token = UserAPIKeyAuth(team_member_models=["mo1", "mo2"]) + token = UserAPIKeyAuth(user_id="test-user", team_member_models=["mo1", "mo2"]) result = get_effective_team_models(team, token) assert set(result) == {"mo1", "mo2"} @@ -95,8 +95,8 @@ class TestGetEffectiveTeamModels: team = LiteLLM_TeamTable( team_id="t1", models=["m1", "d1", "mo_a", "mo_b"], default_models=["d1"] ) - token_a = UserAPIKeyAuth(team_member_models=["mo_a"]) - token_b = UserAPIKeyAuth(team_member_models=["mo_b"]) + token_a = UserAPIKeyAuth(user_id="test-user", team_member_models=["mo_a"]) + token_b = UserAPIKeyAuth(user_id="test-user", team_member_models=["mo_b"]) result_a = get_effective_team_models(team, token_a) result_b = get_effective_team_models(team, token_b) assert set(result_a) == {"d1", "mo_a"} @@ -109,16 +109,29 @@ class TestGetEffectiveTeamModels: litellm.team_model_overrides_enabled = False monkeypatch.delenv("TEAM_MODEL_OVERRIDES", raising=False) team = LiteLLM_TeamTable(team_id="t1", models=["m1"], default_models=["d1"]) - token = UserAPIKeyAuth(team_member_models=["mo1"]) + token = UserAPIKeyAuth(user_id="test-user", team_member_models=["mo1"]) result = get_effective_team_models(team, token) assert result == ["m1"] + def test_service_key_no_user_id_gets_full_team_pool(self): + """Service/bot keys (no user_id) should get full team.models, not defaults.""" + team = LiteLLM_TeamTable( + team_id="t1", + models=["m1", "m2", "m3"], + default_models=["m1"], + ) + # Service key: valid_token with no user_id + token = UserAPIKeyAuth(user_id=None) + result = get_effective_team_models(team, token) + # Should get full team pool, NOT just defaults + assert result == ["m1", "m2", "m3"] + def test_deduplication(self): """Overlapping models are deduplicated.""" team = LiteLLM_TeamTable( team_id="t1", models=["m1", "shared", "extra"], default_models=["shared"] ) - token = UserAPIKeyAuth(team_member_models=["shared", "extra"]) + token = UserAPIKeyAuth(user_id="test-user", team_member_models=["shared", "extra"]) result = get_effective_team_models(team, token) assert set(result) == {"shared", "extra"} assert len(result) == 2 # no duplicates @@ -129,7 +142,7 @@ class TestGetEffectiveTeamModels: def test_no_team_object_with_token(self): """No team object but token has defaults → uses token values.""" - token = UserAPIKeyAuth(team_default_models=["td1"], team_member_models=["mo1"]) + token = UserAPIKeyAuth(user_id="test-user", team_default_models=["td1"], team_member_models=["mo1"]) result = get_effective_team_models(None, token) assert set(result) == {"td1", "mo1"} @@ -163,7 +176,7 @@ class TestCanTeamAccessModelWithOverrides: team = LiteLLM_TeamTable( team_id="t1", models=["m1", "d1", "mo1"], default_models=["d1"] ) - token = UserAPIKeyAuth(team_member_models=["mo1"]) + token = UserAPIKeyAuth(user_id="test-user", team_member_models=["mo1"]) assert await can_team_access_model( model="d1", team_object=team, llm_router=None, valid_token=token ) @@ -177,7 +190,7 @@ class TestCanTeamAccessModelWithOverrides: team = LiteLLM_TeamTable( team_id="t1", models=["m1", "m2", "d1", "mo1"], default_models=["d1"] ) - token = UserAPIKeyAuth(team_member_models=["mo1"]) + token = UserAPIKeyAuth(user_id="test-user", team_member_models=["mo1"]) with pytest.raises(Exception): await can_team_access_model( model="m2", team_object=team, llm_router=None, valid_token=token @@ -190,12 +203,12 @@ class TestCanTeamAccessModelWithOverrides: team_id="t1", models=["m1", "d1", "mo1"], default_models=["d1"] ) # With override - token_with = UserAPIKeyAuth(team_member_models=["mo1"]) + token_with = UserAPIKeyAuth(user_id="test-user", team_member_models=["mo1"]) assert await can_team_access_model( model="mo1", team_object=team, llm_router=None, valid_token=token_with ) # After override removal (empty member models) - token_without = UserAPIKeyAuth(team_member_models=[]) + token_without = UserAPIKeyAuth(user_id="test-user", team_member_models=[]) with pytest.raises(Exception): await can_team_access_model( model="mo1", @@ -211,7 +224,7 @@ class TestCanTeamAccessModelWithOverrides: team_id="t1", models=["m1"], default_models=["m1"] ) # Member has stale override for "m2" which is no longer in team.models - token = UserAPIKeyAuth(team_member_models=["m2"]) + token = UserAPIKeyAuth(user_id="test-user", team_member_models=["m2"]) # effective = union(["m1"], ["m2"]) capped by team.models=["m1"] → ["m1"] with pytest.raises(Exception): await can_team_access_model( @@ -226,7 +239,7 @@ class TestCanTeamAccessModelWithOverrides: team_id="t1", models=["m1"] # no default_models ) # Member has ONLY stale overrides — none are in team.models - token = UserAPIKeyAuth(team_member_models=["stale1", "stale2"]) + token = UserAPIKeyAuth(user_id="test-user", team_member_models=["stale1", "stale2"]) result = get_effective_team_models(team, token) # Should fall back to team.models=["m1"], NOT [] (allow all) assert result == ["m1"] @@ -264,7 +277,7 @@ class TestCanTeamAccessModelWithOverrides: litellm.team_model_overrides_enabled = False monkeypatch.delenv("TEAM_MODEL_OVERRIDES", raising=False) team = LiteLLM_TeamTable(team_id="t1", models=["m1"], default_models=["d1"]) - token = UserAPIKeyAuth(team_member_models=["mo1"]) + token = UserAPIKeyAuth(user_id="test-user", team_member_models=["mo1"]) # Should use team.models=["m1"], not effective models assert await can_team_access_model( model="m1", team_object=team, llm_router=None, valid_token=token @@ -290,7 +303,7 @@ class TestKeyGenerationEnforcement: team = LiteLLM_TeamTable( team_id="t1", models=["m1", "m2", "m3"], default_models=["m1"] ) - token = UserAPIKeyAuth(team_member_models=["m2"]) + token = UserAPIKeyAuth(user_id="test-user", team_member_models=["m2"]) effective = self._get_effective(team, token) # Simulate key-gen validation: requested models must be subset of effective @@ -303,7 +316,7 @@ class TestKeyGenerationEnforcement: team = LiteLLM_TeamTable( team_id="t1", models=["m1", "m2", "m3"], default_models=["m1"] ) - token = UserAPIKeyAuth(team_member_models=["m2"]) + token = UserAPIKeyAuth(user_id="test-user", team_member_models=["m2"]) effective = self._get_effective(team, token) # When no models requested, key should get effective set @@ -314,7 +327,7 @@ class TestKeyGenerationEnforcement: team = LiteLLM_TeamTable( team_id="t1", models=["m1", "m2", "m3"], default_models=["m1"] ) - token = UserAPIKeyAuth(team_member_models=["m2"]) + token = UserAPIKeyAuth(user_id="test-user", team_member_models=["m2"]) effective = self._get_effective(team, token) # all-team-models should resolve to effective set, not team.models