Skip to content

Commit 7912dc3

Browse files
committed
fix: key the rate-limit demos on the visitor, not the proxy
The gallery's rate-limit card promises five requests per ten seconds and did not deliver one on the deployed site. Its middleware took the default bucket key, which is the socket peer, and behind Cloudflare plus Railway that peer is an edge proxy rather than the visitor. The pool has several addresses, each carrying its own full allowance, so the effective limit was five times the pool size and refreshing never produced a 429. Nothing about it looked broken, which is why it survived. Every response still carried an X-RateLimit-Remaining that counted down correctly inside its own bucket. The tell only shows over one keep-alive connection, where the requests share a peer: the count descends there and resets on a fresh connection. Both demos now pass trustProxy: true, so the key is the forwarded client address. The comments say what the default keys on and what a CDN does to it, since this file is copied into every generated app and the old comment's "keyed by client IP by default" is the sentence that made the bug easy to write. The framework limiter needed no change. It behaves correctly on Node and on Bun locally, where the peer really is the visitor.
1 parent d1ecf97 commit 7912dc3

7 files changed

Lines changed: 122 additions & 9 deletions

File tree

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,9 @@ import { rateLimit } from '@webjsdev/server';
109109
export default rateLimit({ window: '1m', max: 60 });
110110
```
111111

112-
Options: `window` (ms or a string like `'1m'`), `max`, `key` (a string prefix or a `(req) => string` function, defaults to the client IP), `message`, `store`, `trustProxy` (honour the forwarded-IP headers; inert while `WEBJS_NO_TRUST_PROXY=1` is set, which outranks it and keeps the limiter on the framework-stamped peer). Over-limit responds `429` with `Retry-After` and `X-RateLimit-*` headers; an allowed response carries the remaining-quota headers too. For multi-instance scaling, set the global store to Redis once at startup.
112+
Options: `window` (ms or a string like `'1m'`), `max`, `key` (a string prefix or a `(req) => string` function, defaults to the framework-stamped socket peer), `message`, `store`, `trustProxy` (honour the forwarded-IP headers; inert while `WEBJS_NO_TRUST_PROXY=1` is set, which outranks it and keeps the limiter on the framework-stamped peer). Over-limit responds `429` with `Retry-After` and `X-RateLimit-*` headers; an allowed response carries the remaining-quota headers too. For multi-instance scaling, set the global store to Redis once at startup.
113+
114+
**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.
113115

114116
## Broadcast
115117

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,27 @@
11
// Per-segment middleware. It sits in the ping/ folder, so it applies ONLY to
22
// /features/rate-limit/ping (its route.ts), not to the demo page one level up.
3-
// rateLimit() returns a standard webjs middleware: return a Response to
4-
// short-circuit (the 429), or call next() to continue. Keyed by client IP by
5-
// default; pass `key` to key by user id, API key, etc.
3+
// rateLimit() returns a standard WebJs middleware: return a Response to
4+
// short-circuit (the 429), or call next() to continue. Pass `key` to bucket by
5+
// user id, API key, or anything else instead of by IP.
6+
//
7+
// `trustProxy: true` is the load-bearing option here, and it is why this demo
8+
// works on the deployed site. WITHOUT it the bucket key is the socket peer,
9+
// which is correct only when the visitor's browser is the thing connecting.
10+
// Behind a CDN or a platform router the peer is that proxy, so every visitor
11+
// sharing one proxy shares one bucket, and (worse for a limiter) a proxy POOL
12+
// hands out one bucket per proxy, which multiplies the real limit by the pool
13+
// size. WITH it the key comes from the forwarded client address instead.
14+
//
15+
// The tradeoff is real and worth knowing before copying this line: the proxy
16+
// in front of you MUST strip an inbound X-Forwarded-For before adding its own,
17+
// or a client can forge the header and pick its own bucket. WEBJS_NO_TRUST_PROXY=1
18+
// also outranks this option and puts the limiter back on the socket peer.
19+
// /docs/rate-limiting has the full threat model.
620
import { rateLimit } from '@webjsdev/server';
721

8-
export default rateLimit({ window: '10s', max: 5, message: 'Slow down: five requests per ten seconds.' });
22+
export default rateLimit({
23+
window: '10s',
24+
max: 5,
25+
trustProxy: true,
26+
message: 'Slow down: five requests per ten seconds.',
27+
});
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { test } from 'node:test';
2+
import assert from 'node:assert/strict';
3+
import { fileURLToPath } from 'node:url';
4+
import { dirname, resolve } from 'node:path';
5+
6+
import { createRequestHandler } from '@webjsdev/server';
7+
import { testRequest } from '@webjsdev/server/testing';
8+
9+
const appDir = resolve(dirname(fileURLToPath(import.meta.url)), '..', '..');
10+
11+
const PING = '/features/rate-limit/ping';
12+
13+
// The demo's own numbers, so a change to the middleware that these tests do not
14+
// notice is a change that made them stale rather than one they tolerated.
15+
const MAX = 5;
16+
17+
// Each test picks its own visitor addresses. The limiter counts into the global
18+
// in-memory cache store, which outlives a handler instance, so two tests sharing
19+
// an address would share a bucket and the second would start already exhausted.
20+
function ping(handle: (req: Request) => Promise<Response>, forwardedFor: string) {
21+
return testRequest(handle, PING, { headers: { 'x-forwarded-for': forwardedFor } });
22+
}
23+
24+
test('the demo limits one visitor to five requests per window', async () => {
25+
const app = await createRequestHandler({ appDir, dev: true });
26+
const visitor = '203.0.113.10';
27+
28+
for (let i = 1; i <= MAX; i += 1) {
29+
const res = await ping(app.handle, visitor);
30+
assert.equal(res.status, 200, `request ${i} is inside the window`);
31+
assert.equal(res.headers.get('x-ratelimit-remaining'), String(MAX - i));
32+
}
33+
34+
const limited = await ping(app.handle, visitor);
35+
assert.equal(limited.status, 429, 'the sixth request is refused');
36+
assert.equal(limited.headers.get('retry-after'), '10');
37+
});
38+
39+
// This is the assertion the deployed bug would have failed. Both visitors reach
40+
// the app through the same proxy, so the socket peer is identical for both and a
41+
// peer-keyed limiter would count them into ONE bucket: exhausting the first
42+
// would refuse the second. Keying on the forwarded address keeps them apart.
43+
//
44+
// Counterfactual, proven at this commit: removing `trustProxy: true` from
45+
// gallery/app/features/rate-limit/ping/middleware.ts fails this test on the last
46+
// assertion (the second visitor gets a 429), while the single-visitor test above
47+
// still passes. That asymmetry is the point, since the single-visitor test is
48+
// what a peer-keyed limiter satisfies too.
49+
test('one visitor exhausting the window does not refuse another behind the same proxy', async () => {
50+
const app = await createRequestHandler({ appDir, dev: true });
51+
const noisy = '203.0.113.20';
52+
const bystander = '203.0.113.21';
53+
54+
for (let i = 0; i < MAX; i += 1) await ping(app.handle, noisy);
55+
assert.equal((await ping(app.handle, noisy)).status, 429, 'the noisy visitor is limited');
56+
57+
const other = await ping(app.handle, bystander);
58+
assert.equal(other.status, 200, 'a different visitor keeps their own window');
59+
assert.equal(other.headers.get('x-ratelimit-remaining'), String(MAX - 1));
60+
});

packages/cli/lib/api-gallery.js

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,26 @@ export async function writeApiGallery(appDir) {
6969
"// Per-segment middleware: it sits beside this route, so it rate-limits ONLY",
7070
"// /api/features/rate-limit. rateLimit() is backed by the pluggable cache store",
7171
"// (in-memory by default; point it at Redis to share the window across nodes).",
72+
"//",
73+
"// `trustProxy: true` decides WHAT gets counted. Without it the bucket key is",
74+
"// the socket peer, which is the visitor only when the browser connects to you",
75+
"// directly. Behind a CDN or a platform router the peer is that proxy, so a",
76+
"// proxy POOL hands out one bucket per proxy and multiplies your real limit by",
77+
"// the pool size. With it the key is the forwarded client address instead.",
78+
"//",
79+
"// It has a precondition: the proxy in front MUST strip an inbound",
80+
"// X-Forwarded-For before adding its own, or a client can forge the header and",
81+
"// pick its own bucket. Serving with nothing in front? Drop the option, since",
82+
"// then the socket peer IS the visitor. WEBJS_NO_TRUST_PROXY=1 outranks it either",
83+
"// way. https://webjs.dev/docs/rate-limiting has the full threat model.",
7284
"import { rateLimit } from '@webjsdev/server';",
7385
"",
74-
"export default rateLimit({ window: '10s', max: 5, message: 'Slow down: five requests per ten seconds.' });",
86+
"export default rateLimit({",
87+
" window: '10s',",
88+
" max: 5,",
89+
" trustProxy: true,",
90+
" message: 'Slow down: five requests per ten seconds.',",
91+
"});",
7592
"",
7693
].join('\n'));
7794
await writeFile(feat('rate-limit', 'route.ts'), [

packages/cli/templates/scripts/clear-gallery.mjs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,13 @@ if (!existsSync(join(root, 'app/features'))) {
5353

5454
// 1) Gallery route trees + example metadata routes. `app/api/auth` is the auth
5555
// card's createAuth handler (it lives at the app root, not under app/features/,
56-
// because createAuth hardcodes /api/auth/*), and `test/auth` is the auth card's
57-
// request-pipeline test, so both are removed here alongside the card.
56+
// because createAuth hardcodes /api/auth/*), and `test/auth` + `test/rate-limit`
57+
// are card-owned request-pipeline tests, so they are removed alongside their
58+
// cards. A card that ships a test under test/ MUST be listed here: the prune
59+
// below only removes test/ once it is EMPTY, so a missed entry silently leaves
60+
// the reset app with a test suite for a card it no longer has.
5861
const galleryPaths = [
59-
'app/features', 'app/examples', 'app/sitemaps', 'app/api/auth', 'test/auth',
62+
'app/features', 'app/examples', 'app/sitemaps', 'app/api/auth', 'test/auth', 'test/rate-limit',
6063
'app/icon.ts', 'app/apple-icon.ts', 'app/manifest.ts', 'app/opengraph-image.ts',
6164
'app/twitter-image.ts', 'app/robots.ts', 'app/sitemap.ts',
6265
'app/global-error.ts', 'app/global-not-found.ts',

test/scaffolds/scaffold-gallery.test.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,12 @@ test('full-stack scaffold ships feature demos and one example app', async () =>
8181
assert.ok(await exists(join(appDir, 'app', 'features', 'broadcast', 'feed', 'route.ts')));
8282
assert.ok(await exists(join(appDir, 'app', 'features', 'rate-limit', 'ping', 'route.ts')));
8383
assert.ok(await exists(join(appDir, 'app', 'features', 'rate-limit', 'ping', 'middleware.ts')));
84+
// The limiter must key on the FORWARDED client, not the socket peer, or the
85+
// demo counts proxies instead of visitors the moment the app is deployed
86+
// behind anything (#1387). This is an emitted file, so assert the generated
87+
// bytes rather than trusting the source it was copied from.
88+
const rateLimitMw = await readFile(join(appDir, 'app', 'features', 'rate-limit', 'ping', 'middleware.ts'), 'utf8');
89+
assert.match(rateLimitMw, /trustProxy:\s*true/, 'gallery rate-limit demo trusts the proxy');
8490
assert.ok(await exists(join(appDir, 'app', 'features', 'file-storage', 'file', '[key]', 'route.ts')));
8591
// Root-only boundaries + metadata image routes (the convention-file demos).
8692
for (const f of ['global-error.ts', 'global-not-found.ts', 'icon.ts', 'apple-icon.ts', 'opengraph-image.ts', 'twitter-image.ts']) {
@@ -318,6 +324,10 @@ test('the api template ships the backend-features showcase, not the UI gallery',
318324
assert.ok(await exists(join(appDir, 'app', 'api', 'features', name, 'route.ts')), `api backend demo ${name}`);
319325
}
320326
assert.ok(await exists(join(appDir, 'app', 'api', 'features', 'rate-limit', 'middleware.ts')), 'rate-limit middleware');
327+
// Same requirement as the UI gallery's copy (#1387), and this one is emitted
328+
// from a string template, so a quoting slip only shows in generated bytes.
329+
const apiRateLimitMw = await readFile(join(appDir, 'app', 'api', 'features', 'rate-limit', 'middleware.ts'), 'utf8');
330+
assert.match(apiRateLimitMw, /trustProxy:\s*true/, 'api rate-limit demo trusts the proxy');
321331
assert.ok(await exists(join(appDir, 'app', 'api', 'features', 'files', '[key]', 'route.ts')), 'file serve route');
322332
assert.ok(await exists(join(appDir, 'modules', 'widgets', 'actions', 'create-widget.server.ts')), 'widgets action');
323333
assert.ok(await exists(join(appDir, 'env.ts')), 'env-validation demo at the app root');

website/app/docs/rate-limiting/page.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ export default rateLimit({ window: '1m', max: 10 });</code-block>
4545
4646
<p><strong>When you're fronted by a reverse proxy or CDN</strong> (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:</p>
4747
48+
<p>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 <code>X-RateLimit-Remaining</code> 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 <code>trustProxy: true</code>.</p>
49+
4850
<code-block>// app/api/auth/middleware.ts
4951
import { rateLimit } from '@webjsdev/server';
5052

0 commit comments

Comments
 (0)