harden(transport): gate --allow-host on the real socket peer, not the Host header (#544)

* harden(transport): gate --allow-host on the real socket peer, not the Host header

When --allow-host widens access to a LAN range, the HTTP transport binds
off loopback and authorization previously rested entirely on is_allowed_host,
which range-checks the client-controlled Host header. A peer outside the
allowed range could pass by spoofing `Host: <an-allowed-ip>`; the real peer
address was never consulted (no scope["client"] / remote_address use anywhere).

Add peer_ip_allowed(): the authoritative gate keyed on the unforgeable socket
peer (scope["client"] for ASGI, remote_address for the WebSocket server). When
an allowlist is set, the peer must be loopback or inside an allowed network;
the Host header is kept as a secondary range filter but is no longer the
authority. Fails closed when opted in and the peer can't be determined.

Loopback-only mode (no --allow-host) is a pass-through — the kernel bind
already guarantees a loopback peer — so behavior there is byte-for-byte
unchanged. The WebSocket port stays loopback-only regardless, so its guard
extracts the peer defensively but never gates on it today.

Adds peer_ip_allowed unit coverage plus middleware/evaluate_loopback tests
for the spoofed-Host-from-foreign-peer case and the fail-closed-on-unknown-peer
case. Addresses advisory GHSA-pp3p-fhg5-f429.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* harden(transport): unwrap IPv4-mapped IPv6 peers in the allow-host gate

Copilot review: on a dual-stack listener the real peer can arrive as an
IPv4-mapped IPv6 address (::ffff:127.0.0.1). ipaddress marks that form
.is_loopback == False and it never matches an IPv4 allowlist network, so a
genuine loopback / in-network peer would be wrongly rejected under --allow-host.
Unwrap .ipv4_mapped before the loopback / allowlist checks. Adds regression
coverage for mapped loopback / in-network / out-of-network peers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(transport): reflect the peer gate in the 403 body + WS guard docstring

Copilot re-review: with the new peer-address gate a request can be refused
solely on its peer IP, but the 403 body said only "non-loopback Host or Origin"
and the make_websocket_request_guard docstring said --allow-host only widens the
Host allowlist. Both now mention the unforgeable peer address. The body keeps
the "DNS rebinding" phrasing the existing test pins.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: dsarno

src/godot_ai/__init__.py

@@ -94,10 +94,12 @@ def main(argv: Sequence[str] | None = None) -> None:
             "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."
+            "off loopback and access is gated on the real socket peer address "
+            "(unforgeable) falling inside these networks ONLY — the Host header "
+            "is range-checked too but is not the authority, and 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(

src/godot_ai/transport/origin_guard.py

@@ -35,6 +35,14 @@
 ``no-cors`` subresources that wouldn't carry an Origin — is refused with
 HTTP 403 long before reaching FastMCP or our session registry.
 
+When the ``--allow-host`` opt-in (#421) binds the transport off loopback,
+the **real socket peer** (``scope["client"]`` / ``remote_address``) is the
+authoritative LAN gate — see :func:`peer_ip_allowed`. The ``Host`` header
+is client-controlled, so a peer outside the allowed range could otherwise
+pass the range check by spoofing ``Host: <an-allowed-ip>``; gating on the
+unforgeable peer address closes that. In the default loopback-only mode the
+peer gate is a pass-through and behavior is byte-for-byte unchanged.
+
 See umbrella #343, finding #1 (audit-v2).
 """
 
@@ -54,7 +62,7 @@
 LOOPBACK_ORIGIN_SCHEMES: frozenset[str] = frozenset({"http", "https", "ws", "wss"})
 
 FORBIDDEN_BODY = (
-    b"forbidden: non-loopback Host or Origin (DNS rebinding guard)\n"
+    b"forbidden: peer, Host, or Origin not permitted (DNS rebinding / --allow-host guard)\n"
     b"see https://github.com/hi-godot/godot-ai issue #345 for details\n"
 )
 FORBIDDEN_BODY_TEXT = FORBIDDEN_BODY.decode("utf-8")
@@ -158,6 +166,45 @@ def _host_ip_in_networks(host_header: str, networks: Sequence[IPNetwork] | None)
     return any(ip in net for net in networks)
 
 
+def peer_ip_allowed(peer_ip: str | None, allowed_networks: Sequence[IPNetwork] | None) -> bool:
+    """Whether the real TCP peer address is permitted to reach the transport.
+
+    This is the authoritative LAN gate. The ``Host`` header is
+    client-controlled — a peer outside an allowed network can spoof
+    ``Host: <an-allowed-ip>`` and pass :func:`is_allowed_host` — so when
+    the transport is bound off loopback (the ``--allow-host`` opt-in, #421)
+    the *socket peer* (``scope["client"]`` for ASGI, ``remote_address`` for
+    the WebSocket server), which the client cannot forge, must itself be
+    loopback or fall inside an allowed network.
+
+    With no allowlist (the default), the transport stays bound to loopback,
+    so the kernel already guarantees a loopback peer and this is a
+    pass-through that keeps the original behavior byte-for-byte — including
+    contexts (e.g. unit scopes) where the peer address isn't populated.
+    When an allowlist *is* set but the peer can't be determined, it fails
+    closed: better to refuse than to fall back to the spoofable header.
+    """
+    if not allowed_networks:
+        return True
+    if not peer_ip:
+        return False
+    ## Strip an IPv6 zone id (``fe80::1%eth0``) before parsing.
+    candidate = peer_ip.split("%", 1)[0].strip()
+    try:
+        ip = ipaddress.ip_address(candidate)
+    except ValueError:
+        return False
+    ## A dual-stack listener can report an IPv4 peer in IPv4-mapped IPv6 form
+    ## (``::ffff:127.0.0.1``), whose ``.is_loopback`` is False and which never
+    ## matches an IPv4 allowlist network. Unwrap it to the real IPv4 address so
+    ## a genuine loopback / in-network peer isn't wrongly rejected.
+    if isinstance(ip, ipaddress.IPv6Address) and ip.ipv4_mapped is not None:
+        ip = ip.ipv4_mapped
+    if ip.is_loopback:
+        return True
+    return any(ip in net for net in allowed_networks)
+
+
 def is_allowed_host(
     host_header: str | None,
     allowed_networks: Sequence[IPNetwork] | None = None,
@@ -230,22 +277,30 @@ def evaluate_loopback(
     origins: list[str],
     sec_fetch_sites: list[str] | None = None,
     allowed_networks: Sequence[IPNetwork] | None = None,
+    peer_ip: str | None = None,
 ) -> bool:
-    """Return True iff the request's headers pass the allowlist.
+    """Return True iff the request passes the allowlist.
 
     Both transports (ASGI middleware + WebSocket ``process_request``)
-    funnel their per-request header extraction through this helper so
-    the duplicate-header smuggling rule, the value-allowlist rule, and
-    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.
+    funnel their per-request extraction through this helper so the
+    duplicate-header smuggling rule, the value-allowlist rule, the
+    Sec-Fetch-Site cross-origin reject rule, and the peer-address gate 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) widens access
+    to named LAN CIDRs. Authorization is anchored on ``peer_ip`` — the real
+    socket peer, which the client cannot forge — not on the ``Host`` header
+    (which it can). The Host header is still range-checked, but only as a
+    secondary filter; the peer gate is what actually keeps out-of-range
+    hosts off the server. The Origin and Sec-Fetch-Site rules are 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 both its peer address and Host IP are allowed.
+
+    When ``allowed_networks`` is None (the loopback-only default), the peer
+    gate is a pass-through and behavior is byte-for-byte unchanged.
     """
     if len(hosts) > 1 or len(origins) > 1:
         return False
@@ -255,7 +310,8 @@ 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, allowed_networks)
+        peer_ip_allowed(peer_ip, allowed_networks)
+        and is_allowed_host(host, allowed_networks)
         and is_allowed_origin(origin)
         and is_allowed_sec_fetch_site(sec_fetch_site)
     )
@@ -300,7 +356,13 @@ 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, self.allowed_networks):
+        ## ASGI populates ``scope["client"]`` with the real ``(host, port)``
+        ## peer — unforgeable, unlike the Host header. ``None`` in unusual
+        ## servers; the peer gate fails closed for it only when opted in.
+        client = scope.get("client")
+        peer_ip = client[0] if client else None
+
+        if evaluate_loopback(hosts, origins, sec_fetch_sites, self.allowed_networks, peer_ip):
             await self.app(scope, receive, send)
             return
         await _send_forbidden(send)
@@ -323,15 +385,15 @@ async def _send_forbidden(send: Send) -> None:
 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.
+    The hook fires before the WebSocket upgrade. When the real peer
+    address, Host, or Origin fails the 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.
+    ``allowed_networks`` (the ``--allow-host`` opt-in, #421) gates the
+    unforgeable peer address (``remote_address``) and widens the Host
+    allowlist identically to the HTTP middleware, so the two transports
+    never diverge.
     """
     networks = list(allowed_networks) if allowed_networks else None
 
@@ -342,7 +404,14 @@ 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, networks):
+        ## ``remote_address`` is the real TCP peer (set before the HTTP
+        ## upgrade) — unforgeable, unlike the Host header. It's a
+        ## ``(host, port[, flowinfo, scopeid])`` tuple, or None if the
+        ## socket is already gone; the peer gate fails closed for None
+        ## only when opted in.
+        remote = getattr(connection, "remote_address", None)
+        peer_ip = remote[0] if remote else None
+        if evaluate_loopback(hosts, origins, sec_fetch_sites, networks, peer_ip):
             return None
         return connection.respond(HTTPStatus.FORBIDDEN, FORBIDDEN_BODY_TEXT)