Merge claude/issue-421-allow-host-lan into beta-clean for testing

Author: trial

src/godot_ai/__init__.py

@@ -85,6 +85,21 @@ def main(argv: Sequence[str] | None = None) -> None:
             "managed servers (CI, manual --reload)."
         ),
     )
+    parser.add_argument(
+        "--allow-host",
+        action="append",
+        metavar="CIDR",
+        default=None,
+        help=(
+            "Expose the server to a named LAN range for remote agents (issue "
+            "#421). Takes a CIDR or bare IP (repeatable, or comma-separated), "
+            "e.g. --allow-host 192.168.1.0/24. When set, both transports bind "
+            "off loopback and the rebinding guard widens its Host allowlist to "
+            "these networks ONLY — browser Origin / Sec-Fetch-Site checks stay "
+            "on. Omit for the default loopback-only behavior. Prefer an SSH "
+            "tunnel / Tailscale on untrusted networks; only name ranges you trust."
+        ),
+    )
     parser.add_argument(
         "--exclude-domains",
         default="",
@@ -105,6 +120,23 @@ def main(argv: Sequence[str] | None = None) -> None:
     except ValueError as exc:
         parser.error(str(exc))
 
+    ## #421: parse --allow-host CIDRs. A typo here fails loudly at startup
+    ## rather than silently binding loopback-only (or worse, wide open).
+    from godot_ai.transport.origin_guard import bind_host_for_networks, parse_allow_hosts
+
+    try:
+        allow_host_networks = parse_allow_hosts(args.allow_host or [])
+    except ValueError as exc:
+        parser.error(str(exc))
+
+    ## Widen the HTTP bind off loopback only when an allowlist is named. The
+    ## DNS-rebinding guard still gates every request by the CIDR(s); binding
+    ## off loopback without the guard would be the footgun this flag avoids.
+    if allow_host_networks and args.transport in ("sse", "streamable-http"):
+        import fastmcp
+
+        fastmcp.settings.host = bind_host_for_networks(allow_host_networks)
+
     from godot_ai.runtime_info import install_pid_file
 
     install_pid_file(args.pid_file)
@@ -129,6 +161,7 @@ def main(argv: Sequence[str] | None = None) -> None:
             port=args.port,
             ws_port=args.ws_port,
             exclude_domains=exclude_domains,
+            allow_host_networks=allow_host_networks,
         )
         return
 
@@ -138,6 +171,7 @@ def main(argv: Sequence[str] | None = None) -> None:
         ws_port=args.ws_port,
         exclude_domains=exclude_domains,
         owner_pid=owner_pid,
+        allow_host_networks=allow_host_networks,
     )
 
     transport_kwargs = {}

src/godot_ai/asgi.py

@@ -15,6 +15,10 @@
 DEV_TRANSPORT_ENV = "GODOT_AI_DEV_TRANSPORT"
 DEV_WS_PORT_ENV = "GODOT_AI_DEV_WS_PORT"
 DEV_EXCLUDE_DOMAINS_ENV = "GODOT_AI_DEV_EXCLUDE_DOMAINS"
