diff --git a/studio/backend/routes/mcp_servers.py b/studio/backend/routes/mcp_servers.py index 6c63bc20ca..200c0ed452 100644 --- a/studio/backend/routes/mcp_servers.py +++ b/studio/backend/routes/mcp_servers.py @@ -32,14 +32,21 @@ router = APIRouter() +def _looks_like_command(value: str) -> bool: + """Whitespace is a one-way signal: a URL can't hold an unencoded space, so a + value with whitespace is definitely a command. No whitespace proves nothing + (a lone token may be a single-arg command or a scheme-less URL).""" + return any(ch.isspace() for ch in value) + + def _validate_url(url: str) -> str: trimmed = (url or "").strip() if not trimmed: raise HTTPException(status_code = 400, detail = "url must not be empty") # When stdio is enabled on this host, a non-HTTP value is a local command. # Reuse this field so stdio servers ride the existing CRUD/storage with no - # schema change. When stdio is disabled the value falls through to the - # http-only validation below, so non-HTTP input is just a bad URL (400). + # schema change. A lone token (example.com, /usr/bin/srv) is ambiguous, so we + # keep the existing behaviour and treat any non-HTTP value as a command here. if stdio_mcp_enabled() and is_stdio(trimmed): try: parts = parse_stdio_command(trimmed) @@ -58,10 +65,15 @@ def _validate_url(url: str) -> str: return trimmed parsed = urlparse(trimmed) if parsed.scheme not in ("http", "https"): - raise HTTPException( - status_code = 400, - detail = "url must start with http:// or https://", + detail = ( + "MCP server address must start with http:// or https:// " + "(for example https://example.com/mcp)." ) + # Host-scoped wording ("this server"), not "desktop only": self-hosted + # hosts can opt in via the env var. + if _looks_like_command(trimmed): + detail += " Running a local command is not enabled on this server." + raise HTTPException(status_code = 400, detail = detail) if not parsed.netloc: raise HTTPException(status_code = 400, detail = "url is missing a host") return trimmed diff --git a/studio/backend/tests/test_mcp_stdio_pr5863.py b/studio/backend/tests/test_mcp_stdio_pr5863.py index d9a9510378..f49edcdaae 100644 --- a/studio/backend/tests/test_mcp_stdio_pr5863.py +++ b/studio/backend/tests/test_mcp_stdio_pr5863.py @@ -200,12 +200,40 @@ def test_validate_url_gate_off_rejects_stdio(monkeypatch): from routes.mcp_servers import _validate_url assert _validate_url("https://example.com/mcp") == "https://example.com/mcp" - for bad in ["npx server", "python -m mod", "ftp://host"]: + # urlparse reads "localhost:8000" scheme as "localhost", so it lands here too. + for bad in [ + "npx server", + "python -m mod", + "ftp://host", + "example.com", + "localhost:8000", + r"C:\node\node.exe server.js", + ]: with pytest.raises(HTTPException) as exc: _validate_url(bad) assert exc.value.status_code == 400 +def test_validate_url_gate_off_message_depends_on_whitespace(monkeypatch): + # The message names a command only when the value has whitespace, and never + # says "desktop app only" (self-hosted hosts can opt in via the env var). + _disable(monkeypatch) + from routes.mcp_servers import _validate_url + + with pytest.raises(HTTPException) as exc: + _validate_url("npx -y @modelcontextprotocol/server-filesystem /tmp") + cmd = exc.value.detail.lower() + assert "http://" in cmd and "https://" in cmd + assert "local command" in cmd + assert "desktop app" not in cmd + + with pytest.raises(HTTPException) as exc: + _validate_url("example.com") + lone = exc.value.detail.lower() + assert "http://" in lone and "https://" in lone + assert "local command" not in lone + + def test_validate_url_gate_on_accepts_stdio(monkeypatch): _enable(monkeypatch) from routes.mcp_servers import _validate_url @@ -217,6 +245,12 @@ def test_validate_url_gate_on_accepts_stdio(monkeypatch): assert _validate_url("npx server --url https://x/mcp") == ( "npx server --url https://x/mcp" ) + # A lone token is ambiguous; keep the prior behaviour and accept it as a + # command rather than guessing it's a URL (no regression for single binaries). + assert ( + _validate_url("/usr/local/bin/my-mcp-server") == "/usr/local/bin/my-mcp-server" + ) + assert _validate_url("mcp-server-sqlite") == "mcp-server-sqlite" # empty / unparseable still rejected for bad in [" ", '"unclosed']: with pytest.raises(HTTPException) as exc: