Surfaced during review of #196 (pre-existing in client_key; #196 mounts the new write brake on the same key path, which is why it is worth hardening).
crates/gitlawb-node/src/rate_limit.rs:198-218 (client_key): in Fly/XForwardedFor modes the limiter key comes entirely from Fly-Client-IP or the rightmost X-Forwarded-For hop, falling back to the socket peer only when the header is absent. There is no check that the socket peer (ConnectInfo) is actually the operator's proxy. If GITLAWB_TRUSTED_PROXY is set but the node's HTTP port is reachable directly (misconfigured firewall, exposed Fly port, second ingress, internal attacker), a client can set Fly-Client-IP: <random> / append any rightmost XFF hop and get a fresh bucket per request, evading the creation/write/push/sync brakes. The unit tests cover header parsing but never assert 'peer not the proxy -> key on peer'.
Fix direction: honor the forwarded header only when the ConnectInfo peer is in a configured proxy CIDR/allowlist; otherwise key on the socket peer (fail closed). Add a 'trust mode set, request from a non-proxy IP' test. Exploitability is deployment-dependent (Fly's edge overwrites Fly-Client-IP), so Medium, not blocker. This is a key-trust gap in the forwarded-IP handling. Hand-traced during the #196 review.
Surfaced during review of #196 (pre-existing in
client_key; #196 mounts the new write brake on the same key path, which is why it is worth hardening).crates/gitlawb-node/src/rate_limit.rs:198-218(client_key): inFly/XForwardedFormodes the limiter key comes entirely fromFly-Client-IPor the rightmostX-Forwarded-Forhop, falling back to the socket peer only when the header is absent. There is no check that the socket peer (ConnectInfo) is actually the operator's proxy. IfGITLAWB_TRUSTED_PROXYis set but the node's HTTP port is reachable directly (misconfigured firewall, exposed Fly port, second ingress, internal attacker), a client can setFly-Client-IP: <random>/ append any rightmost XFF hop and get a fresh bucket per request, evading the creation/write/push/sync brakes. The unit tests cover header parsing but never assert 'peer not the proxy -> key on peer'.Fix direction: honor the forwarded header only when the
ConnectInfopeer is in a configured proxy CIDR/allowlist; otherwise key on the socket peer (fail closed). Add a 'trust mode set, request from a non-proxy IP' test. Exploitability is deployment-dependent (Fly's edge overwritesFly-Client-IP), so Medium, not blocker. This is a key-trust gap in the forwarded-IP handling. Hand-traced during the #196 review.