fix(security): read the forwarded-for entry a trusted proxy wrote, not the caller's

[bgard68]

The defect

Rate limiting partitioned on the leftmost X-Forwarded-For entry. A proxy appends to that header rather than replacing it, so a caller sending X-Forwarded-For: 9.9.9.9 arrives as 9.9.9.9, <real client> — position zero is a value the caller chose. Varying it per request minted a fresh partition every time.

That is throttling defeated with TrustForwardedFor correctly set to true, on the deployed app. It's the same forgery RateLimitOptions already warned about, reached through the other door — and setting that flag right did not close it.

TrustForwardedFor was the setting that got all the attention, and it answers only half the question. Whether to read the header is one decision; which entry of it a proxy actually wrote is the other, and only the second is a trust boundary.

The test that held it in place

The_leftmost_entry_in_a_forwarded_chain_is_the_client()
    Request("10.0.0.1", "198.51.100.9, 10.0.0.8, 10.0.0.1")
    → asserts "198.51.100.9"

It passed. It described the code accurately and the protocol incorrectly, and it read as a specification while documenting a bug.

The fix

Count from the trusted end. With TrustedProxyHops proxies in front the client sits at count - hops; everything to its left is caller-supplied and ignored. A chain shorter than the hop count falls back to the connection address rather than trusting an entry nearer the caller.

The port mattered too. App Service appends the client as ip:port, and the source port is ephemeral. Taking the rightmost entry naively would have partitioned per connection instead of per caller — reopening the identical hole from the other side. Entries are normalised through IPAddress/IPEndPoint, which drops it. IPAddress is tried first so a bare IPv6 address isn't mistaken for a host:port pair on account of its colons.

Values that aren't addresses (unknown, _hidden, junk) are discarded rather than used as keys — they aren't identities, and every caller sending the same placeholder would otherwise share one budget under a name that reads specific.

Changes

ClientAddress	Reads from the trusted end; normalises to a bare address
RateLimitOptions	TrustedProxyHops (default 1; 2 behind a CDN/WAF)
ProxyConfigurationCheck	Third warning: hop count higher than the chain arriving
appsettings.json	Ships the setting, pinned by ShippedConfigurationTests
ClientAddressTests	Wrong assertion replaced; regressions for forgery, port, IPv6, hop counts
Handbook	Entry 46; config chapter now covers which entry, not just whether

A hop count set too high runs off the front of every chain, so each request falls back to the proxy's own address — recreating the global-cap outage while the setting that explains it reads as correct. That's what the new warning is for.

Verification

• The exploit, demonstrated both ways. Pre-fix code returns 9.9.9.9 for the forged chain; new code returns the real client. Run directly against both implementations, not inferred.
• 530 tests pass (360 unit, 75 integration, 95 API) against real PostgreSQL.
• dotnet format --verify-no-changes clean; 0 warnings under -warnaserror.

Not verified: the live spoof test against App Service. This sandbox's network policy blocks azurewebsites.net (403 on CONNECT), so whether the platform normalises the header before the app sees it is unconfirmed from here. The fix is correct either way — it stops depending on that question having a favourable answer.

Provision.ps1 gained a comment only (no pwsh available to syntax-check it); TrustedProxyHops is deliberately not set as an Azure app setting, since the shipped default of 1 is already right for App Service on its own.

---

Generated by Claude Code