diff --git a/README.md b/README.md index ac9daf1ff..462c8413b 100644 --- a/README.md +++ b/README.md @@ -402,8 +402,8 @@ skillspector scan ./my-skill/ --no-llm ### MCP Server Run SkillSpector as a [Model Context Protocol](https://modelcontextprotocol.io) -server so any MCP-capable agent (Claude Code, Codex CLI, Gemini CLI) or remote -runtime can call scanning as a tool and **gate skill/MCP installs on the +server so local MCP-capable agents (Claude Code, Codex CLI, Gemini CLI) +can call scanning as a tool and **gate skill/MCP installs on the result** — turning SkillSpector into a runtime guardrail instead of an out-of-band audit step. @@ -416,7 +416,7 @@ uv tool install --force 'skillspector[mcp] @ git+https://github.com/NVIDIA/skill # FastMCP stdio transport for local CLI agents skillspector mcp -# streamable HTTP/SSE transport for remote / A2A callers +# Streamable HTTP transport on a local loopback interface skillspector mcp --transport http --host 127.0.0.1 --port 8000 ``` @@ -441,11 +441,12 @@ claude mcp add skillspector -- skillspector mcp > **Security — HTTP transport trust model** > > The HTTP transport ships **without authentication**. Any caller that can -> reach the port can invoke `scan_skill`. Over stdio or `127.0.0.1` this is -> the same trust boundary as the CLI. If you bind to a routable interface: +> reach the port can invoke `scan_skill`. HTTP bindings are restricted to +> loopback IPs (`127.0.0.1` or `::1`); `localhost` binds to `127.0.0.1` without +> DNS resolution. Wildcard, routable and other hostname bindings are rejected. > -> - Sit the server behind an authenticating reverse proxy (e.g. nginx + mTLS) -> before exposing it externally. +> - For remote access, put an authenticating reverse proxy (e.g. nginx + mTLS) +> in front of the loopback listener. > - Local paths and `file://` URLs are **automatically rejected** over HTTP to > prevent unauthenticated callers from reading arbitrary host files. Only > remote Git and `.zip` URLs are accepted. diff --git a/src/skillspector/cli.py b/src/skillspector/cli.py index 32560d70a..a0a1996d1 100644 --- a/src/skillspector/cli.py +++ b/src/skillspector/cli.py @@ -3295,13 +3295,15 @@ def mcp( typer.Option( "--transport", "-t", - help="Transport: FastMCP stdio for local CLI agents, http for remote/A2A callers.", + help="Transport: FastMCP stdio for local CLI agents, http for loopback HTTP clients.", case_sensitive=False, ), ] = TransportChoice.stdio, host: Annotated[ str, - typer.Option("--host", help="Host to bind (http transport only)."), + typer.Option( + "--host", help="Loopback IP to bind (http transport only; localhost is accepted)." + ), ] = "127.0.0.1", port: Annotated[ int, @@ -3312,7 +3314,7 @@ def mcp( Run SkillSpector as an MCP server. Exposes a single tool, ``scan_skill``, so any MCP-capable agent (Claude Code, - Codex CLI, Gemini CLI) or remote runtime can scan a skill and gate installs + Codex CLI, Gemini CLI) can scan a skill locally and gate installs on the verdict. Requires the optional mcp extra. Reinstall the GitHub tool package with @@ -3327,7 +3329,7 @@ def mcp( from skillspector.mcp_server import run as run_mcp run_mcp(transport=transport.value, host=host, port=port) - except ModuleNotFoundError as e: + except (ModuleNotFoundError, ValueError) as e: err_console.print(f"[red]Error:[/red] {e}") raise typer.Exit(code=2) from e diff --git a/src/skillspector/mcp_server.py b/src/skillspector/mcp_server.py index 57699867b..90c0ef70b 100644 --- a/src/skillspector/mcp_server.py +++ b/src/skillspector/mcp_server.py @@ -15,8 +15,8 @@ """MCP server exposing SkillSpector scanning as an agent-callable tool. -This lets any MCP-capable agent (Claude Code, Codex CLI, Gemini CLI) or remote -runtime call ``scan_skill`` and gate skill/MCP installs on the verdict, turning +This lets local MCP-capable agents (Claude Code, Codex CLI, Gemini CLI) call +``scan_skill`` and gate skill/MCP installs on the verdict, turning SkillSpector from an out-of-band audit tool into a runtime guardrail. The scan core (:func:`run_scan`) is deliberately independent of the ``mcp`` SDK @@ -28,6 +28,7 @@ from __future__ import annotations from collections.abc import Mapping +from ipaddress import ip_address from pathlib import Path from typing import TYPE_CHECKING, Any @@ -280,7 +281,21 @@ async def scan_skill( def run(transport: str = "stdio", host: str = "127.0.0.1", port: int = 8000) -> None: - """Run the MCP server over ``stdio`` (local agents) or ``http`` (remote/A2A).""" + """Run the MCP server over ``stdio`` or loopback-only ``http``.""" + if transport == "http": + # Never resolve caller-selected names or expose this unauthenticated server. + host = "127.0.0.1" if host.lower() == "localhost" else host + try: + host = str(ip_address(host)) + # FastMCP permits these Host headers, not the full IPv4 loopback range. + loopback = host in {"127.0.0.1", "::1"} + except ValueError: + loopback = False + if not loopback: + raise ValueError( + "HTTP MCP has no authentication and must bind to a loopback IP " + "(127.0.0.1 or ::1). Use an authenticating reverse proxy for remote access." + ) server = build_server(allow_local_targets=transport == "stdio") if transport == "stdio": server.run(transport="stdio") diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 465e130a8..aa650f054 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -96,6 +96,13 @@ def test_cli_version() -> None: assert "v" in result.output +@pytest.mark.parametrize("host", ["0.0.0.0", "::", "example.com"]) +def test_mcp_cli_rejects_exposed_http_binding(host: str) -> None: + result = runner.invoke(app, ["mcp", "--transport", "http", "--host", host]) + assert result.exit_code == 2 + assert "must bind to a loopback IP" in result.output + + def test_discovered_files_tree_renders_untrusted_names_literally() -> None: """File names cannot inject Rich markup or terminal control sequences.""" malicious_name = "\x1b]52;c;SGVsbG8=\x1b\\[conceal]hidden.py" diff --git a/tests/unit/test_mcp_server.py b/tests/unit/test_mcp_server.py index 23cf6733f..01e623a52 100644 --- a/tests/unit/test_mcp_server.py +++ b/tests/unit/test_mcp_server.py @@ -1135,17 +1135,59 @@ def fake_build_server(*, allow_local_targets: bool = False): monkeypatch.setattr(mcp_server, "build_server", fake_build_server) - mcp_server.run(transport=transport, host="0.0.0.0", port=9000) + mcp_server.run(transport=transport, host="127.0.0.1", port=9000) assert captured["allow_local_targets"] is expected_allow_local_targets if transport == "http": - assert server.settings.host == "0.0.0.0" + assert server.settings.host == "127.0.0.1" assert server.settings.port == 9000 server.run.assert_called_once_with(transport="streamable-http") else: server.run.assert_called_once_with(transport="stdio") +@pytest.mark.parametrize( + "host", + [ + "0.0.0.0", + "::", + "10.0.0.1", + "192.0.2.1", + "example.com", + "127.0.0.1.example.com", + "127.1", + "127.3.2.1", + "::1%lo", + "2130706433", + "", + ], +) +def test_http_rejects_non_loopback_before_constructing_server(host, monkeypatch): + build = MagicMock() + monkeypatch.setattr(mcp_server, "build_server", build) + with pytest.raises(ValueError, match="must bind to a loopback IP"): + mcp_server.run(transport="http", host=host) + build.assert_not_called() + + +@pytest.mark.parametrize( + "host, bound", + [ + ("127.0.0.1", "127.0.0.1"), + ("0:0:0:0:0:0:0:1", "::1"), + ("::1", "::1"), + ("localhost", "127.0.0.1"), + ("LOCALHOST", "127.0.0.1"), + ], +) +def test_http_accepts_only_loopback_bindings(host, bound, monkeypatch): + server = SimpleNamespace(settings=SimpleNamespace(host=None, port=None), run=MagicMock()) + monkeypatch.setattr(mcp_server, "build_server", lambda **_: server) + mcp_server.run(transport="http", host=host, port=9000) + assert server.settings.host == bound + server.run.assert_called_once_with(transport="streamable-http") + + def test_run_rejects_unknown_transport_without_allowing_local_targets( monkeypatch: pytest.MonkeyPatch, ) -> None: