feat(api): trusted_proxies + trust_forwarded_for for real-client-IP resolution by neo-wanderer · Pull Request #4534 · librefang/librefang

neo-wanderer · 2026-05-03T15:00:16Z

Summary

Adds two top-level KernelConfig fields that let proxied deployments (Cloudflare Tunnel, nginx, Traefik, …) recover the real client IP from forwarding headers — without being exploitable when no proxy is present. Closes the long-standing TODO referenced in the now-retired rate_limiter::resolve_client_ip doc comment, which deferred header trust pending exactly this allowlist.

trusted_proxies     = ["172.19.0.0/16", "127.0.0.1", "::1"]
trust_forwarded_for = true

When both are set and the TCP peer matches the allowlist, the daemon resolves the real client IP from forwarding headers (preference: CF-Connecting-IP → X-Real-IP → Forwarded (RFC 7239) → rightmost-untrusted hop in X-Forwarded-For) for:

the GCRA rate limiter (per-IP keying)
the auth-login rate limiter (per-IP keying)
the per-IP WebSocket connection cap + the WS connect log line (both agent_ws and terminal_ws)
the dashboard chat SenderContext.user_id (see Migration notes below)

Why

Today, librefang behind a reverse proxy (cloudflared, nginx, Traefik) sees every request originate from the proxy's address. Symptoms:

WS connect log shows client_ip=<proxy> for every browser
The per-IP WS connection cap collapses every browser onto a single shared slot
Brute-force auth limits collapse to "one bucket per proxy" instead of "one bucket per browser"

The existing detect_connection_locality helper in ws.rs parsed the forwarding headers but only used the result for a debug log — it never substituted the real IP for keying. Header trust was previously documented as deliberately removed (see the resolve_client_ip doc comment) because trusting headers without a verified upstream is exploitable: any internet client could rotate X-Forwarded-For per request and defeat the per-IP limiter (the counter never advances past 1 for any unique forged IP).

This PR is the gating mechanism that comment was waiting for.

How it stays safe

Fail-closed by default. Either flag missing → behaviour unchanged. Specifically:

trust_forwarded_for = false (default) → headers ignored, TCP peer used.
trusted_proxies = [] (default) → headers ignored regardless of master switch.
TCP peer not in trusted_proxies → headers ignored. A spoofed X-Forwarded-For from the open internet still hits the limiter on its real source.
All headers malformed → falls back to TCP peer.
X-Forwarded-For walked right-to-left, dropping hops that match trusted_proxies and stopping at the first non-matching value — a malicious leftmost hop cannot poison the result.
Single-value headers (CF-Connecting-IP, X-Real-IP) reject the unspecified address (0.0.0.0, ::); a misconfigured proxy emitting those values cannot poison the slot key with an address that can't be a real client. Loopback / private addresses are accepted by design — the trusted proxy may legitimately pass an internal-network client.
Forwarded for= parameter and the unknown token are matched case-insensitively per RFC 7230 §3.2.4.

Auth-bypass loopback gates intentionally still key on addr.ip(), not the resolved IP — LIBREFANG_ALLOW_NO_AUTH and the no-auth + loopback auto-allow path must never follow a forwarded-header claim. Only rate-limiter keying and the WS slot key adopt the resolved IP.

Implementation

New module crates/librefang-api/src/client_ip.rs with:
- TrustedProxies::compile(&[String]) — hand-rolled CIDR matcher (no new deps), accepts CIDRs ("172.19.0.0/16", "2001:db8::/32") or bare IPs ("127.0.0.1", "::1"); invalid entries are warn-logged and skipped so a single typo can't take down boot.
- resolve_real_client_ip(peer, headers, trusted, trust_flag) — gated header parsing.
- resolve_from_request(req, trusted, trust_flag) — convenience wrapper for axum middleware.
The compiled Arc<TrustedProxies> + trust_forwarded_for flag now live on AppState, built once at boot in server.rs and shared with every consumer:
- GcraState and AuthRateLimitState middlewares
- ws::agent_ws (per-IP WS slot + client_ip log + SenderContext.user_id)
- routes::terminal::terminal_ws (per-IP WS slot + log)
ws.rs::agent_ws uses the resolved IP for both the WS slot key and the client_ip log field.
routes::terminal::terminal_ws mirrors that — was previously keying on addr.ip(), which collapsed every terminal behind cloudflared onto one shared slot.
Old fail-closed-only rate_limiter::resolve_client_ip removed; module-level note points at the new gating.

