I went looking through the rate limiter on a docs search API this weekend and found a comment I’d written explaining exactly how to defeat the code directly underneath it.

Not a TODO. Not a “we should probably fix this someday.” An accurate, well-written security note describing the bypass, sitting three lines above the bypass.

Here’s what it said:

// X-Forwarded-For format: "client, proxy1, proxy2, ..."
// The leftmost IP is the original client IP as reported to the first proxy.
// We take the leftmost IP because it represents the originating client.
// Security note: The leftmost IP can be spoofed if upstream proxies don't
// strip or validate existing X-Forwarded-For headers from incoming requests.
// For higher security, prefer x-real-ip from trusted proxies.
proxyIp =
  request.headers.get('x-forwarded-for')?.split(',')[0]?.trim() ?? null;

Read that in order. Sentence three says we take the leftmost IP. Sentence four says the leftmost IP can be spoofed. Then the code takes the leftmost IP.

Why leftmost is the wrong end

X-Forwarded-For is an append-only chain. Each proxy that handles a request tacks the address it saw onto the right. So the header grows left to right, oldest first.

That means the leftmost value is whatever the original client claimed, and every value to the right of it was written by a piece of infrastructure. If a request arrives at your app with X-Forwarded-For: 203.0.113.9, nothing in that string is evidence of anything. I can put whatever I want there with curl.

The rightmost entries are the trustworthy ones, because your own proxies wrote them. Which is why the correct read is to count backwards a known number of hops, and take the address just before your trusted infrastructure starts.

The old code did the opposite. Worse, the call site made the header trusted by default:

// Rate limiting: 20 requests per minute per IP
// Use 'x-forwarded-for' for common proxy setups.
const rateLimitResult = checkRateLimit(request, {
  maxRequests: 20,
  windowSeconds: 60,
  trustedProxyHeader: 'x-forwarded-for',
});

There’s no environment check there. No “only when deployed behind nginx.” Every deployment, including a plain node ./dist/server/entry.mjs on a box with no proxy in front of it, trusted a header that any client can set.

The limiter allows 20 requests per minute per client identity. If the client picks its own identity, the limit is 20 requests per minute per made-up IP address, and there are about four billion of those. The rate limiter wasn’t weak. It was decorative.

The fix

Three changes, all in one commit.

The header is ignored unless you explicitly opt in with RATE_LIMIT_TRUSTED_PROXY_HEADER. Default identity now comes from clientAddress, which Astro gets from the actual socket and a client can’t forge. And when you do opt in, RATE_LIMIT_TRUSTED_PROXY_HOPS tells the resolver how many entries on the right belong to your infrastructure:

const chain = value.split(',').map((address) => address.trim());
if (chain.length === 0 || chain.some((address) => isIP(address) === 0)) {
  return null;
}

const clientIndex = Math.max(0, chain.length - trustedProxyHops - 1);
return chain[clientIndex] ?? null;

Note the validation on the whole chain, not just the value being extracted. If any entry isn’t a real IP, the header gets thrown out entirely. That’s using isIP from node:net instead of the pair of hand-rolled regexes that used to live there, one of which was a 400-character IPv6 pattern with a comment calling itself “simplified.”

I also bounded the store. It’s an in-memory Map, and buckets are keyed on client identity, so unbounded distinct keys is a memory leak with a friendly interface. It now caps at RATE_LIMIT_MAX_ENTRIES (default 10,000) and evicts the entry with the earliest reset time when full.

One deliberate ugly bit survived. When there’s no direct address and no trusted header, everything buckets together under the literal string unknown. That’s a shared bucket, and yes, one noisy client can exhaust it for everyone in that state. I kept it because the alternative is worse: a unique key per unidentifiable request means the limiter silently turns itself off for exactly the traffic you can’t identify. A limiter that’s too aggressive is a bug report. A limiter that’s off is a bill.

The indexer had the same disease

Same weekend, different file. The content indexer would print this and exit 0:

Successfully indexed 47/52 documents
Failed to index 5 documents

Exit zero. CI green. Deploy proceeds. Five documents missing from the search index, and every automated signal in the pipeline says the build succeeded.

The failure count was right there in the output. Nobody reads the output of a step that passed.

The fix pulled the logic into runContentIndexing(), which returns a 0 | 1 the CLI assigns to process.exitCode, and the new test file asserts the nonzero path on both partial failure and a thrown error. That last case mattered more than the partial one, since a rejected promise mid-index was also landing as a clean exit.

What actually connects these

Both bugs are the same shape, and it isn’t “I forgot to handle an edge case.” Both were mechanisms that reported success while doing nothing.

The rate limiter returned well-formed X-RateLimit-Remaining headers the whole time. The indexer printed a tidy progress log. If you were watching either one from the outside, they looked healthy. That’s the failure mode I now go looking for first, because it’s invisible by construction, and no amount of staring at green dashboards will surface it.

The thing that finally caught both was writing tests that assert on the failure path. Test count went from 153 to 178 across these fixes. Most of those 25 new tests do something unglamorous: they check that the thing fails when it’s supposed to.

A few specifics worth stealing:

1. Trust X-Forwarded-For from the right, never the left, and only with an explicit hop count you configured for that deployment.
2. Validate the entire chain, not just the value you pull out of it. Use isIP from node:net and delete your IP regex.
3. Bound any in-memory store keyed on request-derived data. If an attacker picks the key, the attacker picks your memory ceiling.
4. Make CLI scripts set process.exitCode on partial failure. Printing the failure count isn’t reporting it.
5. Write the test that asserts the failure path. If your suite only covers the happy path, a no-op implementation passes every test you have.

Go read the comments in your own auth and rate limiting code. Not the code, the comments. Past you may have already written the vulnerability report.

Sources

• MDN: X-Forwarded-For — the append-order behavior and its spoofing caveat
• RFC 7239 — the standardized Forwarded header that replaced the de facto X-Forwarded-* set
• Astro API reference — clientAddress, the socket-derived address used as the default identity
• OWASP API4:2023 Unrestricted Resource Consumption — the category both of these fall under
• semantic-docs — the repo, if you want the full diffs in PR #93 and PR #92

I’d appreciate a follow. You can subscribe with your email below. The emails go out once a week, or you can find me on Mastodon at @[email protected].