/** * Tests for createRouteLimiter and the integration shape used by api.ts. * * Closes the U4 test gap (CodeQL js/missing-rate-limiting). Without these, * a refactor that drops the limiter middleware from any route would silently * regress and CodeQL would re-fire — but no test would fail before reaching * CI. * * Two layers of coverage: * 1. Helper unit tests — createRouteLimiter returns distinct middleware * per call, has the right signature, exposes the right error shape. * 2. Integration tests — mount the same factory on a tiny isolated express * app that does fs.readFile (the exact CodeQL sink class) and prove the * 429 fires after the configured limit. windowMs (2 000 ms) is generous * enough that 4 sequential requests fit inside one window even on slow * Windows CI runners; each test uses a fresh limiter so counter state * never carries between tests. */ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest'; import express, { type Express } from 'express'; import http from 'node:http'; import path from 'node:path'; import fs from 'node:fs/promises'; import os from 'node:os'; import { createRouteLimiter } from '../../src/server/validation.js'; import { DEFAULT_TRUST_PROXY, TRUST_PROXY_ENV, resolveTrustProxy, } from '../../src/server/middleware.js'; import { _captureLogger, type LoggerCapture } from '../../src/core/logger.js'; let tmpFile: string; beforeAll(async () => { // Real fs.readFile target so the route does the same kind of FS work // the production routes do — keeps the test honest about what it covers. tmpFile = path.join( await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-ratelimit-')), 'fixture.txt', ); await fs.writeFile(tmpFile, 'hello\n', 'utf-8'); }); afterAll(async () => { await fs.rm(path.dirname(tmpFile), { recursive: true, force: true }); }); // Build a fresh app + server per test so counter state never carries between // tests. windowMs = 2 000 ms gives ample headroom for Windows CI where // sequential loopback HTTP requests can take 50–80 ms each. const buildApp = (limit: number, windowMs = 2000): Express => { const app = express(); app.set('trust proxy', 'loopback, linklocal, uniquelocal'); app.get('/test/file', createRouteLimiter({ windowMs, limit }), async (_req, res) => { const content = await fs.readFile(tmpFile, 'utf-8'); res.json({ content }); }); return app; }; const startServer = (app: Express): Promise<{ server: http.Server; baseUrl: string }> => new Promise((resolve) => { const server = app.listen(0, '127.0.0.1', () => { const addr = server.address(); const baseUrl = typeof addr === 'object' && addr ? `http://127.0.0.1:${addr.port}` : ''; resolve({ server, baseUrl }); }); }); const stopServer = (server: http.Server): Promise => new Promise((resolve) => server.close(() => resolve())); describe('createRouteLimiter — defaults', () => { it('returns a different middleware instance per call (independent counters)', () => { const a = createRouteLimiter(); const b = createRouteLimiter(); expect(a).not.toBe(b); }); it('produces a callable express RequestHandler', () => { const limiter = createRouteLimiter(); expect(typeof limiter).toBe('function'); // 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', () => { let server: http.Server; let baseUrl: string; beforeEach(async () => { ({ server, baseUrl } = await startServer(buildApp(3))); }); afterEach(async () => { await stopServer(server); }); // The exact regression guard CodeQL would re-fire if a maintainer // dropped createRouteLimiter from any of the 4 protected routes: // without the limiter, max+1 requests all return 200. it('lets max requests through and rejects the next one with 429', async () => { for (let i = 1; i <= 3; i++) { const res = await fetch(`${baseUrl}/test/file`); expect(res.status).toBe(200); } const res = await fetch(`${baseUrl}/test/file`); expect(res.status).toBe(429); const body = await res.json(); expect(body.error).toContain('Too many'); }); it('emits draft-7 RateLimit response header (combined form), not legacy X-RateLimit-*', async () => { const res = await fetch(`${baseUrl}/test/file`); expect(res.status).toBe(200); // draft-7: single combined `RateLimit` header in `limit=N, remaining=N, reset=N` shape, // NO individual `X-RateLimit-*` legacy keys. const rateLimitHeader = res.headers.get('ratelimit'); expect(rateLimitHeader).toMatch(/limit=\d+/); expect(rateLimitHeader).toMatch(/remaining=\d+/); expect(rateLimitHeader).toMatch(/reset=\d+/); expect(res.headers.get('x-ratelimit-limit')).toBeNull(); }); it('429 response body uses the project { error } JSON shape', async () => { // Trip the limiter. for (let i = 1; i <= 3; i++) await fetch(`${baseUrl}/test/file`); const res = await fetch(`${baseUrl}/test/file`); expect(res.status).toBe(429); const body = await res.json(); expect(body).toEqual({ error: expect.stringContaining('Too many') }); }); it('429 response includes a Retry-After header so clients can back off', async () => { for (let i = 1; i <= 3; i++) await fetch(`${baseUrl}/test/file`); const res = await fetch(`${baseUrl}/test/file`); expect(res.status).toBe(429); const retryAfter = res.headers.get('retry-after'); expect(retryAfter).toBeTruthy(); // express-rate-limit v8 emits Retry-After in integer-seconds form. The // RFC also allows HTTP-date, but ERL does not use that shape; if a // future version switches, this assertion needs an HTTP-date branch. const seconds = Number(retryAfter); expect(Number.isFinite(seconds) && seconds >= 0).toBe(true); }); it('window resets after windowMs — counter does not carry across windows', async () => { // Trip the limiter. for (let i = 1; i <= 3; i++) await fetch(`${baseUrl}/test/file`); const tripped = await fetch(`${baseUrl}/test/file`); expect(tripped.status).toBe(429); // Wait for the window to roll over (2 000 ms window + 200 ms margin). await new Promise((r) => setTimeout(r, 2200)); const reset = await fetch(`${baseUrl}/test/file`); expect(reset.status).toBe(200); }); }); // Behavioral pin replacing the prior `expect(DEFAULT_RATE_LIMIT_RPM).toBe(60)` // constant assertion — that test pinned the magic number, this test pins the // observable contract that the production default does not 429 at typical // interactive load. describe('createRouteLimiter — production default', () => { it('default policy permits 60 requests in a minute (no opts override)', async () => { // Build an app that uses the production-default limiter (no opts override). // 60 requests is well under the default 60 rpm/IP, so all should pass. // Going to 61 would 429 but takes the full window to test deterministically; // the contract we want pinned here is "default does not throttle interactive // use" — the 429 path is already covered by the integration tests above. const { server, baseUrl } = await startServer( (() => { const app = express(); app.set('trust proxy', 'loopback, linklocal, uniquelocal'); app.get('/test/file', createRouteLimiter(), async (_req, res) => { const content = await fs.readFile(tmpFile, 'utf-8'); res.json({ content }); }); return app; })(), ); try { // Send 60 requests — all should succeed under the default policy. for (let i = 1; i <= 60; i++) { const res = await fetch(`${baseUrl}/test/file`); if (res.status !== 200) { throw new Error(`request ${i}/60 returned ${res.status} under default policy`); } } } finally { await stopServer(server); } }); }); // Production-wiring assertions — proves each of the 4 protected routes in // api.ts actually has rate-limit middleware. Closes the gap reviewers flagged // where a maintainer could drop createRouteLimiter from a route and no test // would fail (only CodeQL would re-fire next scan). // // Walks the express router stack on a real createServer-built app, finds // each protected route by method+path, and asserts the middleware chain // includes the express-rate-limit handler. This is intentionally a // structural check (not behavioral) — the behavioral guarantees are // covered by the integration tests above. describe('production routes — rate-limit middleware wiring', () => { // Small structural check that does not require booting the full server // (which depends on LadybugDB, MCP transport, fork(), etc.). We grep the // api.ts source for the createRouteLimiter call adjacent to each route // registration. If a future refactor drops the call, the regex no longer // matches and the test fails. // // This is admittedly a light-weight check, but it is enough to catch the // single most likely regression (someone removes the middleware while // editing the route handler) without dragging in the full server boot. let apiSource: string; beforeAll(async () => { apiSource = await fs.readFile( path.join(__dirname, '..', '..', 'src', 'server', 'api.ts'), 'utf-8', ); }); it('GET /api/file is wired with createRouteLimiter', () => { expect(apiSource).toMatch(/app\.get\('\/api\/file',\s*createRouteLimiter\(/); }); it('GET /api/grep is wired with createRouteLimiter', () => { expect(apiSource).toMatch(/app\.get\('\/api\/grep',\s*createRouteLimiter\(/); }); it('DELETE /api/repo is wired with createRouteLimiter', () => { expect(apiSource).toMatch(/app\.delete\('\/api\/repo',\s*createRouteLimiter\(/); }); it('DELETE /api/repo resolves the registry row without pruning unusable storage', () => { expect(apiSource).toMatch( /resolveRepo\(repoName,\s*false,\s*undefined,\s*\{\s*validateStorage:\s*false\s*\}\)/, ); expect(apiSource).toMatch( /listRegisteredRepos\(\{\s*validate:\s*options\.validateStorage !== false,\s*\}\)/, ); }); it('GET /api/repo is wired with createRouteLimiter', () => { expect(apiSource).toMatch(/app\.get\('\/api\/repo',\s*createRouteLimiter\(/); }); it('GET /api/repos is wired with createRouteLimiter', () => { // Carries a limiter because it spawns a `git rev-list` per registered repo // to answer freshness — an unauthenticated GET whose cost scales with the // number of indexed repos. Pinned here so dropping the limiter fails a test // before it has to be caught by CodeQL (#3232). expect(apiSource).toMatch(/app\.get\('\/api\/repos',\s*createRouteLimiter\(/); }); it('POST /api/analyze is wired with createRouteLimiter', () => { // Tolerate Prettier wrapping the registration across lines (it does once // the route carries extra middleware like requireTrustedOrigin). expect(apiSource).toMatch(/app\.post\(\s*'\/api\/analyze',\s*createRouteLimiter\(/); }); it('POST /api/embed is wired with createRouteLimiter', () => { // Tolerate Prettier wrapping the registration across lines (it does once // the route carries extra middleware like requireTrustedOrigin). expect(apiSource).toMatch(/app\.post\(\s*'\/api\/embed',\s*createRouteLimiter\(/); }); it('SPA fallback is wired with createRouteLimiter', () => { expect(apiSource).toMatch(/app\.get\(SPA_FALLBACK_REGEX,\s*createRouteLimiter\(/); }); it('GET /api/health is registered (Docker healthcheck, #1147)', () => { expect(apiSource).toMatch(/app\.get\('\/api\/health',\s*\(_req,\s*res\)\s*=>/); }); it('does not register a bare wildcard OPTIONS route under Express 5', () => { expect(apiSource).not.toContain("app.options('*'"); expect(apiSource).not.toMatch(/app\.options\(\s*'\/\*'/); }); // Source-level because createServer listens and cannot be built here. Kept // deliberately loose: the effective-value describe below covers resolution, // so all this has to pin down is that createServer routes the env var // through resolveTrustProxy rather than setting a literal. it('createServer reads trust proxy from GITNEXUS_TRUST_PROXY', () => { expect(apiSource).toMatch( /app\.set\(\s*'trust proxy'\s*,\s*resolveTrustProxy\([^)]*TRUST_PROXY_ENV/, ); }); it('does not register Express-4-only app.options("*") (Express 5 path-to-regexp)', () => { expect(apiSource).not.toMatch(/app\.options\(\s*'\*'/); expect(apiSource).not.toMatch(/app\.options\(\s*'\/\*'/); }); it('sets PNA header middleware before cors (preflight must include Allow-Private-Network)', () => { expect(apiSource).toMatch( /Access-Control-Allow-Private-Network[\s\S]*?app\.use\(\s*\n?\s*cors\(/, ); }); it('embed route flushes WAL via flushWAL, not inline executeQuery (#1376)', () => { // The embed handler must call the consolidated helper, not hand-roll // its own try/catch around executeQuery('CHECKPOINT'). expect(apiSource).toMatch(/await flushWAL\(\)/); expect(apiSource).not.toMatch(/executeQuery\('CHECKPOINT'\)/); }); }); // 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\)/); }); }); // The effective `trust proxy` Express ends up with, rather than the text of the // line that sets it. `createServer` listens and installs signal handlers, so it // cannot be built here; this mirrors its one `app.set` expression instead. describe('trust proxy — effective value from GITNEXUS_TRUST_PROXY', () => { const saved = process.env[TRUST_PROXY_ENV]; // The rejection cases below warn; capture keeps them out of the suite output. let cap: LoggerCapture; beforeEach(() => { cap = _captureLogger(); }); afterEach(() => { cap.restore(); if (saved === undefined) delete process.env[TRUST_PROXY_ENV]; else process.env[TRUST_PROXY_ENV] = saved; }); const effectiveTrustProxy = (value: string | undefined): unknown => { if (value === undefined) delete process.env[TRUST_PROXY_ENV]; else process.env[TRUST_PROXY_ENV] = value; const app = express(); app.set('trust proxy', resolveTrustProxy(process.env[TRUST_PROXY_ENV])); return app.get('trust proxy'); }; it('defaults to the loopback-scoped list when the env var is unset', () => { expect(DEFAULT_TRUST_PROXY).toBe('loopback, linklocal, uniquelocal'); expect(effectiveTrustProxy(undefined)).toBe('loopback, linklocal, uniquelocal'); }); it('carries a configured hop count through to Express', () => { expect(effectiveTrustProxy('3')).toBe(3); }); it('carries a configured proxy list through to Express', () => { expect(effectiveTrustProxy('10.0.0.0/8, 127.0.0.1')).toBe('10.0.0.0/8, 127.0.0.1'); }); // Express 5 compiles `trust proxy` inside `app.set`, so an unvalidated bad // value would throw during createServer instead of resolving to a default. it('never hands Express a value it rejects', () => { expect(() => effectiveTrustProxy('garbage')).not.toThrow(); expect(effectiveTrustProxy('garbage')).toBe(DEFAULT_TRUST_PROXY); expect(effectiveTrustProxy('9'.repeat(400))).toBe(DEFAULT_TRUST_PROXY); }); // The behaviour the describe block below demonstrates is why: `true` cannot // reach Express through the env var at all. it('never hands Express `true`', () => { expect(effectiveTrustProxy('true')).toBe(DEFAULT_TRUST_PROXY); expect(effectiveTrustProxy('yes')).toBe(DEFAULT_TRUST_PROXY); expect(effectiveTrustProxy('on')).toBe(DEFAULT_TRUST_PROXY); }); }); // Why resolveTrustProxy rejects `true`: Express then reads the leftmost // X-Forwarded-For entry, which the client controls, so rotating it hands the // limiter a fresh key per request — unbounded, in front of the two routes that // spawn workers. A hop count reads from the right instead and is immune. Both // apps below see the same requests; only the setting differs. `true` is set // directly here, since the env var can no longer produce it. describe('trust proxy — a rotating X-Forwarded-For defeats `true` but not a hop count', () => { const buildProxiedApp = (trustProxy: boolean | number): Express => { const app = express(); app.set('trust proxy', trustProxy); app.get('/test/file', createRouteLimiter({ windowMs: 2000, limit: 2 }), async (_req, res) => { const content = await fs.readFile(tmpFile, 'utf-8'); res.json({ content }); }); return app; }; // Leftmost entry rotates per request; the rightmost (the hop the loopback // proxy claims to have received from) stays fixed. const statusesUnderRotatingForwardedFor = async (app: Express): Promise => { const { server, baseUrl } = await startServer(app); try { const statuses: number[] = []; for (let i = 1; i <= 4; i++) { const res = await fetch(`${baseUrl}/test/file`, { headers: { 'X-Forwarded-For': `203.0.113.${i}, 198.51.100.7` }, }); statuses.push(res.status); } return statuses; } finally { await stopServer(server); } }; it('collapses to a single limiter key with a hop count of 1', async () => { expect(await statusesUnderRotatingForwardedFor(buildProxiedApp(1))).toEqual([ 200, 200, 429, 429, ]); }); it('gets a fresh limiter key per request with `true`', async () => { expect(await statusesUnderRotatingForwardedFor(buildProxiedApp(true))).toEqual([ 200, 200, 200, 200, ]); }); });