harden(transport): gate --allow-host on the real socket peer, not the Host header#544
Merged
Conversation
… 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>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the LAN opt-in (--allow-host) for the HTTP transport by ensuring authorization is keyed on the real socket peer address (ASGI scope["client"] / WebSocket remote_address) rather than the client-controlled Host header, closing a spoofing bypass described in GHSA-pp3p-fhg5-f429.
Changes:
- Add
peer_ip_allowed()and use it as the authoritative gate when--allow-hostis enabled. - Update
evaluate_loopback()and the ASGI middleware to incorporate the peer-address gate before existing Host/Origin/Sec-Fetch-Site checks. - Expand unit tests to cover spoofed Host from a foreign peer, missing peer fail-closed behavior, and IPv6 zone-id handling.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/godot_ai/transport/origin_guard.py |
Introduces peer-based allowlist gating and threads peer IP through both HTTP middleware and the shared allowlist evaluator. |
tests/unit/test_origin_guard.py |
Updates middleware harness to provide ASGI scope["client"] and adds coverage for peer-gating scenarios. |
src/godot_ai/__init__.py |
Updates --allow-host CLI help text to reflect peer-address-based authorization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+193
to
+199
| try: | ||
| ip = ipaddress.ip_address(candidate) | ||
| except ValueError: | ||
| return False | ||
| if ip.is_loopback: | ||
| return True | ||
| return any(ip in net for net in allowed_networks) |
Comment on lines
+706
to
+710
| def test_peer_ip_allowed_loopback_always_ok_when_opted_in() -> None: | ||
| nets = parse_allow_hosts(["192.168.1.0/24"]) | ||
| assert peer_ip_allowed("127.0.0.1", nets) is True | ||
| assert peer_ip_allowed("::1", nets) is True | ||
|
|
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>
Comment on lines
+44
to
48
| peer gate is a pass-through and behavior is byte-for-byte unchanged. | ||
|
|
||
| See umbrella #343, finding #1 (audit-v2). | ||
| """ | ||
|
|
Comment on lines
+407
to
+411
| ## ``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. |
…string 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Wave-1 hardening (advisory
GHSA-pp3p-fhg5-f429). When--allow-hostwidens access to a LAN range, the HTTP transport binds off loopback and authorization previously rested entirely onis_allowed_host, which range-checks the client-controlledHostheader. A peer outside the allowed range could pass simply by spoofingHost: <an-allowed-ip>— the real socket peer was never consulted.Fix
peer_ip_allowed()— the authoritative gate keyed on the unforgeable socket peer (scope["client"]for ASGI,remote_addressfor the WebSocket server). With an allowlist set, the peer must be loopback or inside an allowed network.evaluate_loopback()now ANDs the peer gate ahead of the existing Host/Origin/Sec-Fetch-Site checks.Hostis kept as a secondary range filter but is no longer the authority.Compatibility
--allow-host) is a pass-through — the kernel bind already guarantees a loopback peer, so behavior there is byte-for-byte unchanged.Tests
peer_ip_allowedunit block (pass-through, in/out-of-network, loopback, fail-closed-on-unknown, unparseable, IPv6 zone-id strip).evaluate_loopbackcases: spoofed in-rangeHostfrom a foreign peer → 403; opted-in with unknown peer → 403; loopback peer on a LAN-bound server → 200.Part of the pre-release Wave-1 hardening set.