Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions gitnexus/src/server/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -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.
*
Expand All @@ -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,
});
Expand Down
35 changes: 35 additions & 0 deletions gitnexus/test/unit/rate-limit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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\)/);
});
});
Loading