diff --git a/test/auth-security.test.ts b/test/auth-security.test.ts index e7a52c25..2f6e633d 100644 --- a/test/auth-security.test.ts +++ b/test/auth-security.test.ts @@ -8,7 +8,7 @@ * 6. Logout endpoint invalidates session * 7. Settings schema rejects unknown fields * - * Port: 3160 (auth tests), 3161 (loopback no-auth tests), 3162 (network override tests) + * Port: ephemeral (`new WebServer(0, …)`, read back through `boundPort`) */ import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach, vi } from 'vitest'; import { WebServer } from '../src/web/server.js'; @@ -16,11 +16,6 @@ import { TmuxManager } from '../src/tmux-manager.js'; import { SettingsUpdateSchema } from '../src/web/schemas.js'; import { getHookSecret, HOOK_SECRET_HEADER } from '../src/config/hook-secret.js'; -const AUTH_PORT = 3160; -const NOAUTH_PORT = 3161; -const NETWORK_OVERRIDE_PORT = 3162; -const AUTH_RATE_LIMIT_PORT = 3220; -const NOAUTH_NETWORK_PORT = 3221; const TEST_USER = 'admin'; const TEST_PASS = 'test-password-12345'; @@ -30,12 +25,12 @@ function basicAuthHeader(user: string, pass: string): string { return 'Basic ' + Buffer.from(`${user}:${pass}`).toString('base64'); } -async function startAuthServer(port: number): Promise<{ server: WebServer; baseUrl: string }> { +async function startAuthServer(): Promise<{ server: WebServer; baseUrl: string }> { process.env.CODEMAN_PASSWORD = TEST_PASS; process.env.CODEMAN_USERNAME = TEST_USER; - const server = new WebServer(port, false, true); + const server = new WebServer(0, false, true); await server.start(); - return { server, baseUrl: `http://localhost:${port}` }; + return { server, baseUrl: `http://localhost:${server.boundPort}` }; } async function getSessionCookie(baseUrl: string): Promise { @@ -66,9 +61,9 @@ describe('Auth Security', () => { beforeAll(async () => { process.env.CODEMAN_PASSWORD = TEST_PASS; process.env.CODEMAN_USERNAME = TEST_USER; - server = new WebServer(AUTH_PORT, false, true); + server = new WebServer(0, false, true); await server.start(); - baseUrl = `http://localhost:${AUTH_PORT}`; + baseUrl = `http://localhost:${server.boundPort}`; }); afterAll(async () => { @@ -194,7 +189,7 @@ describe('Auth Security', () => { let rateBaseUrl: string; beforeEach(async () => { - ({ server: rateServer, baseUrl: rateBaseUrl } = await startAuthServer(AUTH_RATE_LIMIT_PORT)); + ({ server: rateServer, baseUrl: rateBaseUrl } = await startAuthServer()); }); afterEach(async () => { @@ -347,7 +342,7 @@ describe('No-Auth Server Startup Policy', () => { delete process.env.CODEMAN_PASSWORD; delete process.env.CODEMAN_USERNAME; delete process.env.CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK; - server = new WebServer(NOAUTH_PORT, false, true, '127.0.0.1'); + server = new WebServer(0, false, true, '127.0.0.1'); await server.start(); }); @@ -359,7 +354,7 @@ describe('No-Auth Server Startup Policy', () => { }); it('allows loopback requests without auth when no password is configured', async () => { - const res = await fetch(`http://localhost:${NOAUTH_PORT}/api/status`); + const res = await fetch(`http://localhost:${server.boundPort}/api/status`); expect(res.status).toBe(200); }); @@ -368,10 +363,10 @@ describe('No-Auth Server Startup Policy', () => { // bind without a password no longer refuses to start — it starts and warns, // pointing at how to secure it. See docs/security-architecture.md. const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); - const networkServer = new WebServer(NOAUTH_NETWORK_PORT, false, true, '0.0.0.0'); + const networkServer = new WebServer(0, false, true, '0.0.0.0'); await expect(networkServer.start()).resolves.toBeUndefined(); - const res = await fetch(`http://localhost:${NOAUTH_NETWORK_PORT}/api/status`); + const res = await fetch(`http://localhost:${networkServer.boundPort}/api/status`); expect(res.status).toBe(200); const warned = warnSpy.mock.calls.flat().join('\n'); @@ -393,10 +388,10 @@ describe('No-Auth Server Startup Policy', () => { }); it('allows non-loopback startup with the explicit unauthenticated-network override', async () => { - const networkServer = new WebServer(NETWORK_OVERRIDE_PORT, false, true, '0.0.0.0', undefined, true); + const networkServer = new WebServer(0, false, true, '0.0.0.0', undefined, true); await networkServer.start(); - const res = await fetch(`http://localhost:${NETWORK_OVERRIDE_PORT}/api/status`); + const res = await fetch(`http://localhost:${networkServer.boundPort}/api/status`); expect(res.status).toBe(200); await networkServer.stop(); }); diff --git a/test/cod54-hook-event-auth.test.ts b/test/cod54-hook-event-auth.test.ts index 10144207..bba51ee1 100644 --- a/test/cod54-hook-event-auth.test.ts +++ b/test/cod54-hook-event-auth.test.ts @@ -19,7 +19,7 @@ * - tunnel NOT running + good secret → not 401 (allowed) * - rate limiting: rapid unauthorized hook POSTs eventually 429 * - * Port: 3230 (tunnel-running), 3231 (tunnel-down), 3232 (rate-limit) + * Port: ephemeral (`new WebServer(0, …)`, read back through `boundPort`) */ import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; import { WebServer } from '../src/web/server.js'; @@ -28,9 +28,6 @@ import { TunnelManager } from '../src/tunnel-manager.js'; import { getHookSecret, HOOK_SECRET_HEADER } from '../src/config/hook-secret.js'; import { AUTH_FAILURE_MAX } from '../src/config/auth-config.js'; -const TUNNEL_UP_PORT = 3230; -const TUNNEL_DOWN_PORT = 3231; -const RATE_LIMIT_PORT = 3232; const TEST_USER = 'admin'; const TEST_PASS = 'cod54-test-password'; @@ -58,9 +55,9 @@ describe('COD-54 hook-event auth — tunnel running requires secret', () => { process.env.CODEMAN_USERNAME = TEST_USER; // Force the middleware's tunnel check to report "running". isRunningSpy = vi.spyOn(TunnelManager.prototype, 'isRunning').mockReturnValue(true); - server = new WebServer(TUNNEL_UP_PORT, false, true); + server = new WebServer(0, false, true); await server.start(); - baseUrl = `http://localhost:${TUNNEL_UP_PORT}`; + baseUrl = `http://localhost:${server.boundPort}`; }); afterAll(async () => { @@ -97,9 +94,9 @@ describe('COD-91 hook-event auth — tunnel down ALSO requires the secret', () = process.env.CODEMAN_USERNAME = TEST_USER; // Tunnel NOT running — loopback-only normal prod case. isRunningSpy = vi.spyOn(TunnelManager.prototype, 'isRunning').mockReturnValue(false); - server = new WebServer(TUNNEL_DOWN_PORT, false, true); + server = new WebServer(0, false, true); await server.start(); - baseUrl = `http://localhost:${TUNNEL_DOWN_PORT}`; + baseUrl = `http://localhost:${server.boundPort}`; }); afterAll(async () => { @@ -130,9 +127,9 @@ describe('COD-54 hook-event auth — rate limiting', () => { process.env.CODEMAN_USERNAME = TEST_USER; // Tunnel running so unauthorized (no-secret) hook POSTs are rejected and counted. isRunningSpy = vi.spyOn(TunnelManager.prototype, 'isRunning').mockReturnValue(true); - server = new WebServer(RATE_LIMIT_PORT, false, true); + server = new WebServer(0, false, true); await server.start(); - baseUrl = `http://localhost:${RATE_LIMIT_PORT}`; + baseUrl = `http://localhost:${server.boundPort}`; }); afterAll(async () => { diff --git a/test/multiuser-auth.test.ts b/test/multiuser-auth.test.ts index 25128914..fd9c7204 100644 --- a/test/multiuser-auth.test.ts +++ b/test/multiuser-auth.test.ts @@ -1,12 +1,12 @@ /** - * @fileoverview Phase 2 multi-user auth integration tests (live server, port 3170+). + * @fileoverview Phase 2 multi-user auth integration tests (live server, ephemeral port). * * Verifies the multi-user auth branch end to end: per-user Basic verify, cookie * identity, wrong-password / disabled-user rejection, the mustChangePassword * lockbox + self-service change, per-account rate limiting, and QR identity binding * (tunnel-manager unit level). Single-user auth is covered by auth-security.test.ts. * - * Ports: 3170 (multi-user server), 3171 (rate-limit server). + * Ports: ephemeral (`new WebServer(0, …)`, read back through `boundPort`). */ import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; @@ -21,9 +21,6 @@ import { AUTH_FAILURE_MAX } from '../src/config/auth-config.js'; vi.spyOn(TmuxManager, 'isTmuxAvailable').mockReturnValue(true); -const PORT = 3170; -const RATE_PORT = 3171; - function basic(user: string, pass: string): string { return 'Basic ' + Buffer.from(`${user}:${pass}`).toString('base64'); } @@ -68,7 +65,7 @@ beforeAll(async () => { const { updateUser } = await import('../src/user-store.js'); await updateUser('carol', { disabled: true }); - server = new WebServer(PORT, false, true); + server = new WebServer(0, false, true); await server.start(); }); @@ -84,7 +81,7 @@ afterAll(async () => { await fs.rm(spacesDir, { recursive: true, force: true }).catch(() => {}); }); -const url = (p: string) => `http://localhost:${PORT}${p}`; +const url = (p: string) => `http://localhost:${server.boundPort}${p}`; describe('multi-user auth', () => { it('rejects unauthenticated requests', async () => { @@ -161,9 +158,9 @@ describe('multi-user auth', () => { }); it('verify-first: a correct password is never rate-limited and self-heals failures (#17)', async () => { - rateServer = new WebServer(RATE_PORT, false, true); + rateServer = new WebServer(0, false, true); await rateServer.start(); - const rurl = (p: string) => `http://localhost:${RATE_PORT}${p}`; + const rurl = (p: string) => `http://localhost:${rateServer.boundPort}${p}`; // Nine wrong passwords (one below the cap) are each rejected 401 — not throttled yet. for (let i = 0; i < AUTH_FAILURE_MAX - 1; i++) { diff --git a/test/qr-auth.test.ts b/test/qr-auth.test.ts index a8bb9150..7cd67677 100644 --- a/test/qr-auth.test.ts +++ b/test/qr-auth.test.ts @@ -14,14 +14,12 @@ * 12. QR auth bypass in auth middleware * 13. GET /api/tunnel/qr SVG endpoint (auth/no-auth, caching, errors) * - * Port: 3162 (qr-auth tests), 3163 (qr-svg endpoint tests) + * Port: ephemeral (`new WebServer(0, …)`, read back through `boundPort`) */ import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest'; import { TunnelManager } from '../src/tunnel-manager.js'; import { WebServer } from '../src/web/server.js'; -const QR_AUTH_PORT = 3162; -const QR_SVG_PORT = 3163; const TEST_PASS = 'qr-test-pass-xyz'; const TEST_USER = 'admin'; @@ -312,9 +310,9 @@ describe('QR Auth Integration', () => { beforeAll(async () => { process.env.CODEMAN_PASSWORD = TEST_PASS; process.env.CODEMAN_USERNAME = TEST_USER; - server = new WebServer(QR_AUTH_PORT, false, true); + server = new WebServer(0, false, true); await server.start(); - baseUrl = `http://localhost:${QR_AUTH_PORT}`; + baseUrl = `http://localhost:${server.boundPort}`; }); afterAll(async () => { @@ -608,9 +606,9 @@ describe('QR SVG Endpoint (GET /api/tunnel/qr)', () => { beforeAll(async () => { process.env.CODEMAN_PASSWORD = TEST_PASS; process.env.CODEMAN_USERNAME = TEST_USER; - server = new WebServer(QR_SVG_PORT, false, true); + server = new WebServer(0, false, true); await server.start(); - baseUrl = `http://localhost:${QR_SVG_PORT}`; + baseUrl = `http://localhost:${server.boundPort}`; }); afterAll(async () => { diff --git a/test/routes/voice-routes.test.ts b/test/routes/voice-routes.test.ts index d1cd8886..4cef6a13 100644 --- a/test/routes/voice-routes.test.ts +++ b/test/routes/voice-routes.test.ts @@ -11,7 +11,7 @@ * - the socket refuses exactly what the status endpoint calls unavailable, * - a cross-site upgrade cannot open a stream on the operator's subscription. * - * Port: 3230 (routes), 3231 (mock upstream) + * Port: ephemeral (`port: 0` for the routes and the mock upstream) */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; @@ -20,6 +20,7 @@ import fastifyWebsocket from '@fastify/websocket'; import WebSocket, { WebSocketServer } from 'ws'; import { mkdirSync, writeFileSync, rmSync } from 'node:fs'; import { join } from 'node:path'; +import type { AddressInfo } from 'node:net'; import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js'; import { registerVoiceRoutes, _resetVoiceStreamCountForTesting } from '../../src/web/routes/voice-routes.js'; import { MAX_CONCURRENT_STREAMS } from '../../src/config/voice.js'; @@ -47,8 +48,9 @@ function removeCredentials(): void { rmSync(join(testHome(), '.claude', '.credentials.json'), { force: true }); } -const PORT = 3230; -const UPSTREAM_PORT = 3231; +/** Both assigned by the OS on every listen (`port: 0`); see beforeEach. */ +let PORT = 0; +let UPSTREAM_PORT = 0; const TOKEN = 'sk-ant-oat01-voice-route-test'; /** State captured by the mock upstream, so tests can assert what Codeman sent. */ @@ -118,7 +120,7 @@ describe('voice-routes', () => { voiceEnabled = true; capture = { headers: {}, url: '', binaryFrames: [], textFrames: [], socket: null }; - upstream = new WebSocketServer({ port: UPSTREAM_PORT, host: '127.0.0.1' }); + upstream = new WebSocketServer({ port: 0, host: '127.0.0.1' }); upstream.on('connection', (socket, req) => { capture.headers = req.headers; capture.url = req.url ?? ''; @@ -129,6 +131,7 @@ describe('voice-routes', () => { }); }); await new Promise((resolve) => upstream.once('listening', resolve)); + UPSTREAM_PORT = (upstream.address() as AddressInfo).port; process.env.CODEMAN_VOICE_STREAM_BASE = `ws://127.0.0.1:${UPSTREAM_PORT}`; writeCredentials(Date.now() + 3_600_000); @@ -138,7 +141,8 @@ describe('voice-routes', () => { ctx = createMockRouteContext(); ctx.getClaudeVoiceEnabled = (async () => voiceEnabled) as typeof ctx.getClaudeVoiceEnabled; registerVoiceRoutes(app, ctx as never, () => ({ bindHost: '127.0.0.1', allowedHosts: [], tunnelHost: null })); - await app.listen({ port: PORT, host: '127.0.0.1' }); + await app.listen({ port: 0, host: '127.0.0.1' }); + PORT = (app.server.address() as AddressInfo).port; }); afterEach(async () => { diff --git a/test/routes/ws-routes.test.ts b/test/routes/ws-routes.test.ts index 2ca5110d..6c478cd9 100644 --- a/test/routes/ws-routes.test.ts +++ b/test/routes/ws-routes.test.ts @@ -7,10 +7,11 @@ * * @dependency test/mocks/mock-route-context.ts (createMockRouteContext) * @dependency src/web/routes/ws-routes.ts (registerWsRoutes) - * Port: 3170 (ws-routes tests) + * Port: ephemeral (`listen({ port: 0 })`) */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import type { AddressInfo } from 'node:net'; import Fastify, { type FastifyInstance } from 'fastify'; import fastifyWebsocket from '@fastify/websocket'; import WebSocket from 'ws'; @@ -18,7 +19,8 @@ import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js import { registerWsRoutes } from '../../src/web/routes/ws-routes.js'; import { MAX_INPUT_LENGTH } from '../../src/config/terminal-limits.js'; -const PORT = 3170; +/** Assigned by the OS on every listen (`port: 0`); see beforeEach. */ +let PORT = 0; /** Helper: open a WebSocket connection and wait for it to reach OPEN state. */ function connectWs(path: string, timeoutMs = 5000): Promise { @@ -86,7 +88,8 @@ describe('ws-routes', () => { ctx = createMockRouteContext({ sessionId: 'ws-test-session' }); registerWsRoutes(app, ctx as never, () => ({ bindHost: '127.0.0.1', allowedHosts: [], tunnelHost: null })); - await app.listen({ port: PORT, host: '127.0.0.1' }); + await app.listen({ port: 0, host: '127.0.0.1' }); + PORT = (app.server.address() as AddressInfo).port; }); afterEach(async () => { diff --git a/test/test-ports-guard.test.ts b/test/test-ports-guard.test.ts new file mode 100644 index 00000000..20c7d8b8 --- /dev/null +++ b/test/test-ports-guard.test.ts @@ -0,0 +1,141 @@ +/** + * @fileoverview Static guard: a test builds `WebServer` on an ephemeral port. + * + * A fixed port is a red suite on any machine where something else holds it, and a + * collision between two runs on one host (two worktrees, or CI plus a local run): the + * suite runs files serially (`fileParallelism: false`), so the four port pairs #440 + * found never met inside one run, only across runs. `new WebServer(0, …)` binds + * whatever the OS hands out and `boundPort` reads it back, so there is nothing left + * to collide on. + * + * A WebServer built under test/ whose port argument is not the literal `0` fails — + * `new WebServer(…)`, a class declared `extends WebServer`, or a destructured alias + * (`{ WebServer: T }`) — unless the file is in LEGACY_FIXED_PORT_FILES, the files that + * construct one with a non-zero port today (a few never call `start()`); the follow-up + * sweep converts them. A converted file cannot stay listed: an entry whose file no + * longer matches fails too. + * + * Not covered: a helper that takes the port as a parameter is checked at the helper, + * not at its callers (test/mobile/helpers/server.ts is listed, so the mobile tests + * calling `createTestServer(PORT)` are not checked), `import { WebServer as X }`, and + * `new mod.WebServer(…)`. Raw `listen({ port: N })` and `new WebSocketServer({ port: N })` + * belong to the sweep. + * + * Port: N/A (pure static analysis). + */ +import { readdirSync, readFileSync, statSync } from 'node:fs'; +import { join, relative, sep } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; + +const TEST_ROOT = fileURLToPath(new URL('.', import.meta.url)); + +/** Predates the guard; converted in the follow-up sweep. Shrink only. */ +const LEGACY_FIXED_PORT_FILES = new Set( + [ + 'admin-routes.test.ts', + 'base-path-server.test.ts', + 'capture-geometry-retry.browser.test.ts', + 'capture-load-window.browser.test.ts', + 'case-custom-path.browser.test.ts', + 'doctor-settings.browser.test.ts', + 'edge-cases.test.ts', + 'file-link-click.test.ts', + 'git-status.browser.test.ts', + 'hooks-config.test.ts', + 'http-contract.test.ts', + 'inline-rename.test.ts', + 'integration-flows.test.ts', + 'key-tester.browser.test.ts', + 'mobile/helpers/server.ts', + 'opencode-resize.test.ts', + 'operation-lightspeed.test.ts', + 'ownership-scoping.test.ts', + 'pane-exit-sweep.test.ts', + 'paste-image-dir-shared.test.ts', + 'perf-browser.test.ts', + 'quick-start.test.ts', + 'ralph-integration.test.ts', + 'scheduled-runs.test.ts', + 'security-regression.test.ts', + 'session-cleanup.test.ts', + 'session-pane-exit.test.ts', + 'session.test.ts', + 'shift-enter-keypress.browser.test.ts', + 'split-pane-auto-collapse.browser.test.ts', + 'split-pane-orchestration.browser.test.ts', + 'split-pane-terminal.browser.test.ts', + 'sse-cors-headers.test.ts', + 'sse-events.test.ts', + 'sse-routing-remote.test.ts', + 'sse-subscription-filter.test.ts', + 'static-cache-headers.test.ts', + 'terminal-copy-shortcut.test.ts', + 'terminal-keycode229-recovery.browser.test.ts', + 'webgl-fallback.test.ts', + 'webhook-settings.browser.test.ts', + 'webview-lost-root-frame.test.ts', + 'webview-sse.test.ts', + ].map((p) => p.split('/').join(sep)) +); + +function testFiles(dir: string): string[] { + const out: string[] = []; + for (const name of readdirSync(dir)) { + const full = join(dir, name); + if (statSync(full).isDirectory()) { + if (name !== 'node_modules') out.push(...testFiles(full)); + } else if (name.endsWith('.ts')) { + out.push(full); + } + } + return out; +} + +/** + * First argument of every `new WebServer(` in `source`, trimmed (may span lines) — and of + * every `new X(` where the same file makes X a WebServer: `class X extends WebServer`, or + * a destructured alias `{ WebServer: X }` (how `quick-start.test.ts` builds its server). + */ +function webServerPortArgs(source: string): string[] { + const classes = [ + 'WebServer', + ...[...source.matchAll(/class\s+(\w+)\s+extends\s+WebServer\b/g)].map((m) => m[1]), + ...[...source.matchAll(/\bWebServer\s*:\s*(\w+)/g)].map((m) => m[1]), + ]; + return classes.flatMap((name) => + [...source.matchAll(new RegExp(`new ${name}\\(\\s*([^,)]*)`, 'g'))].map((m) => m[1].trim()) + ); +} + +const SELF = fileURLToPath(import.meta.url); +const scanned = testFiles(TEST_ROOT) + .filter((f) => f !== SELF) + .map((file) => ({ rel: relative(TEST_ROOT, file), args: webServerPortArgs(readFileSync(file, 'utf8')) })); +const fixed = (args: string[]) => args.some((a) => a !== '0'); + +describe('test servers bind an ephemeral port', () => { + it('reads the first argument the way a reader would', () => { + expect(webServerPortArgs('new WebServer(0, false, true)')).toEqual(['0']); + expect(webServerPortArgs('new WebServer(\n PORT,\n false)')).toEqual(['PORT']); + expect(webServerPortArgs('new WebServer(3162, false)')).toEqual(['3162']); + expect(webServerPortArgs('new WebServer()')).toEqual(['']); + expect(webServerPortArgs('class T extends WebServer {}\nconst s = new T(3299, false);')).toEqual(['3299']); + expect(webServerPortArgs('const { WebServer: T } = mod;\nreturn new T(port, false);')).toEqual(['port']); + }); + + it('no test outside the legacy list builds WebServer on a fixed port', () => { + const offenders = scanned + .filter((f) => fixed(f.args) && !LEGACY_FIXED_PORT_FILES.has(f.rel)) + .map( + (f) => + `${f.rel}: new WebServer(${f.args.find((a) => a !== '0')}, …) — use new WebServer(0, …) and server.boundPort` + ); + expect(offenders).toEqual([]); + }); + + it('the legacy list only names files that still need converting', () => { + const stale = [...LEGACY_FIXED_PORT_FILES].filter((rel) => !scanned.some((f) => f.rel === rel && fixed(f.args))); + expect(stale).toEqual([]); + }); +});