Repository navigation
Upgrade to fastmcp 4 and use its native host/origin guard - #18
Conversation
The Streamable HTTP transport served any request that reached the listener without checking the Host or Origin header. The MCP Streamable HTTP specification requires servers to validate Origin, so that a page the operator visits cannot reach a loopback-bound listener through DNS rebinding and drive the server's tools with the operator's API key. fastmcp 4.0.2 ships HostOriginGuardMiddleware for exactly this, but it is off by default (`http_host_origin_protection` defaults to False), so this upgrades the pin and enables the guard explicitly. It is set to True rather than "auto": "auto" only validates when the listener is bound to loopback, which would silently drop the check on the exposed deployment that needs it most. With True, reaching the server under any other name requires an explicit GCORE_ALLOWED_HOSTS entry. - Host must match GCORE_ALLOWED_HOSTS; loopback names are always accepted and entries are glob patterns. Unlisted hosts get 421. - Origin, when present, must match GCORE_ALLOWED_ORIGINS, empty by default. Unlisted origins get 403. Requests with no Origin header are unaffected, so non-browser MCP clients keep working unchanged. The stdio transport is untouched. Client authentication is still not implemented; the docstring no longer implies it is coming and the README says so plainly. Upgrading fastmcp across two majors needed two mechanical fixes in existing code: `fastmcp.tools.tool` is now `fastmcp.tools`, and `Tool.to_mcp_tool().model_dump()` returns snake_case keys, so the schema tests ask for `by_alias=True` to keep asserting on the wire format.
742afb0 to
8afdb97
Compare
There was a problem hiding this comment.
🟡 Changes recommended
There are a few correctness/maintainability issues to address (docstring contract mismatch and a likely dev-install dependency gap for httpx).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Upgrades the server to FastMCP v4 and enables FastMCP’s native Host/Origin guard for the HTTP transport, aligning the implementation with the MCP Streamable HTTP security requirements and preventing DNS rebinding-style access from browser contexts.
Changes:
- Bumped
fastmcpto>=4.0.2,<5and updated imports / schema test expectations for v4 (Toolimport path andmodel_dump(by_alias=True)). - Enabled FastMCP host/origin protection for HTTP mode in
main(), sourcing allow-lists from newGCORE_ALLOWED_HOSTS/GCORE_ALLOWED_ORIGINSenv vars. - Added end-to-end HTTP transport security tests and documented the new configuration in the README.
File summaries
| File | Description |
|---|---|
gcore_mcp_server/server.py |
Enables FastMCP host/origin guard for HTTP transport and wires allow-lists from env. |
gcore_mcp_server/config/settings.py |
Adds env var constants and helper to parse comma-separated allow-lists. |
tests/test_http_security.py |
New end-to-end tests for Host/Origin guard behavior + allow-list parsing. |
tests/test_schema.py |
Updates FastMCP Tool import and schema dumps to use aliases under v4. |
tests/test_e2e_smoke.py |
Updates FastMCP Tool import and schema dumps to use aliases under v4. |
README.md |
Documents Host/Origin validation behavior and the new allow-list env vars. |
pyproject.toml |
Upgrades FastMCP dependency and adds httpx to the dev dependency group. |
Review details
- Files reviewed: 7/8 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Declare httpx in the legacy `[project.optional-dependencies].dev` extra as well as the dependency group, so `pip install -e .[dev]` also gets it; the tests use starlette's TestClient, which needs it. - State get_allow_list's full contract: unset, empty and whitespace-only all return None. - The test helper builds the app with the same guard configuration main() passes to mcp.run(); say that rather than claiming it does what main() does.
pedrodeoliveira
left a comment
pedrodeoliveira
left a comment
There was a problem hiding this comment.
GCORE_TRANSPORT=sse is still accepted by _TRANSPORT_MAP, but FastMCP 4 does not install HostOriginGuardMiddleware for its SSE app. main() passes these three guard arguments for every non-stdio transport, yet FastMCP.http_app() only consumes them in the http/streamable-http branch; the sse branch calls create_sse_app() without them. Consequently an SSE deployment remains reachable with arbitrary Host/Origin headers, despite the module and README claiming HTTP requests are protected. The added tests instantiate the default streamable-HTTP app and therefore do not catch this path. Please either remove/disable the sse transport, or explicitly wrap its ASGI app with equivalent Host/Origin protection and add an SSE regression test.
Separately, the required FOSSA check currently fails with a new policy violation from this dependency refresh: uv.lock moves idna from 3.10 to 3.19, which FOSSA classifies as Unicode-3.0. docutils was already failing on main, but the idna finding is introduced by this PR and needs a dependency resolution or policy decision before merge.
FastMCP only installs HostOriginGuardMiddleware in its streamable-HTTP app. Its http_app() forwards host_origin_protection and the allow-lists in the http/streamable-http branch only; the sse branch calls create_sse_app(), which takes no such arguments. So GCORE_TRANSPORT=sse would have started a listener with none of the Host/Origin validation the module and README claim for HTTP. SSE is removed from the accepted transports and rejected explicitly rather than left to the unknown-value fallback: falling back to stdio would turn a misconfiguration into a process that silently reads stdin instead of the network listener the operator asked for. The process now logs the reason and exits 2. Transport resolution moves to config.settings as resolve_transport(), a pure function, so it can be tested without importing server.py's module-level client construction. Tests: the refusal itself, the alias table, the unknown-value fallback, and one test pinning the upstream fact that the SSE app ignores the guard arguments -- so if FastMCP ever guards SSE, that test fails and the refusal can be revisited.
Findings from an external review of the previous commits. A bare `mcp.run()` on the stdio branch let FastMCP choose the transport from its own settings: `FASTMCP_TRANSPORT=http` or `=sse` in the environment (or a .env file) started a listener with no Host/Origin guard, bypassing both the strict-mode guard and the SSE refusal whenever GCORE_TRANSPORT was unset. The stdio branch now passes `transport="stdio"`; a test drives main() with FASTMCP_TRANSPORT=sse and asserts the explicit transport, and fails on the previous code. The allow-lists are now always passed as explicit lists, never None. None made FastMCP substitute FASTMCP_HTTP_ALLOWED_HOSTS / _ORIGINS, a second configuration channel that could widen the policy behind the documented GCORE_* variables. Empty lists keep the built-in defaults and close that channel. Rejecting an unsupported transport moved from import time to main(): the module is imported by tests and tooling (`fastmcp inspect`, the e2e smoke test), and a SystemExit during import terminated those too. Docs corrected against FastMCP's actual policy: the bound address is always accepted (so exposure does not strictly require an allow-list entry), same-origin and loopback origins are always accepted, ports are ignored in host patterns, and IPv6 literal origins cannot be allow-listed because the brackets are read as glob syntax. Only the console entry point applies the policy; `fastmcp run ...:mcp` does not. The SSE sentinel test now asserts the transport's own 404 rather than "not 421/403", so unrelated failures cannot keep it green.
|
Thanks, both points were right. SSE — dropped rather than wrapped (873c69f). idna — can't be pinned around: fastmcp 4 → fastmcp-slim → httpx2 → |
The HTTP transport served anything that reached the listener — no
HostorOrigincheck. The MCP spec requires validatingOrigin, otherwise a page you visit can reach a loopback listener by DNS rebinding and drive the tools with the operator's API key.fastmcp 4.0.2 ships a guard for this, so we upgrade and turn it on instead of writing our own.
Two things worth knowing:
"auto"."auto"only checks when bound to loopback, which skips the check on exactly the exposed setup that needs it.GCORE_ALLOWED_HOSTS/GCORE_ALLOWED_ORIGINSconfigure the allow-lists (glob patterns). Loopback is always allowed; no browser origin is allowed by default. Requests without anOriginheader pass, so normal MCP clients are unaffected. stdio is untouched.Crossing two majors needed two mechanical fixes:
fastmcp.tools.tool→fastmcp.tools, andmodel_dump(by_alias=True)in the schema tests since the keys went snake_case.