relay: client-IP extraction helper for HTTP requests (#51)

[ilmoniemi]

What

Adds relay.ClientIP(r *http.Request, trustForwardedFor bool) string — a pure helper that extracts the source IP from an HTTP request.

• trustForwardedFor=false: returns the host portion of r.RemoteAddr (port stripped via net.SplitHostPort). XFF is ignored even when present.
• trustForwardedFor=true: returns the left-most entry of X-Forwarded-For (whitespace-trimmed). Falls back to r.RemoteAddr when the header is absent, empty, or has only whitespace before the first comma.
• Returns "" only when no usable source is available (SplitHostPort failed and either XFF was not consulted or yielded nothing). Callers MUST treat "" as deny — the rate-limit wiring ticket enforces this.
• No canonicalisation: zone-id and IPv4-in-IPv6 forms pass through verbatim per the docstring.

Issue

Closes #51 (per the AC, the wiring change to cmd/pyrycode-relay/main.go is explicitly out of scope for this ticket).

Testing

New internal/relay/client_ip_test.go (package relay) with a table-driven test that exercises every row from the architecture spec:

• RemoteAddr IPv4 with port, IPv6 bracketed, IPv6 loopback, malformed (no port), empty
• XFF_Disabled_HeaderIgnored
• XFF_Enabled single / multi-entry / leading whitespace
• XFF absent / empty / whitespace-only first entry → fallback to RemoteAddr
• Both XFF and RemoteAddr unusable → ""

go test -race ./..., go vet ./..., go build ./cmd/... all clean.

Architecture compliance

• Single exported pure function in internal/relay/, matching the spec's "Single exported function" section and the convention established by healthz.go (short doc-comment, no init state).
• net.SplitHostPort (not manual splitting) per the spec and the issue's technical notes.
• strings.IndexByte rather than strings.SplitN — avoids the slice allocation per the spec.
• r.Header.Get (single-header semantics); multi-header XFF deferred to the wiring ticket per § Open questions.
• Tests live in package relay (not relay_test) per repo convention.
• Pre-existing unexported remoteHost helper in server_endpoint.go is left untouched, as the spec mandates (logging vs rate-limit contracts diverge).
• Security review's adversarial walk: docstring carries the log-quoting note (finding 5, SHOULD FIX deferred to wiring ticket).

🤖 Generated with Claude Code

[ilmoniemi]

Code Review: #51

Decision: PASS

Findings

None.

Summary

Faithful implementation of the architect's spec — the production code is essentially the docstring-and-body block from the spec verbatim, and the test table maps 1:1 to the spec's required-cases table (all 13 rows present, names match).

Verified locally:

• go test -race ./internal/relay/ -run TestClientIP -v — all 13 subtests PASS
• go vet ./... — clean
• No callers of ClientIP exist (per AC; wiring belongs to the rate-limit ticket)
• Existing unexported remoteHost helper in server_endpoint.go:131-136 left untouched, as the spec mandates

Security-sensitive checks (label-gated):

• Architect security-review section present in docs/specs/architecture/51-client-ip-helper.md with PASS verdict and a thorough adversarial walk (8 scenarios)
• Helper handles no tokens/secrets; no file I/O; no subprocess; no crypto; no bare ListenAndServe
• No // #nosec annotations
• Security review's SHOULD-FIX-deferred finding (log-quoting, item 5) is honored in client_ip.go:30-31 via the docstring's log-injection note — the belt-and-suspenders move the spec suggested
• Empty-string deny semantics matched: net.SplitHostPort failure → "", XFF-only-whitespace → fallback → possibly "". Both paths covered by tests (RemoteAddr_MalformedNoPort, RemoteAddr_Empty, XFF_Enabled_RemoteAddrAlsoMalformed_ReturnsEmpty)

Spec-implementation parity spot-checks:

• client_ip.go:34 — r.Header.Get (not Header.Values), as specified for single-header-only XFF
• client_ip.go:35 — strings.IndexByte (not SplitN), avoids slice allocation per spec
• client_ip.go:38 — TrimSpace on the comma-stripped value, enabling the whitespace-only-first-entry fallback row
• client_ip.go:43 — net.SplitHostPort for both IPv4 host:port and bracketed [ipv6]:port, verified by RemoteAddr_IPv4_WithPort / RemoteAddr_IPv6_Bracketed / RemoteAddr_Loopback_IPv6

CI: test job green; security job still in progress at review time — non-blocking given the audit above, but the dispatcher should still see it green before merge.