diff --git a/.agents/skills/webjs/references/built-ins.md b/.agents/skills/webjs/references/built-ins.md index a82cf44f9..3aaabb757 100644 --- a/.agents/skills/webjs/references/built-ins.md +++ b/.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. diff --git a/gallery/app/features/rate-limit/ping/middleware.ts b/gallery/app/features/rate-limit/ping/middleware.ts index f858c621f..7d82c8871 100644 --- a/gallery/app/features/rate-limit/ping/middleware.ts +++ b/gallery/app/features/rate-limit/ping/middleware.ts @@ -12,10 +12,22 @@ // 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'; @@ -23,5 +35,6 @@ export default rateLimit({ window: '10s', max: 5, trustProxy: true, + clientIpHeader: 'cf-connecting-ip', message: 'Slow down: five requests per ten seconds.', }); diff --git a/gallery/app/features/route-handler/data/route.ts b/gallery/app/features/route-handler/data/route.ts index 773031f04..f553ea0a9 100644 --- a/gallery/app/features/route-handler/data/route.ts +++ b/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 diff --git a/gallery/test/rate-limit/rate-limit.test.ts b/gallery/test/rate-limit/rate-limit.test.ts index 2e50f30ad..82e5124d1 100644 --- a/gallery/test/rate-limit/rate-limit.test.ts +++ b/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, 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, 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'); +}); diff --git a/packages/cli/lib/api-gallery.js b/packages/cli/lib/api-gallery.js index a78d90111..e4cfdd001 100644 --- a/packages/cli/lib/api-gallery.js +++ b/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({", diff --git a/packages/server/index.d.ts b/packages/server/index.d.ts index eebb19842..bc094db9d 100644 --- a/packages/server/index.d.ts +++ b/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; diff --git a/packages/server/src/rate-limit.js b/packages/server/src/rate-limit.js index e30a94046..6b966fefe 100644 --- a/packages/server/src/rate-limit.js +++ b/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) => Promise} */ @@ -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_' ); diff --git a/packages/server/test/rate-limit/rate-limit.test.js b/packages/server/test/rate-limit/rate-limit.test.js index 7917e699d..ee3d19fb7 100644 --- a/packages/server/test/rate-limit/rate-limit.test.js +++ b/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); +}); diff --git a/test/bun/rate-limit-client-ip.mjs b/test/bun/rate-limit-client-ip.mjs new file mode 100644 index 000000000..ed950fe60 --- /dev/null +++ b/test/bun/rate-limit-client-ip.mjs @@ -0,0 +1,105 @@ +/** + * `rateLimit`'s client-IP resolution is identical under Node and Bun (#1389). + * + * Why per runtime. The resolution reads the request's headers AND, when no + * forwarded header applies, the framework-stamped peer, and the two listener + * shells stamp that peer in DIFFERENT ways: the node:http shell sets the + * `x-webjs-remote-ip` header on a rebuilt Request, while the Bun shell stamps it + * out of band through a WeakMap so it does not have to clone one (#756). A + * resolution change can therefore be correct on one shell and wrong on the + * other, and the fallback rungs are exactly where that shows. + * + * What is pinned, in order of what it protects: + * + * - the NAMED header wins over `X-Forwarded-For`. This is the reported bug: + * behind a CDN the leftmost XFF entry is the CDN's egress address, which is + * pinned per connection, so a limiter keyed on it hands out one bucket per + * connection and refuses nobody. + * - a BLANK named header falls through to the peer rather than resolving to + * `''`, which would be one bucket shared by every visitor whose proxy sent + * the header empty. + * - the named header is INERT without `trustProxy`, since naming a wire header + * to trust is itself the trust decision. + * - the DEFAULT chain is unchanged, so apps that name no header keep the + * resolution they already have. + * + * A plain assert script (not `*.test.mjs`, so the node:test runner does not + * double-run it); it exits non-zero on failure. Run from the repo root so the + * bare `@webjsdev/server` specifier resolves to the workspace package. + */ +import assert from 'node:assert/strict'; +import { clientIp, stampRemoteIp } from '@webjsdev/server'; + +const runtime = process.versions.bun ? `bun ${process.versions.bun}` : `node ${process.versions.node}`; + +// The header shape a Cloudflare plus Railway deploy really receives: the CDN +// egress in XFF's first position, the visitor in CF-Connecting-IP, and a +// carrier-grade-NAT router address as the socket peer. +const CDN_EGRESS = '172.68.1.9'; +const VISITOR = '203.0.113.44'; +const PEER = '100.64.0.3'; + +function cdnRequest(extra = {}) { + return new Request('http://x/', { + headers: { 'x-forwarded-for': `${CDN_EGRESS}, ${PEER}`, 'cf-connecting-ip': VISITOR, ...extra }, + }); +} + +// --- the named header wins --------------------------------------------------- + +assert.equal( + clientIp(cdnRequest(), { trustProxy: true, header: 'cf-connecting-ip' }), + VISITOR, + `${runtime}: the named header must resolve to the visitor`, +); + +assert.equal( + clientIp(cdnRequest(), { trustProxy: true }), + CDN_EGRESS, + `${runtime}: the default chain must still read XFF leftmost`, +); + +// --- the fallback rungs, which is where the two shells differ ---------------- + +// Peer via the node shell's header. Both runtimes accept this form, since the +// Bun shell's WeakMap is consulted first and simply has no entry here. +const headerStamped = new Request('http://x/', { headers: { 'x-webjs-remote-ip': PEER } }); +assert.equal( + clientIp(headerStamped, { trustProxy: true, header: 'cf-connecting-ip' }), + PEER, + `${runtime}: a missing named header falls back to the header-stamped peer`, +); + +// Peer via the Bun shell's out-of-band stamp. `stampRemoteIp` is the documented +// embedded-adapter entry point onto the same path. +const oobStamped = stampRemoteIp(new Request('http://x/'), PEER); +assert.equal( + clientIp(oobStamped, { trustProxy: true, header: 'cf-connecting-ip' }), + PEER, + `${runtime}: a missing named header falls back to the out-of-band peer`, +); + +const blank = new Request('http://x/', { + headers: { 'cf-connecting-ip': ' ', 'x-webjs-remote-ip': PEER }, +}); +assert.equal( + clientIp(blank, { trustProxy: true, header: 'cf-connecting-ip' }), + PEER, + `${runtime}: a blank named header must not become a shared bucket key`, +); + +assert.equal( + clientIp(new Request('http://x/'), { trustProxy: true, header: 'cf-connecting-ip' }), + '_anon_', + `${runtime}: nothing to read at all resolves to the anon fallback`, +); + +// --- the option cannot grant trust on its own -------------------------------- + +assert.equal( + clientIp(cdnRequest({ 'x-webjs-remote-ip': PEER }), { header: 'cf-connecting-ip' }), + PEER, + `${runtime}: the named header is inert without trustProxy`, +); + +console.log(`[rate-limit-client-ip] ${runtime}: client-IP resolution parity OK`); diff --git a/test/bun/rate-limit-client-ip.test.mjs b/test/bun/rate-limit-client-ip.test.mjs new file mode 100644 index 000000000..cfe29eaa5 --- /dev/null +++ b/test/bun/rate-limit-client-ip.test.mjs @@ -0,0 +1,14 @@ +/** + * Run the cross-runtime client-IP resolution proof (#1389) under WHICHEVER + * runtime executes the suite. Picked up by the root `node --test` runner (so + * `npm test` exercises the Node path); CI reaches the Bun path through + * `node scripts/run-bun-tests.js`, which auto-discovers `test/bun/*.test.mjs` + * and re-runs them under `bun`. The proof is a plain assert script + * (`rate-limit-client-ip.mjs`, not `*.test.mjs`, so the runner does not + * double-run it); importing it runs it and throws on any failure. + */ +import { test } from 'node:test'; + +test('rateLimit resolves the client IP identically on this runtime (#1389)', async () => { + await import('./rate-limit-client-ip.mjs'); +}); diff --git a/website/app/docs/rate-limiting/page.ts b/website/app/docs/rate-limiting/page.ts index 226f20c1a..3fae9b102 100644 --- a/website/app/docs/rate-limiting/page.ts +++ b/website/app/docs/rate-limiting/page.ts @@ -36,6 +36,7 @@ export default rateLimit({ window: '1m', max: 10 });
  • max: maximum requests per window. Default: 60.
  • key: a string prefix or a function (req) => string that returns a unique key per client. Default: the framework-stamped socket IP, see Behind a proxy below.
  • trustProxy: when true, the default key resolution honours the leftmost X-Forwarded-For entry, then CF-Connecting-IP, then X-Real-IP, before falling back to the socket IP. Default: false. Inert while WEBJS_NO_TRUST_PROXY=1 is set, which outranks it. See Behind a proxy below for the threat model.
    • +
  • clientIpHeader: name the ONE forwarded header carrying the visitor, e.g. 'cf-connecting-ip'. Requires trustProxy: true, and replaces the chain above rather than extending it. Usually required behind a CDN, see Behind a CDN, name the header.
  • message: error message in the 429 response body. Default: 'Too Many Requests'.
  • store: override the cache store (e.g. a dedicated Redis instance for rate limits).
    • @@ -45,14 +46,29 @@ export default rateLimit({ window: '1m', max: 10 });

    When you're fronted by a reverse proxy or CDN (Cloudflare, nginx, Caddy, Railway, Fly, Render, Vercel, Heroku), the socket IP is the proxy, not the user. Every request shares the same IP and the limiter buckets everyone together. Opt in to forwarded-header parsing:

    -

    A proxy POOL fails the other way, and it is the failure you are more likely to hit, because it does not look like a failure at all. Each proxy in the pool is a separate peer, so each gets its own full allowance and your effective limit is the configured one multiplied by the pool size. The headers stay plausible throughout: every response carries a X-RateLimit-Remaining that counts down correctly for its own bucket, so the limiter reads as working while no visitor is ever refused. The tell is that a fresh connection restarts the count while requests sharing one keep-alive connection do count down. This is what shipped in the feature gallery's rate-limit demo, which is why that demo now sets trustProxy: true.

    +

    A proxy POOL fails the other way, and it is the failure you are more likely to hit, because it does not look like a failure at all. Each proxy in the pool is a separate peer, so each gets its own full allowance and your effective limit is the configured one multiplied by the pool size. The headers stay plausible throughout: every response carries a X-RateLimit-Remaining that counts down correctly for its own bucket, so the limiter reads as working while no visitor is ever refused. The tell is that a fresh connection restarts the count while requests sharing one keep-alive connection do count down. This is what shipped in the feature gallery's rate-limit demo, which is why that demo now sets trustProxy: true and names its header.

    // app/api/auth/middleware.ts import { rateLimit } from '@webjsdev/server'; export default rateLimit({ window: '1m', max: 10, trustProxy: true }); -

    With trustProxy: true, the limiter reads the leftmost X-Forwarded-For entry, then CF-Connecting-IP, then X-Real-IP, then the stamped socket IP, then '_anon_'. Your reverse proxy MUST strip any inbound X-Forwarded-For from the wire before adding its own; otherwise trustProxy re-introduces the spoofability it exists to defend against. Cloudflare, Fly, Railway, Render, and Vercel all strip by default. Nginx and Caddy strip only if explicitly configured (proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for in nginx).

    +

    Behind a CDN, name the header

    + +

    trustProxy: true alone is often NOT enough, and this is the part that costs people a debugging session. The default chain starts at the leftmost X-Forwarded-For entry, which behind Cloudflare is Cloudflare's own EGRESS address, not the visitor. Those are pinned per connection, so you get one bucket per connection: a page that pings on a button click counts down correctly and looks fixed, while every fresh connection starts a new window and nobody is ever refused. Name the header that actually carries the visitor:

    + + export default rateLimit({ + window: '1m', + max: 10, + trustProxy: true, + clientIpHeader: 'cf-connecting-ip', +}); + +

    When clientIpHeader is set it is the ONLY wire header read, falling back to the stamped socket IP and then '_anon_'. A blank value falls through rather than becoming a bucket key every visitor shares, and a comma chain is split, so a proxy that appends to the header cannot mint a bucket per hop. It requires trustProxy: true, because naming a header to trust IS the trust decision.

    + +

    WebJs does not prefer CF-Connecting-IP for you, and the reason is worth stating: Cloudflare OVERWRITES that header, which makes it unforgeable behind Cloudflare and forgeable everywhere else. Preferring it globally would let a client on an nginx or bare-platform deploy send CF-Connecting-IP and outrank the X-Forwarded-For the real proxy set. Which header is trustworthy is a fact about your topology, so your app states it. Name the one YOUR edge sets and overwrites: cf-connecting-ip for Cloudflare, x-real-ip for a typical nginx setup, and the leftmost X-Forwarded-For entry (the default, no option needed) when a single trusted proxy sets that chain.

    + +

    With trustProxy: true and no clientIpHeader, the limiter reads the leftmost X-Forwarded-For entry, then CF-Connecting-IP, then X-Real-IP, then the stamped socket IP, then '_anon_'. Your reverse proxy MUST strip any inbound X-Forwarded-For from the wire before adding its own; otherwise trustProxy re-introduces the spoofability it exists to defend against. Cloudflare, Fly, Railway, Render, and Vercel all strip by default. Nginx and Caddy strip only if explicitly configured (proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for in nginx).

    WEBJS_NO_TRUST_PROXY=1 OUTRANKS this option. That env var is the operator's statement that nothing trusted sits in front of the container, and it governs every forwarded-header read in the framework, so while it is set trustProxy: true is ignored and the limiter keys on the stamped socket IP (or '_anon_' when there is none), logging one warning per process. The switch can only ever subtract trust, never grant it. Setting both is a misconfiguration, and it costs you: every visitor behind the proxy shares one bucket, because the proxy is the only peer the socket ever sees. Unset the env var on a genuinely proxied deploy.