fix(backend): stop IPv6 callers bypassing the password-reset rate limit (#84)

The QA stack has been logging ERR_ERL_KEY_GEN_IPV6 at every boot, and express-rate-limit was right to complain.

keyByCallerAndEmail built its key from req.ip raw. For an IPv4 caller that is one address and the limiter worked as intended. For an IPv6 caller it is the full 128 bits — and a residential IPv6 customer is delegated an entire prefix and can source every request from a different address inside it at no cost. Keyed that way, each request counted as a new caller and the allowance of five per fifteen minutes never bound at all.

That matters more here than it would elsewhere, because of what this limiter is for. Its own comment says it: without one, anyone can make the server send unlimited mail to any address they choose. For IPv6 clients there effectively was no limiter, while the code read as though there were.

The caller half of the key now goes through express-rate-limit's ipKeyGenerator, which groups IPv6 by prefix and returns IPv4 unchanged. The helper's default is /56 rather than /64, and that default is kept deliberately: /56 covers a whole delegated site, so an attacker cannot escape their bucket by moving within their own allocation. It does mean several households behind one delegation share an allowance — acceptable only because the key also carries the email address, so they collide just when targeting the same account. The reasoning sits next to the code, because a future reader tightening it to /64 would silently reopen the hole.

keyByCallerAndEmail is now exported so it can be tested directly. The limiter's allowance is still not asserted anywhere, and should not be: its store is process-wide, so a test that exhausts it leaks into every later test from the same address and fails something unrelated later. The key function is pure, and it is where the bug was.

Verified by firing the guard rather than reasoning about it: building main and loading the module reproduces the ValidationError, and the same load with this change is silent. Seven new unit tests cover an IPv4 caller unchanged, two addresses in one delegation collapsing to a single key, separate delegations staying apart, an IPv4-mapped address keying the same as the plain IPv4 one, email normalisation, a non-string email, and a request with no address at all.

86 unit tests pass, 144 integration, lint 0 errors and 8 warnings — unchanged.

Closes #84
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

backend/src/rateLimit.ts

@@ -1,4 +1,4 @@
-import rateLimit from 'express-rate-limit';
+import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
 import { Request } from 'express';
 
 // First rate limiting in the codebase. The password-reset endpoints need it
@@ -20,9 +20,30 @@ const MAX_REQUESTS = 5;
 // This key only makes sense on a request that carries an email. Applying the
 // same limiter to an endpoint without one collapses every caller into a single
 // `ip:` bucket, which is a shared allowance rather than a per-caller one.
-function keyByCallerAndEmail(req: Request): string {
+//
+// The caller half goes through ipKeyGenerator rather than using req.ip raw.
+// A raw IPv6 address is the full 128 bits, but a residential IPv6 customer is
+// delegated an entire prefix and can source every request from a different
+// address inside it for free — so keyed on the exact address this limiter
+// counted each request as a new caller and never bound at all. That is not a
+// small miss: this limiter is the only thing stopping anyone making the server
+// send unlimited mail to any address they choose. express-rate-limit reported
+// it as ERR_ERL_KEY_GEN_IPV6 on every boot; see #84.
+//
+// The helper's default groups IPv6 by /56 rather than /64. That is the
+// deliberate choice: /56 covers a whole delegated site, so an attacker cannot
+// escape the bucket by moving within their own allocation. It does mean
+// several households behind one delegation share an allowance — acceptable
+// here only because the key also contains the email address, so they collide
+// just when targeting the same account. IPv4 is returned unchanged.
+//
+// Exported for the unit test. The limiter's own allowance is not worth
+// asserting in a test — its store is process-wide, so exhausting it leaks into
+// every later test from the same address — but the key function is pure and
+// is where the bug actually was.
+export function keyByCallerAndEmail(req: Request): string {
   const email = typeof req.body?.email === 'string' ? req.body.email.toLowerCase().trim() : '';
-  return `${req.ip}:${email}`;
+  return `${ipKeyGenerator(req.ip ?? '')}:${email}`;
 }
 
 export const passwordResetRequestLimiter = rateLimit({

backend/tests/unit/rateLimitKey.test.ts

@@ -0,0 +1,66 @@
+import { Request } from 'express';
+import { keyByCallerAndEmail } from '../../src/rateLimit';
+
+// The limiter's allowance is deliberately not asserted anywhere. Its store is
+// in-memory and process-wide, so a test that exhausts it leaks that state into
+// every later test keyed on the same address and produces an order-dependent
+// failure somewhere else entirely. The key function is pure, and the IPv6
+// bypass in #84 lived here rather than in the allowance, so this is both the
+// safe thing to test and the thing worth testing.
+function requestFrom(ip: string | undefined, email?: unknown): Request {
+  return { ip, body: email === undefined ? {} : { email } } as unknown as Request;
+}
+
+describe('keyByCallerAndEmail', () => {
+  it('leaves an IPv4 caller as it is', () => {
+    expect(keyByCallerAndEmail(requestFrom('203.0.113.5', 'someone@example.com')))
+      .toBe('203.0.113.5:someone@example.com');
+  });
+
+  // The bug #84 fixed. Before this, these two produced different keys, so an
+  // IPv6 caller got a fresh allowance for every address in their own prefix —
+  // which is to say, no limit at all.
+  it('gives two addresses in one IPv6 delegation the same key', () => {
+    const first = keyByCallerAndEmail(requestFrom('2001:db8:abcd:0100::1', 'target@example.com'));
+    const second = keyByCallerAndEmail(requestFrom('2001:db8:abcd:01ff::9', 'target@example.com'));
+
+    expect(first).toBe(second);
+    expect(first).toBe('2001:db8:abcd:100::/56:target@example.com');
+  });
+
+  // The other half of the same claim: grouping must not be so coarse that
+  // unrelated callers share one allowance.
+  it('keeps separate IPv6 delegations apart', () => {
+    const inside = keyByCallerAndEmail(requestFrom('2001:db8:abcd:0100::1', 'target@example.com'));
+    const elsewhere = keyByCallerAndEmail(requestFrom('2001:db8:abcd:0200::1', 'target@example.com'));
+
+    expect(inside).not.toBe(elsewhere);
+  });
+
+  // Node reports an IPv4 client as an IPv4-mapped IPv6 address in some
+  // configurations. It must key the same either way, or the same caller would
+  // get two allowances depending on how the socket happened to be opened.
+  it('treats an IPv4-mapped address as the IPv4 caller it is', () => {
+    expect(keyByCallerAndEmail(requestFrom('::ffff:203.0.113.5', 'someone@example.com')))
+      .toBe(keyByCallerAndEmail(requestFrom('203.0.113.5', 'someone@example.com')));
+  });
+
+  it('normalizes the email so case and padding cannot buy a second allowance', () => {
+    expect(keyByCallerAndEmail(requestFrom('203.0.113.5', '  SomeOne@Example.COM  ')))
+      .toBe('203.0.113.5:someone@example.com');
+  });
+
+  it('uses an empty email rather than stringifying a non-string one', () => {
+    expect(keyByCallerAndEmail(requestFrom('203.0.113.5', { not: 'a string' })))
+      .toBe('203.0.113.5:');
+    expect(keyByCallerAndEmail(requestFrom('203.0.113.5')))
+      .toBe('203.0.113.5:');
+  });
+
+  // Only reachable if `trust proxy` were misconfigured, but it must not throw
+  // from inside the limiter if it ever happens.
+  it('survives a request with no address at all', () => {
+    expect(keyByCallerAndEmail(requestFrom(undefined, 'someone@example.com')))
+      .toBe(':someone@example.com');
+  });
+});