fix: honor key access_group_ids when team restricts models - #26275
Conversation
Two model-access gates run per request in `common_checks` and they're asymmetric: `can_key_call_model` falls back to the key's `access_group_ids`, but `can_team_access_model` only looks at `team.models` + `team.access_group_ids`. A key granted a model via its own access group on a model-restricted team is silently denied at the team gate. Wrap `can_team_access_model` in try/except in `common_checks`: on `team_model_access_denied`, consult a new `_key_access_group_grants_model` helper that expands `valid_token.access_group_ids` via the existing `_get_models_from_access_groups` and checks via `_can_object_call_model`. Re-raise if the key's access groups don't grant the model. Any other exception propagates unchanged. Effect: request allowed if `team allows X` OR `key's access group grants X`, making the two gates symmetric. Test: add three unit tests for `_key_access_group_grants_model` covering: group covers model, key has no groups, group resolves but does not cover model.
Greptile SummaryThis PR fixes a legitimate gap in the access-group model: when a team's allow-list blocks a model, the key's own Confidence Score: 5/5Safe to merge; the two findings are P2 observability and naming suggestions that do not affect correctness or security. No P0 or P1 issues found. The authorization logic is sound: the access-group override is gated on the group itself listing the key's team or token, preventing escalation. The null-guard on No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/auth/auth_checks.py | Adds _key_access_group_grants_model helper and wraps can_team_access_model in try/except; logic is sound but the helper produces no observability output when it silently returns False, making auth failures hard to diagnose. |
| tests/proxy_unit_tests/test_auth_checks.py | Adds 6 well-structured unit tests covering happy paths, early-return, model-not-covered, foreign-group bypass, and exception tolerance; all mock-only (no real network calls). |
Reviews (2): Last reviewed commit: "fix: gate key access_group override on g..." | Re-trigger Greptile
There was a problem hiding this comment.
High: Team model restriction bypass via key access_group_ids
This PR allows a key's access_group_ids to override team-level model restrictions in common_checks. Previously, the team model check was a hard boundary — if the team couldn't access a model, the request was denied regardless of key-level permissions. Now, if can_team_access_model denies access, the code falls through to check whether the key's access_group_ids grant the model, effectively letting key-level access groups override team restrictions.
A team member who can generate or update keys (via /key/generate or /key/update) can set access_group_ids to reference any access group, including groups created by admins for other teams with broader model access. There is no server-side validation that a key's access_group_ids must be a subset of its team's allowed access groups or models.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
A team member could set any access_group_ids on their key (e.g. a group assigned only to a different team) and override the team's model restriction. Intersect the key's access_group_ids with team_object.access_group_ids in _key_access_group_grants_model so foreign groups are dropped before model expansion. Adds a regression test that asserts expansion is never called for foreign groups.
Replaces the previous intersect-with-team.access_group_ids check, which made the override unreachable in practice (the team-gate fallback already covered every case the intersection allowed). The override now resolves each of the key's access_group_ids via get_access_object and accepts the group only if its assigned_team_ids includes the key's team_id, or its assigned_key_ids includes the key's token. This fulfills the original ask (a key can extend a team's allow-list via a group the admin granted to that team or that specific key) while still rejecting foreign groups referenced by team members of other teams.
|
@greptileai review |
…itellm_fix-ag-not-resolved
85d426c
into
litellm_internal_staging
Behavior
common_checksnow wrapscan_team_access_modelin try/except. Onteam_model_access_denied, it consults a new_key_access_group_grants_modelhelper.The helper resolves each of the key's
access_group_idsviaget_access_object(cache-first, same path team-side fallback already uses) and accepts a group only if either:assigned_team_idsincludes the key'steam_id, orassigned_key_idsincludes the key's token.Accepted groups'
access_model_namesare unioned and the requested model is checked against that set. If found, the team gate's denial is overridden. If no group authorizes the caller, or none of the authorized groups grants the model, the originalteam_model_access_deniedis re-raised.This means the access group itself is the source of truth for which teams and keys can use it — keeping the team-as-owner boundary while not requiring the team's
access_group_idsfield to be populated for the override to fire.Before / after
Test plan
tests/proxy_unit_tests/test_auth_checks.py— 6 new unit tests for_key_access_group_grants_model:assigned_team_idsincludes the team) → Trueassigned_key_idsincludes the key's token) → Trueaccess_group_ids→ False (early return)get_access_objectraises → False (treated as no authorization)test_auth_checks.pypass; black-formatted.Out of scope (follow-up)
/key/generateand/key/updateso the API rejects unauthorizedaccess_group_idsupfront instead of silently dropping them at request time.<AccessGroupSelector />so it only shows groups whoseassigned_team_ids/assigned_key_idsauthorize the key.access_group_idson the key detail view.assigned_team_ids/assigned_key_idseditors on the access group form.