Files
EventSnap/e2e/specs/01-auth/rate-limit-shared-nat.spec.ts
fabi 32dfe6874a test(e2e): make nine red specs assert the contracts the code actually implements
The e2e suite had never been run during this audit. It failed 9 of 256; seven of those
predated the audit's changes, established by building a stack from a clean HEAD worktree
and running the same specs against it rather than guessing.

Most were stale assertions rather than product defects:

- quota.spec solved for a target limit using the observed uploader count, but the divisor is
  max(active, estimated_guest_count, 1) and that config seeds at 100 — so every limit it
  aimed for came out 100x small and every "within quota" upload 413'd.
- rate-limit-shared-nat destructured `ticket` from a 429 body and fetched with
  `ticket=undefined`, turning the 429 under test into an unrelated 401. It also faked a
  release with no archive on disk, so the mint's pre-check 404'd and the per-day limiter was
  never reached; it now does a real release and asserts 200 rather than "not 429".
- ddos allowed only [200,429] from ten concurrent streams, so it failed on the very defence
  it exercises: four tickets per session survive and the rest correctly 401. Now asserts
  exactly four, which a tightened cap or an inverted eviction order would catch.
- auth-tampering asserted a throttled IP is refused EVEN with the correct password. That
  contract was deliberately removed — it let any phone on the venue NAT lock the operator
  out of their own admin panel, with a circular escape hatch. Inverted, plus a new check
  that a success does not refill an attacker's bucket.
- moderation-ui assumed a ban leaves a comment "stuck on screen"; `list_for_upload` filters
  banned authors, so it is hidden from everyone including the host. Now pins the pair that
  matters — the ban hides it, and the host's permanent removal survives an unban — and the
  UI leg it used to own is restored as a separate test on a reachable comment.

The export specs mint with `?kind=` now that a download ticket is bound to one archive, and
four of them assert the mint's 404 rather than the download's: with the kind always known,
the pre-check refuses up front instead of after charging a daily download for an archive
that cannot be served.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-11 22:44:48 +02:00