Test plan

27 unit tests in client_ip: CIDR v4/v6, bare IPs, unmasked input (e.g. 10.0.0.5/8 → still matches 10.0.0.0/8), invalid-entry skip, all four header preferences, RFC 7239 bracketed v6 + obfuscated _token form, multi-header XFF concatenation, v4:port suffix, bracketed [v6]:port and bare [v6] in XFF, case-insensitive for= parameter (4 variants), case-insensitive unknown token (3 variants), single-value-header unspecified-address rejection (4 variants) + loopback/private acceptance (5 variants), malformed-fallback, all-trusted chain, WS-slot-key spoof regression (untrusted peer rotating XFF collapses to one key) + WS-slot-key separation (two browsers behind the same trusted proxy get distinct keys).
3 integration tests in rate_limiter:
- browsers behind a trusted proxy get independent buckets (XFF respected)
- rotating XFF from an untrusted peer does not bypass the limiter (spoof regression)
- master switch off ignores XFF even from a trusted peer
Existing librefang-api lib tests (619) + integration tests still pass
cargo clippy --workspace --all-targets -- -D warnings clean
cargo fmt --check clean

Docs

CHANGELOG.md [Unreleased] ### Added updated.
docs/src/app/configuration/core/page.mdx top-level fields table extended (English).
docs/src/app/zh/configuration/core/page.mdx Chinese mirror translated — would appreciate a native-speaker review on the wording, since this is a security-sensitive feature and the nuance around "fail-closed" and "rightmost-untrusted hop" matters.

Migration notes for operators

