diff --git a/gitnexus/src/server/validation.ts b/gitnexus/src/server/validation.ts index 54bf73d4f4..bae6a9ad07 100644 --- a/gitnexus/src/server/validation.ts +++ b/gitnexus/src/server/validation.ts @@ -19,7 +19,7 @@ */ import path from 'node:path'; -import rateLimit, { type RateLimitRequestHandler } from 'express-rate-limit'; +import rateLimit, { type RateLimitRequestHandler, ipKeyGenerator } from 'express-rate-limit'; import type { Request } from 'express'; /** @@ -138,6 +138,9 @@ export interface RouteLimiterOverrides { * - keyGenerator: req.ip with a socket.remoteAddress fallback so abruptly * closed connections do not trigger ERR_ERL_UNDEFINED_IP_ADDRESS * (which would 500 the request via Express's default error handler). + * The IP is passed through `ipKeyGenerator` so IPv6 addresses are + * normalised to their /56 subnet — without this, each IPv6 address + * gets its own counter and the limit is trivially bypassed (#1360). * Caller must wire `app.set('trust proxy', ...)` correctly — see * createServer in api.ts. * @@ -151,7 +154,10 @@ export function createRouteLimiter(opts?: RouteLimiterOverrides): RateLimitReque standardHeaders: 'draft-7', legacyHeaders: false, passOnStoreError: true, - keyGenerator: (req: Request) => req.ip ?? req.socket?.remoteAddress ?? 'unknown', + keyGenerator: (req: Request) => { + const ip = req.ip ?? req.socket?.remoteAddress; + return ip ? ipKeyGenerator(ip) : 'unknown'; + }, message: { error: 'Too many requests, please try again later.' }, ...opts, }); diff --git a/gitnexus/test/unit/rate-limit.test.ts b/gitnexus/test/unit/rate-limit.test.ts index 5a79e60852..b939c271e1 100644 --- a/gitnexus/test/unit/rate-limit.test.ts +++ b/gitnexus/test/unit/rate-limit.test.ts @@ -78,6 +78,16 @@ describe('createRouteLimiter — defaults', () => { // express middleware signature is (req, res, next) — 3 args. expect(limiter.length).toBe(3); }); + + // Regression guard for #1360 — createRouteLimiter must not throw + // ERR_ERL_KEY_GEN_IPV6. The validation fires at construction time + // (inside `rateLimit()`), so a simple `createRouteLimiter()` call is + // the canary: if the keyGenerator references `req.ip` without using + // `ipKeyGenerator`, the `rateLimit()` constructor throws before the + // middleware is ever invoked. + it('does not throw ERR_ERL_KEY_GEN_IPV6 on construction (#1360)', () => { + expect(() => createRouteLimiter()).not.toThrow(); + }); }); describe('createRouteLimiter — integration with a real route', () => { @@ -252,3 +262,28 @@ describe('production routes — rate-limit middleware wiring', () => { ); }); }); + +// Structural guard for #1360 — validates that the validation module uses +// `ipKeyGenerator` so IPv6 addresses are normalised to their /56 subnet. +// Without this, each IPv6 address gets an independent counter and the +// rate-limit is trivially bypassed. The construction-time test above +// catches the same regression behaviourally; this source-grep test catches +// it structurally so the failure message is immediately obvious. +describe('validation.ts — IPv6 key normalisation (#1360)', () => { + let validationSource: string; + + beforeAll(async () => { + validationSource = await fs.readFile( + path.join(__dirname, '..', '..', 'src', 'server', 'validation.ts'), + 'utf-8', + ); + }); + + it('imports ipKeyGenerator from express-rate-limit', () => { + expect(validationSource).toMatch(/import.*ipKeyGenerator.*from\s+'express-rate-limit'/); + }); + + it('keyGenerator body calls ipKeyGenerator', () => { + expect(validationSource).toMatch(/ipKeyGenerator\(ip\)/); + }); +});