Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions redisvl/cli/mcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -129,7 +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.
"""
if transport not in ("sse", "streamable-http") 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:
Expand Down Expand Up @@ -164,7 +173,7 @@ async def _serve(
print(warning, file=sys.stderr)

transport_kwargs = {}
if transport in ("sse", "streamable-http"):
if transport in _HTTP_TRANSPORT_CHOICES:
transport_kwargs["host"] = host
transport_kwargs["port"] = port

Expand Down
35 changes: 26 additions & 9 deletions redisvl/mcp/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -145,18 +146,34 @@ async def run_async(
outermost, rejecting DNS-rebinding requests before auth or tool handlers.
``stdio`` is untouched.
"""
# Resolved the way FastMCP resolves it, so `run_async()` with no
# argument is recorded as the stdio it will actually serve. Imported
# here because this module stays importable without the `mcp` extra.
# Resolved the way FastMCP resolves it, because both the guard and the
# tenant-injection check have 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

self._transport = (
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 transport in ("sse", "streamable-http"):
host = transport_kwargs.get("host", "127.0.0.1")
port = transport_kwargs.get("port", 8000)
self._transport = resolved

if resolved in HTTP_TRANSPORTS:
Comment thread
cursor[bot] marked this conversation as resolved.
# 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 []
Expand Down
6 changes: 6 additions & 0 deletions redisvl/mcp/transport_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
Expand Down
103 changes: 103 additions & 0 deletions tests/unit/test_mcp/test_transport_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,3 +237,106 @@ 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


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)]
Loading