Skip to content

fix(public): read x-forwarded-for only through a trusted proxy - #627

Open
Evanfeenstra wants to merge 1 commit into
mainfrom
fix/xff-trusted-proxies
Open

Evanfeenstra wants to merge 1 commit into
mainfrom
fix/xff-trusted-proxies

Conversation

@Evanfeenstra

Copy link
Copy Markdown
Contributor

Summary

visitorAddress is the key every per-address budget on the public surface counts under: /public reads (PUBLIC_REQUESTS_PER_MINUTE), launches (PUBLIC_LAUNCHES_PER_MINUTE), tool calls (PUBLIC_TOOL_CALLS_PER_MINUTE), the credit challenge's invoices and claims, /llms.txt, and the index's listing requests and pings. It used to take the FIRST x-forwarded-for hop from any client. A client that reached strut with no proxy overwriting that header could name a new address on every request and get a fresh budget each time.

It now reads x-forwarded-for only when the request came from a trusted proxy:

  • The socket's peer (c.env.incoming.socket.remoteAddress) is the visitor, unless it is a trusted proxy. IPv4-mapped IPv6 (::ffff:a.b.c.d) is treated as the IPv4 address.
  • When the peer is trusted, the header is walked from the RIGHT, skipping trusted hops, and the first untrusted hop is the visitor. If every hop is trusted, the leftmost one is used.
  • With no socket (an in-process app.fetch), the visitor is unknown and the header is not read.

Trusted by default: loopback, link-local and private addresses (RFC 1918, IPv6 unique-local). This is Express's loopback, linklocal, uniquelocal. A proxy on the same box or container network connects from one of these. Trusting nothing would put every visitor behind such a proxy into one budget.

Override: STRUT_TRUSTED_PROXIES (comma-separated IPs and CIDRs, or none), read at boot through bootEnv(). createStrut({ trustedProxies }) takes precedence over it. Either one replaces the default set rather than adding to it. An entry that is not an IP or a CIDR fails the boot.

All of the above lives in the new src/visitor-address.ts. createStrut builds one visitorAddress from the trusted set and passes it to publicRoutes (new optional PublicRouteDeps.visitorAddress) and to its own callers (/llms.txt, /index/requests, /index/ping, /public/mcp). The exported visitorAddress in public-routes.ts is gone. It was not exported from the package.

I grepped for other readers of x-forwarded-for, x-real-ip and forwarded. Every address read already went through visitorAddress, including the credit address/who and the public agents' who. The visitor hash in public-surface.ts (visitorOfVisit) comes from x-strut-visit, not from an address. x-forwarded-proto and x-forwarded-host (link origin, the Secure cookie flag) are not the visitor address, so they are unchanged.

Behavior changes for operators

  • Proxy on a public address: list it in STRUT_TRUSTED_PROXIES. Otherwise every visitor behind it shares the proxy's single budget.
  • Untrusted clients connecting from private addresses (a LAN, a shared container network) with no proxy in between: set STRUT_TRUSTED_PROXIES to the proxy's exact address, or none. The default trusts any private peer.

These budgets are budgets, not auth. Priced items still need paid credit, and the in-flight and cost caps are unchanged.

Docs

  • specs/API.md §17: how the visitor address is chosen.
  • plans/paid-runs.md §3: a "visitor address" paragraph.
  • plans/index-listing.md: points at the new file.
  • AGENTS.md: a file-tree entry and a STRUT_TRUSTED_PROXIES env row.

Tests

  • src/visitor-address.test.ts (new):
    • A spoofed header from an untrusted (public) peer is ignored.
    • A trusted peer with XFF: spoof, real yields real.
    • Multi-proxy chains, and all hops trusted.
    • The default set's members and non-members, IPv4 and IPv6.
    • IPv4-mapped peers and hops, and zone ids.
    • No socket gives unknown.
    • Env parsing (unset, blank, none, list) and invalid entries throwing.
    • Reading the peer off c.env.incoming through Hono.
  • src/public-runs.test.ts, through the real launch route:
    • 61 launches from one public peer, each with a different spoofed x-forwarded-for: the 61st answers 429 (too many launches: 60 a minute).
    • The same client over ::ffff: is still over budget. Another peer has its own budget.
    • Through a private proxy, each forwarded visitor has its own budget, and hops a client wrote before the proxy's are ignored.
    • STRUT_TRUSTED_PROXIES=none makes even loopback's header unread.
    • createStrut({ trustedProxies }) takes precedence over the env and replaces the default set.
    • A bad entry fails the boot.
  • src/public-surface.test.ts: the /public request budget, through a trusted proxy and from a direct client.
  • src/public-tools.test.ts: its helper now sends calls through a loopback proxy, so each call still names its own visitor.

With the old visitorAddress behavior restored, the four new route-level tests fail.

Ran locally: full suite (npm test): 4351 tests, 4297 pass, 0 fail, 54 skipped. tsc --noEmit is clean.

visitorAddress, the key of every per-address budget on the public
surface (requests, launches, tool calls, credit invoices and claims),
took the first x-forwarded-for hop from any client. A client that
reached strut with no proxy overwriting the header could name a new
address each request and get a fresh budget each time.

Now the socket's peer is the visitor unless it is a trusted proxy.
Through a trusted proxy the header is walked from the right, skipping
trusted hops; every hop trusted gives the leftmost. IPv4-mapped peers
are normalized. With no socket the visitor is "unknown" and the header
is not read.

The default trusted set is loopback, link-local and private (Express's
loopback, linklocal, uniquelocal), where a proxy on the same box or
container network connects from. STRUT_TRUSTED_PROXIES (IPs/CIDRs, or
"none") or createStrut({ trustedProxies }) replaces it. A bad entry
fails the boot.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant