fix(mcp): reject local targets over HTTP transport

src/skillspector/mcp_server.py

@@ -27,6 +27,7 @@
 
 from __future__ import annotations
 
+from pathlib import Path
 from typing import TYPE_CHECKING, Any
 
 from skillspector import __version__
@@ -44,11 +45,33 @@
 VALID_FORMATS = ("json", "markdown", "sarif", "terminal")
 
 
+def _is_local_target(target: str) -> bool:
+    """Return True when ``target`` names local filesystem content."""
+    stripped = target.strip()
+    if stripped.startswith("file://"):
+        return True
+    if stripped.startswith(("http://", "https://", "git@", "ssh://", "git+ssh://")):
+        return False
+    if stripped.startswith(("\\\\", "//")):
+        return True
+
+    try:
+        candidate = Path(stripped).expanduser()
+    except RuntimeError:
+        return True
+    if candidate.is_absolute() or candidate.drive:
+        return True
+    if "://" in stripped:
+        return False
+    return candidate.exists()
+
+
 async def run_scan(
     target: str,
     *,
     use_llm: bool = True,
     output_format: str = "json",
+    allow_local_targets: bool = True,
     yara_rules_dir: str | None = None,
 ) -> dict[str, Any]:
     """Invoke the SkillSpector graph and return a structured verdict.
@@ -61,6 +84,9 @@ async def run_scan(
             what actually happened.
         output_format: Format of the embedded ``report`` string. One of
             :data:`VALID_FORMATS`.
+        allow_local_targets: Whether local filesystem targets are allowed.
+            HTTP MCP calls set this to ``False`` so routable servers do not
+            accept caller-controlled local paths.
         yara_rules_dir: Optional directory of additional YARA rules.
 
     Returns:
@@ -72,6 +98,11 @@ async def run_scan(
     """
     if output_format not in VALID_FORMATS:
         raise ValueError(f"output_format must be one of {VALID_FORMATS}, got {output_format!r}")
+    if not allow_local_targets:
+        local_target = _is_local_target(target)
+        local_yara_rules = yara_rules_dir is not None and _is_local_target(yara_rules_dir)
+        if local_target or local_yara_rules:
+            raise ValueError("local targets are disabled for this MCP transport")
 
     llm_available, _ = is_llm_available()
     llm_used = use_llm and llm_available
