Repository navigation
fix(mcp): refuse stdio MCP transport unless LITELLM_ENABLE_STDIO_MCP is set - #44194
devin-ai-integration[bot] wants to merge 4 commits into
Conversation
…is set Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
| if not litellm_constants.MCP_STDIO_TRANSPORT_ENABLED: | ||
| raise HTTPException(status_code=403, detail=litellm_constants.MCP_STDIO_DISABLED_MESSAGE) |
There was a problem hiding this comment.
|
|
||
| for server_name, raw_server_config in mcp_servers_config.items(): | ||
| server_config: MCPServerConfig = raw_server_config | ||
| self._warn_if_stdio_transport_disabled(server_name, server_config) |
There was a problem hiding this comment.
Database servers lack startup warning This warning runs only for config-file servers. Existing database-backed stdio servers are also disabled after upgrade, but their reload path emits no warning, making the change harder for operators to diagnose
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!
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| from typing_extensions import ReadOnly | ||
|
|
||
| import litellm | ||
| import litellm.constants as litellm_constants |
| ) | ||
| from typing_extensions import NotRequired, ReadOnly, Required, TypedDict | ||
|
|
||
| import litellm.constants as litellm_constants |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Closing in favor of #44066, which ships the same stdio gate with a fuller Admin UI. Yuneng owns this change going forward |
TLDR
Problem this solves:
python -c ...andnpx <any package>run arbitrary codeHow it solves it:
LITELLM_ENABLE_STDIO_MCP=trueat deploy timeenvironment_variablesfrom config or the DB can no longer set it (orLITELLM_MCP_STDIO_EXTRA_COMMANDS)Intentional product change: proxies that run stdio MCP servers today lose them on upgrade until the operator sets
LITELLM_ENABLE_STDIO_MCP=truein the proxy's process environment. Registering a stdio server through the UI or API now returns 422 with that instruction. stdio servers that already exist in config or the DB stay listed, log a warning at startup, and return 403 when usedUser Flow
Before: a proxy admin, or an attacker who got a proxy admin key, can run any command on the proxy host
"transport": "stdio", "command": "python3", "args": ["-c", "<any code>"]python3is on the allowlistAfter: the same request is refused unless the operator opted in at deploy time
stdio MCP transport is disabled on this proxy ... set LITELLM_ENABLE_STDIO_MCP=true in the proxy's process environmentforbidden, 403LITELLM_ENABLE_STDIO_MCPthroughenvironment_variablesin config or/config/updateis skipped with a warning, so proxy admin no longer means host code executionPre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/unit/<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
The proxy runs DB-free on 127.0.0.1:4000 with master key
sk-1234and a config that declares one stdio server,stdio_proof. It is a tiny JSON-RPC stdio server started withpython3 -cand exposes one tool,proof_echoAfter (77299dd)
Gate off (default)
uv run --no-sync litellm --config proof_stdio_proxy.yaml --port 4000withLITELLM_ENABLE_STDIO_MCPunset. The startup log printsMCP server 'stdio_proof' uses stdio transport, which is disabled by default. Set LITELLM_ENABLE_STDIO_MCP=true in the proxy's process environment to enable itcurl -H 'Authorization: Bearer sk-1234' http://127.0.0.1:4000/mcp-rest/tools/listreturns{"tools":[],"error":"partial_failure","message":"Failed to get tools from servers: stdio_proof: forbidden","server_outcomes":{"stdio_proof":{"status":"forbidden","http_status":403}}}curl -X POST http://127.0.0.1:4000/v1/mcp/server -H 'Authorization: Bearer sk-1234' -H 'Content-Type: application/json' -d '{"server_name":"stdio_gate_test","transport":"stdio","command":"python3","args":["-m","test_server"]}'returns422withValue error, stdio MCP transport is disabled on this proxy. stdio servers run a command on the proxy host, so they are off by default. To allow them, the operator must set LITELLM_ENABLE_STDIO_MCP=true in the proxy's process environment. Alternatively, run the server behind a stdio-to-HTTP bridge and register it as an HTTP MCP serverGate on
LITELLM_ENABLE_STDIO_MCP=true uv run --no-sync litellm --config proof_stdio_proxy.yaml --port 4000curl .../mcp-rest/tools/listreturnsproof_echowith"server_outcomes":{"stdio_proof":{"status":"ok","tool_count":1}}curl -X POST http://127.0.0.1:4000/mcp-rest/tools/call -H 'Authorization: Bearer sk-1234' -H 'Content-Type: application/json' -d '{"server_id":"stdio-proof","name":"proof_echo","arguments":{}}'returns200with"text":"stdio proof server reached"Type
🐛 Bug Fix
Caveats (if any)
Severe
LITELLM_ENABLE_STDIO_MCPMedium
/get/ui_settingsreportsenable_stdio_mcp: true. That value is read-only and comes from the env var. Editing a server that already uses stdio still shows the option. Covered by component tests, but not yet checked in a running dashboard/config/update, but the live proof doesn't, because that endpoint needs a DBenvironment_variablespath now skips every_BLOCKED_ENV_KEYSentry (PATH,LD_PRELOAD, ...), as the config-file path already didLow
LITELLM_ENABLE_STDIO_MCP=true, because those suites run stdio servers on purpose. The new integration test that checks stdio is off by default, and the/bin/shallowlist assertion, haven't been run against a live integration gateway yetLink to Devin session: https://app.devin.ai/sessions/55147bfac539478f9080942ee7f5b155
Open in Devin Desktop: https://app.devin.ai/desktop/session/55147bfac539478f9080942ee7f5b155?variant=devin