diff --git a/src/main/runtime/relay/relay-control-client-options.ts b/src/main/runtime/relay/relay-control-client-options.ts index 5b98737d11a..75d5c512098 100644 --- a/src/main/runtime/relay/relay-control-client-options.ts +++ b/src/main/runtime/relay/relay-control-client-options.ts @@ -21,5 +21,8 @@ export type RelayControlClientOptions = { onPendingChanged?: () => void createSocket?: (url: string, relayJwt: string) => WebSocket connectDeadlineMs?: number + // Why: settable apart from connectDeadlineMs so a test can stall the connect + // phase alone and prove which of the two bounds fired. + handshakeTimeoutMs?: number silenceLimitMs?: number } diff --git a/src/main/runtime/relay/relay-control-client.ts b/src/main/runtime/relay/relay-control-client.ts index 0c863cce3ad..984427fff68 100644 --- a/src/main/runtime/relay/relay-control-client.ts +++ b/src/main/runtime/relay/relay-control-client.ts @@ -9,7 +9,6 @@ import { RelayHostChallengeMessageSchema, RelayHostHelloAckMessageSchema, RelayPingMessageSchema, - RELAY_HOST_CAPABILITY_HEADERS, encodeRelayHostHello, parseRelayControlMessage, type RelayHostHelloAckMessage, @@ -23,6 +22,7 @@ import { RelayControlSilenceWatchdog } from './relay-control-silence-watchdog' import { closeRelayControlSocket } from './relay-control-socket-close' +import { createRelayControlSocket } from './relay-control-socket-factory' import { controlWebSocketUrl } from './relay-control-url' type RelayControlState = 'idle' | 'opening' | 'proving' | 'active' | 'draining' | 'closed' @@ -55,11 +55,13 @@ export class RelayControlClient { this.createSocket = options.createSocket ?? ((url, token) => - new WebSocket(url, { - headers: { authorization: `Bearer ${token}`, ...RELAY_HOST_CAPABILITY_HEADERS }, - perMessageDeflate: false, - maxPayload: 64 * 1024 - })) + createRelayControlSocket( + url, + token, + options.handshakeTimeoutMs ?? + options.connectDeadlineMs ?? + RELAY_CONTROL_CONNECT_DEADLINE_MS + )) } connect(): Promise { diff --git a/src/main/runtime/relay/relay-control-connect-transport-bound.test.ts b/src/main/runtime/relay/relay-control-connect-transport-bound.test.ts new file mode 100644 index 00000000000..7d47c0e09fc --- /dev/null +++ b/src/main/runtime/relay/relay-control-connect-transport-bound.test.ts @@ -0,0 +1,93 @@ +import { createHash } from 'node:crypto' +import { readFileSync } from 'node:fs' +import { createServer, type Server, type Socket } from 'node:net' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import nacl from 'tweetnacl' +import { isRemoteRuntimeConnectTimeout } from '../../../shared/remote-runtime-connect-bound' +import { RelayControlClient } from './relay-control-client' + +const servers = new Set() +const sockets = new Set() + +afterEach(async () => { + for (const socket of sockets) { + socket.destroy() + } + sockets.clear() + await Promise.all( + [...servers].map( + (server) => + new Promise((resolve) => { + server.close(() => resolve()) + }) + ) + ) + servers.clear() +}) + +/** + * Accepts TCP and never answers the HTTP upgrade, so the socket never opens. + * The incumbent relay test stalls the *proving* phase instead, which the + * transport bound cannot see — only this shape distinguishes the two bounds. + */ +async function listenSilentUpgradeServer(): Promise { + const server = createServer((socket) => { + sockets.add(socket) + socket.once('close', () => sockets.delete(socket)) + }) + servers.add(server) + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + const address = server.address() + if (address === null || typeof address === 'string') { + throw new Error('expected a TCP address') + } + return `http://127.0.0.1:${address.port}` +} + +function buildClient(cellUrl: string, overrides: { handshakeTimeoutMs?: number }) { + const keypair = nacl.box.keyPair() + return new RelayControlClient({ + cellUrl, + relayJwt: 'scoped-token', + relayHostId: createHash('sha256').update(keypair.publicKey).digest('base64url').slice(0, 16), + assignmentEpoch: 1, + identity: { userId: 'user-1', profileId: 'profile-1', organizationId: 'org-1' }, + keypair: { ...keypair, publicKeyB64: Buffer.from(keypair.publicKey).toString('base64') }, + appVersion: '1.2.3', + onConnectionOpen: vi.fn(), + onDrain: vi.fn(), + onClose: vi.fn(), + // Why: 50x the transport bound, so whichever error arrives names the bound + // that produced it rather than the one that merely exists. + connectDeadlineMs: 5_000, + ...overrides + }) +} + +describe('relay control connect transport bound', () => { + it('bounds a connect that never opens, and names the transport bound', async () => { + const cellUrl = await listenSilentUpgradeServer() + const client = buildClient(cellUrl, { handshakeTimeoutMs: 100 }) + + const error = await client.connect().then( + () => null, + (reason: unknown) => reason as Error + ) + + expect(error).toBeInstanceOf(Error) + // The transport bound fired, not the class deadline that also covers this. + expect(isRemoteRuntimeConnectTimeout(error)).toBe(true) + expect(error?.message).not.toContain('relay_control_connect_timeout') + }) + + // Why: defence in depth only works if both bounds survive. Either one removed + // as "redundant" leaves a phase uncovered. + it('keeps both bounds, since each covers a phase the other cannot', () => { + const client = readFileSync(join(__dirname, 'relay-control-client.ts'), 'utf8') + const factory = readFileSync(join(__dirname, 'relay-control-socket-factory.ts'), 'utf8') + expect(factory).toContain('remoteRuntimeConnectOptions(') + expect(client).toContain('this.connectTimer = setTimeout(') + expect(client).toContain("new Error('relay_control_connect_timeout')") + }) +}) diff --git a/src/main/runtime/relay/relay-control-socket-factory.ts b/src/main/runtime/relay/relay-control-socket-factory.ts new file mode 100644 index 00000000000..5f7d3b90d6c --- /dev/null +++ b/src/main/runtime/relay/relay-control-socket-factory.ts @@ -0,0 +1,35 @@ +import WebSocket from 'ws' +import { remoteRuntimeConnectOptions } from '../../../shared/remote-runtime-connect-bound' +import { RELAY_HOST_CAPABILITY_HEADERS } from './relay-control-protocol' + +/** + * Builds the relay control socket with a transport-level connect bound. + * + * Why this exists separately from `connectDeadlineMs` in `RelayControlClient`: + * that deadline bounds the whole handshake including the host proof, but it can + * only run once the socket object exists, and it is the class's to arm. A + * black-holed relay never opens and never errors, so the connect sub-phase + * needs its own bound at the transport — the same one the remote-runtime + * transports use. Both are kept deliberately: they cover different phases and + * neither is redundant. See #18191. + * + * Keeping the construction here means a caller that reaches for the relay + * control socket gets the bound, rather than re-deriving an unbounded one. + */ +export function createRelayControlSocket( + url: string, + relayJwt: string, + connectBoundMs: number +): WebSocket { + return new WebSocket( + url, + remoteRuntimeConnectOptions( + { + headers: { authorization: `Bearer ${relayJwt}`, ...RELAY_HOST_CAPABILITY_HEADERS }, + perMessageDeflate: false, + maxPayload: 64 * 1024 + }, + connectBoundMs + ) + ) +} diff --git a/src/shared/remote-runtime-connect-bound.test.ts b/src/shared/remote-runtime-connect-bound.test.ts new file mode 100644 index 00000000000..2d8cc98feef --- /dev/null +++ b/src/shared/remote-runtime-connect-bound.test.ts @@ -0,0 +1,159 @@ +import { existsSync, readFileSync, readdirSync } from 'node:fs' +import { createServer, type Server, type Socket } from 'node:net' +import { basename, join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { generateKeyPair, publicKeyToBase64 } from './e2ee-crypto' +import type { RemoteRuntimeClientError } from './remote-runtime-client-error' +import { + REMOTE_RUNTIME_CONNECT_TIMEOUT_MS, + isRemoteRuntimeConnectTimeout, + remoteRuntimeConnectFailureMessage, + remoteRuntimeConnectOptions +} from './remote-runtime-connect-bound' +import { openRemoteRuntimeWebSocket } from './remote-runtime-request-websocket' + +const servers = new Set() +const sockets = new Set() + +const RELAY_CONTROL_SOCKET_FACTORY = join('relay', 'relay-control-socket-factory.ts') + +/** + * Files whose WebSocket construction must carry the connect bound. The shared + * remote-runtime transports are swept by prefix; sites outside this directory + * are listed explicitly so adding one is a deliberate act rather than a glob + * accident. Other WebSocket sites (relay data transport, emulator control) + * carry their own bounds and are deliberately not covered here. + */ +function coveredSocketSources(): string[] { + const shared = readdirSync(__dirname) + .filter( + (name) => + name.startsWith('remote-runtime-') && name.endsWith('.ts') && !name.includes('.test.') + ) + .map((name) => join(__dirname, name)) + const relaySocketFactory = join( + __dirname, + '..', + 'main', + 'runtime', + 'relay', + 'relay-control-socket-factory.ts' + ) + if (!existsSync(relaySocketFactory)) { + throw new Error(`connect-bound ratchet lost its relay site: ${relaySocketFactory}`) + } + return [...shared, relaySocketFactory] +} + +afterEach(async () => { + for (const socket of sockets) { + socket.destroy() + } + sockets.clear() + await Promise.all( + [...servers].map( + (server) => + new Promise((resolve) => { + server.close(() => resolve()) + }) + ) + ) + servers.clear() +}) + +/** + * Accepts TCP but never answers the HTTP upgrade, which is the same silent + * stall a black-holed host produces and is bounded by the same `ws` timer. + */ +async function listenSilentUpgradeServer(): Promise { + const server = createServer((socket) => { + sockets.add(socket) + socket.once('close', () => sockets.delete(socket)) + }) + servers.add(server) + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + const address = server.address() + if (address === null || typeof address === 'string') { + throw new Error('expected a TCP address') + } + return `ws://127.0.0.1:${address.port}` +} + +describe('remote runtime connect bound', () => { + it('bounds the production connect with a finite handshake timeout', () => { + const options = remoteRuntimeConnectOptions({ maxPayload: 1024 }) + expect(Number.isFinite(options.handshakeTimeout)).toBe(true) + expect(options.handshakeTimeout).toBe(REMOTE_RUNTIME_CONNECT_TIMEOUT_MS) + expect(options.maxPayload).toBe(1024) + }) + + // Why: the bound only helps if every Node-side remote-runtime socket carries + // it; a new transport that calls `new WebSocket` directly reintroduces #18191. + it('routes every covered WebSocket construction through the bounded options', () => { + const offenders: string[] = [] + let scannedConstructions = 0 + for (const path of coveredSocketSources()) { + const source = readFileSync(path, 'utf8') + const constructions = source.split('new WebSocket(').length - 1 + const bounded = source.split('remoteRuntimeConnectOptions(').length - 1 + scannedConstructions += constructions + if (constructions > bounded) { + offenders.push(`${basename(path)}: ${constructions} WebSocket(s), ${bounded} bounded`) + } + } + expect(offenders).toEqual([]) + // Guards against the scan silently matching nothing and passing vacuously. + expect(scannedConstructions).toBeGreaterThan(0) + // Guards the relay site specifically: an allowlist that quietly stopped + // resolving a path would still satisfy the count above. + expect( + coveredSocketSources().some((path) => path.endsWith(RELAY_CONTROL_SOCKET_FACTORY)) + ).toBe(true) + }) + + it('reports an unanswered host as unreachable rather than as an empty result', async () => { + const endpoint = await listenSilentUpgradeServer() + const keyPair = generateKeyPair() + const onError = vi.fn() + const onTextFrame = vi.fn() + + const opened = openRemoteRuntimeWebSocket( + { + v: 2, + endpoint, + deviceToken: 'device-token', + publicKeyB64: publicKeyToBase64(keyPair.publicKey) + }, + { onClose: vi.fn(), onError, onTextFrame }, + 150 + ) + if (!opened.ok) { + throw opened.error + } + + await vi.waitFor(() => expect(onError).toHaveBeenCalledTimes(1), { + timeout: 5_000 + }) + + // The bounded path was taken: a connect failure, not a silent empty answer. + const error = onError.mock.calls[0][1] as RemoteRuntimeClientError + expect(error.code).toBe('remote_runtime_unavailable') + expect(error.message).toContain(endpoint) + expect(error.message).toContain('unverifiable') + expect(onTextFrame).not.toHaveBeenCalled() + + // Loss of contact is never evidence the host's work stopped. + expect(error.message).not.toMatch(/\b(exited|gone|stopped|empty|no terminals)\b/i) + + opened.socket.cleanup() + opened.socket.ws.terminate() + }) + + it('only calls an elapsed handshake a connect timeout', () => { + expect(isRemoteRuntimeConnectTimeout(new Error('Opening handshake has timed out'))).toBe(true) + expect(isRemoteRuntimeConnectTimeout(new Error('connect ECONNREFUSED'))).toBe(false) + expect(remoteRuntimeConnectFailureMessage(new Error('connect ECONNREFUSED'), 'ws://h')).toBe( + 'Could not connect to the remote Orca runtime.' + ) + }) +}) diff --git a/src/shared/remote-runtime-connect-bound.ts b/src/shared/remote-runtime-connect-bound.ts new file mode 100644 index 00000000000..6e8b9c947f4 --- /dev/null +++ b/src/shared/remote-runtime-connect-bound.ts @@ -0,0 +1,52 @@ +import type { ClientOptions } from 'ws' + +/** + * Connect-phase bound for the Node-side remote-runtime WebSocket transports. + * + * Why: a host that is powered off or firewalled black-holes the TCP SYN, so the + * socket neither opens nor errors. Without this the only bound is the caller's + * whole-request timeout (60s in the CLI), which reads to the user as a frozen + * terminal. `ws` applies `handshakeTimeout` across TCP connect *and* the HTTP + * upgrade, so one option covers both silent stalls. + * + * The value matches `CONNECT_TIMEOUT_MS` in + * `src/renderer/src/web/web-runtime-connection-transport.ts`, which already + * bounded the browser transport; this brings the Node transports in line. + */ +export const REMOTE_RUNTIME_CONNECT_TIMEOUT_MS = 12_000 + +/** The `ws` message for an elapsed `handshakeTimeout`; matched, never thrown by us. */ +const WS_HANDSHAKE_TIMEOUT_MESSAGE = 'Opening handshake has timed out' + +export function remoteRuntimeConnectOptions( + options?: TOptions, + connectTimeoutMs: number = REMOTE_RUNTIME_CONNECT_TIMEOUT_MS +): TOptions & { handshakeTimeout: number } { + return { + ...(options ?? ({} as TOptions)), + handshakeTimeout: connectTimeoutMs + } +} + +export function isRemoteRuntimeConnectTimeout(error: unknown): boolean { + return error instanceof Error && error.message === WS_HANDSHAKE_TIMEOUT_MESSAGE +} + +/** + * Why: per `docs/reference/ssh-execution-boundary.md`, loss of contact is never + * evidence that remote work stopped. This message says the host did not answer + * and stops there — it must not imply the host's terminals are gone. + */ +export function remoteRuntimeConnectFailureMessage( + error: unknown, + endpoint: string, + connectTimeoutMs: number = REMOTE_RUNTIME_CONNECT_TIMEOUT_MS +): string { + if (!isRemoteRuntimeConnectTimeout(error)) { + return 'Could not connect to the remote Orca runtime.' + } + return ( + `Could not reach the remote Orca runtime at ${endpoint} within ${connectTimeoutMs}ms. ` + + 'The host did not answer, so anything running on it is unverifiable.' + ) +} diff --git a/src/shared/remote-runtime-request-socket.ts b/src/shared/remote-runtime-request-socket.ts index 2ace6b4816f..cb0a04c2898 100644 --- a/src/shared/remote-runtime-request-socket.ts +++ b/src/shared/remote-runtime-request-socket.ts @@ -16,6 +16,10 @@ import { ignoreSettledRemoteRuntimeSocketError } from './remote-runtime-client-handshake' import { RemoteRuntimeClientError } from './remote-runtime-client-error' +import { + remoteRuntimeConnectFailureMessage, + remoteRuntimeConnectOptions +} from './remote-runtime-connect-bound' import { REMOTE_RUNTIME_MAX_WEBSOCKET_FRAME_BYTES, serializeRemoteRuntimePayload, @@ -183,7 +187,10 @@ export async function sendRemoteRuntimeRequestOnSocket( } try { - ws = new WebSocket(pairing.endpoint, { maxPayload: REMOTE_RUNTIME_MAX_WEBSOCKET_FRAME_BYTES }) + const connectOptions = remoteRuntimeConnectOptions({ + maxPayload: REMOTE_RUNTIME_MAX_WEBSOCKET_FRAME_BYTES + }) + ws = new WebSocket(pairing.endpoint, connectOptions) } catch (error) { const message = error instanceof Error ? error.message : String(error) finishError( @@ -201,11 +208,11 @@ export async function sendRemoteRuntimeRequestOnSocket( ) } - function onError(): void { + function onError(error: Error): void { finishError( new RemoteRuntimeClientError( 'remote_runtime_unavailable', - 'Could not connect to the remote Orca runtime.', + remoteRuntimeConnectFailureMessage(error, pairing.endpoint), { pairingStage: router.pairingStage } ) ) diff --git a/src/shared/remote-runtime-request-websocket.ts b/src/shared/remote-runtime-request-websocket.ts index e5b0ad8212d..85d028a680c 100644 --- a/src/shared/remote-runtime-request-websocket.ts +++ b/src/shared/remote-runtime-request-websocket.ts @@ -7,6 +7,10 @@ import { publicKeyToBase64 } from './e2ee-crypto' import { RemoteRuntimeClientError } from './remote-runtime-client' +import { + remoteRuntimeConnectFailureMessage, + remoteRuntimeConnectOptions +} from './remote-runtime-connect-bound' import { invalidRemoteRuntimeResponseError, remoteRuntimeUnavailableError @@ -30,9 +34,12 @@ export type RemoteRuntimeWebSocketCallbacks = { export function openRemoteRuntimeWebSocket( pairing: PairingOffer, - callbacks: RemoteRuntimeWebSocketCallbacks + callbacks: RemoteRuntimeWebSocketCallbacks, + // Why: overridable so the connect-bound regression test can pin the behaviour + // without spending the production budget of wall-clock time. + connectTimeoutMs?: number ): { ok: true; socket: RemoteRuntimeWebSocket } | { ok: false; error: RemoteRuntimeClientError } { - const opened = createSocket(pairing) + const opened = createSocket(pairing, connectTimeoutMs) if (!opened.ok) { return opened } @@ -49,10 +56,12 @@ export function openRemoteRuntimeWebSocket( }) ) } - const onError = (): void => { + const onError = (error: Error): void => { callbacks.onError( ws, - remoteRuntimeUnavailableError('Could not connect to the remote Orca runtime.') + remoteRuntimeUnavailableError( + remoteRuntimeConnectFailureMessage(error, pairing.endpoint, connectTimeoutMs) + ) ) } const onClose = (code: number, reason: Buffer): void => callbacks.onClose(ws, code, reason) @@ -100,7 +109,8 @@ export function openRemoteRuntimeWebSocket( function ignoreLateSocketError(): void {} function createSocket( - pairing: PairingOffer + pairing: PairingOffer, + connectTimeoutMs?: number ): | { ok: true; ws: WebSocket; keyPair: ReturnType } | { ok: false; error: RemoteRuntimeClientError } { @@ -119,7 +129,11 @@ function createSocket( } } try { - return { ok: true, ws: new WebSocket(pairing.endpoint), keyPair } + return { + ok: true, + ws: new WebSocket(pairing.endpoint, remoteRuntimeConnectOptions(undefined, connectTimeoutMs)), + keyPair + } } catch (error) { const message = error instanceof Error ? error.message : String(error) return { diff --git a/src/shared/remote-runtime-subscription-transport.ts b/src/shared/remote-runtime-subscription-transport.ts index 9376cbbb9a5..ce68aac5340 100644 --- a/src/shared/remote-runtime-subscription-transport.ts +++ b/src/shared/remote-runtime-subscription-transport.ts @@ -14,6 +14,10 @@ import { ignoreSettledRemoteRuntimeSocketError } from './remote-runtime-client-handshake' import { RemoteRuntimeClientError } from './remote-runtime-client-error' +import { + remoteRuntimeConnectFailureMessage, + remoteRuntimeConnectOptions +} from './remote-runtime-connect-bound' import { isRemoteRuntimeBinaryFrameWithinLimit, REMOTE_RUNTIME_MAX_WEBSOCKET_FRAME_BYTES, @@ -225,11 +229,12 @@ export async function subscribeRemoteRuntimeTransport( callbacks.onClose?.() } + const connectOptions = remoteRuntimeConnectOptions({ + maxPayload: REMOTE_RUNTIME_MAX_WEBSOCKET_FRAME_BYTES, + ...(options?.perMessageDeflate === false ? { perMessageDeflate: false } : {}) + }) try { - ws = new WebSocket(pairing.endpoint, { - maxPayload: REMOTE_RUNTIME_MAX_WEBSOCKET_FRAME_BYTES, - ...(options?.perMessageDeflate === false ? { perMessageDeflate: false } : {}) - }) + ws = new WebSocket(pairing.endpoint, connectOptions) } catch (error) { const message = error instanceof Error ? error.message : String(error) fail(new RemoteRuntimeClientError('invalid_argument', `Invalid remote endpoint: ${message}`)) @@ -242,11 +247,11 @@ export async function subscribeRemoteRuntimeTransport( ) } - function onError(): void { + function onError(error: Error): void { fail( new RemoteRuntimeClientError( 'remote_runtime_unavailable', - 'Could not connect to the remote Orca runtime.' + remoteRuntimeConnectFailureMessage(error, pairing.endpoint) ) ) }