@@ -136,10 +167,11 @@ async def run_scan(
             cleanup_result(result)
 
 
-def build_server(name: str = "skillspector") -> FastMCP:
+def build_server(name: str = "skillspector", *, allow_local_targets: bool = False) -> FastMCP:
     """Construct the FastMCP server exposing the ``scan_skill`` tool.
 
     Requires the optional ``mcp`` dependency (``pip install 'skillspector[mcp]'``).
+    Local targets stay disabled unless the caller selects a trusted transport.
     """
     try:
         from mcp.server.fastmcp import FastMCP
@@ -169,14 +201,19 @@ async def scan_skill(
         actually ran, so a low score from a static-only scan is not mistaken for
         a clean full scan.
         """
-        return await run_scan(target, use_llm=use_llm, output_format=output_format)
+        return await run_scan(
+            target,
+            use_llm=use_llm,
+            output_format=output_format,
+            allow_local_targets=allow_local_targets,
+        )
 
     return server
 
 
 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)."""
-    server = build_server()
+    server = build_server(allow_local_targets=transport == "stdio")
     if transport == "stdio":
         server.run(transport="stdio")
     elif transport == "http":

tests/unit/test_mcp_server.py

@@ -19,6 +19,8 @@
 import os
 import sys
 from pathlib import Path
+from types import SimpleNamespace
+from unittest.mock import AsyncMock, MagicMock
 
 import pytest
 
@@ -222,6 +224,195 @@ async def failed_execution_result(state: dict, config: dict) -> dict:
     assert verdict["execution_successful"] is False
 
 
+async def test_run_scan_rejects_local_target_when_disallowed(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """HTTP-style scans reject local targets before the graph is invoked."""
+    graph_ainvoke = AsyncMock()
+    monkeypatch.setattr(mcp_server.graph, "ainvoke", graph_ainvoke)
+    monkeypatch.setattr(mcp_server, "is_llm_available", lambda: (False, "no llm"))
+
+    with pytest.raises(ValueError, match="local targets are disabled"):
+        await run_scan(str(tmp_path), allow_local_targets=False)
+
+    assert graph_ainvoke.await_count == 0
+
+
+async def test_run_scan_rejects_file_url_when_local_targets_disallowed(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """The same HTTP guard rejects file:// targets before any scan runs."""
+    graph_ainvoke = AsyncMock()
+    monkeypatch.setattr(mcp_server.graph, "ainvoke", graph_ainvoke)
+    monkeypatch.setattr(mcp_server, "is_llm_available", lambda: (False, "no llm"))
+
+    with pytest.raises(ValueError, match="local targets are disabled"):
+        await run_scan(tmp_path.as_uri(), allow_local_targets=False)
+
+    assert graph_ainvoke.await_count == 0
+
+
+async def test_run_scan_rejects_local_yara_rules_when_targets_are_disallowed(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """The HTTP policy covers local YARA configuration as well as scan targets."""
+    graph_ainvoke = AsyncMock()
+    monkeypatch.setattr(mcp_server.graph, "ainvoke", graph_ainvoke)
+    monkeypatch.setattr(mcp_server, "is_llm_available", lambda: (False, "no llm"))
+
+    with pytest.raises(ValueError, match="local targets are disabled"):
+        await run_scan(
+            "https://example.com/skills/safe.git",
+            allow_local_targets=False,
+            yara_rules_dir=str(tmp_path),
+        )
+
+    assert graph_ainvoke.await_count == 0
+
+
+@pytest.mark.parametrize(
+    ("target", "expected"),
+    [
+        (r"\\server\share\skill", True),
+        ("//server/share/skill", True),
+        ("git@github.com:NVIDIA/SkillSpector.git", False),
+        ("ssh://git@github.com/NVIDIA/SkillSpector.git", False),
+        ("git+ssh://git@github.com/NVIDIA/SkillSpector.git", False),
+        ("custom://example/skill", False),
+    ],
+)
+def test_is_local_target_classifies_protocol_edges(target: str, expected: bool) -> None:
+    """Classifier treats UNC-style paths as local and known remote schemes as remote."""
+    assert mcp_server._is_local_target(target) is expected
+
+
+def test_is_local_target_checks_relative_paths_from_cwd(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Existing relative paths are local; missing relative paths stay unresolved."""
+    (tmp_path / "skill").mkdir()
+    monkeypatch.chdir(tmp_path)
+
+    assert mcp_server._is_local_target("skill") is True
+    assert mcp_server._is_local_target("missing-skill") is False
+
+
+def test_is_local_target_fails_closed_when_home_cannot_be_resolved(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Unresolvable tilde paths remain local instead of leaking a runtime error."""
+
+    def fail_to_expanduser(self: Path) -> Path:
+        raise RuntimeError("Could not determine home directory")
+
+    monkeypatch.setattr(Path, "expanduser", fail_to_expanduser)
+
+    assert mcp_server._is_local_target("~nosuchuser/skill") is True
+
+
+async def test_run_scan_allows_remote_target_when_local_targets_disallowed(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Remote HTTP targets still reach the resolver path when local targets are blocked."""
+    graph_ainvoke = AsyncMock(
+        return_value={
+            "risk_score": 0,
+            "risk_severity": "low",
+            "risk_recommendation": "safe",
+            "filtered_findings": [],
+            "report_body": "ok",
+        }
+    )
+    monkeypatch.setattr(mcp_server.graph, "ainvoke", graph_ainvoke)
+    monkeypatch.setattr(mcp_server, "is_llm_available", lambda: (False, "no llm"))
+
+    target = "https://example.com/skills/safe.git"
+    result = await run_scan(target, allow_local_targets=False)
+
+    assert result["target"] == target
+    assert graph_ainvoke.await_count == 1
+    assert graph_ainvoke.await_args.args[0]["input_path"] == target
+
+
+async def test_run_scan_keeps_default_local_target_compatibility(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """The default run_scan path still accepts local targets."""
+    graph_ainvoke = AsyncMock(
+        return_value={
+            "risk_score": 0,
+            "risk_severity": "low",
+            "risk_recommendation": "safe",
+            "filtered_findings": [],
+            "report_body": "ok",
+        }
+    )
+    monkeypatch.setattr(mcp_server.graph, "ainvoke", graph_ainvoke)
+    monkeypatch.setattr(mcp_server, "is_llm_available", lambda: (False, "no llm"))
+
+    result = await run_scan(str(tmp_path))
+
+    assert result["target"] == str(tmp_path)
+    assert graph_ainvoke.await_count == 1
+    assert graph_ainvoke.await_args.args[0]["input_path"] == str(tmp_path)
+
+
+@pytest.mark.parametrize(
+    ("transport", "expected_allow_local_targets"),
+    [("stdio", True), ("http", False)],
+)
+def test_run_passes_transport_local_target_policy(
+    transport: str,
+    expected_allow_local_targets: bool,
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """run() keeps stdio local scans available and disables them for HTTP."""
+    captured: dict[str, bool] = {}
+    server = SimpleNamespace(
+        settings=SimpleNamespace(host=None, port=None),
+        run=MagicMock(),
+    )
+
+    def fake_build_server(*, allow_local_targets: bool = False):
+        captured["allow_local_targets"] = allow_local_targets
+        return server
+
+    monkeypatch.setattr(mcp_server, "build_server", fake_build_server)
+
+    mcp_server.run(transport=transport, host="0.0.0.0", 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.port == 9000
+        server.run.assert_called_once_with(transport="streamable-http")
+    else:
+        server.run.assert_called_once_with(transport="stdio")
+
+
+def test_run_rejects_unknown_transport_without_allowing_local_targets(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Unknown transports fail closed before a server can start."""
+    captured: dict[str, bool] = {}
+    server = SimpleNamespace(
+        settings=SimpleNamespace(host=None, port=None),
+        run=MagicMock(),
+    )
+
+    def fake_build_server(*, allow_local_targets: bool = False):
+        captured["allow_local_targets"] = allow_local_targets
+        return server
+
+    monkeypatch.setattr(mcp_server, "build_server", fake_build_server)
+
+    with pytest.raises(ValueError, match="transport must be"):
+        mcp_server.run(transport="sse")
+
+    assert captured["allow_local_targets"] is False
+    server.run.assert_not_called()
+
+
 async def test_build_server_registers_scan_skill() -> None:
     """build_server wires up the scan_skill tool (requires the mcp extra)."""
     pytest.importorskip("mcp")
@@ -231,6 +422,25 @@ async def test_build_server_registers_scan_skill() -> None:
     assert "scan_skill" in {tool.name for tool in tools}
 
 
+async def test_build_server_disables_local_targets_by_default(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Direct server construction remains fail-closed before transport selection."""
+    pytest.importorskip("mcp")
+    from mcp.server.fastmcp.exceptions import ToolError
+
+    graph_ainvoke = AsyncMock()
+    monkeypatch.setattr(mcp_server.graph, "ainvoke", graph_ainvoke)
+    monkeypatch.setattr(mcp_server, "is_llm_available", lambda: (False, "no llm"))
+
+    server = mcp_server.build_server()
+
+    with pytest.raises(ToolError, match="local targets are disabled"):
+        await server.call_tool("scan_skill", {"target": str(tmp_path)})
+
+    assert graph_ainvoke.await_count == 0
+
+
 async def test_mcp_stdio_initialize_registers_scan_skill() -> None:
     """The real stdio CLI must initialize and expose the scan_skill tool."""
     pytest.importorskip("mcp")