Repository navigation
fix(mcp): let proxy admins assign MCP servers to teamless keys - #31126
Conversation
Greptile SummaryThis PR fixes an asymmetry between the key-create/update validation path and the runtime path for MCP server assignment: proxy admins can now assign non-
Confidence Score: 5/5The change is safe to merge. The bypass is gated on two conditions (team_obj is None and is_proxy_admin), both server-authoritative — is_proxy_admin comes from the authenticated session, not user-supplied request data. The teamed-key path is untouched and four targeted unit tests pin the regression. The fix is minimal and well-scoped: three small additions to the function signature and two union operations inside the existing guard blocks. The is_proxy_admin flag is derived consistently from the same expression at both call sites. There are no structural changes to the auth flow, no new DB queries, and no modifications to existing tests. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/management_helpers/object_permission_utils.py | Adds is_proxy_admin parameter to validate_key_mcp_servers_against_team; when the caller is a proxy admin and the key has no team, the requested servers/groups are folded into the allowed set so the subset check always passes cleanly. |
| litellm/proxy/management_endpoints/key_management_endpoints.py | Threads is_proxy_admin (derived from user_api_key_dict.user_role) into both key-create and key-update call sites; the flag propagation is consistent and correctly sourced from the authenticated caller. |
| tests/test_litellm/proxy/management_helpers/test_object_permission_utils.py | Adds four new unit tests covering proxy-admin private-server assignment, proxy-admin access-group assignment, non-admin rejection, and admin-still-bounded-by-team-scope; all use mocks with no real network calls. |
Reviews (1): Last reviewed commit: "fix(mcp): let proxy admins assign MCP se..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes an asymmetry between the key create/update path and the runtime path for MCP server access:
Confidence Score: 5/5Safe to merge; the bypass is narrowly scoped to teamless keys with a verified proxy-admin caller, the runtime path already grants the same access, and team-scoped keys are untouched. The change is small and well-bounded. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/management_helpers/object_permission_utils.py | Adds is_proxy_admin parameter to validate_key_mcp_servers_against_team; when the key has no team and the caller is a proxy admin, requested servers and access groups are folded into the allowed set so subset checks pass — consistent with the existing teamless-toolset pattern at line 596. |
| litellm/proxy/management_endpoints/key_management_endpoints.py | Threads is_proxy_admin from user_api_key_dict.user_role into both call sites: _common_key_generation_helper (key create) and _validate_mcp_servers_for_key_update (key update); no other logic changed. |
| tests/test_litellm/proxy/management_helpers/test_object_permission_utils.py | Adds four new async unit tests covering: proxy admin assigns private server to teamless key (pass), non-admin still gets 403, proxy admin assigns access group to teamless key (pass), and proxy admin on a team-scoped key is still bounded by team scope. |
Reviews (2): Last reviewed commit: "fix(mcp): let proxy admins assign MCP se..." | Re-trigger Greptile
Creating or updating a key with a specific (non-allow_all_keys) MCP
server or access group failed with a 403 when the key had no team:
Key is not in a team. Only globally available (allow_all_keys) MCP
servers can be assigned
validate_key_mcp_servers_against_team computed the allowed set as
team servers + allow_all_keys servers. For a teamless key the team
set is empty, so the allowed set collapsed to just allow_all_keys
servers and any explicitly-picked server or access group was rejected.
This was asymmetric with runtime: get_allowed_mcp_servers honors a
teamless key's own object_permission.mcp_servers verbatim, with no
team gate and no allow_all_keys filter. So the create/update path
refused to persist a grant the run path would have served.
Thread is_proxy_admin into the validator from both call sites
(/key/generate and /key/update). When a key has no team and the
caller is a proxy admin, the requested servers and access groups are
folded into the allowed set so the existing subset checks pass. A
proxy admin can already reach every MCP server, so there is nothing
to escalate. Non-admins and every team-scoped key are unchanged.
Resolves LIT-3815
8dd7601 to
2adeb41
Compare
Relevant issues
Admins cannot create teamless keys with MCP server access
Linear ticket
LIT-3815
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewCI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
https://www.loom.com/share/fc8401ceb867497ab3db2f87fd2d3b12
Run against a live proxy with a proxy-admin key (
$KEY) and the id of a specific, non-allow_all_keysMCP server ($SERVER)Before the fix, creating a teamless key that references the server is rejected
returns the ticket's error
After the fix, the same request returns a created key, and the key can call the server's tools at runtime
Edge cases verified against the same proxy: a non-admin caller still gets the 403; a proxy admin can likewise assign an access group to a teamless key
Type
🐛 Bug Fix
Changes
In short: a proxy admin couldn't attach a specific MCP server to a teamless key — the request was rejected with a 403, so the only way to grant a non-
allow_all_keysserver was to put the key on a team first. After this fix the admin can create a teamless key, attach the MCP server (on/key/generateor later via/key/update), and the key can actually use that server's tools at runtime. Attaching the grant and using it now line up.Creating or updating a key that references a specific (non-
allow_all_keys) MCP server or access group failed with a 403 when the key had no team. The error advertised on the ticket was "Key is not in a team. Only globally available (allow_all_keys) MCP servers can be assigned"The root cause is an asymmetry between the create/update path and the runtime path.
validate_key_mcp_servers_against_teamcomputed the allowed set as the team's servers plus theallow_all_keysservers; for a teamless key the team set is empty, so the allowed set collapsed to justallow_all_keysand any explicitly-selected server or access group was rejected. The runtime path tells a different story:get_allowed_mcp_servershonors a teamless key's ownobject_permission.mcp_serversverbatim, with no team gate and noallow_all_keysfilter. So the create path refused to persist a grant the run path would have happily servedThe fix threads
is_proxy_admininto the validator from both call sites (/key/generateand/key/update). When a key has no team and the caller is a proxy admin, the requested servers and access groups are folded into the allowed set so the existing subset checks pass. A proxy admin can already reach every MCP server, so there is nothing to escalate; the grant is scoped to exactly what the admin selected. Non-admins and every team-scoped key are unchanged, so the override is strictlyteam_obj is None and is_proxy_admin. The team-scoped branch is deliberately left alone because runtime intersects a teamed key with its team, so an out-of-team grant there would be silently dropped anywayTests live in the mapped file
tests/test_litellm/proxy/management_helpers/test_object_permission_utils.py: a proxy admin can assign a private server to a teamless key, a proxy admin can assign an access group to a teamless key, a non-admin still gets the 403, and a proxy admin is still bounded by team scope on a teamed key. Neutralizing the fix makes the two positive tests fail with the exact ticket error, so they pin the regression