301 lines
13 KiB
TypeScript
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
/**
* Regression guard — the door must not close on a venue behind one NAT.
*
* `/join` was throttled 5 per 60s keyed purely on the client IP. Every guest at a venue
* arrives from the same public IP (that is what a NAT is), so the whole party shared one
* bucket: 12 guests scanning the QR code within a few seconds meant 5 got in and 7 were
* turned away — with no Retry-After to tell them when to try again. `/feed` (60/min) and
* `/export` (3/DAY) had the identical defect.
*
* These ran green for the same structural reason every time: the e2e reseed forces every
* limiter toggle OFF before each test, so nothing here was ever exercised. Enable them
* explicitly, exactly as 02-upload/rate-limit does.
*/
import { test, expect } from '../../fixtures/test';
import { BASE } from '../../helpers/env';
import { seedUpload } from '../../helpers/seed';
test.describe('Rate limits — guests behind a shared NAT', () => {
test('a dozen guests can all join from one IP, and 429s carry Retry-After', async ({
api,
adminToken,
}) => {
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
join_rate_enabled: 'true',
});
// Twelve DISTINCT guests, same source IP — the arrival burst at a real party.
const names = Array.from({ length: 12 }, (_, i) => `NatGuest${i}`);
const results = await Promise.all(
names.map((display_name) =>
fetch(`${BASE}/api/v1/join`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ display_name }),
})
)
);
const rejected = results.filter((r) => r.status === 429);
expect(
rejected.length,
`all 12 guests must get in from one IP; ${rejected.length} were turned away`
).toBe(0);
expect(results.every((r) => r.status === 201)).toBe(true);
});
test('one guest retrying their own name is still throttled, and told for how long', async ({
api,
adminToken,
}) => {
// The per-name bucket must still bite — otherwise the NAT fix would have simply
// removed the anti-spam limit rather than re-keyed it.
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
join_rate_enabled: 'true',
});
const attempt = () =>
fetch(`${BASE}/api/v1/join`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ display_name: 'RepeatOffender' }),
});
// 5 per 60s for the same (ip, name): the first succeeds (201), the next four collide
// with the taken name (409), and the sixth exhausts the bucket.
const codes: number[] = [];
for (let i = 0; i < 6; i++) codes.push((await attempt()).status);
expect(codes[0], 'the first join should succeed').toBe(201);
expect(codes.at(-1), 'the 6th attempt on one name must be throttled').toBe(429);
const throttled = await attempt();
expect(throttled.status).toBe(429);
const retryAfter = throttled.headers.get('retry-after');
expect(retryAfter, '429 must tell the client when to come back').toBeTruthy();
expect(Number(retryAfter)).toBeGreaterThan(0);
expect(Number(retryAfter)).toBeLessThanOrEqual(60);
});
test('the feed limit is per-user, not per-IP', async ({ api, adminToken, guest }) => {
// Two guests, one IP. With a limit of 3/min an IP key would let the first guest's
// three reads starve the second entirely.
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
feed_rate_enabled: 'true',
feed_rate_per_min: '3',
});
const a = await guest('FeedHog');
const b = await guest('FeedVictim');
const read = (jwt: string) =>
fetch(`${BASE}/api/v1/feed`, { headers: { Authorization: `Bearer ${jwt}` } });
// Guest A burns their whole allowance.
for (let i = 0; i < 3; i++) expect((await read(a.jwt)).status).toBe(200);
expect((await read(a.jwt)).status, "A's own 4th read is throttled").toBe(429);
// Guest B must be entirely unaffected.
expect((await read(b.jwt)).status, 'B must not inherit As exhausted bucket').toBe(200);
});
test('one guest sweeping /recover cannot lock the venue — or the host — out of PIN recovery', async ({
api,
adminToken,
guest,
}) => {
// The sharpest version of this file's whole premise. `/recover` has a cross-name failure
// budget keyed on IP, meant to catch someone sweeping the public name list. Behind the venue
// NAT that budget is SHARED BY THE ENTIRE PARTY, and it used to be checked before the account
// was even looked up — so it refused a correct PIN.
//
// That is the host's problem specifically: hosts are promoted guests whose only credential is
// a 4-digit PIN, so /recover is their only way back in after losing a session. A guest posting
// invented names could deny it to everyone, indefinitely, for the price of ~2 requests/minute.
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
recover_rate_enabled: 'true',
// Raise the per-IP VOLUME ceiling out of the way. It defaults to 30/min, and the
// cross-name FAILURE budget under test is also 30 — so the sweep below would trip the
// volume limiter first and this test would pass for the wrong reason (a 429 that proves
// nothing about whether a correct PIN survives a spent failure budget).
recover_ip_rate_per_min: '500',
});
const victim = await guest('RecoverVictim');
// Burn the shared per-IP budget with names that do not exist — the cheapest sweep, and the
// one that needs no knowledge of the guest list at all.
for (let i = 0; i < 35; i++) {
await fetch(`${BASE}/api/v1/recover`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ display_name: `Ghost${i}-${Date.now()}`, pin: '0000' }),
});
}
// A real guest, on that same IP, with their REAL PIN, must still get in.
const res = await fetch(`${BASE}/api/v1/recover`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ display_name: 'RecoverVictim', pin: victim.pin }),
});
expect(
res.status,
'a correct PIN must survive a spent cross-name budget — otherwise any guest can lock the ' +
'host out of the only login path they have'
).toBe(200);
// ...and the sweep is still answered as a sweep: a WRONG pin gets 429, not a bare 401, so the
// budget still does its job on the traffic it was built for.
const wrong = await fetch(`${BASE}/api/v1/recover`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ display_name: 'RecoverVictim', pin: '0001' }),
});
expect(wrong.status, 'wrong PINs from an exhausted IP are still throttled').toBe(429);
});
test('the export limit is per-user — one guest cannot spend the whole venues quota', async ({
api,
adminToken,
guest,
host,
}) => {
// The sharpest case: 3 downloads per DAY on an IP key meant the 4th guest to fetch
// their keepsake was locked out until tomorrow.
//
// A REAL release, not `setExportReleased`. `/export/ticket` pre-validates that the archive is
// actually servable and answers 404 without charging the limiter — deliberately, so a guest
// never spends one of their three daily downloads on an archive that cannot be served. With
// only the released FLAG set and no archive on disk, every mint here 404'd and the per-day
// limiter under test was never reached at all.
await seedUpload(host.jwt, { caption: 'for the keepsake' });
expect(
(
await fetch(`${BASE}/api/v1/host/gallery/release`, {
method: 'POST',
headers: { Authorization: `Bearer ${host.jwt}` },
})
).status
).toBe(204);
await expect
.poll(
async () => {
const s = await (
await fetch(`${BASE}/api/v1/export/status`, {
headers: { Authorization: `Bearer ${host.jwt}` },
})
).json();
return s.released === true && s.zip?.status === 'done';
},
{ timeout: 90_000, intervals: [500] }
)
.toBe(true);
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
export_rate_enabled: 'true',
export_rate_per_day: '1',
});
// The per-day export limit is charged at the MINT, not at the download: the ticket endpoint is
// the authenticated chokepoint, while `/export/zip` authenticates by ticket alone so a resumed
// transfer doesn't spend another of the guest's daily allowance. So a throttled guest is
// refused with 429 at `/export/ticket` and never reaches the archive.
//
// This helper used to destructure `ticket` from that 429 body regardless, then fetch with
// `ticket=undefined` — turning the 429 under test into an unrelated 401 from the download
// endpoint. Surface the mint's refusal instead; that IS the throttle.
const mintAndFetch = async (jwt: string) => {
const minted = await fetch(`${BASE}/api/v1/export/ticket?kind=zip`, {
method: 'POST',
headers: { Authorization: `Bearer ${jwt}` },
});
if (!minted.ok) return minted;
const { ticket } = await minted.json();
return fetch(`${BASE}/api/v1/export/zip?ticket=${encodeURIComponent(ticket)}`);
};
const a = await guest('ExportFirst');
const b = await guest('ExportSecond');
// A spends their single daily allowance — on a real archive, so this is a genuine 200 rather
// than merely "not 429", which would have been satisfied by any error at all.
expect((await mintAndFetch(a.jwt)).status, 'As first download must succeed').toBe(200);
expect((await mintAndFetch(a.jwt)).status, 'As second download is throttled').toBe(429);
// B shares A's IP and must still get their keepsake.
expect((await mintAndFetch(b.jwt)).status, 'B must not be locked out by As download').toBe(
200
);
// And the host too, for good measure.
expect((await mintAndFetch(host.jwt)).status).toBe(200);
});
});
test.describe('Rate limits — /recover name cycling', () => {
test('cycling names from one IP hits the ceiling, while one name is still throttled', async ({
api,
adminToken,
}) => {
// /recover is keyed `recover:{ip}:{name}` — right for its job (stopping someone who
// knows a display name from burning the victim's 3-strike PIN counter), but the name is
// ATTACKER-CHOSEN, so cycling names minted a fresh bucket every time. Behind it sits a
// cost-12 bcrypt verify, including an unconditional throwaway one for unknown names, so
// a name generator was the cheapest way to make the server hash forever.
//
// Squeeze the ceiling so the flood is reproducible without firing 30+ requests.
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
recover_rate_enabled: 'true',
recover_ip_rate_per_min: '5',
});
const attempt = (name: string) =>
fetch(`${BASE}/api/v1/recover`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ display_name: name, pin: '0000' }),
});
// Every name is distinct, so the per-name bucket can never fire — only the ceiling can.
const codes: number[] = [];
for (let i = 0; i < 12; i++) codes.push((await attempt(`Unbekannt${i}_${Date.now()}`)).status);
expect(
codes.filter((c) => c === 429).length,
'name cycling must be capped by the per-IP ceiling'
).toBeGreaterThan(0);
const throttled = await attempt(`Unbekannt99_${Date.now()}`);
expect(throttled.status).toBe(429);
expect(Number(throttled.headers.get('retry-after'))).toBeGreaterThan(0);
});
test('the per-name bucket still protects a real account', async ({ api, adminToken, guest }) => {
// The ceiling must not have REPLACED the anti-guessing control. With a generous ceiling,
// repeated wrong PINs against ONE name must still be shut down by the per-name bucket.
const victim = await guest('PinVictim');
await api.patchConfig(adminToken, {
rate_limits_enabled: 'true',
recover_rate_enabled: 'true',
recover_ip_rate_per_min: '1000',
});
const attempt = () =>
fetch(`${BASE}/api/v1/recover`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ display_name: victim.displayName, pin: '9999' }),
});
const codes: number[] = [];
for (let i = 0; i < 7; i++) codes.push((await attempt()).status);
expect(codes.at(-1), 'guessing one name must still be throttled').toBe(429);
});
});