From 1d0371cc050dece7d5a248c171f0e30a5ea946b6 Mon Sep 17 00:00:00 2001 From: Vishal Bala Date: Fri, 2 Oct 2026 11:46:09 +0200 Subject: [PATCH 1/2] fix(mcp): apply the Host/Origin guard to every HTTP transport `run_async` added the DNS-rebinding middleware only when the transport argument was literally "sse" or "streamable-http". FastMCP serves two more cases over HTTP: "http", which is FastMCP's own default HTTP transport name, and an omitted transport, which FastMCP resolves from `fastmcp.settings.transport` (settable through FASTMCP_TRANSPORT). In both, the server bound an HTTP port with no Host or Origin validation. The `rvl mcp` CLI was not affected, because its `--transport` choices are only stdio, sse and streamable-http. An application embedding the server and calling `run_async` itself was. The HTTP transport names are now defined once, as `HTTP_TRANSPORTS` in `transport_security`, and `run_async` tests the transport after resolving an omitted one the way FastMCP does. The CLI's two checks use the same set, so adding "http" to its choices later cannot silently skip the auth warning or the bind arguments. The CLI imports the set lazily, as it does the rest of the MCP code, so `rvl` stays usable without the `mcp` extra. The regression test drives `run_async` with every transport name and with an omitted one resolving to each, and fails three of its seven cases against the previous code. --- redisvl/cli/mcp.py | 9 +++- redisvl/mcp/server.py | 11 ++++- redisvl/mcp/transport_security.py | 6 +++ .../unit/test_mcp/test_transport_security.py | 45 +++++++++++++++++++ 4 files changed, 68 insertions(+), 3 deletions(-) diff --git a/redisvl/cli/mcp.py b/redisvl/cli/mcp.py index 31f988091..952a18888 100644 --- a/redisvl/cli/mcp.py +++ b/redisvl/cli/mcp.py @@ -129,7 +129,10 @@ def _check_http_auth(cls, transport, host, auth_enabled, allow_unauthenticated): No-op for stdio or when auth is enabled. Warns for loopback binds and fails closed for non-loopback binds unless explicitly allowed. """ - if transport not in ("sse", "streamable-http") or auth_enabled: + # Imported here so `rvl` stays usable without the optional `mcp` extra. + from redisvl.mcp.transport_security import HTTP_TRANSPORTS + + if transport not in HTTP_TRANSPORTS or auth_enabled: return None if host not in cls._LOOPBACK_HOSTS and not allow_unauthenticated: @@ -163,8 +166,10 @@ async def _serve( if warning: print(warning, file=sys.stderr) + from redisvl.mcp.transport_security import HTTP_TRANSPORTS + transport_kwargs = {} - if transport in ("sse", "streamable-http"): + if transport in HTTP_TRANSPORTS: transport_kwargs["host"] = host transport_kwargs["port"] = port diff --git a/redisvl/mcp/server.py b/redisvl/mcp/server.py index 14097b1f0..c9eeda120 100644 --- a/redisvl/mcp/server.py +++ b/redisvl/mcp/server.py @@ -23,6 +23,7 @@ from redisvl.mcp.tools.search import register_search_tool from redisvl.mcp.tools.upsert import register_upsert_tool from redisvl.mcp.transport_security import ( + HTTP_TRANSPORTS, build_host_origin_middleware, resolve_transport_security_config, ) @@ -118,7 +119,15 @@ async def run_async( outermost, rejecting DNS-rebinding requests before auth or tool handlers. ``stdio`` is untouched. """ - if transport in ("sse", "streamable-http"): + # Resolved the way FastMCP resolves it: an omitted transport falls back + # to `fastmcp.settings.transport`, which can name an HTTP transport, so + # testing the raw argument would leave that server unguarded. + # Imported here because this module stays importable without the + # `mcp` extra. + import fastmcp + + resolved = transport if transport is not None else fastmcp.settings.transport + if resolved in HTTP_TRANSPORTS: host = transport_kwargs.get("host", "127.0.0.1") port = transport_kwargs.get("port", 8000) guard = build_host_origin_middleware(self._transport_security, host, port) diff --git a/redisvl/mcp/transport_security.py b/redisvl/mcp/transport_security.py index 9018e4bf8..1f875548b 100644 --- a/redisvl/mcp/transport_security.py +++ b/redisvl/mcp/transport_security.py @@ -19,6 +19,12 @@ from redisvl.mcp.config import MCPTransportSecurityConfig, _substitute_env from redisvl.mcp.settings import MCPSettings +# Every transport name FastMCP serves over HTTP. `"http"` is FastMCP's own +# default HTTP name, distinct from `"streamable-http"` though served the same +# way, and testing for only the two names the CLI offers left the guard off +# for it. Defined once so the server and the CLI cannot drift apart again. +HTTP_TRANSPORTS = frozenset({"http", "sse", "streamable-http"}) + # Hosts that only ever refer to the local machine. Kept in sync with # ``redisvl.cli.mcp.MCP._LOOPBACK_HOSTS``. _LOOPBACK_HOSTS = frozenset({"127.0.0.1", "::1", "localhost"}) diff --git a/tests/unit/test_mcp/test_transport_security.py b/tests/unit/test_mcp/test_transport_security.py index 209371bc5..90b62e9a7 100644 --- a/tests/unit/test_mcp/test_transport_security.py +++ b/tests/unit/test_mcp/test_transport_security.py @@ -237,3 +237,48 @@ def test_build_middleware_merges_configured_hosts(): kwargs = built[0].kwargs assert "proxy.internal:8000" in kwargs["allowed_hosts"] assert "127.0.0.1:8000" in kwargs["allowed_hosts"] + + +# --- which transports get the guard ------------------------------------------- + + +@pytest.mark.parametrize( + "transport, configured_default, guarded", + [ + ("streamable-http", "stdio", True), + ("sse", "stdio", True), + # FastMCP's own default HTTP name, served exactly like streamable-http. + ("http", "stdio", True), + # An omitted transport falls back to fastmcp.settings.transport, which + # can name an HTTP transport just as an explicit argument can. + (None, "streamable-http", True), + (None, "http", True), + ("stdio", "stdio", False), + (None, "stdio", False), + ], +) +def test_every_http_transport_is_served_behind_the_guard( + monkeypatch, transport, configured_default, guarded +): + fastmcp = pytest.importorskip( + "fastmcp", reason="fastmcp not installed (install redisvl[mcp])" + ) + from redisvl.mcp.server import RedisVLMCPServer + + served: dict = {} + + async def record_serve(self, transport=None, show_banner=None, **kwargs): + served["middleware"] = kwargs.get("middleware") + + monkeypatch.setattr(fastmcp.FastMCP, "run_async", record_serve) + monkeypatch.setattr(fastmcp.settings, "transport", configured_default) + guard = object() + monkeypatch.setattr( + "redisvl.mcp.server.build_host_origin_middleware", lambda *args: [guard] + ) + server = RedisVLMCPServer.__new__(RedisVLMCPServer) + server._transport_security = MCPTransportSecurityConfig() + + asyncio.run(server.run_async(transport=transport, host="0.0.0.0", port=8000)) + + assert (served["middleware"] == [guard]) is guarded From 259105ba1ab9a9b69a1250aa2776fb5aaca9f170 Mon Sep 17 00:00:00 2001 From: Vishal Bala Date: Mon, 5 Oct 2026 15:31:09 +0200 Subject: [PATCH 2/2] fix(mcp): resolve the transport and bind address as each FastMCP does Three corrections to the Host/Origin guard change. `fastmcp.settings.transport` only exists from FastMCP 3.1, but RedisVL supports `fastmcp>=2.0.0`. On 2.x through 3.0, reading it raised `AttributeError` before anything was served, breaking the default stdio path. Those versions always resolved an omitted transport to stdio, so that is now the fallback when the setting is absent. Checked against the 2.0.0, 2.10.0, 2.14.0, 3.0.0 and 3.1.0 wheels. The guard built its host allowlist from a hardcoded 127.0.0.1:8000 whenever no host or port was passed, while FastMCP binds `fastmcp.settings.host` and `port` (FASTMCP_HOST / FASTMCP_PORT) in that case. On a settings-driven non-loopback bind the guard would have rejected legitimate Host headers. It now resolves both the way FastMCP does, with explicit arguments still taking precedence. The CLI imported the server's transport set from `redisvl.mcp`, which its tests replace with a non-package stub, so the import resolved only when the submodule happened to be cached -- six of the fourteen CLI tests failed when run alone. The CLI goes back to importing nothing from `redisvl.mcp`, and instead derives its `--transport` choices and both HTTP-only checks from one local tuple, which keeps them from drifting apart. --- redisvl/cli/mcp.py | 20 ++++--- redisvl/mcp/server.py | 30 +++++++--- .../unit/test_mcp/test_transport_security.py | 58 +++++++++++++++++++ 3 files changed, 92 insertions(+), 16 deletions(-) diff --git a/redisvl/cli/mcp.py b/redisvl/cli/mcp.py index 952a18888..9a5caaa6f 100644 --- a/redisvl/cli/mcp.py +++ b/redisvl/cli/mcp.py @@ -5,6 +5,15 @@ import inspect import sys +# The HTTP transports the CLI offers. One tuple drives both the `--transport` +# choices and every HTTP-only branch below, so offering another HTTP transport +# here cannot silently skip the unauthenticated-bind check or the bind +# arguments. Deliberately local: the CLI imports nothing from `redisvl.mcp` at +# module level, so `rvl` stays usable without the optional `mcp` extra. The +# server's own set, which also covers FastMCP's `"http"`, is +# `redisvl.mcp.transport_security.HTTP_TRANSPORTS`. +_HTTP_TRANSPORT_CHOICES = ("sse", "streamable-http") + class _MCPArgumentParser(argparse.ArgumentParser): """ArgumentParser variant that reports usage errors with exit code 2.""" @@ -51,7 +60,7 @@ def __init__(self): parser.add_argument( "--transport", help="Transport protocol (default: stdio)", - choices=["stdio", "sse", "streamable-http"], + choices=["stdio", *_HTTP_TRANSPORT_CHOICES], default="stdio", ) parser.add_argument( @@ -129,10 +138,7 @@ def _check_http_auth(cls, transport, host, auth_enabled, allow_unauthenticated): No-op for stdio or when auth is enabled. Warns for loopback binds and fails closed for non-loopback binds unless explicitly allowed. """ - # Imported here so `rvl` stays usable without the optional `mcp` extra. - from redisvl.mcp.transport_security import HTTP_TRANSPORTS - - if transport not in HTTP_TRANSPORTS or auth_enabled: + if transport not in _HTTP_TRANSPORT_CHOICES or auth_enabled: return None if host not in cls._LOOPBACK_HOSTS and not allow_unauthenticated: @@ -166,10 +172,8 @@ async def _serve( if warning: print(warning, file=sys.stderr) - from redisvl.mcp.transport_security import HTTP_TRANSPORTS - transport_kwargs = {} - if transport in HTTP_TRANSPORTS: + if transport in _HTTP_TRANSPORT_CHOICES: transport_kwargs["host"] = host transport_kwargs["port"] = port diff --git a/redisvl/mcp/server.py b/redisvl/mcp/server.py index c9eeda120..12313192e 100644 --- a/redisvl/mcp/server.py +++ b/redisvl/mcp/server.py @@ -119,17 +119,31 @@ async def run_async( outermost, rejecting DNS-rebinding requests before auth or tool handlers. ``stdio`` is untouched. """ - # Resolved the way FastMCP resolves it: an omitted transport falls back - # to `fastmcp.settings.transport`, which can name an HTTP transport, so - # testing the raw argument would leave that server unguarded. - # Imported here because this module stays importable without the - # `mcp` extra. + # Resolved the way FastMCP resolves it, because the guard has to know + # what will actually be served. An omitted transport falls back to + # `fastmcp.settings.transport` (FASTMCP_TRANSPORT), which can name an + # HTTP transport; FastMCP before 3.1 has no such setting and always + # served stdio, so that is the fallback. Imported here because this + # module stays importable without the `mcp` extra. import fastmcp - resolved = transport if transport is not None else fastmcp.settings.transport + settings = getattr(fastmcp, "settings", None) + resolved = ( + transport + if transport is not None + else getattr(settings, "transport", "stdio") + ) if resolved in HTTP_TRANSPORTS: - host = transport_kwargs.get("host", "127.0.0.1") - port = transport_kwargs.get("port", 8000) + # The allowlist must match the address FastMCP binds, which falls + # back to settings (FASTMCP_HOST / FASTMCP_PORT) the same way. + # Hardcoded defaults would reject legitimate Host headers on a + # settings-driven bind. + host: Any = transport_kwargs.get("host") + if host is None: + host = getattr(settings, "host", "127.0.0.1") + port: Any = transport_kwargs.get("port") + if port is None: + port = getattr(settings, "port", 8000) guard = build_host_origin_middleware(self._transport_security, host, port) if guard: existing = transport_kwargs.get("middleware") or [] diff --git a/tests/unit/test_mcp/test_transport_security.py b/tests/unit/test_mcp/test_transport_security.py index 90b62e9a7..bf8ceb052 100644 --- a/tests/unit/test_mcp/test_transport_security.py +++ b/tests/unit/test_mcp/test_transport_security.py @@ -282,3 +282,61 @@ async def record_serve(self, transport=None, show_banner=None, **kwargs): asyncio.run(server.run_async(transport=transport, host="0.0.0.0", port=8000)) assert (served["middleware"] == [guard]) is guarded + + +def _run_with_settings(monkeypatch, settings, **run_kwargs): + """Run `run_async` against substitute FastMCP settings, recording the guard.""" + fastmcp = pytest.importorskip( + "fastmcp", reason="fastmcp not installed (install redisvl[mcp])" + ) + from redisvl.mcp.server import RedisVLMCPServer + + built: list = [] + served: dict = {} + + async def record_serve(self, transport=None, show_banner=None, **kwargs): + served["middleware"] = kwargs.get("middleware") + + monkeypatch.setattr(fastmcp.FastMCP, "run_async", record_serve) + monkeypatch.setattr(fastmcp, "settings", settings) + monkeypatch.setattr( + "redisvl.mcp.server.build_host_origin_middleware", + lambda config, host, port: built.append((host, port)) or ["guard"], + ) + server = RedisVLMCPServer.__new__(RedisVLMCPServer) + server._transport_security = MCPTransportSecurityConfig() + asyncio.run(server.run_async(**run_kwargs)) + return built, served.get("middleware") + + +def test_an_omitted_transport_serves_stdio_on_fastmcp_without_the_setting( + monkeypatch, +): + # `settings.transport` arrived in FastMCP 3.1; before it an omitted + # transport was always stdio. The supported range starts at 2.0. + from types import SimpleNamespace + + built, middleware = _run_with_settings( + monkeypatch, SimpleNamespace(host="127.0.0.1", port=8000), transport=None + ) + assert built == [] + assert middleware is None + + +def test_the_guard_allowlists_the_address_fastmcp_actually_binds(monkeypatch): + # FastMCP binds FASTMCP_HOST / FASTMCP_PORT when no host or port is passed, + # so a hardcoded 127.0.0.1:8000 allowlist would reject legitimate Host + # headers on that bind. + from types import SimpleNamespace + + settings = SimpleNamespace(transport="streamable-http", host="0.0.0.0", port=9123) + + built, middleware = _run_with_settings(monkeypatch, settings, transport=None) + assert built == [("0.0.0.0", 9123)] + assert middleware == ["guard"] + + # An explicit host and port still take precedence over settings. + built, _ = _run_with_settings( + monkeypatch, settings, transport="http", host="10.0.0.5", port=7000 + ) + assert built == [("10.0.0.5", 7000)]