Skip to content

fix(mcp): restrict unauthenticated HTTP listeners to loopback - #744

Merged
rng1995 merged 12 commits into
mainfrom
yashraj/restrict-mcp-http-loopback
Oct 5, 2026
Merged

rng1995 merged 12 commits into
mainfrom
yashraj/restrict-mcp-http-loopback

Conversation

@yashrajp22

Copy link
Copy Markdown
Collaborator

Unauthenticated HTTP MCP listeners could be exposed with a wildcard or routable bind address. Reject those bindings before creating the server, retain loopback HTTP and stdio, and map localhost to 127.0.0.1 without DNS resolution. Remote access requires an authenticating reverse proxy in front of the loopback listener.

Validation: 329 MCP/CLI tests; fresh installed-wheel checks reproduce the baseline wildcard listener and confirm candidate exit 2 with no listener. Real HTTP initialize/local-target rejection and stdio sample scans still work.

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SkillSpector Review]

Hi @yashrajp22, thank you for making the unauthenticated HTTP transport refuse to listen beyond loopback!

Value and readiness: run() now validates the bind address before it builds the FastMCP server.

  • Rejected: 0.0.0.0, ::, routable IPs, hostnames, and short or integer IPv4 forms (127.1, 2130706433).
  • Accepted: 127.0.0.1, ::1, and localhost, which maps to 127.0.0.1 without a DNS lookup.

The CLI turns a rejection into exit code 2 with a clear message. The default (127.0.0.1) and stdio are unchanged. This closes the exposure and is ready for final maintainer review; the notes below are optional.

Material findings

  1. [Non-blocking] src/skillspector/mcp_server.py:289: is_loopback accepts the whole 127.0.0.0/8 range, and test_http_accepts_only_loopback_bindings locks in 127.3.2.1.

    • Why that bind does not work: build_server() calls FastMCP(name) with its default host. In mcp 1.29, FastMCP then turns on Host-header checking that allows only 127.0.0.1:*, localhost:* and [::1]:*. A server bound to 127.3.2.1 starts (on Linux) and then answers every request with 421 "Invalid Host header".
    • Suggestion: The README and the error message already say "127.0.0.1 or ::1". Accepting exactly those two, plus the localhost alias, would make the code, the docs and FastMCP's allowlist agree.
  2. [Non-blocking] Some wording still describes remote callers:

    • the mcp command docstring (src/skillspector/cli.py:3317, "or remote runtime can scan a skill")
    • the module docstring (src/skillspector/mcp_server.py:18)
    • README.md:405

    Consider "local agents" there, or a pointer to the reverse-proxy pattern. The next release notes should also mention that skillspector mcp --transport http --host 0.0.0.0 now exits with code 2.

PIC tradeoffs: There is no authenticated HTTP mode, and this PR adds no opt-in override. So off-loopback HTTP is now impossible in every configuration, including a container whose authenticating proxy runs in a separate network namespace.

In practice, little working use is lost. On current main, FastMCP already rejected (421) any request whose Host header was not a loopback name. Remote access therefore already depended on a same-host proxy forwarding to 127.0.0.1, which still works and which the README now documents. An explicit opt-in for container deployments could come later, ideally together with real authentication.

Verification and gaps:

  • Code path: I traced skillspector mcp → run(). The check runs before build_server(). FASTMCP_* environment variables cannot override the bind, because run() sets settings.host explicitly after validation.
  • No other HTTP entry point: streamable_http_app and sse_app are not used, and the Docker image does not install the mcp extra.
  • IPv6: ::1 is accepted. :: (unspecified) is rejected. Bracketed [::1] is rejected by ip_address, and uvicorn expects the bare form anyway.
  • FastMCP behaviour: I read the FastMCP auto-enable block and transport_security._validate_host in the upstream v1.29.0 tag, which matches the mcp>=1.29.0 pin.
  • Tests: they cover rejection of 9 hosts before the server is built, the accepted bindings (including ::1, localhost and LOCALHOST), and CLI exit code 2. I did not run them, per policy.
  • CI: all 6 checks passed on 4a39cc8. The bot merges of main (latest 206c644) leave this PR's diff unchanged (identical patch-id). Their CI runs are action_required and need maintainer approval before merge.
  • Conflicts: git merge-tree against main is clean, and there are no conflicts with #736, #737, #738 or #572. #736 changes other hunks of mcp_server.py, tests/unit/test_mcp_server.py and (in its follow-up) the same README note, so landing this one first is simplest.

Decision: Approved (reviewed head 206c644c7c0224fb245da65188750f87b806122b)

@yashrajp22

Copy link
Copy Markdown
Collaborator Author

Thanks for the helpful notes! The HTTP listener now accepts only 127.0.0.1, ::1 and the localhost alias, matching the Host headers FastMCP accepts. I also updated the wording to describe local agents. All 78 MCP tests pass, including rejection of other 127/8 addresses and normalization of the full IPv6 loopback spelling.

@rng1995
rng1995 merged commit 067f309 into main Oct 5, 2026
@rng1995
rng1995 deleted the yashraj/restrict-mcp-http-loopback branch October 5, 2026 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants