From 09a97b0a058f0ec0f63aa060260eb6586bd3ec70 Mon Sep 17 00:00:00 2001 From: m4air Date: Wed, 30 Sep 2026 09:50:29 -0700 Subject: [PATCH 1/2] feat(runtime-rpc): Node WebSocket lifecycle extraction and a required-port bind mode Port the #16741 T1 transport pieces onto the current Node-only transport. The per-socket lifecycle (pre-auth timer, heartbeat enrolment, finalize on close/error), over-capacity rejection and shutdown move out of ws-transport.ts into node-websocket-lifecycle.ts, and the size/connection caps into websocket-transport-limits.ts. WebSocketTransport gains an opt-in strictPort that binds only the configured port and fails closed instead of walking the fallback ladder; OrcaRuntimeRpcServer exposes it as requirePinnedWsPort, which also skips the persisted fallback port and stops the Unix socket transport before rethrowing, so a caller whose tunnel forwards one fixed port never runs on a port nobody can reach. Porting note (#16741 a68b6f3531, T1): - Taken: node-websocket-lifecycle.ts, websocket-transport-limits.ts, WebSocketTransport strictPort, the ws-transport strict-port test, runtime-rpc-state/-pairing-types/-lifecycle requirePinnedWsPort. - Adapted: the lifecycle reads the message/close handlers through getters (the PR snapshotted them per connection) so a handler registered after start still reaches earlier sockets; binary frames convert through a checked RawData branch instead of `as Buffer`; the original Why comments move with the code; stop() keeps a single Node path (no AggregateError) and keeps the PR's closeAllConnections(), now covered by a test that an unanswered HTTP request no longer holds stop() open; the over-capacity test calls the extracted function instead of casting to a private. - Dropped: every bun-websocket-* file and the Bun branches in ws-transport.ts (D9.4); WEBSOCKET_TRANSPORT_MAX_BACKPRESSURE_BYTES (only the Bun transport read it); the relaxed EACCES `port`-field check in isPortListenFallbackError (it covered Bun's listen errors, Node's carry the port). - Not here: the orcad-entry wiring that sets requirePinnedWsPort for `--port` (T6, managed SSH deploy); the transportGeneration context plumbing (its only readers are T3/T7); holdIdleRuntimeEnvironmentCalls (its only caller is the T5 reconciliation coordinator). Wire compatibility: none. The bind policy is server-local, the option defaults off, and no RPC field, method or frame changes. --- .../runtime/rpc/node-websocket-lifecycle.ts | 153 ++++++++++++++ .../runtime/rpc/websocket-transport-limits.ts | 5 + src/main/runtime/rpc/ws-transport.test.ts | 41 +++- src/main/runtime/rpc/ws-transport.ts | 189 +++++------------- .../runtime-rpc-required-ws-port.test.ts | 81 ++++++++ .../runtime-rpc/runtime-rpc-lifecycle.ts | 9 +- .../runtime-rpc/runtime-rpc-pairing-types.ts | 2 + .../runtime/runtime-rpc/runtime-rpc-state.ts | 3 + 8 files changed, 337 insertions(+), 146 deletions(-) create mode 100644 src/main/runtime/rpc/node-websocket-lifecycle.ts create mode 100644 src/main/runtime/rpc/websocket-transport-limits.ts create mode 100644 src/main/runtime/runtime-rpc-required-ws-port.test.ts diff --git a/src/main/runtime/rpc/node-websocket-lifecycle.ts b/src/main/runtime/rpc/node-websocket-lifecycle.ts new file mode 100644 index 000000000000..e37e38375dad --- /dev/null +++ b/src/main/runtime/rpc/node-websocket-lifecycle.ts @@ -0,0 +1,153 @@ +import type { Server as HttpServer } from 'node:http' +import type { Server as HttpsServer } from 'node:https' +import type { WebSocket, WebSocketServer } from 'ws' +import type { RemoteRuntimeServerHeartbeat } from './remote-runtime-server-heartbeat' + +type WebSocketMessagePayload = string | Uint8Array +export type WebSocketMessageHandler = { + bivarianceHack( + msg: WebSocketMessagePayload, + reply: (response: string) => void, + ws: WebSocket + ): void +}['bivarianceHack'] + +export type WebSocketConnectionCloseHandler = ( + clientId: string | null, + ws: WebSocket, + hasOtherConnections: boolean +) => void + +// Why: WS connections are long-lived and multiplex many RPCs by `id`; auth and dispatch are delegated to the message handler. +export function attachNodeWebSocketLifecycle(args: { + ws: WebSocket + heartbeat: RemoteRuntimeServerHeartbeat + preAuthTimeoutMs: number + preAuthTimers: WeakMap> + clientIds: Map + heartbeatConnections: Set + // Why: read lazily so a handler registered after start still reaches sockets accepted earlier. + getMessageHandler: () => WebSocketMessageHandler | null + getConnectionCloseHandler: () => WebSocketConnectionCloseHandler | null +}): void { + const { ws } = args + let finalized = false + const onPong = (): void => args.heartbeat.noteAlive(ws) + const onMessage = (data: WebSocket.RawData, isBinary: boolean): void => { + // Why: any inbound frame counts as proof of life, so an actively-talking client isn't reaped mid-request. + args.heartbeat.noteAlive(ws) + const message = + typeof data === 'string' ? data : isBinary ? toBinaryPayload(data) : data.toString() + args.getMessageHandler()?.( + message, + (response) => { + // Why: mobile clients disconnect often; guard the write so we don't throw on a dead socket. + if (ws.readyState === ws.OPEN) { + ws.send(response) + } + }, + ws + ) + } + const finalize = (): void => { + if (finalized) { + return + } + finalized = true + ws.off('pong', onPong) + ws.off('message', onMessage) + ws.off('close', finalize) + ws.off('error', onError) + clearNodeWebSocketPreAuthTimer(ws, args.preAuthTimers) + args.heartbeatConnections.delete(ws) + if (args.heartbeatConnections.size === 0) { + args.heartbeat.stop() + } + const clientId = args.clientIds.get(ws) ?? null + args.clientIds.delete(ws) + const hasOtherConnections = + clientId !== null && Array.from(args.clientIds.values()).includes(clientId) + args.getConnectionCloseHandler()?.(clientId, ws, hasOtherConnections) + } + const onError = (): void => { + // Why: close isn't guaranteed after every error path; finalize here too so pre-auth E2EE state and connection ids can't leak. + finalize() + ws.close() + } + const preAuthTimer = setTimeout(() => { + if (!args.clientIds.has(ws)) { + // Why: a silent auto-ponging client would otherwise hold a finite mobile slot forever without starting the E2EE handshake. + ws.terminate() + } + }, args.preAuthTimeoutMs) + preAuthTimer.unref?.() + args.preAuthTimers.set(ws, preAuthTimer) + ws.on('pong', onPong) + ws.on('message', onMessage) + // Why: clean up connection-scoped state (e.g. mobile-fit overrides) so a dropped phone doesn't leave orphaned phone-fit on desktop. + ws.on('close', finalize) + ws.on('error', onError) + // Why: install lifecycle ownership before periodic heartbeat ticks can observe this socket. + args.heartbeatConnections.add(ws) + args.heartbeat.noteAlive(ws) + if (args.heartbeatConnections.size === 1) { + // Unauthenticated sockets are protected by the pre-auth timeout; heartbeat probes begin only + // after E2EE binds a client id, avoiding control frames during the handshake. + args.heartbeat.start(() => args.clientIds.keys()) + } +} + +function toBinaryPayload(data: Exclude): Uint8Array { + return Array.isArray(data) ? Buffer.concat(data) : new Uint8Array(data) +} + +export function clearNodeWebSocketPreAuthTimer( + ws: WebSocket, + preAuthTimers: WeakMap> +): void { + const timer = preAuthTimers.get(ws) + if (timer) { + clearTimeout(timer) + preAuthTimers.delete(ws) + } +} + +export async function stopNodeWebSocketTransport(args: { + wss: WebSocketServer | null + httpServer: HttpServer | HttpsServer | null + heartbeat: RemoteRuntimeServerHeartbeat + heartbeatConnections: Set +}): Promise { + args.heartbeat.stop() + args.heartbeatConnections.clear() + if (args.wss) { + for (const client of args.wss.clients) { + // Why: a half-open mobile socket may never answer a close frame, which keeps httpServer.close pending. + client.terminate() + } + args.wss.close() + } + const httpServer = args.httpServer + if (httpServer) { + await new Promise((resolve, reject) => { + httpServer.close((error) => { + if (error) { + reject(error) + return + } + resolve() + }) + // Why: idle keep-alive static-web connections would otherwise hold close() open. + httpServer.closeAllConnections() + }) + } +} + +// Why: force-terminate soon after the 1013 close since a half-open phone may never ack and would hold the descriptor past the WS cap; the 'error' listener absorbs a reset while closing. +export function rejectNodeWebSocketOverCapacity(ws: WebSocket): void { + ws.on('error', () => {}) + ws.close(1013, 'Maximum connections reached') + const terminateTimer = setTimeout(() => ws.terminate(), 1_000) + terminateTimer.unref?.() + ws.once('close', () => clearTimeout(terminateTimer)) +} diff --git a/src/main/runtime/rpc/websocket-transport-limits.ts b/src/main/runtime/rpc/websocket-transport-limits.ts new file mode 100644 index 000000000000..e2e0b3d16ce2 --- /dev/null +++ b/src/main/runtime/rpc/websocket-transport-limits.ts @@ -0,0 +1,5 @@ +export const WEBSOCKET_TRANSPORT_MAX_MESSAGE_BYTES = 1024 * 1024 +// Why: one desktop remote-host client can hold many concurrent streams, so keep the cap high enough that stale streams don't starve control RPCs. +export const WEBSOCKET_TRANSPORT_MAX_CONNECTIONS = 128 +// Why: bound pre-upgrade descriptor use above the WS cap so raw sockets can't grow without bound. +export const WEBSOCKET_TRANSPORT_MAX_TCP_CONNECTIONS = WEBSOCKET_TRANSPORT_MAX_CONNECTIONS * 2 diff --git a/src/main/runtime/rpc/ws-transport.test.ts b/src/main/runtime/rpc/ws-transport.test.ts index 45798a89528b..90be4229d422 100644 --- a/src/main/runtime/rpc/ws-transport.test.ts +++ b/src/main/runtime/rpc/ws-transport.test.ts @@ -1,10 +1,12 @@ import { EventEmitter } from 'node:events' import { mkdtempSync } from 'node:fs' +import { connect } from 'node:net' import { tmpdir } from 'node:os' import { join } from 'node:path' import { describe, expect, it, afterEach, vi } from 'vitest' import WebSocket from 'ws' import { WebSocketTransport } from './ws-transport' +import { rejectNodeWebSocketOverCapacity } from './node-websocket-lifecycle' import { loadOrCreateTlsCertificate } from '../tls-certificate' // Why: disable TLS verification for self-signed certs in tests. @@ -433,9 +435,7 @@ describe('WebSocketTransport', () => { vi.useFakeTimers() try { - ;(transport as unknown as { rejectOverCapacity(ws: WebSocket): void }).rejectOverCapacity( - serverSocket! - ) + rejectNodeWebSocketOverCapacity(serverSocket!) vi.advanceTimersByTime(1_000) } finally { vi.useRealTimers() @@ -455,6 +455,24 @@ describe('WebSocketTransport', () => { await transport.stop() }) + it('stops while an HTTP client holds an unanswered request open', async () => { + const transport = new WebSocketTransport({ host: '127.0.0.1', port: 0 }) + transports.push(transport) + await transport.start() + const socket = connect(transport.resolvedPort, '127.0.0.1') + await new Promise((resolve) => socket.once('connect', () => resolve())) + socket.on('error', () => {}) + // Why: the server's automatic 100 Continue proves the request is parsed and in flight. + const continued = new Promise((resolve) => socket.once('data', () => resolve())) + socket.write( + 'POST /unanswered HTTP/1.1\r\nHost: 127.0.0.1\r\nContent-Length: 1\r\nExpect: 100-continue\r\n\r\n' + ) + await continued + + await transport.stop() + socket.destroy() + }) + it('is safe to stop without starting', async () => { const { transport } = await createTransport() await transport.stop() @@ -669,6 +687,23 @@ describe('WebSocketTransport', () => { expect(transport.resolvedPort).toBe(fallbackPort) }) + it('fails closed when a managed tunnel requires the configured port', async () => { + const preferredHolder = new WebSocketTransport({ host: '127.0.0.1', port: 0 }) + transports.push(preferredHolder) + await preferredHolder.start() + const transport = new WebSocketTransport({ + host: '127.0.0.1', + port: preferredHolder.resolvedPort, + fallbackPort: await reserveFreePort(), + preferPinnedPort: true, + strictPort: true + }) + transports.push(transport) + + await expect(transport.start()).rejects.toMatchObject({ code: 'EADDRINUSE' }) + expect(transport.resolvedHost).toBeNull() + }) + it('binds the preferred port when the persisted fallback is taken', async () => { const fallbackHolder = new WebSocketTransport({ host: '127.0.0.1', port: 0 }) transports.push(fallbackHolder) diff --git a/src/main/runtime/rpc/ws-transport.ts b/src/main/runtime/rpc/ws-transport.ts index 3ba8a17fe607..81e76b1bb09b 100644 --- a/src/main/runtime/rpc/ws-transport.ts +++ b/src/main/runtime/rpc/ws-transport.ts @@ -4,22 +4,22 @@ import { createServer as createHttpServer, type Server as HttpServer } from 'nod import { WebSocketServer, type WebSocket } from 'ws' import type { RpcTransport } from './transport' import { createStaticWebClientHandler } from './static-web-client-handler' +import { + attachNodeWebSocketLifecycle, + clearNodeWebSocketPreAuthTimer, + rejectNodeWebSocketOverCapacity, + stopNodeWebSocketTransport, + type WebSocketConnectionCloseHandler, + type WebSocketMessageHandler +} from './node-websocket-lifecycle' import { RemoteRuntimeServerHeartbeat } from './remote-runtime-server-heartbeat' +import { + WEBSOCKET_TRANSPORT_MAX_CONNECTIONS, + WEBSOCKET_TRANSPORT_MAX_MESSAGE_BYTES, + WEBSOCKET_TRANSPORT_MAX_TCP_CONNECTIONS +} from './websocket-transport-limits' -const MAX_WS_MESSAGE_BYTES = 1024 * 1024 -// Why: one desktop remote-host client can hold many concurrent streams, so keep the cap high enough that stale streams don't starve control RPCs. -const MAX_WS_CONNECTIONS = 128 -// Why: bound pre-upgrade descriptor use above the WS cap so raw sockets can't grow without bound. -const MAX_TCP_CONNECTIONS = MAX_WS_CONNECTIONS * 2 const PRE_AUTH_TIMEOUT_MS = 10_000 -type WebSocketMessagePayload = string | Uint8Array -type WebSocketMessageHandler = { - bivarianceHack( - msg: WebSocketMessagePayload, - reply: (response: string) => void, - ws: WebSocket - ): void -}['bivarianceHack'] // Why: mobile clients background-suspend sockets with no TCP FIN, leaving half-opens that otherwise only the OS keepalive (~2h) reaps; a 15s ping/pong sweep bounds that to ~60s (clients auto-pong per RFC 6455), since a reap needs consecutive unanswered probes rather than one (STA-3320). const HEARTBEAT_INTERVAL_MS = 15_000 @@ -41,6 +41,8 @@ export type WebSocketTransportOptions = { fallbackPort?: number // Why: serve --port clients dial the pinned port; prefer it first so a stale fallback can't steal the pin (issue #8535). Default keeps fallback-first (STA-1511). preferPinnedPort?: boolean + // Why: managed SSH tunnels dial one configured remote port and cannot follow a fallback. + strictPort?: boolean } export class WebSocketTransport implements RpcTransport { @@ -53,12 +55,11 @@ export class WebSocketTransport implements RpcTransport { private readonly staticRoot: string | undefined private readonly fallbackPort: number | undefined private readonly preferPinnedPort: boolean + private readonly strictPort: boolean private httpServer: HttpsServer | HttpServer | null = null private wss: WebSocketServer | null = null private messageHandler: WebSocketMessageHandler | null = null - private connectionCloseHandler: - | ((clientId: string | null, ws: WebSocket, hasOtherConnections: boolean) => void) - | null = null + private connectionCloseHandler: WebSocketConnectionCloseHandler | null = null // Why: maps each socket to its authenticated clientId so close can report which device disconnected. private wsClientIds = new Map() private heartbeatConnections = new Set() @@ -74,7 +75,8 @@ export class WebSocketTransport implements RpcTransport { preAuthTimeoutMs, staticRoot, fallbackPort, - preferPinnedPort + preferPinnedPort, + strictPort }: WebSocketTransportOptions) { this.host = host this.port = port @@ -83,12 +85,13 @@ export class WebSocketTransport implements RpcTransport { this.heartbeat = new RemoteRuntimeServerHeartbeat( heartbeatIntervalMs ?? HEARTBEAT_INTERVAL_MS, heartbeatNow, - MAX_WS_CONNECTIONS + WEBSOCKET_TRANSPORT_MAX_CONNECTIONS ) this.preAuthTimeoutMs = preAuthTimeoutMs ?? PRE_AUTH_TIMEOUT_MS this.staticRoot = staticRoot this.fallbackPort = fallbackPort this.preferPinnedPort = preferPinnedPort === true + this.strictPort = strictPort === true } onMessage(handler: WebSocketMessageHandler): void { @@ -96,15 +99,13 @@ export class WebSocketTransport implements RpcTransport { } // Why: pass the closing `ws` and whether other sockets share its deviceToken, so client-scoped teardown fires only on the last disconnect. - onConnectionClose( - handler: (clientId: string | null, ws: WebSocket, hasOtherConnections: boolean) => void - ): void { + onConnectionClose(handler: WebSocketConnectionCloseHandler): void { this.connectionCloseHandler = handler } setClientId(ws: WebSocket, clientId: string): void { this.wsClientIds.set(ws, clientId) - this.clearPreAuthTimer(ws) + clearNodeWebSocketPreAuthTimer(ws, this.preAuthTimers) } terminateClientConnections(clientId: string): number { @@ -137,6 +138,10 @@ export class WebSocketTransport implements RpcTransport { if (this.wss) { return } + if (this.strictPort) { + await this.tryListen(this.port) + return + } // Why: bind a persisted fallback first so devices paired to it aren't stranded (STA-1511); serve --port flips to pinned-first (issue #8535); on failure each candidate falls through to OS-assigned port 0. const persistedFallbackPort = @@ -192,16 +197,16 @@ export class WebSocketTransport implements RpcTransport { }) // Why: the WS cap applies only post-upgrade; a separate TCP cap bounds raw/pre-upgrade descriptor use. - httpServer.maxConnections = MAX_TCP_CONNECTIONS + httpServer.maxConnections = WEBSOCKET_TRANSPORT_MAX_TCP_CONNECTIONS const wss = new WebSocketServer({ server: httpServer, - maxPayload: MAX_WS_MESSAGE_BYTES + maxPayload: WEBSOCKET_TRANSPORT_MAX_MESSAGE_BYTES }) wss.on('connection', (ws) => { - if (wss.clients.size > MAX_WS_CONNECTIONS) { - this.rejectOverCapacity(ws) + if (wss.clients.size > WEBSOCKET_TRANSPORT_MAX_CONNECTIONS) { + rejectNodeWebSocketOverCapacity(ws) return } this.handleConnection(ws) @@ -211,130 +216,30 @@ export class WebSocketTransport implements RpcTransport { this.wss = wss } - // Why: force-terminate soon after the 1013 close since a half-open phone may never ack and would hold the descriptor past the WS cap; the 'error' listener absorbs a reset while closing. - private rejectOverCapacity(ws: WebSocket): void { - ws.on('error', () => {}) - ws.close(1013, 'Maximum connections reached') - const terminateTimer = setTimeout(() => ws.terminate(), 1_000) - terminateTimer.unref?.() - ws.once('close', () => clearTimeout(terminateTimer)) - } - async stop(): Promise { const wss = this.wss const httpServer = this.httpServer this.wss = null this.httpServer = null - this.heartbeat.stop() - this.heartbeatConnections.clear() - - if (wss) { - for (const client of wss.clients) { - // Why: a half-open mobile socket may never answer a close frame, which keeps httpServer.close pending. - client.terminate() - } - wss.close() - } - - if (httpServer) { - await new Promise((resolve, reject) => { - httpServer.close((error) => { - if (error) { - reject(error) - return - } - resolve() - }) - }) - } + await stopNodeWebSocketTransport({ + wss, + httpServer, + heartbeat: this.heartbeat, + heartbeatConnections: this.heartbeatConnections + }) } - // Why: WS connections are long-lived and multiplex many RPCs by `id`; auth and dispatch are delegated to the message handler. private handleConnection(ws: WebSocket): void { - let finalized = false - const onPong = (): void => { - this.heartbeat.noteAlive(ws) - } - const onMessage = (data: WebSocket.RawData, isBinary: boolean): void => { - // Why: any inbound frame counts as proof of life, so an actively-talking client isn't reaped mid-request. - this.heartbeat.noteAlive(ws) - const msg = - typeof data === 'string' - ? data - : isBinary - ? new Uint8Array(data as Buffer) - : data.toString() - this.messageHandler?.( - msg, - (response) => { - // Why: mobile clients disconnect often; guard the write so we don't throw on a dead socket. - if (ws.readyState === ws.OPEN) { - ws.send(response) - } - }, - ws - ) - } - const onError = (): void => { - // Why: close isn't guaranteed after every error path; finalize here too so pre-auth E2EE state and connection ids can't leak. - finalizeConnection() - ws.close() - } - const finalizeConnection = (): void => { - if (finalized) { - return - } - finalized = true - ws.off('pong', onPong) - ws.off('message', onMessage) - ws.off('close', finalizeConnection) - ws.off('error', onError) - this.clearPreAuthTimer(ws) - this.heartbeatConnections.delete(ws) - if (this.heartbeatConnections.size === 0) { - this.heartbeat.stop() - } - const clientId = this.wsClientIds.get(ws) ?? null - this.wsClientIds.delete(ws) - const hasOtherConnections = - clientId !== null && Array.from(this.wsClientIds.values()).includes(clientId) - this.connectionCloseHandler?.(clientId, ws, hasOtherConnections) - } - - const preAuthTimer = setTimeout(() => { - if (!this.wsClientIds.has(ws)) { - // Why: a silent auto-ponging client would otherwise hold a finite mobile slot forever without starting the E2EE handshake. - ws.terminate() - } - }, this.preAuthTimeoutMs) - if (typeof preAuthTimer.unref === 'function') { - preAuthTimer.unref() - } - this.preAuthTimers.set(ws, preAuthTimer) - - ws.on('pong', onPong) - ws.on('message', onMessage) - - // Why: clean up connection-scoped state (e.g. mobile-fit overrides) so a dropped phone doesn't leave orphaned phone-fit on desktop. - ws.on('close', finalizeConnection) - ws.on('error', onError) - - // Why: install lifecycle ownership before periodic heartbeat ticks can observe this socket. - this.heartbeatConnections.add(ws) - this.heartbeat.noteAlive(ws) - if (this.heartbeatConnections.size === 1) { - // Unauthenticated sockets are protected by the pre-auth timeout; heartbeat probes begin only - // after E2EE binds a client id, avoiding control frames during the handshake. - this.heartbeat.start(() => this.wsClientIds.keys()) - } - } - - private clearPreAuthTimer(ws: WebSocket): void { - const timer = this.preAuthTimers.get(ws) - if (timer) { - clearTimeout(timer) - this.preAuthTimers.delete(ws) - } + attachNodeWebSocketLifecycle({ + ws, + heartbeat: this.heartbeat, + preAuthTimeoutMs: this.preAuthTimeoutMs, + preAuthTimers: this.preAuthTimers, + clientIds: this.wsClientIds, + heartbeatConnections: this.heartbeatConnections, + getMessageHandler: () => this.messageHandler, + getConnectionCloseHandler: () => this.connectionCloseHandler + }) } } diff --git a/src/main/runtime/runtime-rpc-required-ws-port.test.ts b/src/main/runtime/runtime-rpc-required-ws-port.test.ts new file mode 100644 index 000000000000..420dcbaa1117 --- /dev/null +++ b/src/main/runtime/runtime-rpc-required-ws-port.test.ts @@ -0,0 +1,81 @@ +import { mkdtempSync } from 'node:fs' +import { createServer, type Server } from 'node:net' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { OrcaRuntimeService } from './orca-runtime' +import { OrcaRuntimeRpcServer } from './runtime-rpc' +import { readWsFallbackPort } from './rpc/ws-fallback-port-store' + +vi.mock('../git/worktree', () => ({ + listWorktrees: vi.fn().mockResolvedValue([]), + listWorktreesStrict: vi.fn().mockResolvedValue([]) +})) + +const holders: Server[] = [] + +afterEach(async () => { + await Promise.all( + holders.splice(0).map((holder) => new Promise((resolve) => holder.close(() => resolve()))) + ) +}) + +async function occupyLoopbackPort(): Promise { + const holder = createServer() + holders.push(holder) + await new Promise((resolve) => holder.listen(0, '127.0.0.1', resolve)) + const address = holder.address() + if (!address || typeof address === 'string') { + throw new Error('holder did not bind a TCP port') + } + return address.port +} + +function boundWsPort(server: OrcaRuntimeRpcServer): number | null { + const endpoint = server.getWebSocketEndpoint() + return endpoint ? Number(new URL(endpoint).port) : null +} + +describe('OrcaRuntimeRpcServer required WebSocket port', () => { + it('fails startup instead of falling back when the required port is taken', async () => { + const port = await occupyLoopbackPort() + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-rpc-')) + const server = new OrcaRuntimeRpcServer({ + runtime: new OrcaRuntimeService(), + userDataPath, + enableWebSocket: true, + wsPort: port, + preferPinnedWsPort: true, + requirePinnedWsPort: true + }) + + try { + await expect(server.start()).rejects.toMatchObject({ code: 'EADDRINUSE' }) + expect(boundWsPort(server)).toBeNull() + expect(readWsFallbackPort(userDataPath)).toBeUndefined() + } finally { + await server.stop() + } + }) + + it('still falls back for a pinned but not required port', async () => { + const port = await occupyLoopbackPort() + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-rpc-')) + const server = new OrcaRuntimeRpcServer({ + runtime: new OrcaRuntimeService(), + userDataPath, + enableWebSocket: true, + wsPort: port, + preferPinnedWsPort: true + }) + + await server.start() + try { + const resolvedPort = boundWsPort(server) + expect(resolvedPort).not.toBe(port) + expect(readWsFallbackPort(userDataPath)).toBe(resolvedPort) + } finally { + await server.stop() + } + }) +}) diff --git a/src/main/runtime/runtime-rpc/runtime-rpc-lifecycle.ts b/src/main/runtime/runtime-rpc/runtime-rpc-lifecycle.ts index 9cfe31251b6e..4c595b3bbabf 100644 --- a/src/main/runtime/runtime-rpc/runtime-rpc-lifecycle.ts +++ b/src/main/runtime/runtime-rpc/runtime-rpc-lifecycle.ts @@ -90,7 +90,9 @@ export class RuntimeRpcLifecycle extends RuntimeRpcWebSocketDispatch { port: this.wsPort, preferPinnedPort: this.preferPinnedWsPort, // Why: stable fallback port across restarts keeps paired devices' endpoints valid (STA-1511); wsPort 0 = random (E2E). - ...(this.wsPort !== 0 ? { fallbackPort: readWsFallbackPort(this.userDataPath) } : {}) + ...(this.wsPort !== 0 && !this.requirePinnedWsPort + ? { fallbackPort: readWsFallbackPort(this.userDataPath) } + : {}) }) if (this.wsPort !== 0 && transport.resolvedPort !== this.wsPort) { writeWsFallbackPort(this.userDataPath, transport.resolvedPort) @@ -98,6 +100,10 @@ export class RuntimeRpcLifecycle extends RuntimeRpcWebSocketDispatch { activeTransports.push(transport) transportsMeta.push({ kind: 'websocket', endpoint }) } catch (error) { + if (this.requirePinnedWsPort) { + await socketTransport.stop().catch(() => {}) + throw error + } // Why: WebSocket transport is supplementary; on failure (e.g. port in use) continue with Unix socket only. console.error('[runtime] Failed to start WebSocket transport:', error) this.mobileSocketWiring = null @@ -174,6 +180,7 @@ export class RuntimeRpcLifecycle extends RuntimeRpcWebSocketDispatch { host: options.host, port: options.port, staticRoot: this.webClientRoot, + ...(this.requirePinnedWsPort ? { strictPort: true } : {}), ...(options.fallbackPort !== undefined ? { fallbackPort: options.fallbackPort } : {}), ...(options.preferPinnedPort ? { preferPinnedPort: true } : {}) }) diff --git a/src/main/runtime/runtime-rpc/runtime-rpc-pairing-types.ts b/src/main/runtime/runtime-rpc/runtime-rpc-pairing-types.ts index 4923dec1dec1..da517dd9d925 100644 --- a/src/main/runtime/runtime-rpc/runtime-rpc-pairing-types.ts +++ b/src/main/runtime/runtime-rpc/runtime-rpc-pairing-types.ts @@ -36,6 +36,8 @@ export type OrcaRuntimeRpcServerOptions = { wsPort?: number // Why: true when the caller pinned a port (`orca serve --port`) so bind order prefers it over a stale STA-1511 fallback (#8535). preferPinnedWsPort?: boolean + // Why: a managed SSH tunnel forwards exactly this port, so a fallback bind must fail startup instead. + requirePinnedWsPort?: boolean // Why: STA-2370 — bind the WS listener to all interfaces at startup instead of loopback-until-paired. // Only `orca serve` (explicit remote opt-in) and E2E set this; the desktop app widens lazily on pairing. exposeNetworkByDefault?: boolean diff --git a/src/main/runtime/runtime-rpc/runtime-rpc-state.ts b/src/main/runtime/runtime-rpc/runtime-rpc-state.ts index e7f56ceed13c..20ddbf70e38d 100644 --- a/src/main/runtime/runtime-rpc/runtime-rpc-state.ts +++ b/src/main/runtime/runtime-rpc/runtime-rpc-state.ts @@ -38,6 +38,7 @@ export class RuntimeRpcState { protected readonly enableWebSocket: boolean protected readonly wsPort: number protected readonly preferPinnedWsPort: boolean + protected readonly requirePinnedWsPort: boolean protected readonly exposeNetworkByDefault: boolean protected readonly pinnedBindHost: string | null protected readonly webClientRoot: string | undefined @@ -100,6 +101,7 @@ export class RuntimeRpcState { enableWebSocket = false, wsPort = DEFAULT_WS_PORT, preferPinnedWsPort = false, + requirePinnedWsPort = false, exposeNetworkByDefault = false, pinnedBindHost, webClientRoot, @@ -116,6 +118,7 @@ export class RuntimeRpcState { this.enableWebSocket = enableWebSocket this.wsPort = wsPort this.preferPinnedWsPort = preferPinnedWsPort + this.requirePinnedWsPort = requirePinnedWsPort this.exposeNetworkByDefault = exposeNetworkByDefault this.pinnedBindHost = pinnedBindHost ?? null this.webClientRoot = webClientRoot From d18d20d30de6587df213d2e1a04c8159f2f454e9 Mon Sep 17 00:00:00 2001 From: m4air Date: Wed, 30 Sep 2026 09:55:32 -0700 Subject: [PATCH 2/2] test(runtime-rpc): require a failed pinned-port start to release its Unix socket The required-port startup test only checked the WebSocket side. Also assert no runtime socket file remains, which fails if the lifecycle stops rethrowing without first stopping the Unix socket transport. Porting note (#16741 a68b6f3531, T1): review follow-up; taken: none; adapted: strengthens the ported required-port test; dropped: none. --- src/main/runtime/runtime-rpc-required-ws-port.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/runtime/runtime-rpc-required-ws-port.test.ts b/src/main/runtime/runtime-rpc-required-ws-port.test.ts index 420dcbaa1117..5a264fcc0c00 100644 --- a/src/main/runtime/runtime-rpc-required-ws-port.test.ts +++ b/src/main/runtime/runtime-rpc-required-ws-port.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync } from 'node:fs' +import { mkdtempSync, readdirSync } from 'node:fs' import { createServer, type Server } from 'node:net' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -53,6 +53,8 @@ describe('OrcaRuntimeRpcServer required WebSocket port', () => { await expect(server.start()).rejects.toMatchObject({ code: 'EADDRINUSE' }) expect(boundWsPort(server)).toBeNull() expect(readWsFallbackPort(userDataPath)).toBeUndefined() + // Why: the failed start must also release the Unix socket it opened first, or the endpoint leaks. + expect(readdirSync(userDataPath).filter((name) => name.endsWith('.sock'))).toEqual([]) } finally { await server.stop() }