Repository navigation
fix(mcp): cap an agent key's tools at what the invoking user and team may call - #42478
Conversation
|
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| ) | ||
| ) | ||
|
|
||
| allowed_tools = _as_list( |
There was a problem hiding this comment.
Critical-path database lookups
This caller ceiling adds team and user permission lookups to each applicable MCP tool authorization. The repository forbids new database requests in the critical request path, so this requirement must be satisfied before merging
Rule Used: What: Avoid creating new database requests or Router objects in the critical request path. Why: Creating these objects on every request causes performance degradation and unnecessary resource consumption. (source)
Knowledge Base Used: Proxy authentication and authorization
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Both loaders go through user_api_key_cache and the shared object_permission_id cache, and the server-level caller ceiling already warms them for these same principals earlier in the request, so a warm request reads no rows
There was a problem hiding this comment.
That’s a valid clarification. Because the caller team/user resolvers use user_api_key_cache and the shared object-permission cache, and the existing server-level caller ceiling resolves the same principals earlier in the request, this addition does not introduce additional database queries on the normal warm path. The cold-cache lookup is already part of the existing caller-ceiling flow, so my critical-path database finding does not apply here; I’m withdrawing it.
|
Playground check at 7978d18: the caller-scoped agent lists one tool, the forbidden call is 403, the standalone agent key keeps all three
|
… may call The invoking user's and team's x-litellm-user-id / x-litellm-team-id, echoed back by the agent, already narrowed which MCP servers the agent key could reach, but not which tools on those servers. An agent granted every tool on a server kept them all when acting for a user who may only call a subset. The caller's team and user tool grants now intersect the agent's tool list on each server, mirroring the servers axis, so the headers only ever narrow. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… the tool grant stub Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ceiling Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
7978d18 to
1f8d1d2
Compare
TLDR
Problem this solves:
How it solves it:
User Flow
Before: a user allowed one deepwiki tool asks an agent that holds every deepwiki tool, and the agent uses all of them on the user's behalf
deepwiki(three tools) and grants the internal userdemo-useronlyread_wiki_structureon itdeepwikiserver, and gives the agent its own key/v1/a2a/message/send, selects the agent, and sends "Which tools can you call right now?"x-litellm-user-id: demo-userandx-litellm-team-idheaders the proxy forwarded to itask_wiki_question,read_wiki_contentsandread_wiki_structure, and the agent answers with all threeread_wiki_structure, so the agent reached two tools the user never couldAfter: the same user gets an agent that can only call the one tool they hold
deepwiki(three tools) and grants the internal userdemo-useronlyread_wiki_structureon itdeepwikiserver, and gives the agent its own key/v1/a2a/message/send, selects the agent, and sends "Which tools can you call right now?"x-litellm-user-id: demo-userandx-litellm-team-idheadersread_wiki_structure, and the agent answers with that one toolRelevant issues
Affected release
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@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
Shared setup: one Postgres database,
STORE_MODEL_IN_DB=True, proxy started withpython -m litellm.proxy.proxy_cli --config config.yaml --port 4000 --use_v2_migration_resolverwhereconfig.yamlholds onegpt-4o-minideployment and the master key. Tool calls go to the public DeepWiki MCP server, so every tools/call below is a real upstream call. The Before leg runs the same command from a worktree at the merge base against the same database and fixturesFixtures created once with the master key:
$CALLERbelow stands for-H "x-litellm-user-id: demo-user" -H "x-litellm-team-id: $TEAM_ID", the headers the proxy forwards to an agent and the agent echoes back on its own MCP callsBefore (9cc5b78)
Tool discovery per identity
curl localhost:4000/mcp-rest/tools/list -H "Authorization: Bearer $USER_KEY"read_wiki_structurecurl localhost:4000/mcp-rest/tools/list -H "Authorization: Bearer $AGENT_KEY"ask_wiki_question read_wiki_contents read_wiki_structurecurl localhost:4000/mcp-rest/tools/list -H "Authorization: Bearer $AGENT_KEY" $CALLERask_wiki_question read_wiki_contents read_wiki_structure(acting for the user, the agent still lists the two tools the user does not hold)Tool call on the user's behalf
curl -X POST localhost:4000/mcp-rest/tools/call -H "Authorization: Bearer $AGENT_KEY" $CALLER -d '{"server_id":"deepwiki","name":"deepwiki-ask_wiki_question","arguments":{"repoName":"BerriAI/litellm","question":"What is litellm?"}}'HTTP 200with{"content":[{"type":"text","text":"LiteLLM is an open-source AI Gateway that provides a unified, OpenAI-compatible interface to over 100 ...(a tool the user is not allowed to call ran upstream)curl -X POST localhost:4000/mcp-rest/tools/call -H "Authorization: Bearer $AGENT_KEY" $CALLER -d '{"server_id":"deepwiki","name":"deepwiki-read_wiki_structure","arguments":{"repoName":"BerriAI/litellm"}}'HTTP 200with{"content":[{"type":"text","text":"Available pages for BerriAI/litellm:\n\n- 1 Overview\n - 1.1 Getting Started and Repo Orientation ...After (7978d18)
Tool discovery per identity
curl localhost:4000/mcp-rest/tools/list -H "Authorization: Bearer $USER_KEY"read_wiki_structurecurl localhost:4000/mcp-rest/tools/list -H "Authorization: Bearer $AGENT_KEY"ask_wiki_question read_wiki_contents read_wiki_structure(the agent's own grant is unchanged)curl localhost:4000/mcp-rest/tools/list -H "Authorization: Bearer $AGENT_KEY" $CALLERread_wiki_structure(the intersection of what the agent holds and what the user holds)Tool call on the user's behalf
curl -X POST localhost:4000/mcp-rest/tools/call -H "Authorization: Bearer $AGENT_KEY" $CALLER -d '{"server_id":"deepwiki","name":"deepwiki-ask_wiki_question","arguments":{"repoName":"BerriAI/litellm","question":"What is litellm?"}}'HTTP 403with{"detail":{"error":"Tool 'ask_wiki_question' is not allowed for your key/team on server 'deepwiki'. Contact proxy admin for access."}}curl -X POST localhost:4000/mcp-rest/tools/call -H "Authorization: Bearer $AGENT_KEY" $CALLER -d '{"server_id":"deepwiki","name":"deepwiki-read_wiki_structure","arguments":{"repoName":"BerriAI/litellm"}}'HTTP 200with{"content":[{"type":"text","text":"Available pages for BerriAI/litellm:\n\n- 1 Overview\n - 1.1 Getting Started and Repo Orientation ...Tests
The five new tests in
TestAgentMCPPermissionscover the intersection, a caller with no tool grants, a team grant on a different server, an agent key with no echoed caller, and an unreadable caller team. Mutation check at the tip: deleting the_apply_agent_caller_tool_ceilingcall fromget_allowed_tools_for_serverfailstest_agent_key_acting_for_a_user_only_sees_the_tools_that_user_may_call,test_agent_key_acting_for_a_team_is_capped_at_that_teams_tools_on_the_serverandtest_agent_key_acting_for_a_caller_whose_team_is_unreadable_gets_no_tools; returningallowed_toolsinstead of[]from the except branch fails the unreadable-team test. Restoring the code brings the mapped file back to 411 passedType
🐛 Bug Fix
Caveats (if any)
Medium
x-litellm-user-id/x-litellm-team-idheaders on its MCP calls, same as the existing server-level ceiling. An agent that drops them keeps its full grantend_user. The echoed caller headers are not recorded on those rows; tying them to the human is a follow-up on the logging sideLow
patch.object, the same seam the sibling server-level ceiling tests in this file use, because those loaders read proxy globals with no injection point. GivingMCPRequestHandleran injected loader is a separate refactor across every level, not something this fix should carry_get_team_object_permissiongained a return annotation so the new caller reads a typed value. Same body, no behavior changeFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/0ebe19879668488c889d0a3cc252a58d
Open in Devin Desktop: https://app.devin.ai/desktop/session/0ebe19879668488c889d0a3cc252a58d?variant=devin
Requested by: @yassin-berriai