+## #421: reload runs the app in a uvicorn-supervised subprocess via the
+## ``create_app`` factory, so --allow-host CIDRs ride through as an env var
+## (the comma-joined CIDR strings) rather than a function argument.
+DEV_ALLOW_HOST_ENV = "GODOT_AI_DEV_ALLOW_HOST"
 RELOADABLE_TRANSPORTS = {"sse", "streamable-http"}
 
 STALE_MCP_SESSION_MESSAGE = (
@@ -157,9 +161,17 @@ def create_app():
     """Create the FastMCP ASGI app for uvicorn's reload supervisor."""
     from godot_ai.server import create_server
     from godot_ai.tools.domains import parse_exclude_list
+    from godot_ai.transport.origin_guard import parse_allow_hosts
 
     exclude_domains = parse_exclude_list(os.environ.get(DEV_EXCLUDE_DOMAINS_ENV, ""))
-    server = create_server(ws_port=_get_dev_ws_port(), exclude_domains=exclude_domains)
+    allow_host_networks = parse_allow_hosts(
+        [v for v in os.environ.get(DEV_ALLOW_HOST_ENV, "").split(",") if v]
+    )
+    server = create_server(
+        ws_port=_get_dev_ws_port(),
+        exclude_domains=exclude_domains,
+        allow_host_networks=allow_host_networks,
+    )
     return server.http_app(transport=_get_dev_transport())
 
 
@@ -169,6 +181,7 @@ def run_with_reload(
     port: int,
     ws_port: int,
     exclude_domains: set[str] | None = None,
+    allow_host_networks: list | None = None,
 ) -> None:
     """Run the HTTP transport through uvicorn's supported reload path."""
     if transport not in RELOADABLE_TRANSPORTS:
@@ -177,12 +190,20 @@ def run_with_reload(
     os.environ[DEV_TRANSPORT_ENV] = transport
     os.environ[DEV_WS_PORT_ENV] = str(ws_port)
     os.environ[DEV_EXCLUDE_DOMAINS_ENV] = ",".join(sorted(exclude_domains or set()))
+    ## #421: pass the CIDRs to the factory subprocess as their string forms.
+    os.environ[DEV_ALLOW_HOST_ENV] = ",".join(str(net) for net in (allow_host_networks or []))
+
+    ## Bind off loopback only when an allowlist is named; the guard (rebuilt
+    ## inside create_app from the same env) still gates every request.
+    from godot_ai.transport.origin_guard import bind_host_for_networks
+
+    bind_host = bind_host_for_networks(allow_host_networks) or fastmcp.settings.host
 
     src_dir = str(Path(__file__).resolve().parent.parent)
     uvicorn.run(
         "godot_ai.asgi:create_app",
         factory=True,
-        host=fastmcp.settings.host,
+        host=bind_host,
         port=port,
         log_level=fastmcp.settings.log_level.lower(),
         timeout_graceful_shutdown=2,

src/godot_ai/server.py

@@ -5,7 +5,7 @@
 import asyncio
 import logging
 import time
-from collections.abc import AsyncIterator, Iterable
+from collections.abc import AsyncIterator, Iterable, Sequence
 from contextlib import asynccontextmanager
 from dataclasses import dataclass
 from pathlib import Path
@@ -64,7 +64,11 @@
 from godot_ai.tools.testing import register_testing_tools
 from godot_ai.tools.theme import register_theme_tools
 from godot_ai.tools.ui import register_ui_tools
-from godot_ai.transport.origin_guard import LocalhostOnlyHTTPMiddleware
+from godot_ai.transport.origin_guard import (
+    IPNetwork,
+    LocalhostOnlyHTTPMiddleware,
+    bind_host_for_networks,
+)
 from godot_ai.transport.websocket import GodotWebSocketServer
 
 logger = logging.getLogger(__name__)
@@ -96,23 +100,35 @@ def http_app(self, *args: Any, **kwargs: Any):
         ## Outermost wrap: refuse non-loopback Host/Origin (DNS-rebinding
         ## guard, audit-v2 finding #1). Applied to every HTTP transport
         ## including ``sse`` so ``/godot-ai/status`` and the FastMCP
-        ## endpoints are guarded uniformly.
-        return LocalhostOnlyHTTPMiddleware(app)
+        ## endpoints are guarded uniformly. ``--allow-host`` (#421) widens
+        ## only the Host allowlist to named LAN CIDRs; None = loopback-only.
+        return LocalhostOnlyHTTPMiddleware(app, getattr(self, "_allow_host_networks", None))
 
 
 def create_server(
     ws_port: int = 9500,
     *,
     exclude_domains: Iterable[str] | None = None,
     owner_pid: int | None = None,
+    allow_host_networks: Sequence[IPNetwork] | None = None,
 ) -> FastMCP:
     logging.basicConfig(level=logging.INFO, format="%(name)s | %(message)s")
 
+    ## #421: --allow-host opt-in. When set, expose both transports to the
+    ## named LAN CIDR(s) — bind the WS server off loopback and hand the
+    ## networks to its rebinding guard. None/empty = unchanged loopback-only.
+    ws_bind_host = bind_host_for_networks(allow_host_networks) or "127.0.0.1"
+
     # Capture ws_port in the lifespan closure
     @asynccontextmanager
     async def _lifespan(server: FastMCP) -> AsyncIterator[AppContext]:
         registry = SessionRegistry()
-        ws_server = GodotWebSocketServer(registry, port=ws_port)
+        ws_server = GodotWebSocketServer(
+            registry,
+            port=ws_port,
+            host=ws_bind_host,
+            allowed_networks=allow_host_networks,
+        )
         client = GodotClient(ws_server, registry)
 
         ws_task = asyncio.create_task(ws_server.start())
@@ -252,6 +268,10 @@ def _emit_startup() -> None:
         lifespan=_lifespan,
     )
 
+    ## #421: stash the --allow-host CIDRs where http_app() reads them when it
+    ## installs the rebinding guard middleware. None = loopback-only (default).
+    mcp._allow_host_networks = list(allow_host_networks) if allow_host_networks else None
+
     ## Middleware registration order is load-bearing — do not reorder
     ## without reading the rationale below. Locked by
     ## ``tests/unit/test_server_middleware_order.py``.

src/godot_ai/transport/origin_guard.py

@@ -40,12 +40,16 @@
 
 from __future__ import annotations
 
+import ipaddress
+from collections.abc import Iterable, Sequence
 from http import HTTPStatus
 from typing import Any
 from urllib.parse import urlsplit
 
 from starlette.types import ASGIApp, Receive, Scope, Send
 
+IPNetwork = ipaddress.IPv4Network | ipaddress.IPv6Network
+
 LOOPBACK_HOSTNAMES: frozenset[str] = frozenset({"127.0.0.1", "localhost", "[::1]"})
 LOOPBACK_ORIGIN_SCHEMES: frozenset[str] = frozenset({"http", "https", "ws", "wss"})
 
@@ -85,16 +89,87 @@ def _normalise_host(host: str) -> str:
     return normalised
 
 
-def is_allowed_host(host_header: str | None) -> bool:
-    """Whether ``host_header`` resolves to a loopback name.
+def parse_allow_hosts(values: Iterable[str]) -> list[IPNetwork]:
+    """Parse ``--allow-host`` CLI values into IP networks (issue #421).
+
+    Each value may be a CIDR (``192.168.1.0/24``), a bare IP
+    (``192.168.1.50`` → a /32 or /128), or a comma-separated list of
+    either. ``host_bits`` set on a CIDR are tolerated (``strict=False``)
+    so ``192.168.1.5/24`` is accepted as ``192.168.1.0/24``. Raises
+    ``ValueError`` (with the offending token) on anything unparseable so
+    a typo fails loudly at startup instead of silently exposing nothing.
+    """
+    networks: list[IPNetwork] = []
+    for raw in values:
+        for token in str(raw).split(","):
+            token = token.strip()
+            if not token:
+                continue
+            try:
+                networks.append(ipaddress.ip_network(token, strict=False))
+            except ValueError as exc:
+                raise ValueError(f"invalid --allow-host value {token!r}: {exc}") from exc
+    return networks
+
+
+def bind_host_for_networks(networks: Sequence[IPNetwork] | None) -> str | None:
+    """Bind address that exposes the transports to ``networks`` (issue #421).
+
+    Returns ``None`` when no networks are named so callers keep their
+    loopback default (the byte-for-byte unchanged path). Otherwise returns
+    ``"::"`` when any requested network is IPv6 — on a dual-stack host that
+    also accepts IPv4 — and ``"0.0.0.0"`` for an IPv4-only allowlist, so an
+    IPv6 ``--allow-host`` actually listens on IPv6 instead of silently
+    binding IPv4-only. Centralized so the HTTP bind, the WebSocket bind, and
+    the reload runner can't disagree about where to listen.
+    """
+    if not networks:
+        return None
+    if any(isinstance(net, ipaddress.IPv6Network) for net in networks):
+        return "::"  # noqa: S104 — opt-in, and the guard still gates every request
+    return "0.0.0.0"  # noqa: S104 — same
+
+
+def _host_ip_in_networks(host_header: str, networks: Sequence[IPNetwork] | None) -> bool:
+    """Whether the Host header's IP literal falls inside one of ``networks``.
+
+    Only IP literals match — a DNS name (the shape a rebinding attack
+    presents) never parses to an address, so it can't slip into an
+    allowed network. Bracketed IPv6 (``[192.168..]`` form) is unwrapped
+    by ``_normalise_host`` first.
+    """
+    if not networks:
+        return False
+    candidate = _normalise_host(host_header.strip())
+    if candidate.startswith("[") and candidate.endswith("]"):
+        candidate = candidate[1:-1]
+    try:
+        ip = ipaddress.ip_address(candidate)
+    except ValueError:
+        return False
+    return any(ip in net for net in networks)
+
+
+def is_allowed_host(
+    host_header: str | None,
+    allowed_networks: Sequence[IPNetwork] | None = None,
+) -> bool:
+    """Whether ``host_header`` resolves to a loopback name (or an allowed LAN IP).
 
     Empty or missing returns False — a properly formed HTTP/1.1 request
     always carries a Host header, and refusing the request is safer than
     guessing. The WebSocket guard mirrors this.
+
+    When ``allowed_networks`` is supplied (the ``--allow-host`` opt-in,
+    #421), a Host header whose IP literal falls inside one of those
+    networks is also accepted. ``allowed_networks=None`` (the default)
+    is byte-for-byte the original loopback-only behavior.
     """
     if not host_header:
         return False
-    return _normalise_host(host_header.strip()) in LOOPBACK_HOSTNAMES
+    if _normalise_host(host_header.strip()) in LOOPBACK_HOSTNAMES:
+        return True
+    return _host_ip_in_networks(host_header, allowed_networks)
 
 
 def is_allowed_origin(origin_header: str | None) -> bool:
@@ -146,6 +221,7 @@ def evaluate_loopback(
     hosts: list[str],
     origins: list[str],
     sec_fetch_sites: list[str] | None = None,
+    allowed_networks: Sequence[IPNetwork] | None = None,
 ) -> bool:
     """Return True iff the request's headers pass the allowlist.
 
@@ -155,6 +231,13 @@ def evaluate_loopback(
     the Sec-Fetch-Site cross-origin reject rule are evaluated identically.
     A divergence between the two transports would be a security
     regression — this helper exists to prevent it.
+
+    ``allowed_networks`` (the ``--allow-host`` opt-in, #421) only widens
+    the *Host* allowlist to named LAN CIDRs. The Origin and Sec-Fetch-Site
+    rules are deliberately left untouched: a browser on the LAN sends a
+    non-loopback Origin (rejected) and a foreign Sec-Fetch-Site (rejected),
+    so DNS-rebinding defense survives the opt-in. A native remote agent
+    sends neither header, so it passes once its Host IP is allowed.
     """
     if len(hosts) > 1 or len(origins) > 1:
         return False
@@ -164,7 +247,7 @@ def evaluate_loopback(
     origin = origins[0] if origins else None
     sec_fetch_site = sec_fetch_sites[0] if sec_fetch_sites else None
     return (
-        is_allowed_host(host)
+        is_allowed_host(host, allowed_networks)
         and is_allowed_origin(origin)
         and is_allowed_sec_fetch_site(sec_fetch_site)
     )
@@ -178,8 +261,14 @@ class LocalhostOnlyHTTPMiddleware:
     before any inner middleware. Non-HTTP scopes (lifespan) pass through.
     """
 
-    def __init__(self, app: ASGIApp) -> None:
+    def __init__(
+        self,
+        app: ASGIApp,
+        allowed_networks: Sequence[IPNetwork] | None = None,
+    ) -> None:
         self.app = app
+        # #421: empty/None keeps the loopback-only behavior byte-for-byte.
+        self.allowed_networks = list(allowed_networks) if allowed_networks else None
 
     def __getattr__(self, name: str) -> Any:
         # Mirror StaleMcpSessionDiagnosticMiddleware: FastMCP / uvicorn
@@ -203,7 +292,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
             elif key == b"sec-fetch-site":
                 sec_fetch_sites.append(raw_value.decode("latin-1"))
 
-        if evaluate_loopback(hosts, origins, sec_fetch_sites):
+        if evaluate_loopback(hosts, origins, sec_fetch_sites, self.allowed_networks):
             await self.app(scope, receive, send)
             return
         await _send_forbidden(send)
@@ -223,15 +312,20 @@ async def _send_forbidden(send: Send) -> None:
     await send({"type": "http.response.body", "body": FORBIDDEN_BODY, "more_body": False})
 
 
-def make_websocket_request_guard():
+def make_websocket_request_guard(allowed_networks: Sequence[IPNetwork] | None = None):
     """Return a ``process_request`` hook for ``websockets.asyncio.server.serve``.
 
     The hook fires before the WebSocket upgrade. When Host or Origin
     fails the loopback allowlist the hook synthesizes an HTTP 403 via
     ``connection.respond(...)``; returning that response from
     ``process_request`` aborts the upgrade without ever creating a
     Session.
+
+    ``allowed_networks`` (the ``--allow-host`` opt-in, #421) widens the
+    Host allowlist identically to the HTTP middleware so the two
+    transports never diverge.
     """
+    networks = list(allowed_networks) if allowed_networks else None
 
     async def guard(connection, request):
         ## Use ``get_all`` so a smuggled duplicate (two ``Host:`` lines)
@@ -240,7 +334,7 @@ async def guard(connection, request):
         hosts = list(request.headers.get_all("Host"))
         origins = list(request.headers.get_all("Origin"))
         sec_fetch_sites = list(request.headers.get_all("Sec-Fetch-Site"))
-        if evaluate_loopback(hosts, origins, sec_fetch_sites):
+        if evaluate_loopback(hosts, origins, sec_fetch_sites, networks):
             return None
         return connection.respond(HTTPStatus.FORBIDDEN, FORBIDDEN_BODY_TEXT)