Skip to content

Commit a898f93

Browse files
committed
fix(transport): bind dual-stack for IPv6 --allow-host CIDRs (#421)
Code-review follow-up: the bind was hardcoded 0.0.0.0 (IPv4-only), so an IPv6 --allow-host CIDR set the guard to accept IPv6 hosts but the socket never listened on IPv6. Add a shared bind_host_for_networks() helper (single source of truth for the HTTP, WebSocket, and reload binds, mirroring the shared evaluate_loopback) that binds '::' when any requested network is IPv6 and '0.0.0.0' for an IPv4-only allowlist. https://claude.ai/code/session_01Jq5X4ivngAf1N6r5UX2BVw
1 parent ef29303 commit a898f93

5 files changed

Lines changed: 48 additions & 6 deletions

File tree

src/godot_ai/__init__.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ def main(argv: Sequence[str] | None = None) -> None:
122122

123123
## #421: parse --allow-host CIDRs. A typo here fails loudly at startup
124124
## rather than silently binding loopback-only (or worse, wide open).
125-
from godot_ai.transport.origin_guard import parse_allow_hosts
125+
from godot_ai.transport.origin_guard import bind_host_for_networks, parse_allow_hosts
126126

127127
try:
128128
allow_host_networks = parse_allow_hosts(args.allow_host or [])
@@ -131,11 +131,11 @@ def main(argv: Sequence[str] | None = None) -> None:
131131

132132
## Widen the HTTP bind off loopback only when an allowlist is named. The
133133
## DNS-rebinding guard still gates every request by the CIDR(s); binding
134-
## 0.0.0.0 without the guard would be the footgun this flag avoids.
134+
## off loopback without the guard would be the footgun this flag avoids.
135135
if allow_host_networks and args.transport in ("sse", "streamable-http"):
136136
import fastmcp
137137

138-
fastmcp.settings.host = "0.0.0.0" # noqa: S104
138+
fastmcp.settings.host = bind_host_for_networks(allow_host_networks)
139139

140140
from godot_ai.runtime_info import install_pid_file
141141

src/godot_ai/asgi.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,9 @@ def run_with_reload(
195195

196196
## Bind off loopback only when an allowlist is named; the guard (rebuilt
197197
## inside create_app from the same env) still gates every request.
198-
bind_host = "0.0.0.0" if allow_host_networks else fastmcp.settings.host # noqa: S104
198+
from godot_ai.transport.origin_guard import bind_host_for_networks
199+
200+
bind_host = bind_host_for_networks(allow_host_networks) or fastmcp.settings.host
199201

200202
src_dir = str(Path(__file__).resolve().parent.parent)
201203
uvicorn.run(

src/godot_ai/server.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,11 @@
6464
from godot_ai.tools.testing import register_testing_tools
6565
from godot_ai.tools.theme import register_theme_tools
6666
from godot_ai.tools.ui import register_ui_tools
67-
from godot_ai.transport.origin_guard import IPNetwork, LocalhostOnlyHTTPMiddleware
67+
from godot_ai.transport.origin_guard import (
68+
IPNetwork,
69+
LocalhostOnlyHTTPMiddleware,
70+
bind_host_for_networks,
71+
)
6872
from godot_ai.transport.websocket import GodotWebSocketServer
6973

7074
logger = logging.getLogger(__name__)
@@ -113,7 +117,7 @@ def create_server(
113117
## #421: --allow-host opt-in. When set, expose both transports to the
114118
## named LAN CIDR(s) — bind the WS server off loopback and hand the
115119
## networks to its rebinding guard. None/empty = unchanged loopback-only.
116-
ws_bind_host = "0.0.0.0" if allow_host_networks else "127.0.0.1" # noqa: S104
120+
ws_bind_host = bind_host_for_networks(allow_host_networks) or "127.0.0.1"
117121

118122
# Capture ws_port in the lifespan closure
119123
@asynccontextmanager

src/godot_ai/transport/origin_guard.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,24 @@ def parse_allow_hosts(values: Iterable[str]) -> list[IPNetwork]:
112112
return networks
113113

114114

115+
def bind_host_for_networks(networks: Sequence[IPNetwork] | None) -> str | None:
116+
"""Bind address that exposes the transports to ``networks`` (issue #421).
117+
118+
Returns ``None`` when no networks are named so callers keep their
119+
loopback default (the byte-for-byte unchanged path). Otherwise returns
120+
``"::"`` when any requested network is IPv6 — on a dual-stack host that
121+
also accepts IPv4 — and ``"0.0.0.0"`` for an IPv4-only allowlist, so an
122+
IPv6 ``--allow-host`` actually listens on IPv6 instead of silently
123+
binding IPv4-only. Centralized so the HTTP bind, the WebSocket bind, and
124+
the reload runner can't disagree about where to listen.
125+
"""
126+
if not networks:
127+
return None
128+
if any(isinstance(net, ipaddress.IPv6Network) for net in networks):
129+
return "::" # noqa: S104 — opt-in, and the guard still gates every request
130+
return "0.0.0.0" # noqa: S104 — same
131+
132+
115133
def _host_ip_in_networks(host_header: str, networks: Sequence[IPNetwork] | None) -> bool:
116134
"""Whether the Host header's IP literal falls inside one of ``networks``.
117135

tests/unit/test_origin_guard.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
from godot_ai.transport.origin_guard import (
1515
LocalhostOnlyHTTPMiddleware,
16+
bind_host_for_networks,
1617
evaluate_loopback,
1718
is_allowed_host,
1819
is_allowed_origin,
@@ -566,3 +567,20 @@ async def test_middleware_rejects_lan_host_without_opt_in() -> None:
566567
)
567568
assert inner_called is False
568569
assert sent[0]["status"] == 403
570+
571+
572+
def test_bind_host_for_networks_none_keeps_loopback_default() -> None:
573+
# No opt-in → None so callers keep their loopback default.
574+
assert bind_host_for_networks(None) is None
575+
assert bind_host_for_networks([]) is None
576+
577+
578+
def test_bind_host_for_networks_ipv4_only() -> None:
579+
assert bind_host_for_networks(parse_allow_hosts(["192.168.1.0/24"])) == "0.0.0.0"
580+
581+
582+
def test_bind_host_for_networks_ipv6_present_binds_dual_stack() -> None:
583+
# An IPv6 allowlist must actually listen on IPv6, not silently bind v4-only.
584+
assert bind_host_for_networks(parse_allow_hosts(["fd00::/8"])) == "::"
585+
# Mixed families also bind "::" (dual-stack accepts v4-mapped on Linux).
586+
assert bind_host_for_networks(parse_allow_hosts(["192.168.1.0/24", "fd00::/8"])) == "::"

0 commit comments

Comments
 (0)