Repository navigation
feat(mcp): scan and pin upstream tool descriptions - #43283
Conversation
Run every discovered MCP tool's description and input schema through the
pre_mcp_call guardrails before a listing reaches the client, drop the tools
a guardrail blocks, and serve the guardrail's masked text otherwise. Add
POST and DELETE /v1/mcp/server/{server_id}/pin so an admin can freeze a
server's tool names and descriptions; the gateway serves the pinned catalog
and raises a Slack alert with the diff when the upstream drifts.
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
GitHub Actions
Error
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…pe alerts before sending The guardrail scan now runs on the text the client is about to see: description overrides are applied first, the pinned catalog next, and the scan last, so a masked pinned or override description is served masked and a pinned tool keeps serving its pinned text while the upstream's text is poisoned. The alert signature is recorded before the send and dropped only when that send fails, so a recovery during a slow send is never undone. A tool whose scan payload cannot be built is hidden alone instead of failing the listing. apply_tool_overrides shrinks to apply_display_name_overrides and the MagicMock servers in the MCP tests carry pinned_tools=None.
…ription_scan_and_pin # Conflicts: # tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py
GitHub Actions
Error
|
bugbot run |
…tream description drift
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| user_api_key_auth: UserAPIKeyAuth | None, | ||
| raw_headers: Mapping[str, str] | None, | ||
| ) -> ToolDescriptionScan: | ||
| outcomes: Final = await asyncio.gather( |
There was a problem hiding this comment.
Low: Unbounded guardrail fan-out
An authenticated user can repeatedly request tools/list and trigger one concurrent guardrail execution for every tool returned by each allowed server. There is no catalog-size or concurrency bound here, and these scans use guardrails_only=True, which skips rate limiting, so a large catalog can amplify each listing into hundreds or thousands of external guardrail calls and exhaust proxy connections or guardrail quotas. Bound the tool count, text size, and scan concurrency; cache results by catalog and guardrail configuration; and rate-limit discovery requests.
There was a problem hiding this comment.
At 852ed63, independent execution measured eight scans per catalog, cancellation stopping all eight, and no next batch. Please reassess
PR overviewThis PR adds scanning of upstream MCP tool descriptions and pins approved tool metadata and input schemas for later discovery and invocation. Two security issues remain open. Authenticated users can amplify tool discovery into unbounded guardrail calls, potentially exhausting proxy connections or guardrail quotas, while invocation does not enforce pinned schemas and may accept parameters added after approval. One earlier issue has been addressed, but resource controls and schema enforcement are still needed. Open issues (2)
Fixed/addressed: 1 · PR risk: 5/10 |
|
@veria-ai Sequential catalog scans now cap in-flight requests at one; cancellation stops queued tools. Please verify 24faeb1 resolves this fan-out |
|
@greptileai Please review commit 24faeb1, which serializes catalog guardrail scans and adds regressions for bounded concurrency and cancellation |
|
bugbot run Please review commit 24faeb1 for sequential discovery scans, cancellation cleanup, and preservation of per-tool blocking and masking |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
| }, | ||
| ) | ||
|
|
||
| if server.pinned_tools and match_known_tool_name(name, server, server.pinned_tools) is None: |
There was a problem hiding this comment.
Low: Pinned input schemas are not enforced
This verifies only that the tool name is pinned; the subsequent validation uses allowed_params rather than the matched pin's input_schema. A caller can therefore send parameters added after the pin—such as a new callback URL—even though discovery continues to show the approved schema. Validate invocation arguments against the matched PinnedMCPTool.input_schema before dispatch.
There was a problem hiding this comment.
Pinning documents discovery snapshots and name admission, preserving invocation policy. Please assess schema enforcement against that scope: BerriAI/litellm-docs#1750
There was a problem hiding this comment.
Independent dispatch confirms pinned schemas are discovery snapshots; allowed_params rejects extra arguments with zero upstream calls. Schema enforcement changes existing invocation policy
GitHub Actions
Error
|
@greptileai Please review 852ed63: batches of eight bound concurrency and avoid serial latency, with completion and cancellation regressions |
|
bugbot run Please review 852ed63: bounded parallel catalog scans preserve ordering, masking, blocking and cancellation without serial listing latency |
|
@veria-ai Please review 852ed63: catalog scans now run in bounded batches of eight; regression tests cover completion and cancellation |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
@greptileai Please review e6dc5c5: restricted management views now clear pinned catalogs; focused regressions preserve administrator visibility and original snapshots |
|
bugbot run Please review e6dc5c5: existing sanitizers now hide pinned catalogs from restricted management views, with administrator and source-preservation controls |
|
@veria-ai Please review e6dc5c5: both management sanitizers clear pinned_tools; five focused tests pass, including administrator visibility and source preservation |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
This branch was successfully deployed
4 participants
TLDR
Problem this solves:
How it solves it:
pre_mcp_callUser Flow
Before: a developer receives poisoned tool descriptions despite enabling an MCP guardrail
pre_mcp_callon their gateway{"jsonrpc":"2.0","id":2,"method":"tools/list"}after initialization"server_url":"litellm_proxy"; the provider receives the same poisoned definitionsAfter: the same developer gets definitions inspected before their client or provider receives them
pre_mcp_callon their gateway{"jsonrpc":"2.0","id":2,"method":"tools/list"}after initializationRelevant issues
Existing implementation retained in this PR, including catalog pinning and alerts. Documentation: discovery scanning, pin snapshot semantics
Affected release
Discovery omission reproduced on v1.102.1; its introduction version is not established
Linear ticket
Resolves LIT-8383
Pre-Submission checklist
Screenshots / Proof of Fix
Independent execution used Python 3.12.13 and the same dependency lock at both commits. The controlled, real HTTP MCP server on port 8811 exposes
safe_echo,poisoned_lookupandcanary_result.poisoned_lookupcontainsCANARY-LIT8383in its tool description and nestedquerydescription. A customapply_guardrailblocks that canary or replaces it with[MASKED-LIT8383]; it is enabled by default for the supported LLM/MCP request and response hooks. This tests dispatch and propagation, not a vendor's injection detectorBoth proxy builds use the retained block/mask configurations and actual Anthropic
claude-haiku-4-5API calls for Responses. The provider key comes fromANTHROPIC_API_KEY; the test master key is supplied throughLITELLM_MASTER_KEY. No credentials are included here. Database variables are unset for these discovery casesSource builds run from their own worktree with this command shape, using port 4030 before and 4040 after:
CONFIGselects the retainedconfig_supported.yamlorconfig_mask.yaml; each proxy returned 200 from/health/liveliness. At the final tip, fresh block workers were 6589/6591 and mask workers 6964/6966, with the source worktree/home/ubuntu/repos/litellm-pr43283confirmed in process cwd and loaded source pathsFor each native case, initialize a fresh session using the same commands with
PORT=4030orPORT=4040:The listing command used below is:
The Responses command used below is:
Responses evidence inspects the actual outbound provider tool list in the detailed-debug
Final returned optional paramsentry. The client response's emptytoolsarray is not used as evidence of filteringBefore (40297e6)
Native blocking
Native masking
Responses blocking
OK; the actual provider-bound definitions contain all three tools and both canariesResponses masking
OK; provider-bound definitions contain all three tools, two canaries and no replacementsAfter (e6dc5c5)
Native blocking
hostile-safe_echoandhostile-canary_result, zero canaries and three discovery scansNative masking
Responses blocking
OK; the actual provider-bound definitions contain only the two safe tools, with zero canariesResponses masking
OK; provider-bound definitions contain all three tools, zero canaries and two replacementsVerification
Independent Devin verification through MCP checked out the exact final commit and executed the original scenarios, rather than reviewing local test output. Clean tool calls still succeed, poisoned arguments and results are blocked, and the documented known-name invocation behavior remains unchanged
A source-level concurrency probe measured at most eight active scans per catalog, sixteen across two concurrent listings, and zero active scans after cancellation with no ninth scan started. A 24-tool steady-state case with 0.2-second scans completed in 0.61 seconds; the serial equivalent is 4.8 seconds. The process's first-listing warmup is excluded from that comparison
Current-tip independent tests: 414 management tests passed with 16 expected failures, 25 catalog tests, 37 translation tests, one Responses forwarding test, six concurrency probes and four pin/override probes passed. Pin/override and invocation-schema probes use controlled upstream dependencies; they are not DB-backed live pin-management evidence
Local final-tip validation: 2,125 affected backend tests passed, one skipped and 16 expected failures. All canonical
BASE_REF=40297e62684e48ec9d870198e4e5bb4d88322ab0 make lintgates passed, including whole-tree Ruff/test-tree checks, strict/type/test-quality budgets and basedpyright. Dashboard type tests (four) and a clean production build passed; the single AVIF warning matches the merge-base buildLocal changed executable lines: 282/282 (100%). Touched-function branches: 235/282 (83.33%), including legacy branches in shared methods; the catalog scanner itself covers 116/116 lines and 10/10 branches. Remaining branch gaps are retained and reviewed, with no exclusions or threshold changes. Completed Codecov reports 78.84% repository coverage and 259/264 (98.11%) raw patch coverage. Its five displayed gaps use merge-tree line positions against PR-head source. Matching exact source lines from the tested merge
64b81d5ceb63b8fc2aabb74119e4840b7d88cb81to this head shows 282/282 changed executable lines covered, with no gaps; the raw percentage is retained separatelyThe metadata repair was independently checked through the real FastAPI router and response serialization using in-process HTTP with mocked persistence/authentication. At
852ed63e2b, ordinary users, restricted keys and view-only admins received the raw pin; ate6dc5c5445, all three receivepinned_tools: nullwith HTTP 200. Full admins retain the snapshot. This is controlled integration evidence, not DB-backed E2EGreptile reviewed
e6dc5c5445at 5/5. Veria confirms the metadata repair but retains the two resource-control and invocation-schema concerns below. Bugbot is paused at its team spend limit. CLA reports the inheritedgithub-actions[bot]committer unsigned. All ten required CI checks pass. The optional Python CodeQL job failed at the inherited 2 GiB result-set limit (confirmed in current-tip annotations). This PR is not ready for maintainer reviewType
New feature, bug fix and regression tests
Caveats (if any)
Medium
Low
Final Attestation