fix: let rateLimit name the header carrying the visitor

.agents/skills/webjs/references/built-ins.md

@@ -113,6 +113,8 @@ Options: `window` (ms or a string like `'1m'`), `max`, `key` (a string prefix or
 
 **The default key is the socket PEER, which is the visitor only when the browser connects to you directly.** Deploy behind a CDN or a platform router and the peer is that proxy, so `trustProxy: true` is what a deployed limiter almost always wants. Get it wrong and nothing looks broken: a single shared proxy buckets every visitor together, and a proxy POOL (the common case) hands out one full allowance PER proxy, so the effective limit is multiplied by the pool size while `X-RateLimit-Remaining` still counts down convincingly inside each bucket. Diagnose it by sending the requests over ONE keep-alive connection, which pins them to one peer: counts that descend there but reset on a fresh connection mean you are bucketing proxies. `trustProxy: true` has one precondition, that the proxy in front strips an inbound `X-Forwarded-For` before adding its own (Cloudflare, Railway, Fly, Render, and Vercel do; nginx and Caddy only if configured), or a client can forge the header and choose its own bucket.
 
+**Behind a CDN, `trustProxy: true` alone is usually still wrong, so name the header: `rateLimit({ trustProxy: true, clientIpHeader: 'cf-connecting-ip' })`.** The default chain starts at the leftmost `X-Forwarded-For` entry, which behind Cloudflare is Cloudflare's EGRESS address rather than the visitor. Cloudflare pins one egress IP per connection, so the symptom is a limiter that counts down correctly for a button that pings on one connection and never refuses anyone who opens a new one. When `clientIpHeader` is set it is the only wire header read (falling back to the peer, then `_anon_`), a blank value falls through rather than becoming a key every visitor shares, and a comma chain is split so an appending proxy cannot mint a bucket per hop. The framework will NOT prefer `CF-Connecting-IP` on its own, because Cloudflare overwrites it, which makes it unforgeable behind Cloudflare and forgeable anywhere else: on an nginx or bare-platform deploy a client could then send it and outrank the header the real proxy set. Name the header YOUR edge sets and overwrites.
+
 ## Broadcast
 
 Send data to every WebSocket client connected to a route path, from inside that route's `WS` handler.

gallery/app/features/rate-limit/ping/middleware.ts

@@ -12,16 +12,29 @@
 // hands out one bucket per proxy, which multiplies the real limit by the pool
 // size. WITH it the key comes from the forwarded client address instead.
 //
-// The tradeoff is real and worth knowing before copying this line: the proxy
-// in front of you MUST strip an inbound X-Forwarded-For before adding its own,
-// or a client can forge the header and pick its own bucket. WEBJS_NO_TRUST_PROXY=1
-// also outranks this option and puts the limiter back on the socket peer.
+// `clientIpHeader` then says WHICH forwarded header carries the visitor, and on
+// this deployment it is load-bearing too. Without it the default chain takes the
+// leftmost X-Forwarded-For entry, which behind Cloudflare is Cloudflare's EGRESS
+// address rather than yours. Cloudflare pins an egress IP per connection, so the
+// limiter hands out one bucket per connection: the count descends convincingly
+// while you hold one connection open and resets the moment a new one opens,
+// which is a limiter that limits nobody. Your address is in CF-Connecting-IP, so
+// that is the header this app names.
+//
+// Copying this into your own app? Name the header YOUR proxy sets, and only
+// after checking it cannot be forged past that proxy. Cloudflare overwrites
+// CF-Connecting-IP, which is what makes it safe HERE and unsafe on a deploy that
+// Cloudflare is not in front of. The same precondition applies to the default
+// chain: the proxy MUST strip an inbound X-Forwarded-For before adding its own.
+// Serving with nothing in front? Drop both options, since then the socket peer
+// IS the visitor. WEBJS_NO_TRUST_PROXY=1 outranks all of it.
 // /docs/rate-limiting has the full threat model.
 import { rateLimit } from '@webjsdev/server';
 
 export default rateLimit({
   window: '10s',
   max: 5,
   trustProxy: true,
+  clientIpHeader: 'cf-connecting-ip',
   message: 'Slow down: five requests per ten seconds.',
 });

gallery/app/features/route-handler/data/route.ts

@@ -16,7 +16,15 @@ export async function GET(req: Request) {
   return json({
     ok: true,
     at: new Date(), // a real Date; richFetch decodes it back to a Date, not a string
+    // Two addresses, because behind a proxy they are NOT the same and the
+    // difference is invisible until something depends on it (a rate limiter
+    // did, and bucketed proxies instead of visitors). `ip` is the socket peer,
+    // which is the visitor only when the browser connects to you directly.
+    // `forwardedIp` is what the visitor's own CDN header says, which is what a
+    // limiter or an audit log wants. Deployed behind Cloudflare and Railway,
+    // `ip` is a rotating 100.64.x.x router address while `forwardedIp` is you.
     ip: clientIp(req),
+    forwardedIp: clientIp(req, { trustProxy: true, header: 'cf-connecting-ip' }),
     requestId: requestId(),
     userAgent: headers().get('user-agent') ?? 'unknown',
     // cookies() reads the REQUEST cookies. Report how many are present (a

gallery/test/rate-limit/rate-limit.test.ts

@@ -17,8 +17,15 @@ const MAX = 5;
 // Each test picks its own visitor addresses. The limiter counts into the global
 // in-memory cache store, which outlives a handler instance, so two tests sharing
 // an address would share a bucket and the second would start already exhausted.
-function ping(handle: (req: Request) => Promise<Response>, forwardedFor: string) {
-  return testRequest(handle, PING, { headers: { 'x-forwarded-for': forwardedFor } });
+// The demo names CF-Connecting-IP, because that is the header carrying the
+// visitor on the deployment it runs on. Every request here also carries an
+// X-Forwarded-For that DISAGREES, standing in for the CDN egress address the
+// real deploy puts there, so a test that passes only because the two agree
+// cannot exist.
+function ping(handle: (req: Request) => Promise<Response>, visitor: string, cdnEgress = '172.68.1.9') {
+  return testRequest(handle, PING, {
+    headers: { 'cf-connecting-ip': visitor, 'x-forwarded-for': cdnEgress },
+  });
 }
 
 test('the demo limits one visitor to five requests per window', async () => {
@@ -58,3 +65,27 @@ test('one visitor exhausting the window does not refuse another behind the same
   assert.equal(other.status, 200, 'a different visitor keeps their own window');
   assert.equal(other.headers.get('x-ratelimit-remaining'), String(MAX - 1));
 });
+
+// The half `trustProxy: true` alone did not deliver, and the one the live site
+// disproved (#1389). A CDN gives each connection a different egress address, so
+// one visitor opening several connections arrives with several X-Forwarded-For
+// values and ONE CF-Connecting-IP. Keyed on XFF that visitor gets a fresh bucket
+// per connection and is never refused, which is what shipped and read as working.
+//
+// Counterfactual, proven at this commit: removing `clientIpHeader` from the
+// middleware fails this test at the sixth request AND the two-visitor test
+// above, while the single-visitor test still passes. The one that survives is
+// the one whose requests all carry the same CDN address, which is exactly the
+// blind spot that let the first fix look complete on a real deployment.
+test('one visitor is limited across connections, whatever CDN address they arrive on', async () => {
+  const app = await createRequestHandler({ appDir, dev: true });
+  const visitor = '203.0.113.30';
+
+  for (let i = 1; i <= MAX; i += 1) {
+    const res = await ping(app.handle, visitor, `172.68.9.${i}`);
+    assert.equal(res.status, 200, `request ${i} arrives on its own CDN egress address`);
+  }
+
+  const limited = await ping(app.handle, visitor, '172.68.9.99');
+  assert.equal(limited.status, 429, 'a new CDN egress address does not buy a new window');
+});

packages/cli/lib/api-gallery.js

@@ -81,6 +81,13 @@ export async function writeApiGallery(appDir) {
     "// pick its own bucket. Serving with nothing in front? Drop the option, since",
     "// then the socket peer IS the visitor. WEBJS_NO_TRUST_PROXY=1 outranks it either",
     "// way. https://webjs.dev/docs/rate-limiting has the full threat model.",
+    "//",
+    "// Behind a CDN, add `clientIpHeader` to name the header carrying the visitor,",
+    "// e.g. `clientIpHeader: 'cf-connecting-ip'` behind Cloudflare. The default",
+    "// chain reads the leftmost X-Forwarded-For entry, which behind a CDN is the",
+    "// CDN's egress address; those are pinned per connection, so the limiter ends up",
+    "// handing out a bucket per connection and refusing nobody. It is left unset",
+    "// here because the right header depends on what you deploy behind.",
     "import { rateLimit } from '@webjsdev/server';",
     "",
     "export default rateLimit({",

packages/server/index.d.ts

@@ -635,6 +635,21 @@ export declare function rateLimit(opts?: {
    * that STRIPS inbound `X-Forwarded-For` before adding its own.
    */
   trustProxy?: boolean;
+  /**
+   * Name the ONE forwarded header that carries the visitor, e.g.
+   * `'cf-connecting-ip'` behind Cloudflare. Requires `trustProxy: true`, and
+   * when set it is the only wire header consulted (falling back to the stamped
+   * peer, then `_anon_`).
+   *
+   * Behind a CDN this is usually required for the limiter to work at all. The
+   * default chain takes the leftmost `X-Forwarded-For` entry, which behind
+   * Cloudflare is Cloudflare's EGRESS address, not the visitor, and since that
+   * is pinned per connection the limiter hands out one bucket per connection.
+   * The framework will not prefer `CF-Connecting-IP` on its own: Cloudflare
+   * overwrites it, so it is unforgeable behind Cloudflare and forgeable
+   * anywhere else, which makes the right choice a property of your topology.
+   */
+  clientIpHeader?: string;
 }): Middleware;
 /** Parse a window string (`'1m'`, `'30s'`) to milliseconds. */
 export declare function parseWindow(w: number | string): number;
@@ -645,8 +660,17 @@ export declare function parseWindow(w: number | string): number;
  * `trustProxy: true` reads the forwarded-IP headers instead, UNLESS
  * `WEBJS_NO_TRUST_PROXY=1` is set, which overrides the option back to the
  * stamped peer and logs once per process.
+ *
+ * `header` names the one forwarded header to trust (`'cf-connecting-ip'`
+ * behind Cloudflare) and, when given, is the only wire header read. Use it
+ * whenever a CDN sits in front: the default chain's leftmost
+ * `X-Forwarded-For` entry is then the CDN's egress address rather than the
+ * visitor.
  */
-export declare function clientIp(req: Request, opts?: { trustProxy?: boolean }): string;
+export declare function clientIp(
+  req: Request,
+  opts?: { trustProxy?: boolean; header?: string },
+): string;
 /** Stamp the socket remote address onto a request for `clientIp` to read. */
 export declare function stampRemoteIp(req: Request, remoteAddress: string): void;
 

packages/server/src/rate-limit.js

@@ -40,6 +40,7 @@ let warnedProxyOverride = false;
  *   message?: string,
  *   store?: import('./cache.js').CacheStore,
  *   trustProxy?: boolean,
+ *   clientIpHeader?: string,
  * }} opts
  * @returns {(req: Request, next: () => Promise<Response>) => Promise<Response>}
  */
@@ -50,13 +51,17 @@ export function rateLimit(opts = {}) {
   const keyPrefix = typeof opts.key === 'string' ? opts.key : '';
   const message = opts.message ?? 'Too Many Requests';
   const trustProxy = opts.trustProxy === true;
+  // The header carrying the visitor, when the app knows which one that is.
+  // Inert without `trustProxy: true`, since naming a wire header to trust IS
+  // the trust decision and must not be grantable by a second option.
+  const header = typeof opts.clientIpHeader === 'string' ? opts.clientIpHeader : undefined;
   // Use the provided store, or fall back to the global cache store.
   // Whatever was set via `setStore()` at app startup (in-memory by default).
   const store = opts.store || null;
 
   return async function rateLimitMiddleware(req, next) {
     const s = store || getStore();
-    const raw = keyFn ? await keyFn(req) : clientIp(req, { trustProxy });
+    const raw = keyFn ? await keyFn(req) : clientIp(req, { trustProxy, header });
     const key = `rl:${keyPrefix}${raw}`;
 
     const count = await s.increment(key, windowMs);
@@ -185,10 +190,42 @@ export function propagateTrustedRemoteIp(src, dst) {
  * leaving them in disagreement buckets every visitor behind that proxy onto
  * one key.
  *
+ * `header` names the ONE forwarded header to trust, and it is the option a
+ * CDN deployment needs (#1389). The default chain reads the leftmost
+ * `X-Forwarded-For` entry first, which behind Cloudflare is CLOUDFLARE'S
+ * EGRESS address rather than the visitor: Cloudflare pins an egress IP per
+ * connection, so a limiter keyed on it gives one bucket per connection, which
+ * counts down convincingly and limits nobody. The visitor is in
+ * `CF-Connecting-IP`, and naming it here is how the app says so.
+ *
+ * The framework does NOT reorder the default chain to prefer that header,
+ * because which header is trustworthy is a property of the TOPOLOGY, not of
+ * the framework. Cloudflare overwrites `CF-Connecting-IP`, so it is
+ * unforgeable behind Cloudflare and forgeable everywhere else; preferring it
+ * globally would let a client on an nginx or bare-Railway deploy outrank the
+ * `X-Forwarded-For` the real proxy set. So the app names its header and owns
+ * the claim. When `header` is set it is the only forwarded header consulted,
+ * falling back to the stamped peer and then `_anon_`, and a blank value falls
+ * through rather than becoming a shared literal key.
+ *
  * @param {Request} req
- * @param {{ trustProxy?: boolean }} [opts]
+ * @param {{ trustProxy?: boolean, header?: string }} [opts]
  * @returns {string}
  */
+/**
+ * First entry of a forwarded-IP header, trimmed, or `''` when there is nothing
+ * usable. A blank value must FALL THROUGH rather than resolve: an empty string
+ * as a bucket key is one key shared by every visitor whose proxy sent the
+ * header empty, which is a limiter that throttles strangers together.
+ *
+ * @param {string | null | undefined} raw
+ * @returns {string}
+ */
+function firstForwardedEntry(raw) {
+  if (!raw) return '';
+  return raw.split(',')[0].trim();
+}
+
 export function clientIp(req, opts = {}) {
   if (opts.trustProxy === true && !proxyIsTrusted() && !warnedProxyOverride) {
     warnedProxyOverride = true;
@@ -201,10 +238,18 @@ export function clientIp(req, opts = {}) {
     );
   }
   if (opts.trustProxy === true && proxyIsTrusted()) {
+    if (opts.header) {
+      // One named header, and nothing else from the wire. A chain is still
+      // split on the comma so a proxy that appends to the named header cannot
+      // turn the key into a growing string, which would mint a fresh bucket per
+      // hop and reproduce the very failure this option exists to fix.
+      const named = req.headers.get(String(opts.header).toLowerCase());
+      return firstForwardedEntry(named) || trustedRemoteIp(req) || '_anon_';
+    }
     return (
-      req.headers.get('x-forwarded-for')?.split(',')[0].trim() ||
-      req.headers.get('cf-connecting-ip') ||
-      req.headers.get('x-real-ip') ||
+      firstForwardedEntry(req.headers.get('x-forwarded-for')) ||
+      req.headers.get('cf-connecting-ip')?.trim() ||
+      req.headers.get('x-real-ip')?.trim() ||
       trustedRemoteIp(req) ||
       '_anon_'
     );

packages/server/test/rate-limit/rate-limit.test.js

@@ -385,3 +385,86 @@ test('WEBJS_NO_TRUST_PROXY=1: the default path (no option) is unchanged (#1254)'
     assert.equal(clientIp(req), '9.9.9.9', 'the default path must still read only the stamped peer');
   });
 });
+
+/* ------------------ clientIpHeader: naming the visitor's header ------------------ */
+
+test('clientIpHeader reads ONLY the named header (#1389)', async () => {
+  const { clientIp } = await import('../../src/rate-limit.js');
+  // The shape a Cloudflare deploy actually receives: XFF's leftmost entry is
+  // the CDN's own egress address, and the visitor is in CF-Connecting-IP. The
+  // default chain takes the wrong one of the two, which is the whole bug.
+  const req = new Request('http://x/', {
+    headers: {
+      'x-forwarded-for': '172.68.1.9, 100.64.0.3',
+      'cf-connecting-ip': '203.0.113.44',
+      'x-webjs-remote-ip': '100.64.0.3',
+    },
+  });
+  assert.equal(clientIp(req, { trustProxy: true }), '172.68.1.9', 'default chain still prefers XFF leftmost');
+  assert.equal(
+    clientIp(req, { trustProxy: true, header: 'cf-connecting-ip' }),
+    '203.0.113.44',
+    'the named header wins over XFF',
+  );
+});
+
+test('clientIpHeader is matched case-insensitively and splits a chain', async () => {
+  const { clientIp } = await import('../../src/rate-limit.js');
+  const req = new Request('http://x/', {
+    headers: { 'cf-connecting-ip': ' 203.0.113.44 , 10.0.0.1 ', 'x-webjs-remote-ip': '100.64.0.3' },
+  });
+  // A proxy that APPENDS to the named header must not mint a new bucket per
+  // hop, which is the failure mode the option exists to end.
+  assert.equal(clientIp(req, { trustProxy: true, header: 'CF-Connecting-IP' }), '203.0.113.44');
+});
+
+test('a missing or blank named header falls back to the peer, never to a shared key', async () => {
+  const { clientIp } = await import('../../src/rate-limit.js');
+  const missing = new Request('http://x/', { headers: { 'x-webjs-remote-ip': '100.64.0.3' } });
+  assert.equal(clientIp(missing, { trustProxy: true, header: 'cf-connecting-ip' }), '100.64.0.3');
+
+  // A blank value resolving to '' would be ONE key shared by every visitor
+  // whose proxy sent the header empty, throttling strangers together.
+  const blank = new Request('http://x/', {
+    headers: { 'cf-connecting-ip': '   ', 'x-webjs-remote-ip': '100.64.0.3' },
+  });
+  assert.equal(clientIp(blank, { trustProxy: true, header: 'cf-connecting-ip' }), '100.64.0.3');
+
+  const nothing = new Request('http://x/', { headers: { 'cf-connecting-ip': '' } });
+  assert.equal(clientIp(nothing, { trustProxy: true, header: 'cf-connecting-ip' }), '_anon_');
+});
+
+test('clientIpHeader is inert without trustProxy, and under WEBJS_NO_TRUST_PROXY=1', async () => {
+  const { clientIp } = await import('../../src/rate-limit.js');
+  const req = new Request('http://x/', {
+    headers: { 'cf-connecting-ip': '203.0.113.44', 'x-webjs-remote-ip': '100.64.0.3' },
+  });
+  // Naming a wire header to trust IS the trust decision, so it must not be
+  // grantable by a second option that skips the first.
+  assert.equal(clientIp(req, { header: 'cf-connecting-ip' }), '100.64.0.3');
+  await withNoTrustProxy('1', () => {
+    assert.equal(clientIp(req, { trustProxy: true, header: 'cf-connecting-ip' }), '100.64.0.3');
+  });
+});
+
+test('rateLimit buckets by the named header, not by the CDN egress address', async () => {
+  const mw = rateLimit({ window: '1s', max: 1, trustProxy: true, clientIpHeader: 'cf-connecting-ip' });
+  // Two requests from ONE visitor that arrive through DIFFERENT CDN egress
+  // addresses, which is what a fresh connection produces. They must share a
+  // bucket; keyed on XFF they would not, and the visitor would never be limited.
+  const first = new Request('http://x/', {
+    headers: { 'x-forwarded-for': '172.68.1.9', 'cf-connecting-ip': '203.0.113.44' },
+  });
+  const second = new Request('http://x/', {
+    headers: { 'x-forwarded-for': '172.69.7.2', 'cf-connecting-ip': '203.0.113.44' },
+  });
+  assert.equal((await mw(first, async () => new Response('ok'))).status, 200);
+  assert.equal((await mw(second, async () => new Response('ok'))).status, 429, 'same visitor, second egress');
+
+  // And a genuinely different visitor arriving through the SAME egress keeps
+  // their own window, which is the other half of bucketing correctly.
+  const other = new Request('http://x/', {
+    headers: { 'x-forwarded-for': '172.68.1.9', 'cf-connecting-ip': '198.51.100.7' },
+  });
+  assert.equal((await mw(other, async () => new Response('ok'))).status, 200);
+});