Skip to content

fix(client-ip): align client identity resolution with the reference - #86

Merged
rennf93 merged 1 commit into
masterfrom
fix/client-ip-parity
Oct 15, 2026
Merged

rennf93 merged 1 commit into
masterfrom
fix/client-ip-parity

Conversation

@rennf93

@rennf93 rennf93 commented Oct 15, 2026

Copy link
Copy Markdown
Member

Closes #85

I aligned the client identity the engine resolves with the reference (guard_core/_utils/ip_extraction.py and _ip_in_list). I generated 104 extraction rows and 78 list-matching rows by running the reference functions directly (IPv4-mapped, bracketed and zoned addresses, ports, X-Forwarded-For chains with trusted proxies and depth, untrusted peers, the unix token, malformed entries) and replayed them against the port in the new bin/test_client_ip.php (tables in tests/data/).

What differed on master

26 extraction rows failed. The IPv4 blacklist case from the TypeScript report already worked here (the checks read the canonical identity), so the test pins it. The differences were in the resolver:

  • X-Forwarded-For entries kept their port, empty entries were dropped (which changes the entry the depth selects), and a depth that over-counts the proxy hops did not fall back to the right to left walk.
  • Zoned addresses were not parsed (fe80::1%eth0) and a mapped zoned address rendered in hex instead of the reference form.
  • A mapped peer inside an IPv4 CIDR in trusted_proxies was trusted, and a malformed CIDR reached before a match was skipped. The reference trusts neither.
  • trusted_proxies validation rejected the unix token, so a request with no peer address could not resolve through the chain. Entries now keep the reference spelling (no IPv4-mapped collapse).
  • An untrusted peer sending X-Forwarded-For raised no suspicious_request / spoofing_detected event (handler ip_extraction, canonical peer, redacted chain) and no spoof log, and none of the once-only chain warnings were logged. The route_config check now passes its logger and the event bus to the resolver, and EventBus::sendRequestHandlerEvent is the reference send_agent_event shape.
  • List matching compared a bare entry to the parsed client without canonicalizing the entry, so mapped and bracketed entries and zoned clients did not match. CanonicalIp::ipInList is the reference _ip_in_list. The block-hook payload fallback canonicalizes the raw peer like the reference.

Tests

bin/test_client_ip.php (454 checks, added to the CI runners, the coverage suites and the Makefile) replays both tables, stores the engine identity for each row, and adds engine-level tests: an IPv4 blacklist entry, an IPv4 CIDR and a mapped entry block a client that connects as ::ffff:127.0.0.1; a whitelist CIDR admits and denies mapped clients; a ban stored for the IPv4 form blocks the mapped form; the emergency whitelist admits a mapped client; rate-limit keys are shared by both spellings; a forwarded identity from a trusted proxy and the unix chain reach the checks; a spoofed header from an untrusted peer reaches no check and is reported once. On master the runner reports 26 failing extraction rows, the unix configuration is rejected and the spoof event is missing. One existing assertion in bin/test_state.php pinned the hex rendering of a zoned mapped address and now expects the reference form ::ffff:1.2.3.4%eth0 (checked on Python 3.10 to 3.14).

Verification

PHP 8.5.11 on the host (CI runs 8.2 to 8.4): php -l over src and bin, composer validate --strict, every CI runner (with a private redis-server on a free port, never the shared 6379), the pcov 100% line gate, and composer audit --locked all exit 0. bin/test_support.php needs memory_limit=-1 on this host (it exhausts the 128M default on master too). The coverage waivers move with identical code: SecurityConfig.php 1162 and 1186 to 1171 and 1195, CanonicalIp.php 56 and 108 to 154 and 248 plus 219 (the same inet_pton-after-FILTER_VALIDATE_IP proof), ClientIpResolver.php swaps the six unix lines (now reachable and covered) for one dead trailing return (212, proof in the file). Waived lines go from 87 to 83.

The forwarded chain follows the reference rules (ports and brackets stripped, empty entries kept, right to left walk when the depth over-counts hops, metacharacter and invalid entries rejected), the unix token is accepted by trusted_proxies, an untrusted peer sending X-Forwarded-For is reported as a spoof attempt, trusted proxy matching follows the reference raw-address rules, zoned addresses parse and render like ipaddress, and list matching compares canonical forms. Adds the generated reference expectation tables as a test runner.
@github-actions github-actions Bot added area: pipeline Touches src/Pipeline/ (checks and the pipeline) area: engine Touches src/Engine/, src/Config/, or src/Routing/ area: request Touches src/Request/ or src/Ip/ (request model, client IP, canonicalization) tests Test suite changes ci GitHub Actions workflows labels Oct 15, 2026
@rennf93
rennf93 merged commit aa49d17 into master Oct 15, 2026
27 checks passed
@rennf93
rennf93 deleted the fix/client-ip-parity branch October 15, 2026 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: engine Touches src/Engine/, src/Config/, or src/Routing/ area: pipeline Touches src/Pipeline/ (checks and the pipeline) area: request Touches src/Request/ or src/Ip/ (request model, client IP, canonicalization) ci GitHub Actions workflows tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Client IP resolution differs from the reference for forwarded chains, the unix token and spoof reporting

1 participant