From 335cc32c855fdfa435b72de9ae417f057ce0dfbc Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Tue, 13 Oct 2026 14:10:42 -0700 Subject: [PATCH] fix(ssh): an SSH host comes back after restarting Orca instead of staying offline (STA-10476) (#28398) Co-authored-by: m4air --- src/main/ipc/ssh-app-shutdown.test.ts | 33 ++- src/main/ipc/ssh-ipc-mock-shapes.ts | 1 + src/main/ipc/ssh-ipc-module-mocks.ts | 1 + src/main/ipc/ssh-ipc-test-harness.ts | 2 + src/main/ipc/ssh-shutdown-drain.ts | 9 +- src/main/ssh/orcad-managed-tunnel-manager.ts | 7 + src/main/ssh/orcad-managed-tunnel.test.ts | 30 ++- src/main/ssh/orcad-managed-tunnel.ts | 5 + src/main/ssh/ssh-port-forward.test.ts | 32 +++ src/main/ssh/ssh-port-forward.ts | 5 + .../ssh/system-ssh-forward-process.test.ts | 55 ++++- src/main/ssh/system-ssh-forward-process.ts | 57 ++++- .../ssh/system-ssh-orphaned-forward.test.ts | 197 ++++++++++++++++++ src/main/ssh/system-ssh-orphaned-forward.ts | 175 ++++++++++++++++ 14 files changed, 590 insertions(+), 19 deletions(-) create mode 100644 src/main/ssh/system-ssh-orphaned-forward.test.ts create mode 100644 src/main/ssh/system-ssh-orphaned-forward.ts diff --git a/src/main/ipc/ssh-app-shutdown.test.ts b/src/main/ipc/ssh-app-shutdown.test.ts index 06943fcd989..336a447d4aa 100644 --- a/src/main/ipc/ssh-app-shutdown.test.ts +++ b/src/main/ipc/ssh-app-shutdown.test.ts @@ -13,13 +13,18 @@ vi.mock('../ssh/ssh-connection-manager', () => mocks.sshConnectionManager) vi.mock('../ssh/ssh-channel-multiplexer', () => mocks.sshChannelMultiplexer) vi.mock('../ssh/ssh-port-forward', () => mocks.sshPortForward) vi.mock('../ssh/ssh-port-scanner', () => mocks.sshPortScanner) +const { shutdownTunnelsMock } = vi.hoisted(() => ({ shutdownTunnelsMock: vi.fn() })) +vi.mock('../ssh/orcad-managed-tunnel', async (importOriginal) => ({ + ...(await importOriginal>()), + shutdownOrcadManagedTunnels: shutdownTunnelsMock +})) import { beginSshShutdown } from './ssh-shutdown-drain' import type { SshTarget } from '../../shared/ssh-types' import { quitTeardownStartGate } from '../quit-teardown-start-gate' import { createSshIpcHarness } from './ssh-ipc-test-harness' -const { mockSshStore, mockConnectionManager } = mocks +const { mockSshStore, mockConnectionManager, mockPortForwardManager } = mocks describe('SSH IPC handlers', () => { const harness = createSshIpcHarness(mocks) @@ -42,6 +47,32 @@ describe('SSH IPC handlers', () => { } }) + it('stops every port forward and managed tunnel process before quit finishes', async () => { + mockConnectionManager.disconnectAll.mockResolvedValue(undefined) + let finishForwards!: () => void + mockPortForwardManager.closeAll.mockReturnValue( + new Promise((resolve) => { + finishForwards = resolve + }) + ) + shutdownTunnelsMock.mockResolvedValue(undefined) + quitTeardownStartGate.tryStart({ preventDefault() {} }) + + let finished = false + const shutdown = beginSshShutdown().then(() => { + finished = true + }) + for (let tick = 0; tick < 5; tick++) { + await Promise.resolve() + } + expect(finished).toBe(false) + expect(shutdownTunnelsMock).toHaveBeenCalled() + finishForwards() + await shutdown + + expect(mockPortForwardManager.closeAll).toHaveBeenCalled() + }) + it('joins an in-flight test-connection probe before the final shutdown disconnect', async () => { const target: SshTarget = { id: 'ssh-probe', diff --git a/src/main/ipc/ssh-ipc-mock-shapes.ts b/src/main/ipc/ssh-ipc-mock-shapes.ts index a8fb651610d..9cec29314f3 100644 --- a/src/main/ipc/ssh-ipc-mock-shapes.ts +++ b/src/main/ipc/ssh-ipc-mock-shapes.ts @@ -49,6 +49,7 @@ export type SshPortForwardManagerMock = { removeForwardAndWait: Mock listForwards: Mock removeAllForwards: Mock + closeAll: Mock dispose: Mock setCallbacks: Mock callbacksRef: MockCallbacksRef diff --git a/src/main/ipc/ssh-ipc-module-mocks.ts b/src/main/ipc/ssh-ipc-module-mocks.ts index 1630be06283..86aa80c50a9 100644 --- a/src/main/ipc/ssh-ipc-module-mocks.ts +++ b/src/main/ipc/ssh-ipc-module-mocks.ts @@ -53,6 +53,7 @@ export function createSshIpcMocks(): SshIpcMocks { removeForwardAndWait: vi.fn(), listForwards: vi.fn().mockReturnValue([]), removeAllForwards: vi.fn(), + closeAll: vi.fn(), dispose: vi.fn(), setCallbacks: vi.fn(), callbacksRef: { current: null as unknown } diff --git a/src/main/ipc/ssh-ipc-test-harness.ts b/src/main/ipc/ssh-ipc-test-harness.ts index fc25e36c5a7..17381a3499e 100644 --- a/src/main/ipc/ssh-ipc-test-harness.ts +++ b/src/main/ipc/ssh-ipc-test-harness.ts @@ -72,6 +72,7 @@ export function createSshIpcHarness(mocks: SshIpcMocks): SshIpcHarness { removeForward: vi.fn(), listForwards: vi.fn().mockReturnValue([]), removeAllForwards: vi.fn(), + closeAll: vi.fn(), dispose: vi.fn(), setCallbacks: vi.fn(), callbacksRef: { current: null as unknown } @@ -122,6 +123,7 @@ export function createSshIpcHarness(mocks: SshIpcMocks): SshIpcHarness { mockPortForwardManager.removeForwardAndWait.mockReset() mockPortForwardManager.listForwards.mockReset().mockReturnValue([]) mockPortForwardManager.removeAllForwards.mockReset() + mockPortForwardManager.closeAll.mockReset().mockResolvedValue(undefined) mockPortForwardManager.dispose.mockReset() mockPortForwardManager.setCallbacks.mockReset() mockPortForwardManager.callbacksRef.current = null diff --git a/src/main/ipc/ssh-shutdown-drain.ts b/src/main/ipc/ssh-shutdown-drain.ts index 69e35de07a1..d4855b0460b 100644 --- a/src/main/ipc/ssh-shutdown-drain.ts +++ b/src/main/ipc/ssh-shutdown-drain.ts @@ -3,7 +3,8 @@ import { invalidateConnectAttempt, testConnectionProbes } from './ssh-connect-attempt-registry' -import { connectionManager } from './ssh-ipc-context' +import { connectionManager, portForwardManager } from './ssh-ipc-context' +import { shutdownOrcadManagedTunnels } from '../ssh/orcad-managed-tunnel' // Why one budget for the whole sequence rather than one per phase: an invalidated connect only // observes its cancellation at the next checkpoint, and one blocked in the transport handshake can @@ -63,6 +64,12 @@ function sshShutdownTasks(): SshShutdownTask[] { { targetId: '*transports', promise: connectionManager?.disconnectAll() ?? Promise.resolve() + }, + { + // Why: a system-ssh forward is a separate `ssh -L` child that survives Orca's exit and keeps + // its local port, so the next launch cannot bind a managed tunnel's fixed port. + targetId: '*forwards', + promise: Promise.all([portForwardManager?.closeAll(), shutdownOrcadManagedTunnels()]) } ] } diff --git a/src/main/ssh/orcad-managed-tunnel-manager.ts b/src/main/ssh/orcad-managed-tunnel-manager.ts index 4c012bc1a9a..416dada3822 100644 --- a/src/main/ssh/orcad-managed-tunnel-manager.ts +++ b/src/main/ssh/orcad-managed-tunnel-manager.ts @@ -182,6 +182,13 @@ export class OrcadManagedTunnelManager { await this.forwards.removeForwardAndWait(active.forwardId) } + /** Quit: supersedes every build in flight and waits for each forward process to exit. */ + async shutdown(): Promise { + this.managerGeneration += 1 + this.active.clear() + await this.forwards.closeAll() + } + dispose(): void { this.managerGeneration += 1 this.active.clear() diff --git a/src/main/ssh/orcad-managed-tunnel.test.ts b/src/main/ssh/orcad-managed-tunnel.test.ts index 25b496fe649..ff647208ec3 100644 --- a/src/main/ssh/orcad-managed-tunnel.test.ts +++ b/src/main/ssh/orcad-managed-tunnel.test.ts @@ -85,12 +85,14 @@ function setup(overrides: Partial = {}, targeting?: OrcadManagedTunne }) ) const removeForwardAndWait = vi.fn().mockResolvedValue(null) + const closeAll = vi.fn().mockResolvedValue(undefined) const ensureServing = vi.fn().mockResolvedValue({ state: 'serving' }) // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: a test double for the members the tunnel manager calls. const forwardManager = { setCallbacks: vi.fn(), addForward, removeForwardAndWait, + closeAll, dispose: vi.fn() } as unknown as SshPortForwardManager // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: a test double for the members the tunnel manager calls. @@ -111,6 +113,7 @@ function setup(overrides: Partial = {}, targeting?: OrcadManagedTunne }) return { addForward, + closeAll, connect, connection, ensureServing, @@ -128,7 +131,7 @@ function setup(overrides: Partial = {}, targeting?: OrcadManagedTunne } describe('OrcadManagedTunnelManager initial binding', () => { - it.each(['close', 'dispose', 'reconnect'] as const)( + it.each(['close', 'dispose', 'shutdown', 'reconnect'] as const)( 'removes a late initial forward after %s supersedes setup', async (action) => { const state = setup() @@ -139,6 +142,9 @@ describe('OrcadManagedTunnelManager initial binding', () => { if (action === 'dispose') { state.manager.dispose() } + if (action === 'shutdown') { + await state.manager.shutdown() + } if (action === 'reconnect') { state.setTransportGeneration(4) } @@ -155,6 +161,28 @@ describe('OrcadManagedTunnelManager initial binding', () => { } ) + it('shutdown waits for every forward process to stop, so none outlives quit', async () => { + const state = setup() + await state.manager.ensure(createEnvironment('sshAccess')) + let finishClose!: () => void + state.closeAll.mockReturnValueOnce( + new Promise((resolve) => { + finishClose = resolve + }) + ) + + let done = false + const shutdown = state.manager.shutdown().then(() => { + done = true + }) + await Promise.resolve() + expect(done).toBe(false) + finishClose() + await shutdown + + expect(state.closeAll).toHaveBeenCalledOnce() + }) + it('does not open a forward when canceled while the prior forward is closing', async () => { const state = setup() await state.manager.ensure(createEnvironment('sshAccess')) diff --git a/src/main/ssh/orcad-managed-tunnel.ts b/src/main/ssh/orcad-managed-tunnel.ts index e30017bee8f..c206eb87aff 100644 --- a/src/main/ssh/orcad-managed-tunnel.ts +++ b/src/main/ssh/orcad-managed-tunnel.ts @@ -57,6 +57,11 @@ export function disposeOrcadManagedTunnels(): void { managedTunnels.dispose() } +/** Why on quit: a system-ssh tunnel is its own `ssh -L` process and would outlive Orca. */ +export function shutdownOrcadManagedTunnels(): Promise { + return managedTunnels.shutdown() +} + export function recoverOrcadManagedTunnelsAfterHostResume( userDataPath: string, options: { attempts: number; timeoutMs: number } diff --git a/src/main/ssh/ssh-port-forward.test.ts b/src/main/ssh/ssh-port-forward.test.ts index f4af3a36e6e..a125cd7e915 100644 --- a/src/main/ssh/ssh-port-forward.test.ts +++ b/src/main/ssh/ssh-port-forward.test.ts @@ -428,6 +428,38 @@ describe('SshPortForwardManager', () => { expect(manager.listForwards('conn-2')).toHaveLength(1) }) + it('closeAll waits for every system SSH forward to exit, across connections', async () => { + const onForwardClosed = vi.fn() + manager.setCallbacks({ onForwardClosed }) + let resolveClose!: () => void + const first = createFakeSystemSshForward(3000) + first.close.mockReturnValue( + new Promise((resolve) => { + resolveClose = resolve + }) + ) + const second = createFakeSystemSshForward(3001) + startSystemSshPortForwardProcessMock.mockReturnValueOnce(first).mockReturnValueOnce(second) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the mock implements the system-SSH surface the provider reads. + const conn = createSystemSshConn() as never + await manager.addForward('conn-1', conn, 3000, '127.0.0.1', 8080) + await manager.addForward('conn-2', conn, 3001, '127.0.0.1', 8081) + + let resolved = false + const closing = manager.closeAll().then(() => { + resolved = true + }) + await Promise.resolve() + expect(resolved).toBe(false) + expect(second.close).toHaveBeenCalled() + resolveClose() + await closing + + expect(manager.listForwards()).toHaveLength(0) + first.process.emit('exit', null) + expect(onForwardClosed).not.toHaveBeenCalled() + }) + it('dispose removes all forwards', async () => { const conn = createMockConn() await manager.addForward('conn-1', conn as never, 3000, 'localhost', 8080) diff --git a/src/main/ssh/ssh-port-forward.ts b/src/main/ssh/ssh-port-forward.ts index 4c7b7207825..f7b79bef28f 100644 --- a/src/main/ssh/ssh-port-forward.ts +++ b/src/main/ssh/ssh-port-forward.ts @@ -191,6 +191,11 @@ export class SshPortForwardManager { await Promise.all(stale.map((id) => this.removeForwardAsync(id))) } + /** Stops every forward and waits until each released its port; quit uses it. */ + async closeAll(): Promise { + await Promise.all([...this.forwards.keys()].map((id) => this.removeForwardAsync(id))) + } + dispose(): void { const ids = [...this.forwards.keys()] for (const id of ids) { diff --git a/src/main/ssh/system-ssh-forward-process.test.ts b/src/main/ssh/system-ssh-forward-process.test.ts index ffc9c20975d..f1bb5407e49 100644 --- a/src/main/ssh/system-ssh-forward-process.test.ts +++ b/src/main/ssh/system-ssh-forward-process.test.ts @@ -1,11 +1,18 @@ import { EventEmitter } from 'node:events' import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest' -const { existsSyncMock, spawnMock, connectMock, createServerMock } = vi.hoisted(() => ({ - existsSyncMock: vi.fn(), - spawnMock: vi.fn(), - connectMock: vi.fn(), - createServerMock: vi.fn() +const { existsSyncMock, spawnMock, connectMock, createServerMock, stopOrphanMock } = vi.hoisted( + () => ({ + existsSyncMock: vi.fn(), + spawnMock: vi.fn(), + connectMock: vi.fn(), + createServerMock: vi.fn(), + stopOrphanMock: vi.fn() + }) +) + +vi.mock('./system-ssh-orphaned-forward', () => ({ + stopOrphanedOrcaSshForward: stopOrphanMock })) vi.mock('fs', async (importOriginal) => { @@ -113,6 +120,7 @@ describe('system SSH forward process', () => { spawnMock.mockReset() connectMock.mockReset() createServerMock.mockReset().mockImplementation(createFakeServer) + stopOrphanMock.mockReset().mockResolvedValue(false) mockSystemSshExists() }) @@ -232,6 +240,43 @@ describe('system SSH forward process', () => { expect(spawnMock).not.toHaveBeenCalled() }) + it('stops a forward an earlier Orca orphaned on the port, then binds it', async () => { + const busy = createFakeServer() + busy.listen.mockImplementation(() => { + queueMicrotask(() => + busy.emit('error', Object.assign(new Error('EADDRINUSE'), { code: 'EADDRINUSE' })) + ) + return busy + }) + createServerMock.mockReturnValueOnce(busy) + stopOrphanMock.mockResolvedValue(true) + spawnMock.mockReturnValue(createFakeProcess()) + + const forward = await startSystemSshPortForwardProcess(createTarget(), 5173, '127.0.0.1', 3000) + + expect(stopOrphanMock).toHaveBeenCalledWith(5173, expect.any(Array)) + expect(spawnMock.mock.calls[0][1]).toEqual(stopOrphanMock.mock.calls[0][1]) + expect(forward.localPort).toBe(5173) + expect(spawnMock).toHaveBeenCalledOnce() + }) + + it('keeps the port error when nothing Orca orphaned holds the port', async () => { + const server = createFakeServer() + server.listen.mockImplementation(() => { + queueMicrotask(() => + server.emit('error', Object.assign(new Error('EADDRINUSE'), { code: 'EADDRINUSE' })) + ) + return server + }) + createServerMock.mockReturnValue(server) + + await expect( + startSystemSshPortForwardProcess(createTarget(), 5173, '127.0.0.1', 3000) + ).rejects.toThrow('Local port 127.0.0.1:5173 is not available') + expect(stopOrphanMock).toHaveBeenCalledWith(5173, expect.any(Array)) + expect(spawnMock).not.toHaveBeenCalled() + }) + it('spawns ssh after the requested local forward port preflight succeeds', async () => { const child = createFakeProcess() spawnMock.mockReturnValue(child) diff --git a/src/main/ssh/system-ssh-forward-process.ts b/src/main/ssh/system-ssh-forward-process.ts index 43661d216b0..e39e729deaa 100644 --- a/src/main/ssh/system-ssh-forward-process.ts +++ b/src/main/ssh/system-ssh-forward-process.ts @@ -1,6 +1,7 @@ import { spawn, type ChildProcess } from 'node:child_process' import { connect, createServer } from 'node:net' import { buildSshArgs, findSystemSsh, type SystemSshBuildArgsOptions } from './ssh-system-fallback' +import { stopOrphanedOrcaSshForward } from './system-ssh-orphaned-forward' import type { SshTarget } from '../../shared/ssh-types' export const SYSTEM_SSH_FORWARD_STARTUP_GRACE_MS = 750 @@ -32,7 +33,22 @@ export function spawnSystemSshPortForward( 'No system ssh binary found. Install OpenSSH to use system SSH port forwarding.' ) } + const args = buildSystemSshPortForwardArgs(target, localPort, remoteHost, remotePort, options) + // Why: port-forward ssh processes are not wired to Orca credential prompts; + // system SSH forwards must authenticate via OpenSSH config, agent, or control socket. + return spawn(sshPath, args, { + stdio: ['ignore', 'ignore', 'pipe'], + windowsHide: true + }) +} +export function buildSystemSshPortForwardArgs( + target: SshTarget, + localPort: number, + remoteHost: string, + remotePort: number, + options?: SystemSshBuildArgsOptions +): string[] { const args = buildSshArgs(target, { ...options, suppressOrcaControlMaster: true }) const destinationIndex = args.lastIndexOf('--') // Why `no`: this second login re-reads the host's ssh_config, so a LocalForward/RemoteForward @@ -52,13 +68,7 @@ export function spawnSystemSshPortForward( // destination and optional remote command. args.splice(destinationIndex, 0, ...forwardArgs) } - - // Why: port-forward ssh processes are not wired to Orca credential prompts; - // system SSH forwards must authenticate via OpenSSH config, agent, or control socket. - return spawn(sshPath, args, { - stdio: ['ignore', 'ignore', 'pipe'], - windowsHide: true - }) + return args } export function startSystemSshPortForwardProcess( @@ -68,7 +78,11 @@ export function startSystemSshPortForwardProcess( remotePort: number, options?: SystemSshBuildArgsOptions ): Promise { - return reserveLocalForwardPort(localPort).then((boundPort) => { + const orphanArgs = + localPort === 0 + ? undefined + : buildSystemSshPortForwardArgs(target, localPort, remoteHost, remotePort, options) + return reserveLocalForwardPort(localPort, orphanArgs).then((boundPort) => { const process = spawnSystemSshPortForward(target, boundPort, remoteHost, remotePort, options) stopOnOwnListenFailure(process, boundPort) return { @@ -120,11 +134,32 @@ function stopOnOwnListenFailure(process: ChildProcess, localPort: number): void * The port a system SSH forward will listen on. OpenSSH binds `-L 0` to a port it never * reports, so port 0 is resolved to a free one here first. */ -export function reserveLocalForwardPort(localPort: number): Promise { +export async function reserveLocalForwardPort( + localPort: number, + orphanArgs?: readonly string[] +): Promise { + try { + return await bindLocalForwardPort(localPort) + } catch (error) { + // Why: a fixed port (a managed tunnel's) may be held by the same forward an earlier Orca + // orphaned; `orphanArgs` is that forward's argv, the only process recovery may stop. + const inUse = error instanceof Error && error.cause === 'EADDRINUSE' + if (!orphanArgs || !inUse || !(await stopOrphanedOrcaSshForward(localPort, orphanArgs))) { + throw error + } + return bindLocalForwardPort(localPort) + } +} + +function bindLocalForwardPort(localPort: number): Promise { return new Promise((resolve, reject) => { const server = createServer() - server.once('error', (err: Error) => { - reject(new Error(`Local port 127.0.0.1:${localPort} is not available: ${err.message}`)) + server.once('error', (err: NodeJS.ErrnoException) => { + reject( + new Error(`Local port 127.0.0.1:${localPort} is not available: ${err.message}`, { + cause: err.code + }) + ) }) server.once('listening', () => { const address = server.address() diff --git a/src/main/ssh/system-ssh-orphaned-forward.test.ts b/src/main/ssh/system-ssh-orphaned-forward.test.ts new file mode 100644 index 00000000000..7974409b3ab --- /dev/null +++ b/src/main/ssh/system-ssh-orphaned-forward.test.ts @@ -0,0 +1,197 @@ +import { describe, expect, it, vi } from 'vitest' +import { + findOrphanedOrcaSshForward, + stopOrphanedOrcaSshForward, + type SshForwardProcessRow +} from './system-ssh-orphaned-forward' + +const EXPECTED = [ + '-o', + 'BatchMode=no', + '-T', + '-N', + '-o', + 'ExitOnForwardFailure=no', + '-L', + '127.0.0.1:47123:127.0.0.1:6768', + '--', + 'devbox' +] +// What an earlier version spawned for the same forward: ExitOnForwardFailure=yes. +const ORCA_FORWARD = + '/usr/bin/ssh -o BatchMode=no -T -N -o ExitOnForwardFailure=yes -L 127.0.0.1:47123:127.0.0.1:6768 -- devbox' + +function row(pid: number, ppid: number, command: string, creationTimeMs?: number) { + return { pid, ppid, command, creationTimeMs } satisfies SshForwardProcessRow +} + +describe('findOrphanedOrcaSshForward', () => { + it('finds a reparented Orca forward for this exact forward (macOS/Linux init)', () => { + expect(findOrphanedOrcaSshForward([row(900, 1, ORCA_FORWARD)], EXPECTED, 'darwin')?.pid).toBe( + 900 + ) + }) + + it('finds one reparented to a systemd --user subreaper', () => { + const rows = [row(50, 1, '/usr/lib/systemd/systemd --user'), row(900, 50, ORCA_FORWARD)] + expect(findOrphanedOrcaSshForward(rows, EXPECTED, 'linux')?.pid).toBe(900) + }) + + it("never picks a forward whose Orca is still running (another instance's tunnel)", () => { + const rows = [ + row(400, 1, '/Applications/Orca.app/Contents/MacOS/Orca'), + row(900, 400, ORCA_FORWARD) + ] + expect(findOrphanedOrcaSshForward(rows, EXPECTED, 'darwin')).toBeNull() + }) + + it("never picks the user's own ssh on the port, even an orphaned daemon in Orca's flag order", () => { + const rows = [ + row(900, 1, 'ssh -N -L 127.0.0.1:47123:127.0.0.1:6768 devbox'), + row( + 901, + 1, + 'ssh -o BatchMode=yes -T -N -o ExitOnForwardFailure=yes -L 127.0.0.1:47123:127.0.0.1:5432 -- production-db' + ), + row(902, 1, ORCA_FORWARD.replace('-- devbox', '-- otherbox')), + row(903, 1, ORCA_FORWARD.replace('/usr/bin/ssh ', '/usr/bin/ssh -f -F /home/u/.ssh/prod ')) + ] + expect(findOrphanedOrcaSshForward(rows, EXPECTED, 'darwin')).toBeNull() + }) + + it('never picks a forward for another port that shares a prefix', () => { + const command = ORCA_FORWARD.replace('127.0.0.1:47123:', '127.0.0.1:471230:') + expect(findOrphanedOrcaSshForward([row(900, 1, command)], EXPECTED, 'darwin')).toBeNull() + }) + + it('on Windows, treats a missing or younger parent as gone', () => { + const command = `"C:\\Windows\\System32\\OpenSSH\\ssh.exe" ${ORCA_FORWARD.slice('/usr/bin/ssh '.length)}` + expect( + findOrphanedOrcaSshForward([row(900, 400, command, 1_000)], EXPECTED, 'win32')?.pid + ).toBe(900) + expect( + findOrphanedOrcaSshForward( + [row(400, 4, 'C:\\Program Files\\Other\\other.exe', 2_000), row(900, 400, command, 1_000)], + EXPECTED, + 'win32' + )?.pid + ).toBe(900) + expect( + findOrphanedOrcaSshForward( + [row(400, 4, 'C:\\Orca\\Orca.exe', 500), row(900, 400, command, 1_000)], + EXPECTED, + 'win32' + ) + ).toBeNull() + }) +}) + +function esrch(): Error { + return Object.assign(new Error('gone'), { code: 'ESRCH' }) +} + +describe('stopOrphanedOrcaSshForward', () => { + it('terminates the orphan and waits until it is gone', async () => { + let alive = true + const kill = vi.fn((_pid: number, signal: NodeJS.Signals | 0) => { + if (signal === 0 && !alive) { + throw esrch() + } + if (signal === 'SIGTERM') { + alive = false + } + }) + const stopped = await stopOrphanedOrcaSshForward(47123, EXPECTED, { + listProcesses: async () => [row(900, 1, ORCA_FORWARD)], + kill, + readStartTimeMs: async () => 1_000, + platform: 'darwin' + }) + expect(stopped).toBe(true) + expect(kill).toHaveBeenCalledWith(900, 'SIGTERM') + expect(kill).not.toHaveBeenCalledWith(900, 'SIGKILL') + }) + + it('escalates to SIGKILL only while the same orphan is still there', async () => { + vi.useFakeTimers() + try { + const kill = vi.fn() + const stopping = stopOrphanedOrcaSshForward(47123, EXPECTED, { + listProcesses: async () => [row(900, 1, ORCA_FORWARD)], + kill, + readStartTimeMs: async () => 1_000, + platform: 'darwin' + }) + await vi.advanceTimersByTimeAsync(3_000) + await expect(stopping).resolves.toBe(true) + expect(kill).toHaveBeenCalledWith(900, 'SIGKILL') + } finally { + vi.useRealTimers() + } + }) + + it('never SIGKILLs a pid that was reused after SIGTERM', async () => { + vi.useFakeTimers() + try { + const kill = vi.fn() + const listProcesses = vi + .fn() + .mockResolvedValueOnce([row(900, 1, ORCA_FORWARD)]) + .mockResolvedValue([row(900, 77, '/usr/bin/vim notes.txt'), row(77, 1, '/bin/zsh')]) + const readStartTimeMs = vi.fn().mockResolvedValueOnce(1_000).mockResolvedValue(90_000) + const stopping = stopOrphanedOrcaSshForward(47123, EXPECTED, { + listProcesses, + kill, + readStartTimeMs, + platform: 'darwin' + }) + await vi.advanceTimersByTimeAsync(3_000) + await stopping + expect(kill).toHaveBeenCalledWith(900, 'SIGTERM') + expect(kill).not.toHaveBeenCalledWith(900, 'SIGKILL') + } finally { + vi.useRealTimers() + } + }) + + it('never SIGKILLs when the start time cannot be read, even for the same command', async () => { + vi.useFakeTimers() + try { + const kill = vi.fn() + const stopping = stopOrphanedOrcaSshForward(47123, EXPECTED, { + listProcesses: async () => [row(900, 1, ORCA_FORWARD)], + kill, + readStartTimeMs: async () => null, + platform: 'darwin' + }) + await vi.advanceTimersByTimeAsync(3_000) + await stopping + expect(kill).not.toHaveBeenCalledWith(900, 'SIGKILL') + } finally { + vi.useRealTimers() + } + }) + + it('touches nothing when no orphan holds the port, or the table cannot be read', async () => { + const kill = vi.fn() + await expect( + stopOrphanedOrcaSshForward(47123, EXPECTED, { + listProcesses: async () => [], + kill, + readStartTimeMs: async () => 1_000, + platform: 'darwin' + }) + ).resolves.toBe(false) + await expect( + stopOrphanedOrcaSshForward(47123, EXPECTED, { + listProcesses: async () => { + throw new Error('ps failed') + }, + kill, + readStartTimeMs: async () => 1_000, + platform: 'darwin' + }) + ).resolves.toBe(false) + expect(kill).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/ssh/system-ssh-orphaned-forward.ts b/src/main/ssh/system-ssh-orphaned-forward.ts new file mode 100644 index 00000000000..525e7a6d84f --- /dev/null +++ b/src/main/ssh/system-ssh-orphaned-forward.ts @@ -0,0 +1,175 @@ +/** + * An `ssh -N -L` forward a previous Orca left behind — after a crash, a force-quit, or a version + * that did not stop its forwards on quit — keeps holding the fixed local port a managed tunnel + * binds, so every later start fails with EADDRINUSE. Only a process whose argv is exactly the one + * Orca would spawn for this same forward and host, and whose parent is gone, qualifies: a user's + * own ssh and a live Orca's tunnel never match. + */ +import { getFreshShellForegroundSnapshot } from '../../shared/process-table-snapshot-reader' +import { readWindowsProcessTableFresh } from '../windows/windows-process-table' +import { + PROCESS_START_TIME_TOLERANCE_MS, + readProcessStartTimeMs +} from '../runtime/agent-session-process-identity-probe' + +export type SshForwardProcessRow = { + pid: number + ppid: number + command: string + /** Windows creation time; `startTime` is the posix tables' opaque start marker. */ + creationTimeMs?: number + startTime?: string +} + +type OrphanedForwardDeps = { + listProcesses: () => Promise + kill: (pid: number, signal: NodeJS.Signals | 0) => void + readStartTimeMs: (pid: number) => Promise + platform: NodeJS.Platform +} + +const DEFAULT_DEPS: OrphanedForwardDeps = { + listProcesses: () => + process.platform === 'win32' + ? readWindowsProcessTableFresh() + : getFreshShellForegroundSnapshot(), + kill: (pid, signal) => { + process.kill(pid, signal) + }, + readStartTimeMs: (pid) => readProcessStartTimeMs(pid), + platform: process.platform +} + +const EXIT_POLL_MS = 50 +const TERM_WAIT_MS = 2_000 +const KILL_WAIT_MS = 500 + +// Init and the per-user subreapers an orphan is handed to. +const REAPER_NAMES = new Set(['init', 'launchd', 'systemd']) + +function splitExecutable(command: string): { name: string; rest: string } { + const end = command.startsWith('"') ? command.indexOf('"', 1) + 1 : command.search(/\s|$/) + const first = command.slice(0, end).replaceAll('"', '') + return { + name: (first.split(/[\\/]/).pop() ?? '').toLowerCase(), + rest: command.slice(end).trim() + } +} + +function executableName(command: string): string { + return splitExecutable(command).name +} + +// Why: versions before STA-10471 spawned the same argv with ExitOnForwardFailure=yes. +function normalizeForwardArgv(text: string): string { + return text.replaceAll(/ExitOnForwardFailure=(?:yes|no)/g, 'ExitOnForwardFailure=*') +} + +/** The whole argv after the executable must be Orca's; any extra option is someone else's ssh. */ +function isOrcaForward(command: string, expectedArgs: readonly string[]): boolean { + const { name, rest } = splitExecutable(command) + return ( + (name === 'ssh' || name === 'ssh.exe') && + normalizeForwardArgv(rest) === normalizeForwardArgv(expectedArgs.join(' ')) + ) +} + +function parentIsGone( + row: SshForwardProcessRow, + byPid: Map, + platform: NodeJS.Platform +): boolean { + const parent = byPid.get(row.ppid) + if (platform === 'win32') { + // Windows never reparents; a recycled pid shows up as a parent younger than its child. + return ( + !parent || + (parent.creationTimeMs !== undefined && + row.creationTimeMs !== undefined && + parent.creationTimeMs > row.creationTimeMs) + ) + } + return row.ppid <= 1 || !parent || REAPER_NAMES.has(executableName(parent.command)) +} + +/** `expectedArgs` is the argv after the ssh executable that Orca spawns for this forward. */ +export function findOrphanedOrcaSshForward( + rows: readonly SshForwardProcessRow[], + expectedArgs: readonly string[], + platform: NodeJS.Platform +): SshForwardProcessRow | null { + const byPid = new Map(rows.map((row) => [row.pid, row])) + return ( + rows.find( + (row) => isOrcaForward(row.command, expectedArgs) && parentIsGone(row, byPid, platform) + ) ?? null + ) +} + +function isGone(pid: number, kill: OrphanedForwardDeps['kill']): boolean { + try { + kill(pid, 0) + return false + } catch (error) { + return error instanceof Error && 'code' in error && error.code === 'ESRCH' + } +} + +async function waitUntilGone( + pid: number, + kill: OrphanedForwardDeps['kill'], + ms: number +): Promise { + for (let waited = 0; waited < ms; waited += EXIT_POLL_MS) { + if (isGone(pid, kill)) { + return true + } + await new Promise((resolve) => setTimeout(resolve, EXIT_POLL_MS)) + } + return isGone(pid, kill) +} + +/** True when an orphaned Orca forward was signalled; false means nothing was touched. */ +export async function stopOrphanedOrcaSshForward( + localPort: number, + expectedArgs: readonly string[], + deps: OrphanedForwardDeps = DEFAULT_DEPS +): Promise { + const find = async (): Promise => { + try { + return findOrphanedOrcaSshForward(await deps.listProcesses(), expectedArgs, deps.platform) + } catch { + return null + } + } + const orphan = await find() + if (!orphan) { + return false + } + console.warn( + `[ssh] Stopping a forward a previous Orca left on 127.0.0.1:${localPort} (pid ${orphan.pid})` + ) + const startedAtMs = await deps.readStartTimeMs(orphan.pid) + try { + deps.kill(orphan.pid, 'SIGTERM') + if (await waitUntilGone(orphan.pid, deps.kill, TERM_WAIT_MS)) { + return true + } + // Why: the pid may have exited and been reused while we polled; SIGKILL only on proof that it + // is still the same process, never when its start time cannot be read. + const still = await find() + const stillStartedAtMs = await deps.readStartTimeMs(orphan.pid) + if ( + still?.pid === orphan.pid && + startedAtMs !== null && + stillStartedAtMs !== null && + Math.abs(stillStartedAtMs - startedAtMs) <= PROCESS_START_TIME_TOLERANCE_MS + ) { + deps.kill(orphan.pid, 'SIGKILL') + await waitUntilGone(orphan.pid, deps.kill, KILL_WAIT_MS) + } + } catch { + // Exited between the table read and the signal; the caller's retry decides. + } + return true +}