From 03629c966e719f37251514894c4a8ba2b2d96d34 Mon Sep 17 00:00:00 2001 From: Randalix Date: Thu, 8 Oct 2026 14:56:41 +0200 Subject: [PATCH 1/2] fix(server): report the port actually bound, so `new WebServer(0)` is usable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `port: 0` already bound an ephemeral port at the socket level, but `this.port` stayed 0: the banner printed `:0`, CODEMAN_API_URL pointed panes at `:0`, the docker bridge listener and the unauthenticated-bind warnings read 0, and the route context's `port` was a by-value snapshot taken in setupRoutes(), before listen() runs, so `tunnelManager.start(ctx.port, …)` would have been handed 0. - After `app.listen()`, `this.port` takes the number from `this.app.server.address()` (string/null addresses are left alone). - The route context exposes `port` as a getter, and the cron routes get only the `cron` their CronPort declares instead of a spread copy of the context. - `get boundPort()`: the readonly accessor tests use instead of a private field. test/webserver-bound-port.test.ts compares each reader against the socket's own `address().port`; all three tests fail with only the write-back removed. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/web/server.ts | 24 ++++++++++++++-- test/webserver-bound-port.test.ts | 46 +++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 test/webserver-bound-port.test.ts diff --git a/src/web/server.ts b/src/web/server.ts index 4442e544..2ca28689 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -323,6 +323,14 @@ export class WebServer extends EventEmitter { private store = getStore(); private tabLayouts!: TabLayoutService; private port: number; + + /** + * The port the server is actually listening on once `start()` has resolved — the + * OS-assigned one for `new WebServer(0, …)` — and the constructor's port before that. + */ + get boundPort(): number { + return this.port; + } private host: string; private https: boolean; /** Reverse-proxy sub-path prefix (normalized: '' for root, or '/foo'). */ @@ -746,7 +754,11 @@ export class WebServer extends EventEmitter { saveRespawnConfig: this.saveRespawnConfig.bind(this), // ConfigPort store: this.store, - port: this.port, + // A getter, not a snapshot: this context is built in setupRoutes(), BEFORE + // listen() resolves an ephemeral `port: 0`, and the tunnel start reads it later. + get port() { + return self.port; + }, https: this.https, testMode: this.testMode, serverStartTime: this.serverStartTime, @@ -1154,7 +1166,9 @@ export class WebServer extends EventEmitter { // due times for any persisted jobs, then expose it to its routes. this.cronService = new CronService(ctx); this.cronService.init(); - registerCronRoutes(this.app, { ...ctx, cron: this.cronService }); + // Only what CronPort declares: a spread of ctx would copy `port` by value (the + // pre-listen 0 of an ephemeral bind) into an object nothing keeps in sync. + registerCronRoutes(this.app, { cron: this.cronService }); registerWsRoutes(this.app, ctx, () => this.getHostPolicy()); registerVoiceRoutes(this.app, ctx, () => this.getHostPolicy()); @@ -2909,6 +2923,12 @@ export class WebServer extends EventEmitter { } await this.app.listen({ port: this.port, host: this.host }); + // A `port: 0` bind gets its number from the OS. Everything below and every later + // reader (the banner, CODEMAN_API_URL, the docker bridge listener, the + // unauthenticated-bind warnings, the route context's getter) must see that number, + // not the 0 that was asked for. + const address = this.app.server.address(); + if (address !== null && typeof address === 'object') this.port = address.port; const protocol = this.https ? 'https' : 'http'; const displayHost = this.host === '0.0.0.0' ? 'localhost' : this.host; // The only startup banner: `codeman web` used to print its own copy of this diff --git a/test/webserver-bound-port.test.ts b/test/webserver-bound-port.test.ts new file mode 100644 index 00000000..e81612b9 --- /dev/null +++ b/test/webserver-bound-port.test.ts @@ -0,0 +1,46 @@ +/** + * @fileoverview `new WebServer(0, …)`: the OS picks the port and every reader of + * `this.port` sees that number, not the 0 that was asked for (#440). + * + * Port: ephemeral. + */ +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import type { AddressInfo } from 'node:net'; +import { WebServer } from '../src/web/server.js'; + +describe('WebServer on an ephemeral port', () => { + let server: WebServer; + /** Built BEFORE start(), as setupRoutes() builds it, so a by-value snapshot would read 0. */ + let ctx: { port: number }; + const savedApiUrl = process.env.CODEMAN_API_URL; + /** The socket's own answer, so each test stands on its own instead of trusting boundPort. */ + const socketPort = () => + ((server as unknown as { app: { server: { address(): AddressInfo } } }).app.server.address() as AddressInfo).port; + + beforeAll(async () => { + server = new WebServer(0, false, true); + ctx = (server as unknown as { createRouteContext(): { port: number } }).createRouteContext(); + await server.start(); + }); + + afterAll(async () => { + await server.stop(); + if (savedApiUrl === undefined) delete process.env.CODEMAN_API_URL; + else process.env.CODEMAN_API_URL = savedApiUrl; + }); + + it('reports the port the OS assigned', async () => { + expect(socketPort()).toBeGreaterThan(0); + expect(server.boundPort).toBe(socketPort()); + const res = await fetch(`http://127.0.0.1:${server.boundPort}/api/status`); + expect(res.status).toBe(200); + }); + + it('the route context reads it live (the tunnel start gets the real port, not 0)', () => { + expect(ctx.port).toBe(socketPort()); + }); + + it('exports the real port to the panes as CODEMAN_API_URL', () => { + expect(process.env.CODEMAN_API_URL).toBe(`http://127.0.0.1:${socketPort()}`); + }); +}); From bb4e7943c501a76bddccb7e501198bf4ccd7c1db Mon Sep 17 00:00:00 2001 From: Randalix Date: Thu, 8 Oct 2026 14:56:42 +0200 Subject: [PATCH 2/2] test: bind the port-sharing test servers to ephemeral ports; guard new fixed ports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four ports were shared by two files each — 3162 (qr-auth / auth-security), 3170 (multiuser-auth / routes/ws-routes), 3230 and 3231 (cod54-hook-event-auth / routes/voice-routes). Files run serially (`fileParallelism: false`), so the pairs never met inside one run; they collide between two runs on one host, or with anything else holding the port. All six files now bind port 0 and read the number back (`boundPort` for WebServer, `server.address()` after each listen for the raw Fastify / ws servers). test/test-ports-guard.test.ts fails on a WebServer built under test/ whose port argument is not the literal 0 — `new WebServer(…)`, a subclass, or a destructured alias (`{ WebServer: T }`, as quick-start.test.ts does) — outside a legacy list of the 43 files that construct one with a non-zero port today; the follow-up sweep converts them. A converted file cannot stay listed. What it does not cover (helper parameters, `import { WebServer as X }`, raw listen sites) is written down in it. Co-Authored-By: Claude Opus 5.5 (1M context) --- test/auth-security.test.ts | 31 +++---- test/cod54-hook-event-auth.test.ts | 17 ++-- test/multiuser-auth.test.ts | 15 ++- test/qr-auth.test.ts | 12 +-- test/routes/voice-routes.test.ts | 14 ++- test/routes/ws-routes.test.ts | 9 +- test/test-ports-guard.test.ts | 141 +++++++++++++++++++++++++++++ 7 files changed, 187 insertions(+), 52 deletions(-) create mode 100644 test/test-ports-guard.test.ts 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([]); + }); +});