fix(mcp): extend key access-group union to MCP servers#28890
Conversation
A key's unified access_group_ids now extend the team's MCP scope instead of being capped by it — mirrors the model-side union from LIT-2404. The group's assigned_team_ids / assigned_key_ids still gate the override, so team members can't pull in MCPs via a foreign team's group. Resolves LIT-3189
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR extends the key
Confidence Score: 5/5Safe to merge — the authorization gate (assigned_team_ids/assigned_key_ids) is preserved, the org/end-user/agent ceilings still cap the extended result, and the refactor of the model path is behavior-neutral. The shared helper is a clean extraction with no logic change for the existing model path. The new MCP extras union correctly inherits all downstream ceiling filters. The foreign-group escalation regression test explicitly covers the key security boundary. Cache-first semantics via get_access_object are maintained, and global_mcp_server_manager is an initialized module-level singleton so there is no uninitialized-reference risk at runtime. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/auth/auth_checks.py | Extracts generic get_authorized_resources_from_key_access_groups helper from _key_access_group_grants_model; refactors existing model path to call the shared helper. Logic and deduplication are preserved correctly. |
| litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py | Adds _get_key_access_group_mcp_server_extras and wires it into get_allowed_mcp_servers as a union with the team/key base scope. End-user, agent, and org ceiling intersections correctly apply to the extended set. |
| tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py | Adds 8 focused tests covering the new MCP access-group extras path (team-authorized, key-authorized, no groups, empty grant, foreign-group escalation regression, lookup failure, two end-to-end scenarios). Minor parentheses cleanup in existing assertions only. |
Reviews (1): Last reviewed commit: "fix(mcp): extend key access-group union ..." | Re-trigger Greptile
123a9ce
into
litellm_internal_staging
The previous commit moved key.access_group_ids resolution out of the intersected key ceiling (_get_allowed_mcp_servers_for_key) and into the ungated additive grant path (_get_key_access_group_mcp_server_extras), unioned on top of the team ceiling. Five tests from #28890/#29195 still asserted the old gated / in-key-scope contract and failed: - _get_allowed_mcp_servers_for_key now returns the object_permission ceiling only and never resolves access_group_ids; two tests now assert the group resolver is not called from that path (with and without an object_permission present). - The extras path is ungated, so a group whose assigned_team_ids / assigned_key_ids exclude the caller still contributes its servers. - The end-to-end test asserts the grant surfaces via the extras path rather than the base key path. - Dropped test_key_access_group_ids_empty_returns_no_extras; the empty case is already covered by the extras family's no-groups test.
…gnment (#29313) * fix(mcp): make key.access_group_ids grants additive over team ceiling A key whose unified access_group_ids grant a private MCP server was having that grant intersected against its team's MCP ceiling, so a key in a team scoped to other servers (or with no own scope) lost the granted server entirely. Resolve access_group_ids once as ungated additive grants and union them on top of the key/team ceiling instead of folding them into the key scope that gets intersected. * test(mcp): align key access-group tests with additive-grant model The previous commit moved key.access_group_ids resolution out of the intersected key ceiling (_get_allowed_mcp_servers_for_key) and into the ungated additive grant path (_get_key_access_group_mcp_server_extras), unioned on top of the team ceiling. Five tests from #28890/#29195 still asserted the old gated / in-key-scope contract and failed: - _get_allowed_mcp_servers_for_key now returns the object_permission ceiling only and never resolves access_group_ids; two tests now assert the group resolver is not called from that path (with and without an object_permission present). - The extras path is ungated, so a group whose assigned_team_ids / assigned_key_ids exclude the caller still contributes its servers. - The end-to-end test asserts the grant surfaces via the extras path rather than the base key path. - Dropped test_key_access_group_ids_empty_returns_no_extras; the empty case is already covered by the extras family's no-groups test. * feat(auth): gate member access-group assignment on keys behind opt-in Non-admin team members could attach access_group_ids to keys they create or update, letting them self-grant resources (MCP servers/models) the team admin never intended. Add an opt-in KEY_ACCESS_GROUP_ASSIGNMENT team-member permission (default-deny) enforced at /key/generate and /key/update; proxy and team admins bypass. Surfaces automatically as a checkbox in the team Member Permissions UI. * fix(auth): gate access-group assignment on /key/regenerate too RegenerateKeyRequest inherits access_group_ids and prepare_key_update_data persists it, so a non-admin key owner could self-grant access groups by regenerating. Apply the same opt-in member gate using the existing key's team. * test(auth): cover member access-group gate and additive MCP grants Add unit tests for enforce_member_can_assign_access_groups (deny without opt-in, allow with opt-in, and proxy-admin / team-admin / non-team-key bypasses) and for _get_key_access_group_mcp_server_extras (no-auth and no-resolved-servers return empty, resolved ids are expanded, errors degrade to no grants).
Summary
A key's unified
access_group_idsnow extend the team's MCP scope instead of being capped by it — mirroring the model-side union shipped in LIT-2404. The override is still gated by the access group'sassigned_team_ids/assigned_key_ids, so a team member cannot pull in MCPs via a foreign team's group. End-user / agent / org ceilings remain unchanged and still cap the unioned result.A new shared helper
get_authorized_resources_from_key_access_groups(inauth_checks.py) factors out the per-group fetch + assignment-gate + resource extraction. Both the existing model path and the new MCP path call it with their respectiveresource_field.Screenshots
before

after

Test plan
tests/proxy_unit_tests/test_auth_checks.py::test_key_access_group_grants_model_*(6) — model-side regression after refactor.tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py::test_mcp_key_access_group_extras_*(6) — mirrors the 6 model scenarios on the new MCP helper (team-authorized, key-authorized, no groups, empty grant, foreign-group escalation regression, lookup failure).…::test_get_allowed_mcp_servers_unions_key_access_group_extras/…::test_get_allowed_mcp_servers_no_union_when_no_authorized_extras— end-to-end onget_allowed_mcp_servers.test_auth_checks.py62/62,test_user_api_key_auth_mcp.py123/123.FastMCPbackends: team withmcp_servers=[postgres]and key holding access group withassigned_team_ids=[team],access_mcp_server_ids=[stripe]. BEFORE:GET /v1/mcp/toolsreturns 2 tools (postgres only). AFTER: 4 tools (postgres + stripe).Resolves LIT-3189