mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
434 lines
18 KiB
TypeScript
434 lines
18 KiB
TypeScript
/**
|
||
* 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<void> =>
|
||
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('GET /api/repo is wired with createRouteLimiter', () => {
|
||
expect(apiSource).toMatch(/app\.get\('\/api\/repo',\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<number[]> => {
|
||
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,
|
||
]);
|
||
});
|
||
});
|