fix(mcp): expand toolset grants in shared permission primitives so tools/call honors them - #33612
Conversation
…ols/call honors them
Greptile SummaryFixes a 403 error on
Confidence Score: 5/5Safe to merge; the change is well-scoped and the new behavior is fully covered by mutation-checked tests. The fix centralizes toolset expansion in the two primitives every entry point already consults, so list and call paths are structurally in sync. Keys with no mcp_toolsets see no behavior change (the early-exit guard in each primitive is tested). The team-ceiling intersection is preserved because toolset servers land in key_set, which is intersected with team_set before access-group grants are unioned in. The REST filter tightening from len > 0 to is not None is a correct fix: an empty intersection now blocks all tools rather than silently falling through to allow-all. No pre-existing tests were weakened. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py | Core fix: _get_allowed_mcp_servers_for_key now unions toolset-referenced servers into the key scope (capped by team ceiling), and get_allowed_tools_for_server now unions toolset tools with direct tool grants — both guards that were missing on the call path. |
| litellm/proxy/_experimental/mcp_server/rest_endpoints.py | REST tool-listing filter now delegates to MCPRequestHandler.get_allowed_tools_for_server (the same toolset-aware shared primitive) instead of reading raw mcp_tool_permissions directly; also tightens the filter guard from len > 0 to is not None which correctly blocks empty-intersection cases. |
| litellm/proxy/_experimental/mcp_server/server.py | Removes the _merge_toolset_permissions wrapper and its single call site in _list_mcp_tools; the list path's filter_tools_by_key_team_permissions already delegates to get_allowed_tools_for_server which now handles toolset expansion natively. |
| tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py | Six new tests covering server expansion for toolset-only keys, the no-toolsets fast path (asserts no DB round-trip), end-to-end inheritance through get_allowed_mcp_servers, direct+toolset tool union, over-grant negative control, and allow-all preservation; mutation-safe against dropping either union. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py | Adds test_resolve_toolset_tool_permissions_single_db_fetch_across_checks which pins the within-request dedup contract: two sequential calls to resolve_toolset_tool_permissions with the same IDs result in exactly one list_mcp_toolsets await. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py | Adds TestRestListToolsetFiltering covering the REST list path for a toolset-only key: raw server catalog contains two tools, toolset grants only one, and the test asserts only the granted tool appears in the result. |
Reviews (3): Last reviewed commit: "fix(mcp): route REST tools list filterin..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
bugbot run |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
bugbot run |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b5d38b8. Configure here.
…omes Conflict in _list_mcp_tools: staging (#33612) moved toolset-grant expansion into the shared permission primitives and removed the _merge_toolset_permissions call; resolution applies that removal to this branch's AggregateToolListing structure
Relevant issues
Customer report (Pylon #4335): a key granted access through a toolset can list the toolset's tools but calling any of them returns 403
Linear ticket
Part of LIT-4448 (prerequisite for the entitlement enforcement point; does not resolve the ticket)
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Live proxy on localhost:4012, fresh Postgres, stub streamable-HTTP MCP server ("stubtools") exposing two tools:
lookup_statusanddelete_everything. Fixture: a toolset containing ONLY{stubtools, lookup_status}, and a key whose object_permission grants ONLY that toolset (mcp_toolsets: [<toolset_id>], no mcp_servers, no tool permissions)Before (unfixed, captured at
669ef389b9): the key can list the tool but not call it, on both the MCP protocol path and the REST pathAfter (fixed, captured at
e25cab6ed5): same fixture, same curlsType
🐛 Bug Fix
Changes
A key granted MCP access only through toolsets could list the granted tools but not call them. tools/list ran a toolset expansion step (
_merge_toolset_permissions) before filtering, butcall_mcp_toolcomputed allowed servers from the raw auth object, so the server-level check saw an empty server list and returned 403 before execution. The REST/mcp-rest/tools/callendpoint had the same gap through its own resolverRather than bolting the expansion onto each remaining entry point (there were already four near-duplicate toolset expansion wrappers, and any new entry point would re-introduce the bug), this PR moves toolset awareness into the two shared permission primitives every path already consults:
_get_allowed_mcp_servers_for_keynow unions the servers referenced by the key's toolsets into the key's scope (subject to the same team/org ceilings as any other key-level grant), andget_allowed_tools_for_serverunions the toolset's tool list into the key's per-server tool restrictions. The second half is what prevents over-granting: the call path's tool-level check defaults to allow-all when a server has no restrictions, so expanding servers without tools would have exposed every tool on a toolset-referenced server. With both in place, a toolset key reaches exactly the toolset's tools and nothing elseThe list path's
_merge_toolset_permissionswrapper is deleted; tools/list now gets identical behavior from the shared primitives, so list and call can no longer drift. A review round surfaced the one remaining path still reading raw object_permission for tool filtering: the REST tools list helper, which would have listed every tool on a toolset server; it now consults the same get_allowed_tools_for_server primitive, which also applies the team/agent/org tool ceilings the raw read ignored and fails closed on an empty allowed list, matching the protocol path./toolset/{name}/mcpscoping (_apply_toolset_scope) replaces the object_permission and emptiesmcp_toolsetsbefore the primitives run, so toolset-scoped routes are unaffected; the Responses API MCP handler does its own equivalent merge and is likewise unaffected. Team-levelmcp_toolsetsremains a key-creation ceiling only, matching its behavior on every path before this changeTests: six new cases in the mapped auth test file covering server expansion for a toolset-only key, the no-toolsets fast path (no resolver round-trip), end-to-end inheritance through
get_allowed_mcp_servers, direct+toolset tool union, the over-grant negative control (granted tool allowed, sibling tool on the same server denied), and preservation of allow-all when no restrictions exist. Mutation-checked: dropping the server union, the tool union, or the union gate each fails at least one test. A follow-up commit pins the scope semantics explicitly: toolset grants expand the KEY scope (union within a level) and the team ceiling still intersects it (intersection across levels), killing the mutant where toolset servers escape into the additive grant path. Full MCP suite (393 pre-existing tests) passes with the wrapper removedFinal Attestation
Note
Medium Risk
Changes MCP authorization for keys using toolsets across list, call, and REST; incorrect union/intersection logic could over-grant tools or block valid access, but behavior is heavily regression-tested.
Overview
Fixes toolset-only API keys that could list granted MCP tools but got 403 on call (and inconsistent REST behavior) because toolset expansion lived only on the list path.
mcp_toolsetsis now resolved inside shared auth primitives instead of a one-off merge beforetools/list._get_allowed_mcp_servers_for_keyunions servers from the key’s toolsets into allowed servers (still capped by team/org ceilings).get_allowed_tools_for_serverunions toolset tools with directmcp_tool_permissionsso tool-level checks see the full key scope and toolset-only keys are restricted to toolset tools, not allow-all on the server._merge_toolset_permissionsis removed from the protocol list path; list and call both use the same primitives. REST_get_tools_for_single_serverstops reading rawobject_permissionand callsget_allowed_tools_for_serverso REST listing matches protocol paths and team ceilings.Tests cover toolset-only server access, team intersection, tool union/over-grant guards, REST list filtering, and toolset resolution caching.
Reviewed by Cursor Bugbot for commit b5d38b8. Bugbot is set up for automated code reviews on this repo. Configure here.