GitNexus/gitnexus/test/unit/rate-limit.test.ts
eddie.pan 5ce448a93a
feat(wiki): support local Claude and Codex providers (#1769)
* feat(wiki): support local Claude and Codex providers

* chore(autofix): apply prettier + eslint fixes via /autofix command

* fix(wiki): address local CLI provider review findings

- Add subprocess timeout: LocalCLIConfig gains requestTimeoutMs,
  runLocalCLI sets a kill timer that rejects with an actionable error
  matching the HTTP timeout message format. --timeout is no longer
  silently ignored for claude/codex providers.
- Add windowsHide: true to spawn() to prevent console window flash on
  Windows, matching cursor-client.ts behavior.
- Skip GITNEXUS_MODEL env var for local providers so a user's OpenAI
  model name doesn't cross-contaminate claude/codex CLI invocations.
  Precedence for local providers: --model → savedLocalModel → ''.
- Guard against empty stdout: reject with actionable error when CLI
  exits 0 but produces no output, preventing silent empty wiki pages.

* fix(wiki): address deep-review findings in local CLI providers

- Move empty-output guard from runLocalCLI to per-provider callers so
  Codex can read --output-last-message file even when stdout is empty
- Merge existing config in interactive setup (local + Azure paths) to
  prevent saveCLIConfig from erasing previously saved API keys
- Use StringDecoder for stdout/stderr to handle multi-byte UTF-8 chars
  split across pipe chunk boundaries
- Distinguish ENOENT from non-zero exit in detectLocalCLI so users see
  auth guidance instead of misleading "CLI not found" when the binary
  exists but is not authenticated

* test(wiki): add subprocess contract tests for local CLI providers

Add 21 integration-level tests covering the Claude and Codex subprocess
contracts that wiki-flags.test.ts mocks out:

- Claude argv: -p, --output-format text, --no-session-persistence,
  --model conditional, stdin prompt content, CI=1, windowsHide:true
- Codex argv: exec subcommand, --sandbox read-only, -c approval_policy,
  --output-last-message temp path, --cd, stdin marker, --model
- Timeout: kill timer fires and rejects, no timer when unset
- Codex file fallback: stdout used when file missing, error when both empty
- detectLocalCLI: warn on non-ENOENT, silent on ENOENT
- onChunk: cumulative byte count forwarded

Also register the test in cross-platform-tests.ts SPAWN_CLI section and
fix detectLocalCLI ENOENT detection logic (invert the check so non-ENOENT
errors produce a warning).

* fix(wiki): platform-aware process tree kill and Codex contract snapshot

- Add killChildTree helper that uses taskkill /T /F /PID on Windows to
  terminate the entire process tree (including cmd.exe grandchildren),
  with fallback to child.kill() if taskkill fails or on non-Windows
- Add Codex CLI flag contract snapshot test that locks the exact spawn
  args — any flag rename, reorder, or removal is caught immediately
- Add Windows taskkill tests: success path asserts taskkill called with
  correct PID and /T /F flags, failure path verifies child.kill() fallback

---------

Co-authored-by: eddie.pan2 <eddie.pan2@jtexpress.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Test <test@example.com>
2026-05-25 12:59:48 +01:00

312 lines
13 KiB
TypeScript
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

/**
* 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';
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('POST /api/analyze is wired with createRouteLimiter', () => {
expect(apiSource).toMatch(/app\.post\('\/api\/analyze',\s*createRouteLimiter\(/);
});
it('POST /api/embed is wired with createRouteLimiter', () => {
expect(apiSource).toMatch(/app\.post\('\/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*'\/\*'/);
});
it('createServer wires trust proxy to loopback/linklocal/uniquelocal', () => {
expect(apiSource).toMatch(
/app\.set\(\s*'trust proxy'\s*,\s*'loopback,\s*linklocal,\s*uniquelocal'\s*\)/,
);
});
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\)/);
});
});