merge PR 20053 remote-connect-bound

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
 }

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<RelayHostHelloAckMessage> {

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<Server>()
+const sockets = new Set<Socket>()
+
+afterEach(async () => {
+  for (const socket of sockets) {
+    socket.destroy()
+  }
+  sockets.clear()
+  await Promise.all(
+    [...servers].map(
+      (server) =>
+        new Promise<void>((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<string> {
+  const server = createServer((socket) => {
+    sockets.add(socket)
+    socket.once('close', () => sockets.delete(socket))
+  })
+  servers.add(server)
+  await new Promise<void>((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')")
+  })
+})

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
+    )
+  )
+}

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<Server>()
+const sockets = new Set<Socket>()
+
+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<void>((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<string> {
+  const server = createServer((socket) => {
+    sockets.add(socket)
+    socket.once('close', () => sockets.delete(socket))
+  })
+  servers.add(server)
+  await new Promise<void>((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.'
+    )
+  })
+})

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<TOptions extends ClientOptions>(
+  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.'
+  )
+}

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<TResult>(
     }
 
     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<TResult>(
       )
     }
 
-    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 }
         )
       )

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<typeof generateKeyPair> }
   | { 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 {

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<TResult>(
       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<TResult>(
       )
     }
 
-    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)
         )
       )
     }