diff --git a/docs/user_guide/how_to_guides/mcp.md b/docs/user_guide/how_to_guides/mcp.md index b0a4132b..2d75e469 100644 --- a/docs/user_guide/how_to_guides/mcp.md +++ b/docs/user_guide/how_to_guides/mcp.md @@ -57,6 +57,29 @@ uvx --from redisvl[mcp] rvl mcp --config /path/to/mcp.yaml --transport sse --hos Streamable HTTP and SSE endpoints are **unauthenticated by default**. Binding to a non-loopback host without auth fails closed unless you pass `--allow-unauthenticated`; binding to loopback without auth only warns. For real deployments, enable JWT authentication (see {doc}`mcp_authentication`) rather than using `--allow-unauthenticated`. When not using `--read-only`, the `upsert-records` tool is also exposed to any client that can reach the server. ``` +### Transport Security (Host / Origin Validation) + +On the HTTP transports the server validates the request `Host` and `Origin` headers against allowlists before any tool runs. This is **on by default** and defends against [DNS rebinding](https://en.wikipedia.org/wiki/DNS_rebinding): without it, a malicious web page could rebind its own hostname to `127.0.0.1` and use the victim's browser to reach a local MCP server, even one bound to loopback. + +- **Host:** the allowlist is derived automatically from the bind address. Loopback binds accept `localhost`, `127.0.0.1`, and `[::1]` (with or without the port), so local MCP clients work with no configuration. +- **Origin:** requests with no `Origin` header (typical of non-browser MCP clients) always pass. A request that carries a cross-site `Origin` is rejected unless that origin is explicitly allowlisted. + +You only need to configure this for deployments where the client-visible `Host` differs from the bind address (a reverse proxy, or a `--host 0.0.0.0` bind reached via a public hostname). Use the `REDISVL_MCP_ALLOWED_HOSTS` / `REDISVL_MCP_ALLOWED_ORIGINS` env vars, or a `server.transport_security` block in the config: + +```yaml +server: + redis_url: ${REDIS_URL} + transport_security: + allowed_hosts: [mcp.example.com] # extra Host values to accept + allowed_origins: [https://app.example.com] # browser origins to accept + # allow_any_origin: true # trust upstream Origin validation + # enabled: false # disable when a trusted proxy validates +``` + +```{note} +Behind a reverse proxy or gateway that already validates `Host`/`Origin`, set `server.transport_security.enabled: false` (or `allow_any_origin: true`) so the proxy's rewritten headers are not rejected. +``` + Run it in read-only mode to expose search without upsert: ```bash @@ -83,6 +106,10 @@ You can also control boot settings through environment variables: | `REDISVL_MCP_READ_ONLY` | Disable `upsert-records` when set to `true` | | `REDISVL_MCP_TOOL_SEARCH_DESCRIPTION` | Set the base search tool description text; RedisVL still appends schema-derived typed filter, `exists`, and `return_fields` hints | | `REDISVL_MCP_TOOL_UPSERT_DESCRIPTION` | Override the upsert tool description | +| `REDISVL_MCP_ALLOWED_HOSTS` | Comma-separated extra `Host` header values to accept on HTTP transports (in addition to the bind-derived defaults) | +| `REDISVL_MCP_ALLOWED_ORIGINS` | Comma-separated browser `Origin` values to accept on HTTP transports | +| `REDISVL_MCP_ALLOW_ANY_ORIGIN` | Accept any `Origin` (for trusted-proxy setups) when set to `true` | +| `REDISVL_MCP_TRANSPORT_SECURITY_ENABLED` | Set to `false` to disable Host/Origin validation (e.g. behind a validating proxy) | ## Connect a Remote MCP Client @@ -91,7 +118,7 @@ When using Streamable HTTP or SSE transport, point your MCP client at the server - **Streamable HTTP**: `http://:/mcp` - **SSE**: `http://:/sse` -> **Note:** `` here is the bind address the server was started with. The default `127.0.0.1` only accepts connections from the same machine. To allow connections from other machines, start the server with `--host 0.0.0.0` and use the machine's actual IP or hostname in the client URL. +> **Note:** `` here is the bind address the server was started with. The default `127.0.0.1` only accepts connections from the same machine. To allow connections from other machines, start the server with `--host 0.0.0.0` and use the machine's actual IP or hostname in the client URL. Because a `0.0.0.0` bind has no single canonical `Host`, add the client-visible hostname(s) to `REDISVL_MCP_ALLOWED_HOSTS` (or `server.transport_security.allowed_hosts`), or Host validation will reject those requests. See [Transport Security](#transport-security-host-origin-validation). For example, to configure a remote MCP client to connect to a Streamable HTTP server running on `192.168.1.10:8000`: diff --git a/docs/user_guide/how_to_guides/mcp_authentication.md b/docs/user_guide/how_to_guides/mcp_authentication.md index e09680f7..a6cd7747 100644 --- a/docs/user_guide/how_to_guides/mcp_authentication.md +++ b/docs/user_guide/how_to_guides/mcp_authentication.md @@ -18,6 +18,15 @@ The `stdio` transport is a local subprocess with no network surface and is never authenticated. ``` +```{important} +Authentication is a separate concern from **transport security** (Host/Origin +validation), which is always on for the HTTP transports and defends against DNS +rebinding independently of auth. See +[Transport Security](mcp.md#transport-security-host-origin-validation). Both +layers apply together: auth decides *who* may call; the Host/Origin guard +rejects requests whose claimed authority is not allowlisted. +``` + ## What RedisVL Enforces RedisVL validates a bearer **JWT** that an existing identity provider (IdP) @@ -199,6 +208,11 @@ Use RedisVL's JWT validation for authentication and coarse read/write authorization. Layer a gateway on top when you need per-tenant Redis ACL enforcement. +When such a gateway or reverse proxy terminates the connection and forwards a +rewritten `Host` header, set `server.transport_security.enabled: false` (or +declare the public host via `server.transport_security.allowed_hosts`) so the +proxy's rewritten `Host` is not rejected by the Host/Origin guard. + ## See Also - {doc}`mcp`: run and configure the RedisVL MCP server. diff --git a/redisvl/mcp/__init__.py b/redisvl/mcp/__init__.py index f86933e6..619c040c 100644 --- a/redisvl/mcp/__init__.py +++ b/redisvl/mcp/__init__.py @@ -1,4 +1,4 @@ -from redisvl.mcp.config import MCPConfig, load_mcp_config +from redisvl.mcp.config import MCPConfig, MCPTransportSecurityConfig, load_mcp_config from redisvl.mcp.errors import MCPErrorCode, RedisVLMCPError, map_exception from redisvl.mcp.server import RedisVLMCPServer from redisvl.mcp.settings import MCPSettings @@ -7,6 +7,7 @@ "MCPConfig", "MCPErrorCode", "MCPSettings", + "MCPTransportSecurityConfig", "RedisVLMCPError", "RedisVLMCPServer", "load_mcp_config", diff --git a/redisvl/mcp/config.py b/redisvl/mcp/config.py index 65f53060..c2cd3b46 100644 --- a/redisvl/mcp/config.py +++ b/redisvl/mcp/config.py @@ -165,11 +165,41 @@ def _validate_jwt(self) -> "MCPAuthConfig": return self +class MCPTransportSecurityConfig(BaseModel): + """Host/Origin validation for the MCP server's HTTP transports. + + Only serves requests that target a trusted authority: the ``Host`` header + must be allowlisted and any cross-site ``Origin`` must be too. This blocks + requests that reach the server claiming an authority it does not own -- + DNS-rebinding being the primary motivating case, alongside cross-origin + browser requests and misrouted/proxied traffic. + + Defaults to enabled. The effective host allowlist is the set derived from + the bind address (see :func:`default_allowed_hosts`) unioned with + ``allowed_hosts``. ``stdio`` has no network surface and is never affected. + """ + + enabled: bool = True + # Extra hosts accepted in addition to the bind-derived default allowlist. + # Needed for reverse-proxy / non-loopback deployments whose public Host + # differs from the bind address. + allowed_hosts: list[str] = Field(default_factory=list) + # Exact ``Origin`` values accepted from browser clients. Empty means no + # cross-site browser origin is allowed; requests with no Origin (typical of + # non-browser MCP clients) always pass. + allowed_origins: list[str] = Field(default_factory=list) + # Escape hatch for trusted-proxy setups that terminate/validate Origin + # upstream. Distinct from an empty allowlist so "no origins configured" + # (fail closed on any Origin) differs from "operator disabled the check". + allow_any_origin: bool = False + + class MCPServerConfig(BaseModel): """Server-level bootstrap configuration.""" redis_url: str = Field(..., min_length=1) auth: MCPAuthConfig | None = None + transport_security: MCPTransportSecurityConfig | None = None class MCPIndexSearchConfig(BaseModel): diff --git a/redisvl/mcp/server.py b/redisvl/mcp/server.py index a67618df..d1bc2b6b 100644 --- a/redisvl/mcp/server.py +++ b/redisvl/mcp/server.py @@ -14,6 +14,10 @@ from redisvl.mcp.settings import MCPSettings from redisvl.mcp.tools.search import register_search_tool from redisvl.mcp.tools.upsert import register_upsert_tool +from redisvl.mcp.transport_security import ( + build_host_origin_middleware, + resolve_transport_security_config, +) from redisvl.redis.connection import RedisConnectionFactory, is_version_gte from redisvl.schema import IndexSchema @@ -84,8 +88,39 @@ def __init__(self, settings: MCPSettings): self.auth_config = auth_config self._auth_enabled = auth_provider is not None + # Host/Origin (DNS-rebinding) protection for HTTP transports. Resolved + # here so config errors surface at construction; the bind-derived host + # allowlist is finalized in run_async once host/port are known. + self._transport_security = resolve_transport_security_config( + settings, self._config_path + ) + super().__init__("redisvl", lifespan=self._fastmcp_lifespan, auth=auth_provider) + async def run_async( + self, + transport: Any = None, + show_banner: bool | None = None, + **transport_kwargs: Any, + ) -> None: + """Run the server, injecting Host/Origin validation for HTTP transports. + + The guard is prepended to any caller-supplied middleware so it runs + outermost, rejecting DNS-rebinding requests before auth or tool handlers. + ``stdio`` is untouched. + """ + if transport in ("sse", "streamable-http"): + 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) + if guard: + existing = transport_kwargs.get("middleware") or [] + transport_kwargs["middleware"] = [*guard, *existing] + + await super().run_async( + transport=transport, show_banner=show_banner, **transport_kwargs + ) + async def startup(self) -> None: """Load config, inspect the configured index, and initialize dependencies.""" async with self._transition_lock: diff --git a/redisvl/mcp/settings.py b/redisvl/mcp/settings.py index 62c9ec68..0ee8510b 100644 --- a/redisvl/mcp/settings.py +++ b/redisvl/mcp/settings.py @@ -33,6 +33,13 @@ class MCPSettings(BaseSettings): auth_authorization_claim: str | None = None auth_base_url: str | None = None + # Transport security overrides (``REDISVL_MCP_*``). When any are set they + # take precedence over the YAML ``server.transport_security`` block. + transport_security_enabled: bool | None = None + allowed_hosts: str | None = None + allowed_origins: str | None = None + allow_any_origin: bool | None = None + @classmethod def from_env( cls, @@ -86,3 +93,24 @@ def auth_overrides(self) -> dict[str, Any]: item.strip() for item in env_value.split(",") if item.strip() ] return overrides + + def transport_security_overrides(self) -> dict[str, Any]: + """Return the non-``None`` transport-security fields as a config mapping. + + Comma-separated ``allowed_hosts`` / ``allowed_origins`` are split into + lists. Returns an empty dict when no transport-security env vars are set. + """ + overrides: dict[str, Any] = {} + if self.transport_security_enabled is not None: + overrides["enabled"] = self.transport_security_enabled + if self.allow_any_origin is not None: + overrides["allow_any_origin"] = self.allow_any_origin + for env_value, field in ( + (self.allowed_hosts, "allowed_hosts"), + (self.allowed_origins, "allowed_origins"), + ): + if env_value is not None: + overrides[field] = [ + item.strip() for item in env_value.split(",") if item.strip() + ] + return overrides diff --git a/redisvl/mcp/transport_security.py b/redisvl/mcp/transport_security.py new file mode 100644 index 00000000..9018e4bf --- /dev/null +++ b/redisvl/mcp/transport_security.py @@ -0,0 +1,220 @@ +"""DNS-rebinding protection for the RedisVL MCP server's HTTP transports. + +Provides a pure-ASGI middleware (:class:`HostOriginValidationMiddleware`) that +validates ``Host`` and ``Origin`` against allowlists before any tool call runs, +plus config resolution mirroring :mod:`redisvl.mcp.auth`. See +:class:`redisvl.mcp.config.MCPTransportSecurityConfig` for what this defends +against. + +Imports of Starlette/FastMCP are deferred so this module stays importable +without the optional ``mcp`` extra. Applies only to HTTP transports; ``stdio`` +has no network surface. +""" + +from pathlib import Path +from typing import Any + +import yaml + +from redisvl.mcp.config import MCPTransportSecurityConfig, _substitute_env +from redisvl.mcp.settings import MCPSettings + +# 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"}) +# Bind addresses that listen on every interface and have no single canonical +# Host. For these the default allowlist covers only loopback; operators must +# declare their public host via ``allowed_hosts``. +_WILDCARD_HOSTS = frozenset({"0.0.0.0", "::", ""}) + + +def _to_host_header_form(host: str) -> str: + """Return the form a host takes inside an HTTP ``Host`` header. + + IPv6 literals are bracketed (``::1`` -> ``[::1]``); everything else is + returned unchanged. Already-bracketed values pass through. + """ + if ":" in host and not host.startswith("["): + return f"[{host}]" + return host + + +def _host_variants(host: str, port: int) -> set[str]: + """Return the bare and ``:port`` Host-header forms for one host, lowercased.""" + form = _to_host_header_form(host).lower() + return {form, f"{form}:{port}"} + + +def default_allowed_hosts(host: str, port: int) -> set[str]: + """Compute the default Host allowlist from the bind address. + + Loopback and wildcard binds expand to the full loopback set (localhost, + 127.0.0.1, [::1]) in both bare and ``:port`` forms. A specific non-wildcard + bind allows only that host (bare and ``:port``); operators add any extra + public hosts through config. + """ + allowed: set[str] = set() + if host in _LOOPBACK_HOSTS or host in _WILDCARD_HOSTS: + for loopback in ("localhost", "127.0.0.1", "::1"): + allowed |= _host_variants(loopback, port) + else: + allowed |= _host_variants(host, port) + return allowed + + +def _strip_port(host: str) -> str: + """Return the Host-header value with any trailing ``:port`` removed.""" + if host.startswith("["): + # Bracketed IPv6, e.g. "[::1]" or "[::1]:8000". + closing = host.find("]") + return host[: closing + 1] if closing != -1 else host + if host.count(":") == 1: + return host.rsplit(":", 1)[0] + return host + + +class HostOriginValidationMiddleware: + """Reject HTTP requests whose Host/Origin headers are not allowlisted. + + Non-``http`` scopes pass through untouched. Missing/unknown ``Host`` yields + ``400``; a present but disallowed cross-site ``Origin`` yields ``403``. A + request with no ``Origin`` (typical of non-browser MCP clients) always + passes the origin check. + """ + + def __init__( + self, + app: Any, + *, + allowed_hosts: frozenset[str], + allowed_origins: frozenset[str], + allow_any_origin: bool = False, + ) -> None: + self.app = app + # Allowlists are pre-lowercased so per-request comparison is a plain + # membership test. + self.allowed_hosts = frozenset(h.lower() for h in allowed_hosts) + self.allowed_origins = frozenset(o.lower() for o in allowed_origins) + self.allow_any_origin = allow_any_origin + + async def __call__(self, scope: Any, receive: Any, send: Any) -> None: + if scope["type"] != "http": + await self.app(scope, receive, send) + return + + headers = {key.lower(): value for key, value in scope.get("headers", [])} + + host = headers.get(b"host", b"").decode("latin-1").strip().lower() + if not self._host_allowed(host): + reason = "missing Host header" if not host else "Host not allowed" + # 400 is used (rather than the RFC-pure 421 Misdirected Request) for + # broad client compatibility. + await self._reject(send, 400, reason) + return + + origin = headers.get(b"origin") + if origin is not None: + origin_value = origin.decode("latin-1").strip().lower() + if origin_value and not self._origin_allowed(origin_value): + await self._reject(send, 403, "Origin not allowed") + return + + await self.app(scope, receive, send) + + def _host_allowed(self, host: str) -> bool: + if not host: + return False + return host in self.allowed_hosts or _strip_port(host) in self.allowed_hosts + + def _origin_allowed(self, origin: str) -> bool: + if self.allow_any_origin: + return True + return origin in self.allowed_origins + + @staticmethod + async def _reject(send: Any, status: int, reason: str) -> None: + body = reason.encode("utf-8") + await send( + { + "type": "http.response.start", + "status": status, + "headers": [ + (b"content-type", b"text/plain; charset=utf-8"), + (b"content-length", str(len(body)).encode("latin-1")), + ], + } + ) + await send({"type": "http.response.body", "body": body}) + + +def peek_yaml_transport_security(config_path: str | None) -> dict[str, Any] | None: + """Read only the ``server.transport_security`` block, env-substituted. + + Returns ``None`` when the path is unset/missing or the block is absent. Like + :func:`redisvl.mcp.auth.peek_yaml_auth`, this avoids the full config load so + the guard can be wired before the server lifespan runs. + """ + if not config_path: + return None + path = Path(config_path).expanduser() + if not path.exists(): + return None + try: + with path.open("r", encoding="utf-8") as file: + raw = yaml.safe_load(file) + except yaml.YAMLError: + return None + + server = raw.get("server") if isinstance(raw, dict) else None + block = server.get("transport_security") if isinstance(server, dict) else None + if not isinstance(block, dict): + return None + return _substitute_env(block) + + +def resolve_transport_security_config( + settings: MCPSettings, config_path: str | None = None +) -> MCPTransportSecurityConfig: + """Resolve the effective transport-security config from env over YAML. + + Always returns a config; when nothing is set it defaults to enabled with an + empty operator allowlist (the bind-derived default allowlist is added later, + when the bind address is known). + """ + env_overrides = settings.transport_security_overrides() + yaml_block = peek_yaml_transport_security(config_path) or {} + merged: dict[str, Any] = {**yaml_block, **env_overrides} + return MCPTransportSecurityConfig.model_validate(merged) + + +def build_host_origin_middleware( + config: MCPTransportSecurityConfig, host: str, port: int +) -> list: + """Build the Starlette middleware list for HTTP transport security. + + Returns an empty list when the guard is disabled. Otherwise returns a single + ``Middleware`` wrapping :class:`HostOriginValidationMiddleware`, with the + effective host allowlist (bind-derived defaults unioned with configured + hosts) and configured origins. + """ + if not config.enabled: + return [] + + from starlette.middleware import Middleware + + # Operator-supplied hosts are matched verbatim (lowercased). Supply the + # exact Host-header value expected, e.g. "example.com", "example.com:8000", + # or a bracketed IPv6 literal "[2001:db8::1]". + allowed_hosts = default_allowed_hosts(host, port) | { + entry.strip().lower() for entry in config.allowed_hosts if entry.strip() + } + allowed_origins = frozenset(origin.lower() for origin in config.allowed_origins) + + return [ + Middleware( + HostOriginValidationMiddleware, + allowed_hosts=frozenset(allowed_hosts), + allowed_origins=allowed_origins, + allow_any_origin=config.allow_any_origin, + ) + ] diff --git a/tests/integration/test_mcp/test_transport.py b/tests/integration/test_mcp/test_transport.py index 8f9641ea..a8b9f476 100644 --- a/tests/integration/test_mcp/test_transport.py +++ b/tests/integration/test_mcp/test_transport.py @@ -9,6 +9,7 @@ import socket from pathlib import Path +import httpx import pytest import yaml @@ -237,3 +238,119 @@ async def test_server_read_only_mode_hides_upsert_over_http( await server_task except asyncio.CancelledError: pass + + +# Reasons emitted by HostOriginValidationMiddleware. The MCP layer can also +# return 400 for protocol reasons, so tests match on the guard's plain-text body +# to distinguish a guard rejection from a downstream response. +_GUARD_HOST_REASONS = {"Host not allowed", "missing Host header"} +_GUARD_ORIGIN_REASON = "Origin not allowed" + + +async def _post_mcp(port: int, headers: dict) -> httpx.Response: + """POST a minimal payload to /mcp with custom headers; return the response.""" + async with httpx.AsyncClient(timeout=5.0) as client: + return await client.post( + f"http://127.0.0.1:{port}/mcp", + headers={ + "content-type": "application/json", + "accept": "application/json, text/event-stream", + **headers, + }, + json={"jsonrpc": "2.0", "id": 1, "method": "ping"}, + ) + + +@pytest.mark.asyncio +async def test_transport_security_rejects_spoofed_host( + monkeypatch, transport_index, transport_config_path +): + """DNS-rebinding defense: a request with an untrusted Host is rejected + before reaching any MCP handler.""" + + monkeypatch.setattr( + "redisvl.mcp.server.resolve_vectorizer_class", + lambda class_name: FakeVectorizer, + ) + + port = _find_free_port() + server = RedisVLMCPServer( + MCPSettings(config=transport_config_path(transport_index.schema.index.name)) + ) + + server_task = None + try: + server_task = asyncio.create_task( + server.run_async(transport="streamable-http", host="127.0.0.1", port=port) + ) + await _wait_for_port("127.0.0.1", port) + + # Spoofed Host (the DNS-rebinding attacker's domain) -> rejected by guard. + spoofed = await _post_mcp(port, {"host": "evil.com"}) + assert spoofed.status_code == 400 + assert spoofed.text in _GUARD_HOST_REASONS + + # A cross-site Origin with a valid loopback Host -> rejected by guard. + bad_origin = await _post_mcp( + port, {"host": f"127.0.0.1:{port}", "origin": "https://evil.com"} + ) + assert bad_origin.status_code == 403 + assert bad_origin.text == _GUARD_ORIGIN_REASON + + # Legitimate loopback Host, no Origin -> reaches the app (not a guard + # rejection). The MCP layer may still 400 for protocol reasons, so assert + # the guard did not produce the response. + legit = await _post_mcp(port, {"host": f"127.0.0.1:{port}"}) + assert legit.text not in _GUARD_HOST_REASONS + assert legit.text != _GUARD_ORIGIN_REASON + finally: + if server_task is not None: + server_task.cancel() + try: + await server_task + except asyncio.CancelledError: + pass + + +@pytest.mark.asyncio +async def test_transport_security_allows_configured_origin( + monkeypatch, transport_index, transport_config_path +): + """An operator-allowlisted Origin passes the guard while others are rejected.""" + + monkeypatch.setattr( + "redisvl.mcp.server.resolve_vectorizer_class", + lambda class_name: FakeVectorizer, + ) + monkeypatch.setenv("REDISVL_MCP_ALLOWED_ORIGINS", "https://good.example") + + port = _find_free_port() + server = RedisVLMCPServer( + MCPSettings.from_env( + config=transport_config_path(transport_index.schema.index.name) + ) + ) + + server_task = None + try: + server_task = asyncio.create_task( + server.run_async(transport="streamable-http", host="127.0.0.1", port=port) + ) + await _wait_for_port("127.0.0.1", port) + + host = f"127.0.0.1:{port}" + allowed = await _post_mcp( + port, {"host": host, "origin": "https://good.example"} + ) + assert allowed.text != _GUARD_ORIGIN_REASON + + rejected = await _post_mcp(port, {"host": host, "origin": "https://evil.com"}) + assert rejected.status_code == 403 + assert rejected.text == _GUARD_ORIGIN_REASON + finally: + if server_task is not None: + server_task.cancel() + try: + await server_task + except asyncio.CancelledError: + pass diff --git a/tests/unit/test_mcp/test_transport_security.py b/tests/unit/test_mcp/test_transport_security.py new file mode 100644 index 00000000..209371bc --- /dev/null +++ b/tests/unit/test_mcp/test_transport_security.py @@ -0,0 +1,239 @@ +"""Unit tests for the HTTP transport-security guard (DNS-rebinding defense). + +Drives ``HostOriginValidationMiddleware`` directly with synthetic ASGI +scope/receive/send, and covers the bind-derived default host allowlist. +""" + +import asyncio + +import pytest + +from redisvl.mcp.config import MCPTransportSecurityConfig +from redisvl.mcp.transport_security import ( + HostOriginValidationMiddleware, + _strip_port, + build_host_origin_middleware, + default_allowed_hosts, +) + + +class _RecordingApp: + """ASGI app stand-in that records whether it was invoked.""" + + def __init__(self): + self.called = False + + async def __call__(self, scope, receive, send): + self.called = True + await send({"type": "http.response.start", "status": 200, "headers": []}) + await send({"type": "http.response.body", "body": b"ok"}) + + +def _http_scope(headers: dict[bytes, bytes]) -> dict: + return { + "type": "http", + "headers": [(key, value) for key, value in headers.items()], + } + + +def _run(middleware, scope): + """Drive a middleware once, returning (status, downstream_called).""" + sent: list[dict] = [] + + async def receive(): + return {"type": "http.request", "body": b"", "more_body": False} + + async def send(message): + sent.append(message) + + asyncio.run(middleware(scope, receive, send)) + status = next( + (m["status"] for m in sent if m["type"] == "http.response.start"), None + ) + return status + + +def _middleware(app, *, hosts=("127.0.0.1:8000",), origins=(), allow_any_origin=False): + return HostOriginValidationMiddleware( + app, + allowed_hosts=frozenset(hosts), + allowed_origins=frozenset(origins), + allow_any_origin=allow_any_origin, + ) + + +# --- Host validation --------------------------------------------------------- + + +@pytest.mark.parametrize( + "host", + [b"127.0.0.1:8000", b"127.0.0.1", b"localhost", b"localhost:8000", b"[::1]"], +) +def test_allowed_loopback_host_passes(host): + app = _RecordingApp() + hosts = default_allowed_hosts("127.0.0.1", 8000) + mw = _middleware(app, hosts=hosts) + status = _run(mw, _http_scope({b"host": host})) + assert app.called is True + assert status == 200 + + +def test_spoofed_host_rejected_and_app_not_called(): + app = _RecordingApp() + mw = _middleware(app, hosts=default_allowed_hosts("127.0.0.1", 8000)) + status = _run(mw, _http_scope({b"host": b"evil.com"})) + assert status == 400 + assert app.called is False + + +def test_missing_host_rejected(): + app = _RecordingApp() + mw = _middleware(app) + status = _run(mw, _http_scope({})) + assert status == 400 + assert app.called is False + + +def test_host_is_case_insensitive(): + app = _RecordingApp() + mw = _middleware(app, hosts={"localhost", "localhost:8000"}) + status = _run(mw, _http_scope({b"host": b"LOCALHOST:8000"})) + assert status == 200 + assert app.called is True + + +def test_host_with_default_port_matches_bare_allowlist_entry(): + # Allowlist only carries the bare form; a client that includes a port still + # matches via the port-stripped comparison. + app = _RecordingApp() + mw = _middleware(app, hosts={"example.com"}) + status = _run(mw, _http_scope({b"host": b"example.com:8000"})) + assert status == 200 + + +# --- Origin validation ------------------------------------------------------- + + +def test_absent_origin_passes(): + app = _RecordingApp() + mw = _middleware(app, hosts={"localhost:8000"}) + status = _run(mw, _http_scope({b"host": b"localhost:8000"})) + assert status == 200 + assert app.called is True + + +def test_cross_site_origin_rejected(): + app = _RecordingApp() + mw = _middleware(app, hosts={"localhost:8000"}) + status = _run( + mw, + _http_scope({b"host": b"localhost:8000", b"origin": b"https://evil.com"}), + ) + assert status == 403 + assert app.called is False + + +def test_allowlisted_origin_passes(): + app = _RecordingApp() + mw = _middleware(app, hosts={"localhost:8000"}, origins={"https://good.example"}) + status = _run( + mw, + _http_scope({b"host": b"localhost:8000", b"origin": b"https://good.example"}), + ) + assert status == 200 + assert app.called is True + + +def test_allow_any_origin_passes_any_origin(): + app = _RecordingApp() + mw = _middleware(app, hosts={"localhost:8000"}, allow_any_origin=True) + status = _run( + mw, + _http_scope({b"host": b"localhost:8000", b"origin": b"https://evil.com"}), + ) + assert status == 200 + assert app.called is True + + +def test_origin_is_case_insensitive(): + app = _RecordingApp() + mw = _middleware(app, hosts={"localhost:8000"}, origins={"https://good.example"}) + status = _run( + mw, + _http_scope({b"host": b"localhost:8000", b"origin": b"HTTPS://GOOD.EXAMPLE"}), + ) + assert status == 200 + + +# --- Non-http scopes --------------------------------------------------------- + + +def test_non_http_scope_passes_through(): + app = _RecordingApp() + mw = _middleware(app) + + async def receive(): + return {"type": "websocket.receive"} + + async def send(message): + pass + + asyncio.run(mw({"type": "websocket"}, receive, send)) + assert app.called is True + + +# --- default_allowed_hosts --------------------------------------------------- + + +def test_default_allowed_hosts_loopback_expansion(): + hosts = default_allowed_hosts("127.0.0.1", 8000) + assert {"localhost", "localhost:8000", "127.0.0.1", "127.0.0.1:8000"} <= hosts + assert "[::1]" in hosts and "[::1]:8000" in hosts + + +def test_default_allowed_hosts_specific_host(): + hosts = default_allowed_hosts("192.168.1.10", 9000) + assert hosts == {"192.168.1.10", "192.168.1.10:9000"} + + +def test_default_allowed_hosts_wildcard_is_loopback_only(): + hosts = default_allowed_hosts("0.0.0.0", 8000) + # No synthesized external host; only the loopback set. + assert hosts == default_allowed_hosts("127.0.0.1", 8000) + assert "0.0.0.0" not in hosts + + +def test_default_allowed_hosts_ipv6_bind_is_bracketed(): + hosts = default_allowed_hosts("2001:db8::1", 8000) + assert hosts == {"[2001:db8::1]", "[2001:db8::1]:8000"} + + +@pytest.mark.parametrize( + "raw,expected", + [ + ("127.0.0.1:8000", "127.0.0.1"), + ("127.0.0.1", "127.0.0.1"), + ("[::1]:8000", "[::1]"), + ("[::1]", "[::1]"), + ("localhost", "localhost"), + ], +) +def test_strip_port(raw, expected): + assert _strip_port(raw) == expected + + +# --- build_host_origin_middleware -------------------------------------------- + + +def test_build_middleware_disabled_returns_empty(): + cfg = MCPTransportSecurityConfig(enabled=False) + assert build_host_origin_middleware(cfg, "127.0.0.1", 8000) == [] + + +def test_build_middleware_merges_configured_hosts(): + cfg = MCPTransportSecurityConfig(allowed_hosts=["proxy.internal:8000"]) + built = build_host_origin_middleware(cfg, "0.0.0.0", 8000) + assert len(built) == 1 + kwargs = built[0].kwargs + assert "proxy.internal:8000" in kwargs["allowed_hosts"] + assert "127.0.0.1:8000" in kwargs["allowed_hosts"] diff --git a/tests/unit/test_mcp/test_transport_security_resolution.py b/tests/unit/test_mcp/test_transport_security_resolution.py new file mode 100644 index 00000000..0624c28d --- /dev/null +++ b/tests/unit/test_mcp/test_transport_security_resolution.py @@ -0,0 +1,117 @@ +"""Unit tests for resolving transport-security config from env and YAML. + +Env vars (`REDISVL_MCP_*`) take precedence over the YAML +`server.transport_security` block. Defaults to enabled when nothing is set. +""" + +from pathlib import Path + +import yaml + +from redisvl.mcp.settings import MCPSettings +from redisvl.mcp.transport_security import resolve_transport_security_config + +_TS_ENV_VARS = ( + "REDISVL_MCP_TRANSPORT_SECURITY_ENABLED", + "REDISVL_MCP_ALLOWED_HOSTS", + "REDISVL_MCP_ALLOWED_ORIGINS", + "REDISVL_MCP_ALLOW_ANY_ORIGIN", +) + + +def _write_config(tmp_path: Path, block: dict | None) -> str: + config = { + "server": {"redis_url": "redis://localhost:6379"}, + "indexes": { + "knowledge": { + "redis_name": "docs-index", + "search": {"type": "fulltext"}, + "runtime": {"text_field_name": "content"}, + } + }, + } + if block is not None: + config["server"]["transport_security"] = block + path = tmp_path / "mcp.yaml" + path.write_text(yaml.safe_dump(config)) + return str(path) + + +def _clear_env(monkeypatch): + for var in _TS_ENV_VARS: + monkeypatch.delenv(var, raising=False) + + +def test_defaults_enabled_when_unset(tmp_path, monkeypatch): + _clear_env(monkeypatch) + path = _write_config(tmp_path, block=None) + settings = MCPSettings.from_env(config=path) + cfg = resolve_transport_security_config(settings, path) + assert cfg.enabled is True + assert cfg.allowed_hosts == [] + assert cfg.allowed_origins == [] + assert cfg.allow_any_origin is False + + +def test_resolves_from_yaml(tmp_path, monkeypatch): + _clear_env(monkeypatch) + path = _write_config( + tmp_path, + block={ + "allowed_hosts": ["proxy.internal", "proxy.internal:8000"], + "allowed_origins": ["https://app.example"], + }, + ) + settings = MCPSettings.from_env(config=path) + cfg = resolve_transport_security_config(settings, path) + assert cfg.enabled is True + assert cfg.allowed_hosts == ["proxy.internal", "proxy.internal:8000"] + assert cfg.allowed_origins == ["https://app.example"] + + +def test_env_overrides_yaml(tmp_path, monkeypatch): + _clear_env(monkeypatch) + path = _write_config(tmp_path, block={"allowed_hosts": ["from-yaml.example"]}) + monkeypatch.setenv("REDISVL_MCP_ALLOWED_HOSTS", "from-env.example, other.example") + settings = MCPSettings.from_env(config=path) + cfg = resolve_transport_security_config(settings, path) + assert cfg.allowed_hosts == ["from-env.example", "other.example"] + + +def test_env_disables_guard(tmp_path, monkeypatch): + _clear_env(monkeypatch) + path = _write_config(tmp_path, block={"enabled": True}) + monkeypatch.setenv("REDISVL_MCP_TRANSPORT_SECURITY_ENABLED", "false") + settings = MCPSettings.from_env(config=path) + cfg = resolve_transport_security_config(settings, path) + assert cfg.enabled is False + + +def test_env_allow_any_origin(tmp_path, monkeypatch): + _clear_env(monkeypatch) + path = _write_config(tmp_path, block=None) + monkeypatch.setenv("REDISVL_MCP_ALLOW_ANY_ORIGIN", "true") + settings = MCPSettings.from_env(config=path) + cfg = resolve_transport_security_config(settings, path) + assert cfg.allow_any_origin is True + + +# --- MCPSettings.transport_security_overrides() ------------------------------ + + +def test_overrides_empty_when_unset(monkeypatch): + _clear_env(monkeypatch) + settings = MCPSettings.from_env(config="/tmp/mcp.yaml") + assert settings.transport_security_overrides() == {} + + +def test_overrides_split_comma_separated_lists(monkeypatch): + _clear_env(monkeypatch) + monkeypatch.setenv("REDISVL_MCP_ALLOWED_HOSTS", "a.example, b.example ,") + monkeypatch.setenv("REDISVL_MCP_ALLOWED_ORIGINS", "https://a.example") + monkeypatch.setenv("REDISVL_MCP_TRANSPORT_SECURITY_ENABLED", "false") + settings = MCPSettings.from_env(config="/tmp/mcp.yaml") + overrides = settings.transport_security_overrides() + assert overrides["allowed_hosts"] == ["a.example", "b.example"] + assert overrides["allowed_origins"] == ["https://a.example"] + assert overrides["enabled"] is False