No-op for existing deployments. Both flags default to off; the TCP peer is still used everywhere unless the operator explicitly opts in.
Behind a single proxy (cloudflared, nginx with proxy_set_header X-Forwarded-For, etc.): set trusted_proxies to the proxy's source CIDR (e.g. the docker bridge subnet, or 127.0.0.1 if the proxy is on the host) and trust_forwarded_for = true. Restart.
Direct LAN/internet access alongside a proxy is the threat model the allowlist defends against, so be careful: if the daemon is reachable on 0.0.0.0:4545 directly, an attacker can bypass the proxy entirely. The recommended hardening is to bind api_listen to loopback (already the new install default per Windows automatically initializes after installation #2766) and force public traffic through the trusted proxy.

Behaviour change when both flags flip on

When operators opt in by setting both flags, the dashboard WebSocket
chat (agent_ws) constructs SenderContext.user_id from the resolved
real client IP instead of the proxy's TCP peer address. Any kernel-side
state that keys on user_id therefore re-keys at that moment:

Audit attribution. Audit-log rows that record the requesting user_id
switch from the proxy IP to the real browser IP. New rows in the hash chain
carry the new keying — old rows are unaffected (the chain is append-only).
Channel-sender keying. The dashboard chat publishes through the same
SenderContext plumbing the channel bridges use. Code paths that key
per-user_id continuity off the dashboard chat will see the new IPs as
fresh senders. The persistent dashboard session itself is unaffected
(it uses use_canonical_session: true, which keys on the agent's
canonical session ID, not on user_id).
Per-user_id rate limits / quotas (if you've added any operator-side
quotas keyed on it) similarly re-key.

terminal_ws does not construct a SenderContext, so the only behaviour
change there is the per-IP WS slot key + the connect/reject log line.

If your environment relies on user_id == proxy_ip for any operational
process (alerting, dashboard rules, log filters), update those rules
before enabling the flags.

Adds two top-level KernelConfig fields that let proxied deployments (Cloudflare Tunnel, nginx, Traefik, …) recover the real client IP from forwarding headers without being exploitable when no proxy is present. Closes the long-standing TODO referenced in the now-retired rate_limiter::resolve_client_ip doc comment. trusted_proxies = ["172.19.0.0/16", "127.0.0.1", "::1"] trust_forwarded_for = true When BOTH are set AND the TCP peer matches the allowlist, the daemon resolves the real client IP from forwarding headers (preference: CF-Connecting-IP → X-Real-IP → Forwarded (RFC 7239) → rightmost- untrusted hop in X-Forwarded-For) for: * the GCRA rate limiter (per-IP keying) * the auth-login rate limiter (per-IP keying) * the per-IP WebSocket connection cap + the WS connect log line Fail-closed by default — empty allowlist OR master-switch off OR peer not in allowlist OR malformed headers all collapse back to the TCP peer. A spoofed X-Forwarded-For from an untrusted internet client still hits the limiter on its real source, so the per-IP brute-force properties are preserved. Implementation * New crates/librefang-api/src/client_ip.rs with TrustedProxies (hand-rolled CIDR matcher, no new deps), resolve_real_client_ip, resolve_from_request. * GcraState + new AuthRateLimitState carry Arc<TrustedProxies> + trust_forwarded_for; server.rs compiles the allowlist once at boot and threads it through both middleware layers. * ws.rs::agent_ws uses the resolved IP for the WS slot key and the client_ip log field. * Old fail-closed-only rate_limiter::resolve_client_ip removed. * Auth-bypass loopback gates (no-auth-allow-loopback, LIBREFANG_ALLOW_NO_AUTH) intentionally still key on the TCP peer via addr.ip(), not the resolved IP — auth bypass must never follow a forwarded-header claim. Tests * 19 unit tests in client_ip: CIDR v4/v6, bare IPs, unmasked input, invalid-entry skip, all four header preferences, RFC 7239 bracketed v6 + obfuscated _token, multi-header XFF concatenation, v4:port suffix, malformed-fallback, all-trusted chain. * 3 integration tests in rate_limiter: - browsers behind a trusted proxy get independent buckets - rotating XFF from an untrusted peer does not bypass the limiter - master switch off ignores XFF even from a trusted peer * Existing 611 librefang-api lib + 271 integration tests still pass; workspace cargo clippy --all-targets -D warnings clean.

houko · 2026-05-03T23:12:23Z

Review

Overall: changes requested. The core design (allowlist + master switch, fail-closed by default, untrusted-peer headers ignored) is sound, the test coverage on client_ip.rs and the rate-limit middlewares is genuinely good, and the spoof-regression test is exactly the right thing to gate this change on. A few real defects below — most are missed call sites and parsing corner cases rather than broken core logic.

Findings

[high] crates/librefang-api/src/routes/terminal.rs:743 — terminal WS per-IP cap still keys on the proxy. terminal_ws does let ip = addr.ip(); and passes that into try_acquire_ws_slot, identical to the bug agent_ws was just fixed for. Behind cloudflared every browser opening a terminal collapses onto one slot and max_ws_per_ip traps the whole org on the proxy address. Same fix as agent_ws: compile TrustedProxies from cfg.trusted_proxies and call client_ip::resolve_real_client_ip(addr.ip(), &headers, &trusted, cfg.trust_forwarded_for) before try_acquire_ws_slot. The PR description claims the per-IP WS cap is fixed, but only the agent WS cap is — the terminal WS cap is the same threat model and is unchanged.
[medium] crates/librefang-api/src/ws.rs:457-465 — TrustedProxies::compile() is re-parsed on every WS upgrade. agent_ws calls TrustedProxies::compile(&cfg.trusted_proxies) per upgrade, walking the list and warn-logging on every malformed entry. The boot-time compiled allowlist is already threaded through GcraState / AuthRateLimitState; either (a) thread the same Arc<TrustedProxies> through AppState, or (b) cache it on AppState so the WS path reuses it. As written, a typo in trusted_proxies will spam tracing::warn! once per WS connection.
[medium] crates/librefang-api/src/client_ip.rs:233-271 — XFF :port stripping silently drops bracketed IPv6 with port. The fallback trimmed.rsplit_once(':") strategy works for 1.2.3.4:54321 but for [2001:db8::1]:1234 (rare but seen in the wild from some proxies) parse::<IpAddr> fails on the whole string AND on [2001:db8::1] (brackets are not valid in IpAddr::from_str), so the entry is dropped. Either strip surrounding [...] before retrying or explicitly document that bracketed v6-with-port in XFF is unsupported (RFC 7239 Forwarded already handles it correctly via parse_forwarded_for_param).
[medium] crates/librefang-api/src/client_ip.rs:280-285 — RFC 7239 for= parameter match is case-sensitive. Code only accepts for= and For= prefixes; FOR=, fOr=, etc. are silently skipped. RFC 7230 §3.2.4 makes parameter names case-insensitive. Minor real-world impact (most proxies emit lowercase) but trivial to fix by splitting on = once and comparing the key with eq_ignore_ascii_case("for").
[medium] crates/librefang-api/src/ws.rs:806, 1025 — behavior change unmentioned in the migration notes: SenderContext.user_id for dashboard WS messages flips from proxy IP to real client IP when these flags are enabled. handle_agent_ws stamps user_id: client_ip.to_string() into the kernel SenderContext, so operators who have already deployed behind a proxy and have per-user_id state on the kernel side (session continuity, audit attribution, anything keyed off the channel sender) will see that state effectively re-key when they turn the flags on. Worth calling out explicitly under "migration notes for operators."
[low] crates/librefang-api/src/client_ip.rs:184 — cf-connecting-ip and x-real-ip are not validated against trusted_proxies. Once the peer is trusted, whatever the proxy claims wins, including a private/loopback/unspecified address. That matches the way most reverse-proxy frameworks behave and the threat model assumes a non-malicious trusted proxy, but a defensive sanity-check (reject unspecified 0.0.0.0 / ::) would be cheap insurance against a misconfigured upstream. At minimum the module doc should be explicit that the trusted proxy is fully authoritative for these single-value headers.
[low] crates/librefang-api/src/client_ip.rs:226-231 — single_ip_header only reads the first header instance. HeaderMap::get returns the first value for a name. If a proxy chain emits two X-Real-IP headers (RFC says single-value, but multiple values do appear in the wild), the leftmost wins regardless of which is more authoritative. Probably fine — flagging only because the XFF path goes out of its way to handle get_all and the asymmetry is undocumented.
[low] crates/librefang-api/src/client_ip.rs:289 — RFC 7239 obfuscated-token short-circuits the chain. With Forwarded: for=_obf, for=192.0.2.1 (RFC-legal, multiple list elements) the second element is never inspected because the parser only looks at the first list element. Defensible by design but worth a comment.
[nit] crates/librefang-api/src/server.rs:1219 — drop(proxy_cfg) immediately after copying scalar fields is theatrical. If it is required because something below re-locks the kernel config, a one-line comment explaining the deadlock avoided would help; otherwise drop the drop.
[nit] crates/librefang-api/src/client_ip.rs:289 — unquoted == "unknown" does not match the for=Unknown casing. RFC 7239 says the unknown identifier is case-insensitive; use eq_ignore_ascii_case.

What I liked

The spoof regression auth_rate_limit_ignores_xff_from_untrusted_peer is exactly the test that needs to exist for this feature, and the assertion is sharp. The "master switch off" test similarly nails the kill-switch contract. The doc comments in rate_limiter.rs explaining why the auth-bypass loopback gates intentionally still key on addr.ip() are the kind of thing future contributors will thank you for, and the right-to-left XFF walk + boot-time compile both reflect mature thinking about this class of feature.

Threads `Arc<TrustedProxies>` + `trust_forwarded_for` through `AppState`, compiled once at boot in `server.rs`. The GCRA + auth-login middlewares now read from the cached instance instead of recompiling, and `ws::agent_ws` no longer re-parses the raw config strings (and re-emits the malformed-entry warning) on every WebSocket upgrade. Also adds explicit "Behaviour change" comments at the two `agent_ws` sites that propagate the resolved client IP into `SenderContext.user_id`, so a future reader / operator flipping the new flags on understands that any per-`user_id` kernel state (audit attribution, channel-sender keying, session continuity that keys on it) re-keys from proxy IP to real client IP at that moment. Pure documentation — no behaviour change vs the prior PR commit. Updates the test fixtures (`librefang-testing::test_app::build_state` and the three in-tree `AppState` init sites in `routes/{agents,memory,network}.rs`) to set the new fields to default (header trust off), matching the production default. Addresses review #2 (no per-upgrade recompilation), #5 (migration-notes comments), and #10 (drops the cosmetic `drop(proxy_cfg)` since the allowlist is now read straight off `AppState`).

`terminal_ws` was still keying its per-IP WS slot on `addr.ip()`, which is the same bug `agent_ws` was just fixed for: behind a trusted reverse proxy (cloudflared / nginx / Traefik), the TCP peer is the proxy and every terminal connection from every browser collapses onto a single shared slot — `max_ws_per_ip` then throttles the whole organisation the moment the second tab opens. Mirrors the `agent_ws` fix exactly: pulls the boot-compiled `Arc<TrustedProxies>` and `trust_forwarded_for` flag off `AppState` and calls `client_ip::resolve_real_client_ip` before `try_acquire_ws_slot`. Untrusted peers fall through to `addr.ip()` — a spoofed `X-Forwarded-For` from the open internet still hits the per-IP cap on its real source. `SenderContext.user_id` parity is N/A here: `terminal_ws` does not construct a `SenderContext` (the terminal is a PTY pipe, not a kernel agent message), so the only behavioural surface that needs to swap to the resolved IP is the WS slot key + the rejection log line. Addresses review #1.

Five header-parsing fixes lifted from review: - **XFF: bracketed IPv6 with port** — `[2001:db8::1]:1234` fell off the fallback parser because `parse::<IpAddr>` rejects the bracket form and the prior `rsplit_once(':')` left the brackets attached. The walker now strips a surrounding `[...]` (with or without `:port`) before retry, while still bailing on ambiguous unbracketed v6 + port rather than silently truncating. - **RFC 7239 `for=` parameter case-insensitive** — RFC 7230 §3.2.4 makes parameter names case-insensitive; we were only matching `for=` and `For=`. Now compares the key with `eq_ignore_ascii_case("for")`. - **RFC 7239 `unknown` token case-insensitive** — companion fix; now compares with `eq_ignore_ascii_case("unknown")` so `Unknown` / `UNKNOWN` short-circuit the same way as the lowercase form. - **Single-value headers reject the unspecified address** — a misconfigured proxy that injects `0.0.0.0` / `::` into `cf-connecting-ip` / `x-real-ip` no longer poisons the per-IP slot key with an address that can't be a real client. Loopback and RFC1918/ULA addresses are accepted by design — the trusted proxy may legitimately pass an internal-network client. Doc-comment spells out the policy. - **Documentation comments** for the asymmetries reviewers flagged: `single_ip_header` reads only the first instance of the named header (vs the XFF path which concatenates all); the `Forwarded` parser only inspects the first list element and obfuscated tokens short-circuit by design. Adds 8 new unit tests covering: bracketed-v6 with/without port, case-insensitive `for=` (4 variants), case-insensitive `unknown` (3 variants), unspecified-address rejection on both single-value headers (4 variants), loopback + private acceptance (5 variants), plus two WS-slot-key composition tests asserting the resolver collapses spoof attempts from an untrusted peer down to the real TCP source AND separates real clients behind the same trusted proxy into distinct slot keys. Addresses review #3, #4, #6, #7, #8, #9.

neo-wanderer · 2026-05-04T06:39:55Z

@houko thanks — addressed everything. Pushed three commits on top of the
existing branch (da86af81 → 76b78e74 → b02417cb).

#	Finding	Status	Fix
1	terminal_ws missing real-IP resolution	fixed	`routes/terminal.rs::terminal_ws` now reads `Arc<TrustedProxies>` + `trust_forwarded_for` off `AppState` and calls `client_ip::resolve_real_client_ip` before `try_acquire_ws_slot`. Parity note: terminal_ws does not construct a `SenderContext`, so the slot key + connect/reject log are the only behavioural surfaces — there's no `user_id` to stamp here. (The 2 new `ws_slot_key_*` regression tests in `client_ip` cover the underlying composition for both handlers.)
2	cache compiled `TrustedProxies` on `AppState`	fixed	Added `pub trusted_proxies: Arc<TrustedProxies>` + `pub trust_forwarded_for: bool` on `AppState`, compiled once in `server.rs` (before the `AppState` constructor so the in-tree test fixtures and middlewares all share one Arc). `agent_ws` and `terminal_ws` now read it from there — no more per-upgrade `TrustedProxies::compile` walk and no more re-emitted malformed-entry warnings. The `GcraState`/`AuthRateLimitState` middlewares were already on the same instance; they just no longer re-grab it from `config_ref()` in a separate block.
3	XFF parse: bracketed IPv6 with port	fixed	`parse_xff_rightmost_untrusted` now strips a surrounding `[...]` (with or without `:port`) before retry. Bare unbracketed v6 like `2001:db8::1` continues to bare-parse first. Bails on ambiguous unbracketed `v6 + port` rather than silently truncating. New `xff_bracketed_v6_with_port_parses` and `xff_bracketed_v6_no_port_parses` tests cover it.
4	RFC 7239 `for=` parameter case-insensitive	fixed	Now splits the param on `=` once and compares the key with `eq_ignore_ascii_case("for")`. New `forwarded_rfc7239_for_param_case_insensitive` test covers `FOR=`, `fOr=`, `FoR=`, and `by=...;FOR=...`.
5	Migration note: `SenderContext.user_id` change	fixed	Added explicit `// Behaviour change (...)` comments at both `agent_ws` sites that propagate `client_ip` (the message-handler call site and the `SenderContext.user_id` field). Updated the PR body with a new "Behaviour change when both flags flip on" subsection under Migration notes covering audit attribution, channel-sender keying, and per-`user_id` rate limits / quotas, plus the explicit note that the persistent dashboard session is unaffected (it keys on the canonical session ID, not on `user_id`) and that `terminal_ws` is N/A. No code behaviour change in this PR — just docs / inline notes.
6	Defensive sanity check on single-value headers	fixed	`single_ip_header` now rejects the unspecified address (`0.0.0.0` / `::`). Loopback / RFC1918 / ULA accepted by design (the trusted proxy may pass an internal client). Module doc updated to spell out the policy. New `single_value_header_rejects_unspecified` (4 variants) and `single_value_header_accepts_loopback_and_private` (5 variants) tests.
7	`single_ip_header` only first instance — document	fixed	One-line comment added at `HeaderMap::get` explaining the asymmetry vs the XFF path and that single-value headers are by-spec single-value (Cloudflare docs / RFC).
8	RFC 7239 obfuscated-token short-circuits chain — document	fixed	`// NOTE:` comment at the obfuscated-/unknown-token check spelling out that we deliberately don't peek past it (the proxy redacted the identity intentionally) and callers fall through to XFF instead.
9	Case-insensitive `unknown` token	fixed	Now `eq_ignore_ascii_case("unknown")`. `forwarded_rfc7239_unknown_token_case_insensitive` covers `unknown` / `Unknown` / `UNKNOWN`.
10	`drop(proxy_cfg)` theatrics	fixed (deleted)	Investigated: not load-bearing — `kernel.config_ref()` returns an `arc_swap::Guard`, which is a reference holder, not a lock guard, so there's no deadlock risk. The whole block that needed it is gone too: server.rs now reads `state.trusted_proxies.clone()` / `state.trust_forwarded_for` straight off `AppState`, so no temporary Guard binding exists.

Test results: client_ip::tests 27 pass (up from 19), rate_limiter::tests
26 pass (incl. all three spoof regressions), librefang-api lib 619 +
integration suites all green. cargo clippy --workspace --all-targets -- -D warnings clean.

Side-note on the AppState-threaded TrustedProxies change: it touched
four other AppState init sites (the in-tree fixtures in
routes/{agents,memory,network}.rs and librefang-testing::test_app::build_state).
All four set trusted_proxies = TrustedProxies::default() and
trust_forwarded_for = false, matching the production default — i.e.
test fixtures unaffected behaviourally.

houko · 2026-05-04T06:48:25Z

Re-review after da86af81 → 76b78e74 → b02417cb — all 10 prior findings resolved cleanly. Approving with one new [low] observation.

Spot-checks of the resolution patches:

#	Item	Verified
1	`terminal_ws` real-IP resolution	`routes/terminal.rs:743` reads `state.trusted_proxies` + `state.trust_forwarded_for` and calls `resolve_real_client_ip` before `try_acquire_ws_slot` ✓
2	`TrustedProxies` cached on `AppState`	`server.rs:1219-1230` compiles once + caches; `agent_ws` and `terminal_ws` both consume from `AppState`, no per-upgrade re-parse ✓
3	XFF bracketed v6 + port	`client_ip.rs:281-296` strips `[...]` then optional `:port`; bails on multi-colon ambiguity (so `2001:db8::1` bare-parses without truncation, `[2001:db8::1]:1234` strips correctly). Tests `xff_bracketed_v6_with_port_parses` / `_no_port_parses` cover ✓
4	`for=` case-insensitive	`client_ip.rs:325-330` does `split_once('=') → eq_ignore_ascii_case("for")`. Test covers FOR/fOr/FoR + interleaved with `by=` ✓
5	Migration note for `SenderContext.user_id` behaviour change	Two inline `// Behaviour change` comments in `ws.rs:800-810, 1035-1045` plus the new "Behaviour change when both flags flip on" subsection in the PR body covering audit attribution, channel-sender keying, per-`user_id` quotas, and the explicit "persistent dashboard session unaffected (`use_canonical_session: true` keys on canonical session ID)" callout ✓
6	`single_ip_header` rejects unspecified	`client_ip.rs:250` `is_unspecified()` guard, with comment that loopback / RFC1918 / ULA are accepted by design. Tests `single_value_header_rejects_unspecified` (4) + `_accepts_loopback_and_private` (5) ✓
7	Asymmetry vs XFF (`get` vs `get_all`) documented	Comment at `client_ip.rs:235-240` ✓
8	Obfuscated short-circuit documented	`// NOTE:` comment at `client_ip.rs:334-339` ✓
9	`unknown` token case-insensitive	`eq_ignore_ascii_case("unknown")` at `client_ip.rs:340`. Test covers `unknown` / `Unknown` / `UNKNOWN` ✓
10	Theatrical `drop()` removed	Confirmed gone from `server.rs` — `state.trusted_proxies.clone() / state.trust_forwarded_for` reads off `AppState` directly, no Guard binding to drop ✓

The 27 unit tests + 3 integration tests give strong coverage; auth_rate_limit_keys_on_forwarded_ip_when_proxy_trusted and the spoof regression are both pointed at the right invariants.

One new observation

[low] crates/librefang-api/src/server.rs:1219-1230 and routes/mod.rs:181-187 — trusted_proxies (compiled allowlist) and trust_forwarded_for are snapshotted at boot onto AppState. KernelConfig is held in arc_swap and other call sites in this file (e.g. state.kernel.config_ref().rate_limit.max_ws_per_ip in ws.rs:464) read from it per-request, so they pick up librefang config reload automatically. These two fields will not — operators flipping trust_forwarded_for from off → on, or extending trusted_proxies, won't take effect until the daemon restarts. The compiled allowlist is genuinely expensive to rebuild per upgrade and worth caching, but the inconsistency with max_ws_per_ip deserves either (a) a // CACHED AT BOOT — not hot-reloadable comment on the two AppState fields, or (b) wrapping TrustedProxies in arc_swap::ArcSwap and rebuilding on config notify if you have a hook. Not a blocker; just don't want operators to be surprised when their reload doesn't take.

Nits — no action needed

CI shows OpenAPI Drift fail, log is the same git fetch ... exit code 1 retry-then-die infra flake hitting the other PR (feat(llm-drivers): surface caller IDs as x-librefang-* headers #4548) right now. Re-running the workflow should clear it; nothing actually drifted (the diff doesn't touch any route schema).
Minor: parse_forwarded_for_param could in principle see for = 1.2.3.4 with surrounding OWS around = (RFC 7230 §3.2.4 BWS rule) — split_once('=') then eq_ignore_ascii_case("for") won't match because of the trailing space. In practice no real proxy emits that shape; just flagging.

Re-confirmed clean

Auth-bypass loopback path still keys on addr.ip() (the LIBREFANG_ALLOW_NO_AUTH and no-auth + loopback gates), exactly as the prior round called out — verified in rate_limiter.rs doc comments. The PR is consistent with that intentional split.
Spoof regression auth_rate_limit_ignores_xff_from_untrusted_peer and ws_slot_key_resists_spoof_from_untrusted_peer are both correctly asserting on collapse-to-peer rather than leak-through.
No new dependencies — the CIDR matcher is hand-rolled.

Nice tightening on this round. LGTM modulo the hot-reload note. Happy to approve once CI clears.

github-actions Bot added size/L 250-999 lines changed area/docs Documentation and guides labels May 3, 2026

houko added 6 commits May 4, 2026 00:06

Merge branch 'main' into trusted-proxies

9e1b128

Merge branch 'main' into trusted-proxies

9c76d89

Merge branch 'main' into trusted-proxies

d73fe59

Merge branch 'main' into trusted-proxies

a613f45

Merge branch 'main' into trusted-proxies

931b580

Merge branch 'main' into trusted-proxies

78f1770

houko and others added 5 commits May 4, 2026 08:45

Merge branch 'main' into trusted-proxies

baac5f4

Merge branch 'main' into trusted-proxies

73f5a7c

github-actions Bot added size/XL 1000+ lines changed and removed size/L 250-999 lines changed labels May 4, 2026

houko merged commit 1efd7d3 into librefang:main May 4, 2026
21 of 22 checks passed

This was referenced May 4, 2026

chore(ci): validate (@user) attribution on Unreleased CHANGELOG entries (#3400) #4573

Merged

release: v2026.5.6-beta.9 #4688

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat(api): trusted_proxies + trust_forwarded_for for real-client-IP resolution#4534

feat(api): trusted_proxies + trust_forwarded_for for real-client-IP resolution#4534
houko merged 12 commits into
librefang:mainfrom
neo-wanderer:trusted-proxies

neo-wanderer commented May 3, 2026 •

edited

Loading

Uh oh!

houko commented May 3, 2026

Uh oh!

neo-wanderer commented May 4, 2026

Uh oh!

houko commented May 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

neo-wanderer commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

How it stays safe

Implementation

Test plan

Docs

Migration notes for operators

Behaviour change when both flags flip on

Uh oh!

houko commented May 3, 2026

Review

Findings

What I liked

Uh oh!

neo-wanderer commented May 4, 2026

Uh oh!

houko commented May 4, 2026

One new observation

Nits — no action needed

Re-confirmed clean

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

neo-wanderer commented May 3, 2026 •

edited

Loading