From 42e04268fbfb4a313c59d88ca5ef9b3d5c27bf61 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Tue, 14 Apr 2026 21:29:50 -0400 Subject: [PATCH] fix: harden SSH connection reliability and add passphrase prompt (#636) --- src/main/ipc/ssh-auth-helpers.ts | 68 --- src/main/ipc/ssh-passphrase.ts | 65 +++ src/main/ipc/ssh.ts | 54 +-- src/main/persistence.ts | 21 +- src/main/ssh/ssh-config-parser.test.ts | 104 ++++- src/main/ssh/ssh-config-parser.ts | 70 +++ src/main/ssh/ssh-config-resolver.test.ts | 64 +++ src/main/ssh/ssh-connection-store.ts | 3 +- src/main/ssh/ssh-connection-utils.test.ts | 369 ++++++++++++++++ src/main/ssh/ssh-connection-utils.ts | 401 +++++++----------- src/main/ssh/ssh-connection.test.ts | 23 +- src/main/ssh/ssh-connection.ts | 323 +++++++------- src/main/ssh/ssh-relay-deploy.test.ts | 112 +++++ src/main/ssh/ssh-relay-deploy.ts | 32 +- src/main/ssh/ssh-system-fallback.ts | 2 +- src/preload/api-types.d.ts | 52 +-- src/preload/index.ts | 123 ++---- src/renderer/src/App.tsx | 2 + .../src/components/settings/SshPane.tsx | 36 +- .../settings/SshPassphraseDialog.tsx | 122 ++++++ .../src/components/settings/SshTargetCard.tsx | 38 +- .../src/components/settings/SshTargetForm.tsx | 2 + .../sidebar/SshDisconnectedDialog.tsx | 42 +- .../src/components/sidebar/WorktreeCard.tsx | 9 + .../status-bar/SshStatusSegment.tsx | 212 +++++++++ .../src/components/status-bar/StatusBar.tsx | 98 ++--- .../terminal-pane/TerminalErrorToast.tsx | 38 +- .../components/terminal-pane/TerminalPane.tsx | 1 + .../terminal-pane/pty-connection.ts | 5 + .../components/terminal-pane/pty-transport.ts | 13 +- src/renderer/src/hooks/useIpcEvents.test.ts | 181 +++++++- src/renderer/src/hooks/useIpcEvents.ts | 86 +++- src/renderer/src/store/slices/ssh.ts | 34 +- src/shared/constants.ts | 2 +- src/shared/ssh-types.test.ts | 4 +- src/shared/ssh-types.ts | 4 +- src/shared/types.ts | 2 +- 37 files changed, 2066 insertions(+), 751 deletions(-) delete mode 100644 src/main/ipc/ssh-auth-helpers.ts create mode 100644 src/main/ipc/ssh-passphrase.ts create mode 100644 src/main/ssh/ssh-config-resolver.test.ts create mode 100644 src/main/ssh/ssh-connection-utils.test.ts create mode 100644 src/main/ssh/ssh-relay-deploy.test.ts create mode 100644 src/renderer/src/components/settings/SshPassphraseDialog.tsx create mode 100644 src/renderer/src/components/status-bar/SshStatusSegment.tsx diff --git a/src/main/ipc/ssh-auth-helpers.ts b/src/main/ipc/ssh-auth-helpers.ts deleted file mode 100644 index d9ce9fef6..000000000 --- a/src/main/ipc/ssh-auth-helpers.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { randomUUID } from 'crypto' -import { ipcMain, type BrowserWindow } from 'electron' -import type { SshConnectionCallbacks } from '../ssh/ssh-connection' - -// Why: all three SSH auth callbacks (host-key-verify, auth-challenge, password) -// share the same IPC round-trip pattern: send a prompt event to the renderer, -// wait for a single response on a unique channel, clean up on timeout/close. -// Extracting the pattern into a generic helper avoids triplicating the cleanup -// logic and keeps ssh.ts under the max-lines threshold. -function promptRenderer( - win: BrowserWindow, - sendChannel: string, - sendPayload: Record, - fallback: T -): Promise { - return new Promise((resolve) => { - const responseChannel = `${sendChannel}-response-${randomUUID()}` - const onClosed = () => { - cleanup() - resolve(fallback) - } - const cleanup = () => { - ipcMain.removeAllListeners(responseChannel) - clearTimeout(timer) - win.removeListener('closed', onClosed) - } - const timer = setTimeout(() => { - cleanup() - resolve(fallback) - }, 120_000) - win.webContents.send(sendChannel, { ...sendPayload, responseChannel }) - ipcMain.once(responseChannel, (_event, value: T) => { - cleanup() - resolve(value) - }) - win.once('closed', onClosed) - }) -} - -export function buildSshAuthCallbacks( - getMainWindow: () => BrowserWindow | null -): Pick { - return { - onHostKeyVerify: async (req) => { - const win = getMainWindow() - if (!win || win.isDestroyed()) { - return false - } - return promptRenderer(win, 'ssh:host-key-verify', req, false) - }, - - onAuthChallenge: async (req) => { - const win = getMainWindow() - if (!win || win.isDestroyed()) { - return [] - } - return promptRenderer(win, 'ssh:auth-challenge', req, []) - }, - - onPasswordPrompt: async (targetId: string) => { - const win = getMainWindow() - if (!win || win.isDestroyed()) { - return null - } - return promptRenderer(win, 'ssh:password-prompt', { targetId }, null) - } - } -} diff --git a/src/main/ipc/ssh-passphrase.ts b/src/main/ipc/ssh-passphrase.ts new file mode 100644 index 000000000..a5827e528 --- /dev/null +++ b/src/main/ipc/ssh-passphrase.ts @@ -0,0 +1,65 @@ +import { ipcMain, type BrowserWindow } from 'electron' +import { randomUUID } from 'crypto' +import type { SshCredentialKind } from '../ssh/ssh-connection-utils' + +const CREDENTIAL_TIMEOUT_MS = 120_000 +const pendingRequests = new Map void }>() + +function notifyCredentialResolved( + getMainWindow: () => BrowserWindow | null, + requestId: string +): void { + const win = getMainWindow() + if (win && !win.isDestroyed()) { + win.webContents.send('ssh:credential-resolved', { requestId }) + } +} + +export function requestCredential( + getMainWindow: () => BrowserWindow | null, + targetId: string, + kind: SshCredentialKind, + detail: string +): Promise { + const requestId = randomUUID() + return new Promise((resolve) => { + const timer = setTimeout(() => { + if (pendingRequests.delete(requestId)) { + notifyCredentialResolved(getMainWindow, requestId) + resolve(null) + } + }, CREDENTIAL_TIMEOUT_MS) + + pendingRequests.set(requestId, { + resolve: (value) => { + clearTimeout(timer) + resolve(value) + } + }) + + const win = getMainWindow() + if (win && !win.isDestroyed()) { + win.webContents.send('ssh:credential-request', { requestId, targetId, kind, detail }) + } else { + pendingRequests.delete(requestId) + clearTimeout(timer) + notifyCredentialResolved(getMainWindow, requestId) + resolve(null) + } + }) +} + +export function registerCredentialHandler(getMainWindow: () => BrowserWindow | null): void { + ipcMain.removeHandler('ssh:submitCredential') + ipcMain.handle( + 'ssh:submitCredential', + (_event, args: { requestId: string; value: string | null }) => { + const pending = pendingRequests.get(args.requestId) + if (pending) { + pendingRequests.delete(args.requestId) + notifyCredentialResolved(getMainWindow, args.requestId) + pending.resolve(args.value) + } + } + ) +} diff --git a/src/main/ipc/ssh.ts b/src/main/ipc/ssh.ts index 4c645a14e..8f0ec6090 100644 --- a/src/main/ipc/ssh.ts +++ b/src/main/ipc/ssh.ts @@ -11,10 +11,11 @@ import { registerSshPtyProvider } from './pty' import { registerSshFilesystemProvider } from '../providers/ssh-filesystem-dispatch' import { registerSshGitProvider } from '../providers/ssh-git-dispatch' import { SshPortForwardManager } from '../ssh/ssh-port-forward' -import type { SshTarget, SshConnectionState } from '../../shared/ssh-types' +import type { SshTarget, SshConnectionState, SshConnectionStatus } from '../../shared/ssh-types' +import { isAuthError } from '../ssh/ssh-connection-utils' import { cleanupConnection, wireUpSshPtyEvents, reestablishRelayStack } from './ssh-relay-helpers' -import { buildSshAuthCallbacks } from './ssh-auth-helpers' import { registerSshBrowseHandler } from './ssh-browse' +import { requestCredential, registerCredentialHandler } from './ssh-passphrase' let sshStore: SshConnectionStore | null = null let connectionManager: SshConnectionManager | null = null @@ -32,6 +33,7 @@ const initializedConnections = new Set() // flash "connected" then "disconnected". Suppressing broadcasts during tests // avoids that visual glitch. const testingTargets = new Set() +const explicitRelaySetupTargets = new Set() export function registerSshHandlers( store: Store, @@ -59,7 +61,11 @@ export function registerSshHandlers( sshStore = new SshConnectionStore(store) + registerCredentialHandler(getMainWindow) + const callbacks: SshConnectionCallbacks = { + onCredentialRequest: (targetId, kind, detail) => + requestCredential(getMainWindow, targetId, kind, detail), onStateChange: (targetId: string, state: SshConnectionState) => { if (testingTargets.has(targetId)) { return @@ -76,7 +82,8 @@ export function registerSshHandlers( if ( state.status === 'connected' && state.reconnectAttempt === 0 && - initializedConnections.has(targetId) + initializedConnections.has(targetId) && + !explicitRelaySetupTargets.has(targetId) ) { void reestablishRelayStack( targetId, @@ -86,9 +93,7 @@ export function registerSshHandlers( portForwardManager ) } - }, - - ...buildSshAuthCallbacks(getMainWindow) + } } connectionManager = new SshConnectionManager(callbacks) @@ -129,21 +134,23 @@ export function registerSshHandlers( } let conn + explicitRelaySetupTargets.add(args.targetId) try { conn = await connectionManager!.connect(target) } catch (err) { - // Why: SshConnection.connect() sets its internal state to 'error', but - // the onStateChange callback may have been suppressed or the state may - // not have propagated to the renderer. Explicitly broadcast the error - // so the UI leaves 'connecting'/'host-key-verification'. + // Why: SshConnection.connect() sets its internal state, but the + // onStateChange callback may not have propagated to the renderer. + // Explicitly broadcast so the UI leaves 'connecting'. + const errObj = err instanceof Error ? err : new Error(String(err)) + const status: SshConnectionStatus = isAuthError(errObj) ? 'auth-failed' : 'error' const win = getMainWindow() if (win && !win.isDestroyed()) { win.webContents.send('ssh:state-changed', { targetId: args.targetId, state: { targetId: args.targetId, - status: 'error', - error: err instanceof Error ? err.message : String(err), + status, + error: errObj.message, reconnectAttempt: 0 } }) @@ -151,22 +158,17 @@ export function registerSshHandlers( throw err } - // Deploy relay and establish multiplexer - callbacks.onStateChange(args.targetId, { - targetId: args.targetId, - status: 'deploying-relay', - error: null, - reconnectAttempt: 0 - }) - try { - const { transport } = await deployAndLaunchRelay(conn, (status) => { - const win = getMainWindow() - if (win && !win.isDestroyed()) { - win.webContents.send('ssh:deploy-progress', { targetId: args.targetId, status }) - } + // Deploy relay and establish multiplexer + callbacks.onStateChange(args.targetId, { + targetId: args.targetId, + status: 'deploying-relay', + error: null, + reconnectAttempt: 0 }) + const { transport } = await deployAndLaunchRelay(conn) + const mux = new SshChannelMultiplexer(transport) activeMultiplexers.set(args.targetId, mux) @@ -202,6 +204,8 @@ export function registerSshHandlers( // Relay deployment failed — disconnect SSH await connectionManager!.disconnect(args.targetId) throw err + } finally { + explicitRelaySetupTargets.delete(args.targetId) } return connectionManager!.getState(args.targetId) diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 0e8ba797d..b3f845d84 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -52,6 +52,12 @@ function normalizeSortBy(sortBy: unknown): 'name' | 'recent' | 'repo' { return getDefaultUIState().sortBy } +// Why: old persisted targets predate configHost. Default to label-based lookup +// so imported SSH aliases keep resolving through ssh -G after upgrade. +function normalizeSshTarget(t: SshTarget): SshTarget { + return { ...t, configHost: t.configHost ?? t.label ?? t.host } +} + export class Store { private state: PersistedState private writeTimer: ReturnType | null = null @@ -88,7 +94,7 @@ export class Store { sortBy: normalizeSortBy(parsed.ui?.sortBy) }, workspaceSession: { ...defaults.workspaceSession, ...parsed.workspaceSession }, - sshTargets: parsed.sshTargets ?? [] + sshTargets: (parsed.sshTargets ?? []).map(normalizeSshTarget) } } } catch (err) { @@ -311,18 +317,17 @@ export class Store { // ── SSH Targets ──────────────────────────────────────────────────── getSshTargets(): SshTarget[] { - return this.state.sshTargets ?? [] + return (this.state.sshTargets ?? []).map(normalizeSshTarget) } getSshTarget(id: string): SshTarget | undefined { - return this.state.sshTargets?.find((t) => t.id === id) + const target = this.state.sshTargets?.find((t) => t.id === id) + return target ? normalizeSshTarget(target) : undefined } addSshTarget(target: SshTarget): void { - if (!this.state.sshTargets) { - this.state.sshTargets = [] - } - this.state.sshTargets.push(target) + this.state.sshTargets ??= [] + this.state.sshTargets.push(normalizeSshTarget(target)) this.scheduleSave() } @@ -331,7 +336,7 @@ export class Store { if (!target) { return null } - Object.assign(target, updates) + Object.assign(target, updates, normalizeSshTarget({ ...target, ...updates })) this.scheduleSave() return { ...target } } diff --git a/src/main/ssh/ssh-config-parser.test.ts b/src/main/ssh/ssh-config-parser.test.ts index 563aecfce..9f862ce17 100644 --- a/src/main/ssh/ssh-config-parser.test.ts +++ b/src/main/ssh/ssh-config-parser.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { parseSshConfig, sshConfigHostsToTargets } from './ssh-config-parser' +import { parseSshConfig, sshConfigHostsToTargets, parseSshGOutput } from './ssh-config-parser' vi.mock('os', () => ({ homedir: () => '/home/testuser' @@ -209,3 +209,105 @@ describe('sshConfigHostsToTargets', () => { expect(targets[0].jumpHost).toBe('bastion.example.com') }) }) + +// ── parseSshGOutput ────────────────────────────────────────────────── + +describe('parseSshGOutput', () => { + it('parses hostname, user, port from ssh -G output', () => { + const output = [ + 'hostname 192.168.1.100', + 'user deploy', + 'port 2222', + 'identityfile /home/testuser/.ssh/id_ed25519', + 'forwardagent no' + ].join('\n') + + const result = parseSshGOutput(output) + expect(result.hostname).toBe('192.168.1.100') + expect(result.user).toBe('deploy') + expect(result.port).toBe(2222) + expect(result.identityFile).toEqual(['/home/testuser/.ssh/id_ed25519']) + expect(result.forwardAgent).toBe(false) + }) + + it('collects multiple identity files', () => { + const output = [ + 'hostname example.com', + 'identityfile ~/.ssh/id_ed25519', + 'identityfile ~/.ssh/id_rsa', + 'port 22' + ].join('\n') + + const result = parseSshGOutput(output) + expect(result.identityFile).toEqual([ + '/home/testuser/.ssh/id_ed25519', + '/home/testuser/.ssh/id_rsa' + ]) + }) + + it('parses forwardagent yes', () => { + const output = 'hostname example.com\nforwardagent yes\nport 22' + const result = parseSshGOutput(output) + expect(result.forwardAgent).toBe(true) + }) + + it('defaults port to 22 when missing', () => { + const output = 'hostname example.com' + const result = parseSshGOutput(output) + expect(result.port).toBe(22) + }) + + it('returns empty hostname when missing', () => { + const output = 'user admin\nport 22' + const result = parseSshGOutput(output) + expect(result.hostname).toBe('') + }) + + it('returns undefined user when missing', () => { + const output = 'hostname example.com\nport 22' + const result = parseSshGOutput(output) + expect(result.user).toBeUndefined() + }) + + it('handles empty output', () => { + const result = parseSshGOutput('') + expect(result.hostname).toBe('') + expect(result.port).toBe(22) + expect(result.identityFile).toEqual([]) + }) + + it('skips lines without spaces', () => { + const output = 'hostname example.com\nbadline\nport 22' + const result = parseSshGOutput(output) + expect(result.hostname).toBe('example.com') + expect(result.port).toBe(22) + }) + + it('parses proxycommand and filters "none"', () => { + const output = 'hostname example.com\nproxycommand ssh -W %h:%p bastion\nport 22' + const result = parseSshGOutput(output) + expect(result.proxyCommand).toBe('ssh -W %h:%p bastion') + + const noneOutput = 'hostname example.com\nproxycommand none\nport 22' + const noneResult = parseSshGOutput(noneOutput) + expect(noneResult.proxyCommand).toBeUndefined() + }) + + it('parses proxyjump and filters "none"', () => { + const output = 'hostname example.com\nproxyjump bastion.example.com\nport 22' + const result = parseSshGOutput(output) + expect(result.proxyJump).toBe('bastion.example.com') + + const noneOutput = 'hostname example.com\nproxyjump none\nport 22' + const noneResult = parseSshGOutput(noneOutput) + expect(noneResult.proxyJump).toBeUndefined() + }) + + it('handles ~ expansion in identity file paths', () => { + const output = 'hostname example.com\nidentityfile ~/custom_key\nport 22' + const result = parseSshGOutput(output) + expect(result.identityFile).toEqual(['/home/testuser/custom_key']) + }) +}) + +// Why: resolveWithSshG tests are in ssh-config-resolver.test.ts (max-lines). diff --git a/src/main/ssh/ssh-config-parser.ts b/src/main/ssh/ssh-config-parser.ts index 86c5edf68..e137d05cd 100644 --- a/src/main/ssh/ssh-config-parser.ts +++ b/src/main/ssh/ssh-config-parser.ts @@ -1,4 +1,5 @@ import { readFileSync, existsSync } from 'fs' +import { execFile } from 'child_process' import { join } from 'path' import { homedir } from 'os' import type { SshTarget } from '../../shared/ssh-types' @@ -137,6 +138,7 @@ export function sshConfigHostsToTargets( targets.push({ id: `ssh-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`, label, + configHost: entry.host, host: effectiveHost, port: entry.port ?? 22, username: entry.user ?? '', @@ -148,3 +150,71 @@ export function sshConfigHostsToTargets( return targets } + +// ── ssh -G config resolution ────────────────────────────────────────── + +export type SshResolvedConfig = { + hostname: string + user?: string + port: number + identityFile: string[] + forwardAgent: boolean + proxyCommand?: string + proxyJump?: string +} + +const SSH_G_TIMEOUT_MS = 5000 + +// Why: `ssh -G ` asks OpenSSH to resolve the full effective config +// for a host, including Include directives, Match blocks, wildcard +// inheritance, and ProxyCommand expansion. This gives us correct config +// resolution without reimplementing OpenSSH's complex matching logic. +export function resolveWithSshG(host: string): Promise { + return new Promise((resolve) => { + // Why: '--' prevents a host label starting with '-' from being interpreted + // as an SSH flag (classic argument injection vector). + execFile('ssh', ['-G', '--', host], { timeout: SSH_G_TIMEOUT_MS }, (err, stdout) => { + if (err) { + resolve(null) + return + } + resolve(parseSshGOutput(stdout)) + }) + }) +} + +export function parseSshGOutput(stdout: string): SshResolvedConfig { + const map = new Map() + const identityFiles: string[] = [] + + for (const line of stdout.split('\n')) { + const spaceIdx = line.indexOf(' ') + if (spaceIdx === -1) { + continue + } + const key = line.substring(0, spaceIdx).toLowerCase() + const value = line.substring(spaceIdx + 1).trim() + if (key === 'identityfile') { + identityFiles.push(resolveHomePath(value)) + } else { + map.set(key, value) + } + } + + // Why: `ssh -G` outputs `proxycommand none` / `proxyjump none` when no + // proxy is configured. Treating "none" as real would spawn bad commands. + const rawProxy = map.get('proxycommand') + const proxyCommand = rawProxy && rawProxy !== 'none' ? rawProxy : undefined + const rawJump = map.get('proxyjump') + const proxyJump = rawJump && rawJump !== 'none' ? rawJump : undefined + + return { + hostname: map.get('hostname') ?? '', + user: map.get('user') || undefined, + port: parseInt(map.get('port') ?? '22', 10), + identityFile: identityFiles, + forwardAgent: map.get('forwardagent') === 'yes', + proxyCommand, + proxyJump + } +} diff --git a/src/main/ssh/ssh-config-resolver.test.ts b/src/main/ssh/ssh-config-resolver.test.ts new file mode 100644 index 000000000..bfc948a2c --- /dev/null +++ b/src/main/ssh/ssh-config-resolver.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it, vi } from 'vitest' +import { resolveWithSshG } from './ssh-config-parser' + +vi.mock('os', () => ({ + homedir: () => '/home/testuser' +})) + +vi.mock('child_process', () => ({ + execFile: vi.fn() +})) + +describe('resolveWithSshG', () => { + it('returns parsed config on success', async () => { + const { execFile } = await import('child_process') + const mockExecFile = vi.mocked(execFile) + mockExecFile.mockImplementation( + (_cmd: unknown, _args: unknown, _opts: unknown, cb: unknown) => { + const callback = cb as (err: Error | null, stdout: string) => void + callback(null, 'hostname 10.0.0.1\nuser admin\nport 22') + return undefined as never + } + ) + + const result = await resolveWithSshG('myhost') + expect(result).toBeDefined() + expect(result!.hostname).toBe('10.0.0.1') + expect(result!.user).toBe('admin') + }) + + it('calls ssh -G with the given host', async () => { + const { execFile } = await import('child_process') + const mockExecFile = vi.mocked(execFile) + mockExecFile.mockImplementation( + (_cmd: unknown, _args: unknown, _opts: unknown, cb: unknown) => { + const callback = cb as (err: Error | null, stdout: string) => void + callback(null, 'hostname example.com\nport 22') + return undefined as never + } + ) + + await resolveWithSshG('testserver') + expect(mockExecFile).toHaveBeenCalledWith( + 'ssh', + ['-G', '--', 'testserver'], + expect.objectContaining({ timeout: 5000 }), + expect.any(Function) + ) + }) + + it('returns null when ssh -G fails', async () => { + const { execFile } = await import('child_process') + const mockExecFile = vi.mocked(execFile) + mockExecFile.mockImplementation( + (_cmd: unknown, _args: unknown, _opts: unknown, cb: unknown) => { + const callback = cb as (err: Error | null, stdout: string) => void + callback(new Error('ssh not found'), '') + return undefined as never + } + ) + + const result = await resolveWithSshG('myhost') + expect(result).toBeNull() + }) +}) diff --git a/src/main/ssh/ssh-connection-store.ts b/src/main/ssh/ssh-connection-store.ts index 580ca9ef1..50e758c3e 100644 --- a/src/main/ssh/ssh-connection-store.ts +++ b/src/main/ssh/ssh-connection-store.ts @@ -16,6 +16,7 @@ export class SshConnectionStore { addTarget(target: Omit): SshTarget { const full: SshTarget = { ...target, + configHost: target.configHost ?? target.host, id: `ssh-${Date.now()}-${Math.random().toString(36).slice(2, 8)}` } this.store.addSshTarget(full) @@ -35,7 +36,7 @@ export class SshConnectionStore { * Returns the newly imported targets. */ importFromSshConfig(): SshTarget[] { - const existingLabels = new Set(this.store.getSshTargets().map((t) => t.label)) + const existingLabels = new Set(this.store.getSshTargets().map((t) => t.configHost ?? t.label)) const configHosts = loadUserSshConfig() const newTargets = sshConfigHostsToTargets(configHosts, existingLabels) diff --git a/src/main/ssh/ssh-connection-utils.test.ts b/src/main/ssh/ssh-connection-utils.test.ts new file mode 100644 index 000000000..07237c363 --- /dev/null +++ b/src/main/ssh/ssh-connection-utils.test.ts @@ -0,0 +1,369 @@ +import { afterEach, describe, expect, it, vi, beforeEach } from 'vitest' + +vi.mock('os', () => ({ + homedir: () => '/home/testuser' +})) + +const mockExistsSync = vi.fn().mockReturnValue(false) +const mockReadFileSync = vi.fn() + +vi.mock('fs', () => ({ + existsSync: (...args: unknown[]) => mockExistsSync(...args), + readFileSync: (...args: unknown[]) => mockReadFileSync(...args) +})) + +import { + isTransientError, + isAuthError, + sleep, + shellEscape, + findDefaultKeyFile, + buildConnectConfig, + resolveEffectiveProxy, + CONNECT_TIMEOUT_MS, + INITIAL_RETRY_ATTEMPTS, + INITIAL_RETRY_DELAY_MS, + RECONNECT_BACKOFF_MS +} from './ssh-connection-utils' +import type { SshTarget } from '../../shared/ssh-types' +import type { SshResolvedConfig } from './ssh-config-parser' + +// ── Constants ──────────────────────────────────────────────────────── + +describe('SSH connection constants', () => { + it('CONNECT_TIMEOUT_MS is 30 seconds (matches VS Code)', () => { + expect(CONNECT_TIMEOUT_MS).toBe(30_000) + }) + + it('INITIAL_RETRY_ATTEMPTS is 5', () => { + expect(INITIAL_RETRY_ATTEMPTS).toBe(5) + }) + + it('INITIAL_RETRY_DELAY_MS is 2 seconds', () => { + expect(INITIAL_RETRY_DELAY_MS).toBe(2000) + }) + + it('RECONNECT_BACKOFF_MS has 9 entries', () => { + expect(RECONNECT_BACKOFF_MS).toHaveLength(9) + }) +}) + +// ── isTransientError ───────────────────────────────────────────────── + +describe('isTransientError', () => { + it('returns true for ETIMEDOUT code', () => { + const err = new Error('timed out') as NodeJS.ErrnoException + err.code = 'ETIMEDOUT' + expect(isTransientError(err)).toBe(true) + }) + + it('returns true for ECONNREFUSED code', () => { + const err = new Error('refused') as NodeJS.ErrnoException + err.code = 'ECONNREFUSED' + expect(isTransientError(err)).toBe(true) + }) + + it('returns true for ECONNRESET code', () => { + const err = new Error('reset') as NodeJS.ErrnoException + err.code = 'ECONNRESET' + expect(isTransientError(err)).toBe(true) + }) + + it('returns true for EHOSTUNREACH code', () => { + const err = new Error('host unreachable') as NodeJS.ErrnoException + err.code = 'EHOSTUNREACH' + expect(isTransientError(err)).toBe(true) + }) + + it('returns true for ENETUNREACH code', () => { + const err = new Error('net unreachable') as NodeJS.ErrnoException + err.code = 'ENETUNREACH' + expect(isTransientError(err)).toBe(true) + }) + + it('returns true for EAI_AGAIN code', () => { + const err = new Error('dns') as NodeJS.ErrnoException + err.code = 'EAI_AGAIN' + expect(isTransientError(err)).toBe(true) + }) + + it('returns true for ETIMEDOUT in message (no code)', () => { + expect(isTransientError(new Error('connect ETIMEDOUT 1.2.3.4:22'))).toBe(true) + }) + + it('returns true for ECONNREFUSED in message', () => { + expect(isTransientError(new Error('connect ECONNREFUSED 1.2.3.4:22'))).toBe(true) + }) + + it('returns true for ECONNRESET in message', () => { + expect(isTransientError(new Error('read ECONNRESET'))).toBe(true) + }) + + it('returns false for auth errors', () => { + expect(isTransientError(new Error('All configured authentication methods failed'))).toBe(false) + }) + + it('returns false for generic errors', () => { + expect(isTransientError(new Error('something went wrong'))).toBe(false) + }) +}) + +// ── isAuthError ────────────────────────────────────────────────────── + +describe('isAuthError', () => { + it('returns true for "All configured authentication methods failed"', () => { + expect(isAuthError(new Error('All configured authentication methods failed'))).toBe(true) + }) + + it('returns true for "Authentication failed"', () => { + expect(isAuthError(new Error('Authentication failed'))).toBe(true) + }) + + it('returns true for client-authentication level', () => { + const err = new Error('auth') as Error & { level: string } + err.level = 'client-authentication' + expect(isAuthError(err)).toBe(true) + }) + + it('returns false for transient errors', () => { + expect(isAuthError(new Error('connect ETIMEDOUT'))).toBe(false) + }) +}) + +// ── sleep ──────────────────────────────────────────────────────────── + +describe('sleep', () => { + it('resolves after the given delay', async () => { + const start = Date.now() + await sleep(50) + expect(Date.now() - start).toBeGreaterThanOrEqual(40) + }) +}) + +// ── shellEscape ────────────────────────────────────────────────────── + +describe('shellEscape', () => { + it('wraps string in single quotes', () => { + expect(shellEscape('hello')).toBe("'hello'") + }) + + it('escapes embedded single quotes', () => { + expect(shellEscape("it's")).toBe("'it'\\''s'") + }) + + it('handles empty string', () => { + expect(shellEscape('')).toBe("''") + }) + + it('handles special characters', () => { + expect(shellEscape('foo bar; rm -rf /')).toBe("'foo bar; rm -rf /'") + }) +}) + +// ── findDefaultKeyFile ─────────────────────────────────────────────── + +describe('findDefaultKeyFile', () => { + beforeEach(() => { + mockExistsSync.mockReturnValue(false) + mockReadFileSync.mockReset() + }) + + it('returns undefined when no default keys exist', () => { + expect(findDefaultKeyFile()).toBeUndefined() + }) + + it('returns the first existing key file', () => { + mockExistsSync.mockImplementation((path: unknown) => { + return path === '/home/testuser/.ssh/id_ed25519' + }) + mockReadFileSync.mockReturnValue(Buffer.from('key-contents')) + + const result = findDefaultKeyFile() + expect(result).toBeDefined() + expect(result!.path).toBe('~/.ssh/id_ed25519') + expect(result!.contents).toEqual(Buffer.from('key-contents')) + }) + + it('probes keys in VS Code order: ed25519, rsa, ecdsa, dsa, xmss', () => { + const checkedPaths: string[] = [] + mockExistsSync.mockImplementation((path: unknown) => { + checkedPaths.push(String(path)) + return false + }) + + findDefaultKeyFile() + + expect(checkedPaths).toEqual([ + '/home/testuser/.ssh/id_ed25519', + '/home/testuser/.ssh/id_rsa', + '/home/testuser/.ssh/id_ecdsa', + '/home/testuser/.ssh/id_dsa', + '/home/testuser/.ssh/id_xmss' + ]) + }) + + it('skips unreadable key files and tries next', () => { + mockExistsSync.mockImplementation((path: unknown) => { + return path === '/home/testuser/.ssh/id_ed25519' || path === '/home/testuser/.ssh/id_rsa' + }) + mockReadFileSync.mockImplementation((path: unknown) => { + if (String(path) === '/home/testuser/.ssh/id_ed25519') { + throw new Error('permission denied') + } + return Buffer.from('rsa-key') + }) + + const result = findDefaultKeyFile() + expect(result).toBeDefined() + expect(result!.path).toBe('~/.ssh/id_rsa') + }) +}) + +// ── buildConnectConfig ────────────────────────────────────────────── + +function makeTarget(overrides?: Partial): SshTarget { + return { + id: 'test-1', + label: 'myhost', + host: 'example.com', + port: 22, + username: 'deploy', + ...overrides + } +} + +function makeResolved(overrides?: Partial): SshResolvedConfig { + return { + hostname: '10.0.0.1', + port: 22, + identityFile: [], + forwardAgent: false, + ...overrides + } +} + +describe('buildConnectConfig', () => { + const originalEnv = process.env.SSH_AUTH_SOCK + + beforeEach(() => { + mockExistsSync.mockReturnValue(false) + mockReadFileSync.mockReset() + process.env.SSH_AUTH_SOCK = '/tmp/agent.sock' + }) + + afterEach(() => { + if (originalEnv !== undefined) { + process.env.SSH_AUTH_SOCK = originalEnv + } else { + delete process.env.SSH_AUTH_SOCK + } + }) + + it('uses target host/port/username', () => { + const config = buildConnectConfig(makeTarget(), null) + expect(config.host).toBe('example.com') + expect(config.port).toBe(22) + expect(config.username).toBe('deploy') + }) + + it('falls back to resolved config when target fields are empty', () => { + const config = buildConnectConfig( + makeTarget({ host: '', port: 0, username: '' }), + makeResolved({ hostname: '10.0.0.1', port: 2222, user: 'admin' }) + ) + expect(config.host).toBe('10.0.0.1') + expect(config.port).toBe(2222) + expect(config.username).toBe('admin') + }) + + it('sets readyTimeout to CONNECT_TIMEOUT_MS', () => { + const config = buildConnectConfig(makeTarget(), null) + expect(config.readyTimeout).toBe(30_000) + }) + + it('sets keepaliveInterval to 15s', () => { + const config = buildConnectConfig(makeTarget(), null) + expect(config.keepaliveInterval).toBe(15_000) + }) + + it('uses agent auth when no explicit key and SSH_AUTH_SOCK is set', () => { + const config = buildConnectConfig(makeTarget(), null) + expect(config.agent).toBe('/tmp/agent.sock') + }) + + it('uses keyFile auth when target.identityFile is set', () => { + mockReadFileSync.mockReturnValue(Buffer.from('key')) + const config = buildConnectConfig(makeTarget({ identityFile: '/home/user/.ssh/custom' }), null) + expect(config.privateKey).toEqual(Buffer.from('key')) + expect(config.agent).toBe('/tmp/agent.sock') + }) + + it('uses keyFile auth when resolved identityFile is non-default', () => { + mockReadFileSync.mockReturnValue(Buffer.from('custom-key')) + const config = buildConnectConfig( + makeTarget(), + makeResolved({ identityFile: ['/home/user/.ssh/work_key'] }) + ) + expect(config.privateKey).toEqual(Buffer.from('custom-key')) + expect(config.agent).toBe('/tmp/agent.sock') + }) + + it('uses agent auth when resolved identityFile is a default path (expanded)', () => { + const config = buildConnectConfig( + makeTarget(), + makeResolved({ identityFile: ['/home/testuser/.ssh/id_ed25519'] }) + ) + expect(config.agent).toBe('/tmp/agent.sock') + }) + + it('provides fallback key in agent auth mode', () => { + mockExistsSync.mockImplementation( + (p: unknown) => String(p) === '/home/testuser/.ssh/id_ed25519' + ) + mockReadFileSync.mockReturnValue(Buffer.from('fallback')) + const config = buildConnectConfig(makeTarget(), null) + expect(config.agent).toBe('/tmp/agent.sock') + expect(config.privateKey).toEqual(Buffer.from('fallback')) + }) +}) + +// ── resolveEffectiveProxy ─────────────────────────────────────────── + +describe('resolveEffectiveProxy', () => { + it('returns target.proxyCommand first', () => { + const target = { ...makeTarget(), proxyCommand: 'cloudflared access ssh --hostname %h' } + const resolved = makeResolved({ proxyCommand: 'other' }) + expect(resolveEffectiveProxy(target, resolved)).toEqual({ + kind: 'proxy-command', + command: 'cloudflared access ssh --hostname %h' + }) + }) + + it('falls back to resolved proxyCommand', () => { + expect( + resolveEffectiveProxy(makeTarget(), makeResolved({ proxyCommand: 'ssh -W %h:%p gw' })) + ).toEqual({ + kind: 'proxy-command', + command: 'ssh -W %h:%p gw' + }) + }) + + it('returns structured jump-host config for target.jumpHost', () => { + const target = { ...makeTarget(), jumpHost: 'bastion.example.com' } + expect(resolveEffectiveProxy(target, null)).toEqual({ + kind: 'jump-host', + jumpHost: 'bastion.example.com' + }) + }) + + it('returns structured jump-host config for resolved proxyJump', () => { + expect(resolveEffectiveProxy(makeTarget(), makeResolved({ proxyJump: 'jump.host' }))).toEqual({ + kind: 'jump-host', + jumpHost: 'jump.host' + }) + }) + + it('returns undefined when no proxy is configured', () => { + expect(resolveEffectiveProxy(makeTarget(), null)).toBeUndefined() + }) +}) diff --git a/src/main/ssh/ssh-connection-utils.ts b/src/main/ssh/ssh-connection-utils.ts index 15373929d..02009c15e 100644 --- a/src/main/ssh/ssh-connection-utils.ts +++ b/src/main/ssh/ssh-connection-utils.ts @@ -1,39 +1,33 @@ -import { Client as SshClient } from 'ssh2' -import type { ConnectConfig, ClientChannel } from 'ssh2' -import { type ChildProcess, execFileSync } from 'child_process' -import { readFileSync } from 'fs' -import { createHash } from 'crypto' +import { readFileSync, existsSync } from 'fs' +import { spawn, type ChildProcess } from 'child_process' +import { homedir } from 'os' +import { join } from 'path' +import { Duplex } from 'stream' import type { Socket as NetSocket } from 'net' +import type { ConnectConfig } from 'ssh2' import type { SshTarget, SshConnectionState } from '../../shared/ssh-types' +import type { SshResolvedConfig } from './ssh-config-parser' -// Why: types live here (not ssh-connection.ts) to break a circular import. - -export type HostKeyVerifyRequest = { - host: string - ip: string - fingerprint: string - keyType: string -} - -export type AuthChallengeRequest = { - targetId: string - name: string - instructions: string - prompts: { prompt: string; echo: boolean }[] -} +export type SshCredentialKind = 'passphrase' | 'password' export type SshConnectionCallbacks = { onStateChange: (targetId: string, state: SshConnectionState) => void - onHostKeyVerify: (req: HostKeyVerifyRequest) => Promise - onAuthChallenge: (req: AuthChallengeRequest) => Promise - onPasswordPrompt: (targetId: string) => Promise + onCredentialRequest?: ( + targetId: string, + kind: SshCredentialKind, + detail: string + ) => Promise +} + +export function isPassphraseError(err: Error): boolean { + const msg = err.message.toLowerCase() + return msg.includes('passphrase') || msg.includes('encrypted key') || msg.includes('bad decrypt') } export const INITIAL_RETRY_ATTEMPTS = 5 export const INITIAL_RETRY_DELAY_MS = 2000 export const RECONNECT_BACKOFF_MS = [1000, 2000, 5000, 5000, 10000, 10000, 10000, 30000, 30000] -export const AUTH_CHALLENGE_TIMEOUT_MS = 60_000 -export const CONNECT_TIMEOUT_MS = 15_000 +export const CONNECT_TIMEOUT_MS = 30_000 const TRANSIENT_ERROR_CODES = new Set([ 'ETIMEDOUT', @@ -44,6 +38,15 @@ const TRANSIENT_ERROR_CODES = new Set([ 'EAI_AGAIN' ]) +export function isAuthError(err: Error): boolean { + const msg = err.message.toLowerCase() + return ( + msg.includes('all configured authentication methods failed') || + msg.includes('authentication failed') || + (err as { level?: string }).level === 'client-authentication' + ) +} + export function isTransientError(err: Error): boolean { const code = (err as NodeJS.ErrnoException).code if (code && TRANSIENT_ERROR_CODES.has(code)) { @@ -65,235 +68,161 @@ export function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)) } -// Why: prevents shell injection when interpolating into ProxyCommand. export function shellEscape(s: string): string { return `'${s.replace(/'/g, "'\\''")}'` } -// Why: ssh2 doesn't check known_hosts. Without this, every connection blocks -// on a UI prompt that isn't wired up yet, causing a silent timeout. -function isHostKnown(host: string, port: number): boolean { - try { - const lookup = port === 22 ? host : `[${host}]:${port}` - execFileSync('ssh-keygen', ['-F', lookup], { stdio: 'pipe', timeout: 3000 }) - return true - } catch { - return false - } +function cmdEscape(s: string): string { + return `"${s.replace(/"/g, '""')}"` } -// ── Auth handler state (passed in by the connection) ──────────────── +// Why: ssh2 only tries keys that are explicitly provided. Users with keys in +// standard locations (e.g. ~/.ssh/id_ed25519) but no SSH agent running would +// fail to authenticate. Probing default paths matches VS Code's _findDefaultKeyFile. +const DEFAULT_KEY_NAMES = ['id_ed25519', 'id_rsa', 'id_ecdsa', 'id_dsa', 'id_xmss'] -export type AuthHandlerState = { - agentAttempted: boolean - keyAttempted: boolean - setState: (status: string, error?: string) => void -} +const DEFAULT_KEY_PATHS = DEFAULT_KEY_NAMES.map((name) => `~/.ssh/${name}`) -export type ConnectConfigResult = { - config: ConnectConfig - jumpClient: SshClient | null - proxyProcess: ChildProcess | null -} -export async function buildConnectConfig( - target: SshTarget, - callbacks: SshConnectionCallbacks, - authState: AuthHandlerState -): Promise { - const config: ConnectConfig = { - host: target.host, - port: target.port, - username: target.username, - readyTimeout: CONNECT_TIMEOUT_MS, - keepaliveInterval: 5000, - keepaliveCountMax: 4, +// Why: parseSshGOutput expands ~ to homedir(), so resolved identityFile +// paths won't match the ~/... form in DEFAULT_KEY_PATHS. Pre-expand for +// the comparison in buildConnectConfig. +const EXPANDED_DEFAULT_KEY_PATHS = DEFAULT_KEY_NAMES.map((name) => join(homedir(), '.ssh', name)) - // Why: ssh2's hostVerifier callback form `(key, verify) => void` blocks - // the handshake until `verify(true/false)` is called. We check - // known_hosts first so trusted hosts connect without a UI prompt. - hostVerifier: (key: Buffer, verify: (accept: boolean) => void) => { - if (isHostKnown(target.host, target.port)) { - verify(true) - return - } - - const fingerprint = createHash('sha256').update(key).digest('base64') - const keyType = 'unknown' - - authState.setState('host-key-verification') - callbacks - .onHostKeyVerify({ - host: target.host, - ip: target.host, - fingerprint, - keyType - }) - .then((accepted) => { - verify(accepted) - }) - .catch(() => { - verify(false) - }) - }, - - authHandler: (methodsLeft, _partialSuccess, callback) => { - // ssh2 passes null on the first call, meaning "try whatever you want". - // Treat it as all methods available. - const methods = methodsLeft ?? ['publickey', 'keyboard-interactive', 'password'] - - // Try auth methods in order: agent -> publickey -> keyboard-interactive -> password - // The custom authHandler overrides ssh2's built-in sequence, so we must - // explicitly try agent auth here -- the config.agent field alone is not enough. - if (methods.includes('publickey') && process.env.SSH_AUTH_SOCK && !authState.agentAttempted) { - authState.agentAttempted = true - callback({ - type: 'agent' as const, - agent: process.env.SSH_AUTH_SOCK, - username: target.username - } as never) - return - } - - if (methods.includes('publickey') && target.identityFile && !authState.keyAttempted) { - authState.keyAttempted = true - try { - callback({ - type: 'publickey' as const, - username: target.username, - key: readFileSync(target.identityFile) - } as never) - return - } catch { - // Key file unreadable -- fall through to next method - } - } - - if (methods.includes('keyboard-interactive')) { - callback({ - type: 'keyboard-interactive' as const, - username: target.username, - prompt: async ( - _name: string, - instructions: string, - _lang: string, - prompts: { prompt: string; echo: boolean }[], - finish: (responses: string[]) => void - ) => { - authState.setState('auth-challenge') - - const timeoutPromise = sleep(AUTH_CHALLENGE_TIMEOUT_MS).then(() => null) - const responsePromise = callbacks.onAuthChallenge({ - targetId: target.id, - name: _name, - instructions, - prompts - }) - - const responses = await Promise.race([responsePromise, timeoutPromise]) - - if (!responses) { - finish([]) - return - } - finish(responses) - } - } as never) - return - } - - if (methods.includes('password')) { - callbacks - .onPasswordPrompt(target.id) - .then((password) => { - if (password === null) { - authState.setState('auth-failed', 'Authentication cancelled') - callback(false as never) - return - } - callback({ - type: 'password' as const, - username: target.username, - password - } as never) - }) - .catch(() => { - callback(false as never) - }) - return - } - - authState.setState('auth-failed', 'No supported authentication methods') - callback(false as never) - } - } - - // If an identity file is specified, try it for the initial attempt - if (target.identityFile) { +export function findDefaultKeyFile(): { path: string; contents: Buffer } | undefined { + for (const keyPath of DEFAULT_KEY_PATHS) { + const resolved = keyPath.replace(/^~/, homedir()) try { - config.privateKey = readFileSync(target.identityFile) + if (!existsSync(resolved)) { + continue + } + const contents = readFileSync(resolved) + return { path: keyPath, contents } } catch { - // Will fall through to other auth methods + continue } } + return undefined +} - // Try SSH agent by default +// Why: matches VS Code's _connectSSH auth method selection (lines 606-611, 727-758). +// ssh2 handles the auth negotiation natively — no custom authHandler needed. +export function buildConnectConfig( + target: SshTarget, + resolved: SshResolvedConfig | null +): ConnectConfig { + const effectiveHost = target.host || resolved?.hostname || target.label + const effectivePort = target.port || resolved?.port || 22 + const effectiveUser = target.username || resolved?.user || '' + + const config: Record = { + host: effectiveHost, + port: effectivePort, + username: effectiveUser, + readyTimeout: CONNECT_TIMEOUT_MS, + keepaliveInterval: 15_000 + } + + // Why: always provide agent when available. Unlike VS Code (which has a + // passphrase prompt UI), we can't decrypt passphrase-protected keys at + // runtime. The agent holds decrypted keys, so it must always be a + // fallback even when an explicit key file is also provided. if (process.env.SSH_AUTH_SOCK) { config.agent = process.env.SSH_AUTH_SOCK } - let proxyProcess: ChildProcess | null = null - if (target.proxyCommand) { - const { spawn } = await import('child_process') - const expanded = target.proxyCommand - .replace(/%h/g, shellEscape(target.host)) - .replace(/%p/g, shellEscape(String(target.port))) - .replace(/%r/g, shellEscape(target.username)) - proxyProcess = spawn('/bin/sh', ['-c', expanded], { stdio: ['pipe', 'pipe', 'pipe'] }) - // Why: a single PassThrough used for both directions creates a feedback loop — - // proxy stdout data flows through the PassThrough and gets piped right back to - // proxy stdin. Use a Duplex wrapper where reads come from stdout and writes - // go to stdin independently. - const { Duplex } = await import('stream') - const stream = new Duplex({ - read() {}, - write(chunk, _encoding, cb) { - proxyProcess!.stdin!.write(chunk, cb) - } - }) - proxyProcess.stdout!.on('data', (data) => stream.push(data)) - proxyProcess.stdout!.on('end', () => stream.push(null)) - config.sock = stream as unknown as NetSocket + const resolvedIdentity = resolved?.identityFile?.[0] + const explicitKey = + target.identityFile || + (resolvedIdentity && !EXPANDED_DEFAULT_KEY_PATHS.includes(resolvedIdentity) + ? resolvedIdentity + : undefined) + + if (explicitKey) { + try { + config.privateKey = readFileSync(explicitKey.replace(/^~/, homedir())) + } catch { + // Key unreadable — agent will handle auth if available + } + } else { + const fallback = findDefaultKeyFile() + if (fallback) { + config.privateKey = fallback.contents + } } - // Wire JumpHost: establish an intermediate SSH connection and forward a channel. - // Why: the jump client is returned to the caller so it can be destroyed on - // disconnect — otherwise the intermediate TCP connection leaks. - let jumpClient: SshClient | null = null - if (target.jumpHost && !target.proxyCommand) { - jumpClient = new SshClient() - const jumpConn = jumpClient - await new Promise((resolve, reject) => { - jumpConn.on('ready', () => resolve()) - jumpConn.on('error', (err) => reject(err)) - jumpConn.connect({ - host: target.jumpHost!, - port: 22, - username: target.username, - agent: process.env.SSH_AUTH_SOCK ?? undefined, - readyTimeout: CONNECT_TIMEOUT_MS - }) - }) - const forwardedChannel = await new Promise((resolve, reject) => { - jumpConn.forwardOut('127.0.0.1', 0, target.host, target.port, (err, channel) => { - if (err) { - reject(err) - } else { - resolve(channel) - } - }) - }) - config.sock = forwardedChannel as unknown as NetSocket - } - - return { config, jumpClient, proxyProcess } + return config as ConnectConfig +} + +// Why: ProxyJump and jumpHost are syntactic sugar for ProxyCommand. +// OpenSSH internally converts `ProxyJump bastion` to +// `ProxyCommand ssh -W %h:%p bastion`. We do the same so that ssh2 +// gets a single proxy spawn path regardless of how the tunnel was configured. +export type EffectiveProxy = + | { kind: 'proxy-command'; command: string } + | { kind: 'jump-host'; jumpHost: string } + +export function resolveEffectiveProxy( + target: SshTarget, + resolved: SshResolvedConfig | null +): EffectiveProxy | undefined { + if (target.proxyCommand) { + return { kind: 'proxy-command', command: target.proxyCommand } + } + if (resolved?.proxyCommand) { + return { kind: 'proxy-command', command: resolved.proxyCommand } + } + const jump = target.jumpHost || resolved?.proxyJump + if (jump) { + return { kind: 'jump-host', jumpHost: jump } + } + return undefined +} + +// Why: ssh2 doesn't natively support ProxyCommand. When the SSH config +// specifies one (e.g. `cloudflared access ssh --hostname %h`), we spawn +// the command and bridge its stdin/stdout into a Duplex stream that ssh2 +// uses as its transport socket via `config.sock`. +function getShellSpawnConfig(command: string): { file: string; args: string[] } { + if (process.platform === 'win32') { + const comspec = process.env.ComSpec || 'cmd.exe' + return { file: comspec, args: ['/d', '/s', '/c', command] } + } + return { file: '/bin/sh', args: ['-c', command] } +} + +export function spawnProxyCommand( + proxy: EffectiveProxy, + host: string, + port: number, + user: string +): { process: ChildProcess; sock: NetSocket } { + const proc = + proxy.kind === 'jump-host' + ? // Why: ProxyJump is structured input, not a shell snippet. Spawn ssh + // directly so jump-host values cannot escape through shell parsing. + spawn('ssh', ['-W', `${host}:${port}`, '--', proxy.jumpHost], { stdio: ['pipe', 'pipe', 'pipe'] }) + : (() => { + const escape = process.platform === 'win32' ? cmdEscape : shellEscape + const expanded = proxy.command + .replace(/%h/g, escape(host)) + .replace(/%p/g, escape(String(port))) + .replace(/%r/g, escape(user)) + const shell = getShellSpawnConfig(expanded) + return spawn(shell.file, shell.args, { stdio: ['pipe', 'pipe', 'pipe'] }) + })() + + // Why: a single PassThrough for both directions creates a feedback loop. + // Reads come from the proxy's stdout; writes go to its stdin. + const stream = new Duplex({ + read() {}, + write(chunk, _encoding, cb) { + proc.stdin!.write(chunk, cb) + } + }) + proc.stdout!.on('data', (data) => stream.push(data)) + proc.stdout!.on('end', () => stream.push(null)) + proc.stdin!.on('error', (err) => stream.destroy(err)) + proc.on('error', (err) => stream.destroy(err)) + + return { process: proc, sock: stream as unknown as NetSocket } } diff --git a/src/main/ssh/ssh-connection.test.ts b/src/main/ssh/ssh-connection.test.ts index fb141d643..552e6d66a 100644 --- a/src/main/ssh/ssh-connection.test.ts +++ b/src/main/ssh/ssh-connection.test.ts @@ -36,7 +36,12 @@ vi.mock('./ssh-system-fallback', () => ({ }) })) +vi.mock('./ssh-config-parser', () => ({ + resolveWithSshG: vi.fn().mockResolvedValue(null) +})) + import { SshConnection, SshConnectionManager, type SshConnectionCallbacks } from './ssh-connection' +import { resolveWithSshG } from './ssh-config-parser' import type { SshTarget } from '../../shared/ssh-types' function createTarget(overrides?: Partial): SshTarget { @@ -53,9 +58,6 @@ function createTarget(overrides?: Partial): SshTarget { function createCallbacks(overrides?: Partial): SshConnectionCallbacks { return { onStateChange: vi.fn(), - onHostKeyVerify: vi.fn().mockResolvedValue(true), - onAuthChallenge: vi.fn().mockResolvedValue(['123456']), - onPasswordPrompt: vi.fn().mockResolvedValue('password'), ...overrides } } @@ -138,6 +140,21 @@ describe('SshConnection', () => { await expect(conn.connect()).rejects.toThrow('Connection disposed') }) + + it('resolves OpenSSH config using configHost when present', async () => { + const callbacks = createCallbacks() + const conn = new SshConnection( + createTarget({ + label: 'Friendly Name', + configHost: 'ssh-alias' + }), + callbacks + ) + + await conn.connect() + + expect(resolveWithSshG).toHaveBeenCalledWith('ssh-alias') + }) }) describe('SshConnectionManager', () => { diff --git a/src/main/ssh/ssh-connection.ts b/src/main/ssh/ssh-connection.ts index 0e2066aa4..6c986fbcb 100644 --- a/src/main/ssh/ssh-connection.ts +++ b/src/main/ssh/ssh-connection.ts @@ -1,31 +1,28 @@ +/* eslint-disable max-lines -- Why: SSH connection lifecycle, credential retries, reconnect policy, and transport fallback are intentionally co-located so state transitions stay auditable in one file. */ import { Client as SshClient } from 'ssh2' import type { ChildProcess } from 'child_process' -import type { ClientChannel, SFTPWrapper } from 'ssh2' +import type { ClientChannel, ConnectConfig, SFTPWrapper } from 'ssh2' import type { SshTarget, SshConnectionState, SshConnectionStatus } from '../../shared/ssh-types' import { spawnSystemSsh, type SystemSshProcess } from './ssh-system-fallback' +import { resolveWithSshG } from './ssh-config-parser' import { INITIAL_RETRY_ATTEMPTS, INITIAL_RETRY_DELAY_MS, RECONNECT_BACKOFF_MS, CONNECT_TIMEOUT_MS, isTransientError, + isAuthError, + isPassphraseError, sleep, buildConnectConfig, + resolveEffectiveProxy, + spawnProxyCommand, type SshConnectionCallbacks } from './ssh-connection-utils' -// Why: type definitions live in ssh-connection-utils.ts to break a circular -// import. Re-exported here so existing import sites keep working. -export type { - HostKeyVerifyRequest, - AuthChallengeRequest, - SshConnectionCallbacks -} from './ssh-connection-utils' +export type { SshConnectionCallbacks } from './ssh-connection-utils' export class SshConnection { private client: SshClient | null = null - /** Why: the jump host client must be tracked so it can be torn down on - * disconnect — otherwise the intermediate TCP connection leaks. */ - private jumpClient: SshClient | null = null private proxyProcess: ChildProcess | null = null private systemSsh: SystemSshProcess | null = null private state: SshConnectionState @@ -33,8 +30,9 @@ export class SshConnection { private target: SshTarget private reconnectTimer: ReturnType | null = null private disposed = false - private agentAttempted = false - private keyAttempted = false + private cachedPassphrase: string | null = null + private cachedPassword: string | null = null + private connectGeneration = 0 constructor(target: SshTarget, callbacks: SshConnectionCallbacks) { this.target = target @@ -50,47 +48,25 @@ export class SshConnection { getState(): SshConnectionState { return { ...this.state } } - getClient(): SshClient | null { return this.client } - getTarget(): SshTarget { return { ...this.target } } - /** Open an exec channel. Used by relay deployment to run commands on the remote. */ - async exec(command: string): Promise { - const client = this.client - if (!client) { + async exec(cmd: string): Promise { + if (!this.client) { throw new Error('Not connected') } - return new Promise((resolve, reject) => { - client.exec(command, (err, channel) => { - if (err) { - reject(err) - } else { - resolve(channel) - } - }) - }) + return new Promise((res, rej) => this.client!.exec(cmd, (e, ch) => (e ? rej(e) : res(ch)))) } - /** Open an SFTP session for file transfers (relay deployment). */ async sftp(): Promise { - const client = this.client - if (!client) { + if (!this.client) { throw new Error('Not connected') } - return new Promise((resolve, reject) => { - client.sftp((err, sftp) => { - if (err) { - reject(err) - } else { - resolve(sftp) - } - }) - }) + return new Promise((res, rej) => this.client!.sftp((e, s) => (e ? rej(e) : res(s)))) } async connect(): Promise { @@ -107,7 +83,13 @@ export class SshConnection { } catch (err) { lastError = err instanceof Error ? err : new Error(String(err)) + if (isAuthError(lastError) || isPassphraseError(lastError)) { + this.setState('auth-failed', lastError.message) + throw lastError + } + if (!isTransientError(lastError)) { + this.setState('error', lastError.message) throw lastError } @@ -124,95 +106,136 @@ export class SshConnection { private async attemptConnect(): Promise { this.setState('connecting') - this.agentAttempted = false - this.keyAttempted = false + this.proxyProcess?.kill() + this.proxyProcess = null + const connectGeneration = ++this.connectGeneration - // Why: clean up resources from a prior failed attempt before overwriting. - // Without this, a retry after timeout/auth-failure orphans the old jump - // host TCP connection and proxy child process. - if (this.jumpClient) { - this.jumpClient.end() - this.jumpClient = null + const resolved = await resolveWithSshG(this.target.configHost || this.target.label).catch(() => null) + const config = buildConnectConfig(this.target, resolved) + + // Why: ssh2 doesn't support ProxyCommand/ProxyJump natively. Spawn the + // resolved proxy and pipe its stdin/stdout as config.sock. + const effectiveProxy = resolveEffectiveProxy(this.target, resolved) + if (effectiveProxy) { + const proxy = spawnProxyCommand(effectiveProxy, config.host!, config.port!, config.username!) + this.proxyProcess = proxy.process + config.sock = proxy.sock } - if (this.proxyProcess) { - this.proxyProcess.kill() + + if (this.cachedPassphrase) { + config.passphrase = this.cachedPassphrase + } + if (this.cachedPassword) { + config.password = this.cachedPassword + } + + try { + await this.doSsh2Connect(config, connectGeneration) + } catch (err) { + if (!(err instanceof Error) || !this.callbacks.onCredentialRequest) { + this.proxyProcess?.kill() + this.proxyProcess = null + throw err + } + // Why: prompt for passphrase on encrypted-key error, then retry with + // a fresh proxy socket (ssh2 may have destroyed the original). + if (isPassphraseError(err) && !this.cachedPassphrase) { + const detail = this.target.identityFile || resolved?.identityFile?.[0] || '(unknown)' + const val = await this.callbacks.onCredentialRequest(this.target.id, 'passphrase', detail) + if (val) { + this.cachedPassphrase = val + config.passphrase = val + this.respawnProxy(config, effectiveProxy) + await this.doSsh2Connect(config, connectGeneration) + return + } + } + // Why: prompt for password on auth failure. Check the original error + // (not a retry error) to avoid conflating passphrase vs password failures. + if (isAuthError(err) && !this.cachedPassword) { + const val = await this.callbacks.onCredentialRequest( + this.target.id, + 'password', + config.host || this.target.label + ) + if (val) { + this.cachedPassword = val + config.password = val + this.respawnProxy(config, effectiveProxy) + await this.doSsh2Connect(config, connectGeneration) + return + } + } + this.proxyProcess?.kill() this.proxyProcess = null + throw err } + } - const { config, jumpClient, proxyProcess } = await this.buildConfig() - this.jumpClient = jumpClient - this.proxyProcess = proxyProcess + // Why: ssh2 may destroy the proxy socket on auth failure, so credential + // retries need a fresh proxy process and Duplex stream. + private respawnProxy( + config: ConnectConfig, + proxy: ReturnType | null | undefined + ): void { + if (!proxy) { + return + } + this.proxyProcess?.kill() + const p = spawnProxyCommand(proxy, config.host!, config.port!, config.username!) + this.proxyProcess = p.process + config.sock = p.sock + } + private doSsh2Connect(config: ConnectConfig, connectGeneration: number): Promise { return new Promise((resolve, reject) => { const client = new SshClient() let settled = false - - const timeout = setTimeout(() => { - if (!settled) { - settled = true - client.destroy() - const msg = `Connection timed out after ${CONNECT_TIMEOUT_MS}ms` - this.setState('error', msg) - reject(new Error(msg)) - } - }, CONNECT_TIMEOUT_MS) - - // Why: host key verification is now handled inside the hostVerifier - // callback in buildConnectConfig (ssh-connection-utils.ts). The - // callback form `(key, verify) => void` blocks the handshake until - // the user accepts/rejects, so no separate 'handshake' listener is - // needed here. - client.on('ready', () => { if (settled) { return } + // Why: connect() completion races with explicit disconnect(). Once a + // newer connect attempt or disconnect bumps the generation/disposed + // state, this late ready event must not resurrect the torn-down client. + if (this.disposed || connectGeneration !== this.connectGeneration) { + settled = true + client.end() + client.destroy() + reject(new Error('SSH connection attempt was cancelled')) + return + } settled = true - clearTimeout(timeout) this.client = client + this.proxyProcess = null this.setState('connected') this.setupDisconnectHandler(client) resolve() }) - client.on('error', (err) => { if (settled) { return } settled = true - clearTimeout(timeout) - this.setState('error', err.message) + client.destroy() reject(err) }) - client.connect(config) }) } - private async buildConfig() { - // Why: config-building logic extracted to ssh-connection-utils.ts (max-lines). - return buildConnectConfig(this.target, this.callbacks, { - agentAttempted: this.agentAttempted, - keyAttempted: this.keyAttempted, - setState: (status: string, error?: string) => { - this.setState(status as SshConnectionStatus, error) - } - }) - } - - // Why: both `end` and `close` fire on disconnect. If reconnect succeeds - // between the two events, the second handler would null out the *new* - // connection. Guarding on `this.client === client` prevents that. + // Why: guard on identity so a late event from the old client doesn't + // null out a successful reconnect. private setupDisconnectHandler(client: SshClient): void { - const handleDisconnect = () => { + const onDrop = () => { if (this.disposed || this.client !== client) { return } this.client = null this.scheduleReconnect() } - client.on('end', handleDisconnect) - client.on('close', handleDisconnect) + client.on('end', onDrop) + client.on('close', onDrop) client.on('error', (err) => { if (this.disposed || this.client !== client) { return @@ -227,71 +250,58 @@ export class SshConnection { if (this.disposed || this.reconnectTimer) { return } - const attempt = this.state.reconnectAttempt if (attempt >= RECONNECT_BACKOFF_MS.length) { this.setState('reconnection-failed', 'Max reconnection attempts reached') return } - this.setState('reconnecting') - const delay = RECONNECT_BACKOFF_MS[attempt] - this.reconnectTimer = setTimeout(async () => { this.reconnectTimer = null if (this.disposed) { return } - try { - await this.attemptConnect() - // Why: reset the counter and re-broadcast so the UI shows attempt 0. - // attemptConnect already calls setState('connected'), but the attempt - // counter must be zeroed *before* so the broadcast carries the right value. + // Why: reset reconnectAttempt before attemptConnect so setState('connected') + // broadcasts reconnectAttempt=0, which ssh.ts uses to trigger relay re-establishment. this.state.reconnectAttempt = 0 - this.setState('connected') - } catch { - // Why: increment before scheduleReconnect so the setState('reconnecting') - // call inside it broadcasts the updated attempt number to the UI. - this.state.reconnectAttempt++ + await this.attemptConnect() + } catch (err) { + if (this.disposed) { + return + } + const error = err instanceof Error ? err : new Error(String(err)) + if (isAuthError(error) || isPassphraseError(error)) { + this.setState('auth-failed', error.message) + return + } + if (!isTransientError(error)) { + this.setState('error', error.message) + return + } + this.state.reconnectAttempt = attempt + 1 this.scheduleReconnect() } - }, delay) + }, RECONNECT_BACKOFF_MS[attempt]) } - /** Fall back to system SSH binary when ssh2 cannot handle auth (FIDO2, ControlMaster). */ async connectViaSystemSsh(): Promise { if (this.disposed) { throw new Error('Connection disposed') } - // Why: if connectViaSystemSsh is called again after a prior failed attempt, - // the old process may still be running. Without cleanup, overwriting - // this.systemSsh at line 267 would orphan the old process. - if (this.systemSsh) { - this.systemSsh.kill() - this.systemSsh = null - } + this.systemSsh?.kill() + this.systemSsh = null this.setState('connecting') - try { const proc = spawnSystemSsh(this.target) this.systemSsh = proc - - // Why: two onExit handlers are registered — one for the initial handshake - // (reject the promise on early exit) and one for post-connect reconnection. - // Without a settled flag, an early exit during handshake would fire both, - // causing the reconnection handler to schedule a reconnect for a connection - // that was never established. let settled = false - - // Why: verify the SSH connection succeeded before reporting connected. - // Wait for relay sentinel output or a non-zero exit. await new Promise((resolve, reject) => { const timeout = setTimeout(() => { settled = true + proc.kill() reject(new Error('System SSH connection timed out')) }, CONNECT_TIMEOUT_MS) - proc.stdout.once('data', () => { settled = true clearTimeout(timeout) @@ -303,69 +313,54 @@ export class SshConnection { } settled = true clearTimeout(timeout) - if (code !== 0) { - reject(new Error(`System SSH exited with code ${code}`)) - } + reject( + new Error( + code !== 0 + ? `System SSH exited with code ${code}` + : 'System SSH exited before producing output' + ) + ) }) }) - this.setState('connected') - - // Why: unlike ssh2 Client which emits end/close, the system SSH process - // only signals disconnection through its exit event. Without this handler - // an unexpected exit would leave the connection in 'connected' state with - // no underlying transport. - proc.onExit((_code) => { + // Why: register reconnection handler only after the initial handshake + // succeeds. The onExit registered above guards with `settled` so it + // won't fire a duplicate for exits during the handshake phase. + proc.onExit(() => { if (!this.disposed && this.systemSsh === proc) { this.systemSsh = null this.scheduleReconnect() } }) - return proc } catch (err) { - const msg = err instanceof Error ? err.message : String(err) - this.setState('error', msg) + this.setState('error', err instanceof Error ? err.message : String(err)) throw err } } async disconnect(): Promise { this.disposed = true + this.connectGeneration += 1 if (this.reconnectTimer) { clearTimeout(this.reconnectTimer) - this.reconnectTimer = null - } - if (this.client) { - this.client.end() - this.client = null - } - // Why: the jump host client holds an open TCP connection to the - // intermediate host. Failing to close it would leak the socket. - if (this.jumpClient) { - this.jumpClient.end() - this.jumpClient = null - } - if (this.proxyProcess) { - this.proxyProcess.kill() - this.proxyProcess = null - } - if (this.systemSsh) { - this.systemSsh.kill() - this.systemSsh = null } + this.reconnectTimer = null + this.cachedPassphrase = null + this.cachedPassword = null + this.client?.end() + this.client = null + this.proxyProcess?.kill() + this.proxyProcess = null + this.systemSsh?.kill() + this.systemSsh = null this.setState('disconnected') } private setState(status: SshConnectionStatus, error?: string): void { - this.state = { - ...this.state, - status, - error: error ?? null - } + this.state = { ...this.state, status, error: error ?? null } this.callbacks.onStateChange(this.target.id, { ...this.state }) } } -// Why: extracted to ssh-connection-manager.ts to stay under 300-line max-lines. export { SshConnectionManager } from './ssh-connection-manager' diff --git a/src/main/ssh/ssh-relay-deploy.test.ts b/src/main/ssh/ssh-relay-deploy.test.ts new file mode 100644 index 000000000..bf73d53ca --- /dev/null +++ b/src/main/ssh/ssh-relay-deploy.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('electron', () => ({ + app: { getAppPath: () => '/mock/app' } +})) + +vi.mock('fs', () => ({ + existsSync: vi.fn().mockReturnValue(false), + readFileSync: vi.fn() +})) + +vi.mock('./relay-protocol', () => ({ + RELAY_VERSION: '0.1.0', + RELAY_REMOTE_DIR: '.orca-remote', + parseUnameToRelayPlatform: vi.fn().mockReturnValue('linux-x64'), + RELAY_SENTINEL: 'ORCA-RELAY v0.1.0 READY\n', + RELAY_SENTINEL_TIMEOUT_MS: 10_000 +})) + +vi.mock('./ssh-relay-deploy-helpers', () => ({ + uploadDirectory: vi.fn().mockResolvedValue(undefined), + waitForSentinel: vi.fn().mockResolvedValue({ + write: vi.fn(), + onData: vi.fn(), + onClose: vi.fn() + }), + execCommand: vi.fn().mockResolvedValue('Linux x86_64'), + resolveRemoteNodePath: vi.fn().mockResolvedValue('/usr/bin/node') +})) + +vi.mock('./ssh-connection-utils', () => ({ + shellEscape: (s: string) => `'${s}'` +})) + +import { deployAndLaunchRelay } from './ssh-relay-deploy' +import { execCommand } from './ssh-relay-deploy-helpers' +import type { SshConnection } from './ssh-connection' + +function makeMockConnection(): SshConnection { + return { + exec: vi.fn().mockResolvedValue({ + on: vi.fn(), + stderr: { on: vi.fn() }, + stdin: {}, + stdout: { on: vi.fn() }, + close: vi.fn() + }), + sftp: vi.fn().mockResolvedValue({ + mkdir: vi.fn((_p: string, cb: (err: Error | null) => void) => cb(null)), + createWriteStream: vi.fn().mockReturnValue({ + on: vi.fn((_event: string, cb: () => void) => { + if (_event === 'close') { + setTimeout(cb, 0) + } + }), + end: vi.fn() + }), + end: vi.fn() + }) + } as unknown as SshConnection +} + +describe('deployAndLaunchRelay', () => { + it('calls exec to detect remote platform', async () => { + const conn = makeMockConnection() + const mockExecCommand = vi.mocked(execCommand) + mockExecCommand.mockResolvedValueOnce('Linux x86_64') // uname -sm + mockExecCommand.mockResolvedValueOnce('/home/user') // echo $HOME + mockExecCommand.mockResolvedValueOnce('OK') // check relay exists + mockExecCommand.mockResolvedValueOnce('0.1.0') // version check + + await deployAndLaunchRelay(conn) + + expect(mockExecCommand).toHaveBeenCalledWith(conn, 'uname -sm') + }) + + it('reports progress via callback', async () => { + const conn = makeMockConnection() + const mockExecCommand = vi.mocked(execCommand) + mockExecCommand.mockResolvedValueOnce('Linux x86_64') + mockExecCommand.mockResolvedValueOnce('/home/user') + mockExecCommand.mockResolvedValueOnce('OK') + mockExecCommand.mockResolvedValueOnce('0.1.0') + + const progress: string[] = [] + await deployAndLaunchRelay(conn, (status) => progress.push(status)) + + expect(progress).toContain('Detecting remote platform...') + expect(progress).toContain('Starting relay...') + }) + + it('has a 120-second overall timeout', async () => { + const conn = makeMockConnection() + const mockExecCommand = vi.mocked(execCommand) + + // Make the first exec never resolve + mockExecCommand.mockReturnValueOnce(new Promise(() => {})) + + vi.useFakeTimers() + + // Catch the rejection immediately to avoid unhandled rejection warning + const promise = deployAndLaunchRelay(conn).catch((err: Error) => err) + + await vi.advanceTimersByTimeAsync(121_000) + + const result = await promise + expect(result).toBeInstanceOf(Error) + expect((result as Error).message).toBe('Relay deployment timed out after 120s') + + vi.useRealTimers() + }) +}) diff --git a/src/main/ssh/ssh-relay-deploy.ts b/src/main/ssh/ssh-relay-deploy.ts index a841c5f3a..00e889234 100644 --- a/src/main/ssh/ssh-relay-deploy.ts +++ b/src/main/ssh/ssh-relay-deploy.ts @@ -22,6 +22,12 @@ export type RelayDeployResult = { platform: RelayPlatform } +// Why: individual exec commands have 30s timeouts, but the full deploy +// pipeline (detect platform → check existing → upload → npm install → +// launch) has no overall bound. A hanging `npm install` or slow SFTP +// upload could block the connection indefinitely. +const RELAY_DEPLOY_TIMEOUT_MS = 120_000 + /** * Deploy the relay to the remote host and launch it. * @@ -36,6 +42,24 @@ export type RelayDeployResult = { export async function deployAndLaunchRelay( conn: SshConnection, onProgress?: (status: string) => void +): Promise { + let timeoutHandle: ReturnType + const timeoutPromise = new Promise((_resolve, reject) => { + timeoutHandle = setTimeout(() => { + reject(new Error(`Relay deployment timed out after ${RELAY_DEPLOY_TIMEOUT_MS / 1000}s`)) + }, RELAY_DEPLOY_TIMEOUT_MS) + }) + + try { + return await Promise.race([deployAndLaunchRelayInner(conn, onProgress), timeoutPromise]) + } finally { + clearTimeout(timeoutHandle!) + } +} + +async function deployAndLaunchRelayInner( + conn: SshConnection, + onProgress?: (status: string) => void ): Promise { onProgress?.('Detecting remote platform...') console.log('[ssh-relay] Detecting remote platform...') @@ -50,9 +74,11 @@ export async function deployAndLaunchRelay( // Why: SFTP does not expand `~`, so we must resolve the remote home directory // explicitly. `echo $HOME` over exec gives us the absolute path. const remoteHome = (await execCommand(conn, 'echo $HOME')).trim() - // Why: a malicious or misconfigured remote could return a $HOME containing - // shell metacharacters. Validate it looks like a reasonable path. - if (!remoteHome || !/^\/[a-zA-Z0-9/_.-]+$/.test(remoteHome)) { + // Why: we only interpolate $HOME into single-quoted shell strings later, so + // this validation only needs to reject obviously unsafe control characters. + // Allow spaces and non-ASCII so valid home directories are not rejected. + // oxlint-disable-next-line no-control-regex + if (!remoteHome || !remoteHome.startsWith('/') || /[\u0000\r\n]/.test(remoteHome)) { throw new Error(`Remote $HOME is not a valid path: ${remoteHome.slice(0, 100)}`) } const remoteRelayDir = `${remoteHome}/${RELAY_REMOTE_DIR}/relay-v${RELAY_VERSION}` diff --git a/src/main/ssh/ssh-system-fallback.ts b/src/main/ssh/ssh-system-fallback.ts index bc0786ccb..5c6835031 100644 --- a/src/main/ssh/ssh-system-fallback.ts +++ b/src/main/ssh/ssh-system-fallback.ts @@ -76,7 +76,7 @@ function buildSshArgs(target: SshTarget): string[] { } const userHost = target.username ? `${target.username}@${target.host}` : target.host - args.push(userHost) + args.push('--', userHost) return args } diff --git a/src/preload/api-types.d.ts b/src/preload/api-types.d.ts index 1307a1d46..12cf2f19c 100644 --- a/src/preload/api-types.d.ts +++ b/src/preload/api-types.d.ts @@ -68,6 +68,7 @@ import type { ClaudeUsageSummary } from '../../shared/claude-usage-types' import type { RateLimitState } from '../../shared/rate-limit-types' +import type { SshConnectionState, SshTarget } from '../../shared/ssh-types' import type { CodexUsageBreakdownKind, CodexUsageBreakdownRow, @@ -250,6 +251,7 @@ export type PreloadApi = { env?: Record command?: string connectionId?: string | null + worktreeId?: string }) => Promise<{ id: string }> write: (id: string, data: string) => void resize: (id: string, cols: number, rows: number) => void @@ -529,42 +531,18 @@ export type PreloadApi = { onUpdate: (callback: (state: RateLimitState) => void) => () => void } ssh: { - listTargets: () => Promise - addTarget: (args: { target: Record }) => Promise - updateTarget: (args: { id: string; updates: Record }) => Promise + listTargets: () => Promise + addTarget: (args: { target: Omit }) => Promise + updateTarget: (args: { id: string; updates: Partial> }) => Promise removeTarget: (args: { id: string }) => Promise - importConfig: () => Promise - connect: (args: { targetId: string }) => Promise + importConfig: () => Promise + connect: (args: { targetId: string }) => Promise disconnect: (args: { targetId: string }) => Promise - getState: (args: { targetId: string }) => Promise + getState: (args: { targetId: string }) => Promise testConnection: (args: { targetId: string - }) => Promise<{ success: boolean; error?: string; state?: unknown }> - onStateChanged: (callback: (data: { targetId: string; state: unknown }) => void) => () => void - onHostKeyVerify: ( - callback: (data: { - host: string - ip: string - fingerprint: string - keyType: string - responseChannel: string - }) => void - ) => () => void - respondHostKeyVerify: (args: { channel: string; accepted: boolean }) => void - onAuthChallenge: ( - callback: (data: { - targetId: string - name: string - instructions: string - prompts: { prompt: string; echo: boolean }[] - responseChannel: string - }) => void - ) => () => void - respondAuthChallenge: (args: { channel: string; responses: string[] }) => void - onPasswordPrompt: ( - callback: (data: { targetId: string; responseChannel: string }) => void - ) => () => void - respondPassword: (args: { channel: string; password: string | null }) => void + }) => Promise<{ success: boolean; error?: string; state?: SshConnectionState }> + onStateChanged: (callback: (data: { targetId: string; state: SshConnectionState }) => void) => () => void addPortForward: (args: { targetId: string localPort: number @@ -578,5 +556,15 @@ export type PreloadApi = { entries: { name: string; isDirectory: boolean }[] resolvedPath: string }> + onCredentialRequest: ( + callback: (data: { + requestId: string + targetId: string + kind: 'passphrase' | 'password' + detail: string + }) => void + ) => () => void + onCredentialResolved: (callback: (data: { requestId: string }) => void) => () => void + submitCredential: (args: { requestId: string; value: string | null }) => Promise } } diff --git a/src/preload/index.ts b/src/preload/index.ts index 276b5cb53..4a5418174 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -11,6 +11,7 @@ import type { } from '../shared/types' import type { RuntimeStatus, RuntimeSyncWindowGraph } from '../shared/runtime-types' import type { RateLimitState } from '../shared/rate-limit-types' +import type { SshConnectionState, SshTarget } from '../shared/ssh-types' import { ORCA_EDITOR_SAVE_DIRTY_FILES_EVENT, type EditorSaveDirtyFilesDetail @@ -1123,113 +1124,46 @@ const api = { }, ssh: { - listTargets: (): Promise => ipcRenderer.invoke('ssh:listTargets'), + listTargets: (): Promise => ipcRenderer.invoke('ssh:listTargets'), - addTarget: (args: { target: Record }): Promise => + addTarget: (args: { target: Omit }): Promise => ipcRenderer.invoke('ssh:addTarget', args), - updateTarget: (args: { id: string; updates: Record }): Promise => + updateTarget: ( + args: { id: string; updates: Partial> } + ): Promise => ipcRenderer.invoke('ssh:updateTarget', args), removeTarget: (args: { id: string }): Promise => ipcRenderer.invoke('ssh:removeTarget', args), - importConfig: (): Promise => ipcRenderer.invoke('ssh:importConfig'), + importConfig: (): Promise => ipcRenderer.invoke('ssh:importConfig'), - connect: (args: { targetId: string }): Promise => + connect: (args: { targetId: string }): Promise => ipcRenderer.invoke('ssh:connect', args), disconnect: (args: { targetId: string }): Promise => ipcRenderer.invoke('ssh:disconnect', args), - getState: (args: { targetId: string }): Promise => + getState: (args: { targetId: string }): Promise => ipcRenderer.invoke('ssh:getState', args), testConnection: (args: { targetId: string - }): Promise<{ success: boolean; error?: string; state?: unknown }> => + }): Promise<{ success: boolean; error?: string; state?: SshConnectionState }> => ipcRenderer.invoke('ssh:testConnection', args), onStateChanged: ( - callback: (data: { targetId: string; state: unknown }) => void + callback: (data: { targetId: string; state: SshConnectionState }) => void ): (() => void) => { const listener = ( _event: Electron.IpcRendererEvent, - data: { targetId: string; state: unknown } + data: { targetId: string; state: SshConnectionState } ) => callback(data) ipcRenderer.on('ssh:state-changed', listener) return () => ipcRenderer.removeListener('ssh:state-changed', listener) }, - onHostKeyVerify: ( - callback: (data: { - host: string - ip: string - fingerprint: string - keyType: string - responseChannel: string - }) => void - ): (() => void) => { - const listener = ( - _event: Electron.IpcRendererEvent, - data: { - host: string - ip: string - fingerprint: string - keyType: string - responseChannel: string - } - ) => callback(data) - ipcRenderer.on('ssh:host-key-verify', listener) - return () => ipcRenderer.removeListener('ssh:host-key-verify', listener) - }, - - respondHostKeyVerify: (args: { channel: string; accepted: boolean }): void => { - ipcRenderer.send(args.channel, args.accepted) - }, - - onAuthChallenge: ( - callback: (data: { - targetId: string - name: string - instructions: string - prompts: { prompt: string; echo: boolean }[] - responseChannel: string - }) => void - ): (() => void) => { - const listener = ( - _event: Electron.IpcRendererEvent, - data: { - targetId: string - name: string - instructions: string - prompts: { prompt: string; echo: boolean }[] - responseChannel: string - } - ) => callback(data) - ipcRenderer.on('ssh:auth-challenge', listener) - return () => ipcRenderer.removeListener('ssh:auth-challenge', listener) - }, - - respondAuthChallenge: (args: { channel: string; responses: string[] }): void => { - ipcRenderer.send(args.channel, args.responses) - }, - - onPasswordPrompt: ( - callback: (data: { targetId: string; responseChannel: string }) => void - ): (() => void) => { - const listener = ( - _event: Electron.IpcRendererEvent, - data: { targetId: string; responseChannel: string } - ) => callback(data) - ipcRenderer.on('ssh:password-prompt', listener) - return () => ipcRenderer.removeListener('ssh:password-prompt', listener) - }, - - respondPassword: (args: { channel: string; password: string | null }): void => { - ipcRenderer.send(args.channel, args.password) - }, - addPortForward: (args: { targetId: string localPort: number @@ -1250,7 +1184,38 @@ const api = { }): Promise<{ entries: { name: string; isDirectory: boolean }[] resolvedPath: string - }> => ipcRenderer.invoke('ssh:browseDir', args) + }> => ipcRenderer.invoke('ssh:browseDir', args), + + onCredentialRequest: ( + callback: (data: { + requestId: string + targetId: string + kind: 'passphrase' | 'password' + detail: string + }) => void + ): (() => void) => { + const listener = ( + _event: Electron.IpcRendererEvent, + data: { + requestId: string + targetId: string + kind: 'passphrase' | 'password' + detail: string + } + ) => callback(data) + ipcRenderer.on('ssh:credential-request', listener) + return () => ipcRenderer.removeListener('ssh:credential-request', listener) + }, + + onCredentialResolved: (callback: (data: { requestId: string }) => void): (() => void) => { + const listener = (_event: Electron.IpcRendererEvent, data: { requestId: string }) => + callback(data) + ipcRenderer.on('ssh:credential-resolved', listener) + return () => ipcRenderer.removeListener('ssh:credential-resolved', listener) + }, + + submitCredential: (args: { requestId: string; value: string | null }): Promise => + ipcRenderer.invoke('ssh:submitCredential', args) } } diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index dd0deb219..29397f1af 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -23,6 +23,7 @@ import WorktreeJumpPalette from './components/WorktreeJumpPalette' import { StatusBar } from './components/status-bar/StatusBar' import { UpdateCard } from './components/UpdateCard' import { ZoomOverlay } from './components/ZoomOverlay' +import { SshPassphraseDialog } from './components/settings/SshPassphraseDialog' import { useGitStatusPolling } from './components/right-sidebar/useGitStatusPolling' import { setRuntimeGraphStoreStateGetter, @@ -696,6 +697,7 @@ function App(): React.JSX.Element { + ) diff --git a/src/renderer/src/components/settings/SshPane.tsx b/src/renderer/src/components/settings/SshPane.tsx index c52dd0aca..9c7d9d31d 100644 --- a/src/renderer/src/components/settings/SshPane.tsx +++ b/src/renderer/src/components/settings/SshPane.tsx @@ -50,20 +50,34 @@ export function SshPane(_props: SshPaneProps): React.JSX.Element { const [showForm, setShowForm] = useState(false) const [editingId, setEditingId] = useState(null) const [form, setForm] = useState(EMPTY_FORM) - const [testing, setTesting] = useState(null) + const [testingIds, setTestingIds] = useState>(new Set()) const [pendingRemove, setPendingRemove] = useState<{ id: string; label: string } | null>(null) - const loadTargets = useCallback(async () => { + const setSshTargetLabels = useAppStore((s) => s.setSshTargetLabels) + + const loadTargets = useCallback(async (opts?: { signal?: AbortSignal }) => { try { const result = (await window.api.ssh.listTargets()) as SshTarget[] + if (opts?.signal?.aborted) { + return + } setTargets(result) + const labels = new Map() + for (const t of result) { + labels.set(t.id, t.label) + } + setSshTargetLabels(labels) } catch { - toast.error('Failed to load SSH targets') + if (!opts?.signal?.aborted) { + toast.error('Failed to load SSH targets') + } } - }, []) + }, [setSshTargetLabels]) useEffect(() => { - void loadTargets() + const abortController = new AbortController() + void loadTargets({ signal: abortController.signal }) + return () => abortController.abort() }, [loadTargets]) const handleSave = async (): Promise => { @@ -80,6 +94,7 @@ export function SshPane(_props: SshPaneProps): React.JSX.Element { const target = { label: form.label.trim() || `${form.username}@${form.host}`, + configHost: form.configHost.trim() || form.host.trim(), host: form.host.trim(), port, username: form.username.trim(), @@ -126,6 +141,7 @@ export function SshPane(_props: SshPaneProps): React.JSX.Element { setEditingId(target.id) setForm({ label: target.label, + configHost: target.configHost ?? target.host, host: target.host, port: String(target.port), username: target.username, @@ -153,7 +169,7 @@ export function SshPane(_props: SshPaneProps): React.JSX.Element { } const handleTest = async (targetId: string): Promise => { - setTesting(targetId) + setTestingIds((prev) => new Set(prev).add(targetId)) try { const result = await window.api.ssh.testConnection({ targetId }) if (result.success) { @@ -164,7 +180,11 @@ export function SshPane(_props: SshPaneProps): React.JSX.Element { } catch (err) { toast.error(err instanceof Error ? err.message : 'Test failed') } finally { - setTesting(null) + setTestingIds((prev) => { + const next = new Set(prev) + next.delete(targetId) + return next + }) } } @@ -238,7 +258,7 @@ export function SshPane(_props: SshPaneProps): React.JSX.Element { key={target.id} target={target} state={sshConnectionStates.get(target.id)} - testing={testing === target.id} + testing={testingIds.has(target.id)} onConnect={(id) => void handleConnect(id)} onDisconnect={(id) => void handleDisconnect(id)} onTest={(id) => void handleTest(id)} diff --git a/src/renderer/src/components/settings/SshPassphraseDialog.tsx b/src/renderer/src/components/settings/SshPassphraseDialog.tsx new file mode 100644 index 000000000..bbc632612 --- /dev/null +++ b/src/renderer/src/components/settings/SshPassphraseDialog.tsx @@ -0,0 +1,122 @@ +import React, { useCallback, useEffect, useRef, useState } from 'react' +import { toast } from 'sonner' +import { Button } from '@/components/ui/button' +import { Input } from '@/components/ui/input' +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle +} from '@/components/ui/dialog' +import { useAppStore } from '@/store' + +export function SshPassphraseDialog(): React.JSX.Element | null { + const request = useAppStore((s) => s.sshCredentialQueue[0] ?? null) + const targetLabels = useAppStore((s) => s.sshTargetLabels) + const removeRequest = useAppStore((s) => s.removeSshCredentialRequest) + const [value, setValue] = useState('') + const [submitting, setSubmitting] = useState(false) + const inputRef = useRef(null) + + const open = request !== null + + const requestId = request?.requestId + useEffect(() => { + if (requestId) { + setValue('') + setSubmitting(false) + requestAnimationFrame(() => inputRef.current?.focus()) + } + }, [requestId]) + + const handleSubmit = useCallback(async () => { + if (!request || !value) { + return + } + setSubmitting(true) + try { + await window.api.ssh.submitCredential({ requestId: request.requestId, value }) + removeRequest(request.requestId) + } catch (err) { + toast.error(err instanceof Error ? err.message : 'Failed to submit SSH credential') + setSubmitting(false) + } + }, [request, value, removeRequest]) + + const handleCancel = useCallback(async () => { + if (request) { + setSubmitting(true) + try { + await window.api.ssh.submitCredential({ requestId: request.requestId, value: null }) + removeRequest(request.requestId) + } catch (err) { + toast.error(err instanceof Error ? err.message : 'Failed to cancel SSH credential request') + setSubmitting(false) + } + } + }, [request, removeRequest]) + + if (!request) { + return null + } + + const label = targetLabels.get(request.targetId) ?? request.targetId + const isPassword = request.kind === 'password' + + return ( + !isOpen && void handleCancel()}> + + + + {isPassword ? 'SSH Password' : 'SSH Key Passphrase'} + + + {isPassword ? ( + <> + Enter the password for {label} + + ) : ( + <> + Enter the passphrase for {label} + + )} + + +
+ + setValue(e.target.value)} + onKeyDown={(e) => { + if (e.key === 'Enter') { + e.preventDefault() + void handleSubmit() + } + }} + placeholder={isPassword ? 'Enter password' : 'Enter passphrase'} + className="h-8 text-sm" + disabled={submitting} + /> +
+ + + + + + + ) +} diff --git a/src/renderer/src/components/settings/SshTargetCard.tsx b/src/renderer/src/components/settings/SshTargetCard.tsx index 972daaf75..85c83c84e 100644 --- a/src/renderer/src/components/settings/SshTargetCard.tsx +++ b/src/renderer/src/components/settings/SshTargetCard.tsx @@ -1,3 +1,4 @@ +import { useState } from 'react' import { Loader2, MonitorSmartphone, Pencil, Server, Trash2, Wifi, WifiOff } from 'lucide-react' import type { SshTarget, @@ -11,8 +12,6 @@ import { Button } from '../ui/button' export const STATUS_LABELS: Record = { disconnected: 'Disconnected', connecting: 'Connecting\u2026', - 'host-key-verification': 'Verifying host key\u2026', - 'auth-challenge': 'Authenticating\u2026', 'auth-failed': 'Auth failed', 'deploying-relay': 'Deploying relay\u2026', connected: 'Connected', @@ -26,8 +25,6 @@ export function statusColor(status: SshConnectionStatus): string { case 'connected': return 'bg-emerald-500' case 'connecting': - case 'host-key-verification': - case 'auth-challenge': case 'deploying-relay': case 'reconnecting': return 'bg-yellow-500' @@ -41,9 +38,7 @@ export function statusColor(status: SshConnectionStatus): string { } export function isConnecting(status: SshConnectionStatus): boolean { - return ['connecting', 'host-key-verification', 'auth-challenge', 'deploying-relay'].includes( - status - ) + return ['connecting', 'deploying-relay', 'reconnecting'].includes(status) } // ── SshTargetCard ──────────────────────────────────────────────────── @@ -70,6 +65,23 @@ export function SshTargetCard({ onRemove }: SshTargetCardProps): React.JSX.Element { const status: SshConnectionStatus = state?.status ?? 'disconnected' + const [actionInFlight, setActionInFlight] = useState<'connect' | 'disconnect' | null>(null) + + const handleConnect = (): void => { + if (actionInFlight) { + return + } + setActionInFlight('connect') + Promise.resolve(onConnect(target.id)).finally(() => setActionInFlight(null)) + } + + const handleDisconnect = (): void => { + if (actionInFlight) { + return + } + setActionInFlight('disconnect') + Promise.resolve(onDisconnect(target.id)).finally(() => setActionInFlight(null)) + } return (
@@ -95,8 +107,9 @@ export function SshTargetCard({ {showReconnect && ( - + ) : status === 'connected' ? ( + + ) : null} +
+ ) +} + +export function SshStatusSegment({ + compact, + iconOnly +}: { + compact: boolean + iconOnly: boolean +}): React.JSX.Element | null { + const sshConnectionStates = useAppStore((s) => s.sshConnectionStates) + const sshTargetLabels = useAppStore((s) => s.sshTargetLabels) + const setActiveView = useAppStore((s) => s.setActiveView) + const openSettingsTarget = useAppStore((s) => s.openSettingsTarget) + + const targets = Array.from(sshTargetLabels.entries()).map(([id, label]) => { + const state = sshConnectionStates.get(id) + return { id, label, status: (state?.status ?? 'disconnected') as SshConnectionStatus } + }) + + if (targets.length === 0) { + return null + } + + const statuses = targets.map((t) => t.status) + const overall = overallStatus(statuses) + const anyConnecting = overall === 'connecting' + + return ( + + + + + +
+ SSH Connections +
+ {targets.map((t) => ( + + ))} + + { + openSettingsTarget({ pane: 'general', repoId: null, sectionId: 'ssh' }) + setActiveView('settings') + }} + > + Manage SSH… + + + + ) +} diff --git a/src/renderer/src/components/status-bar/StatusBar.tsx b/src/renderer/src/components/status-bar/StatusBar.tsx index 12dd8b3c0..46bf17174 100644 --- a/src/renderer/src/components/status-bar/StatusBar.tsx +++ b/src/renderer/src/components/status-bar/StatusBar.tsx @@ -23,6 +23,7 @@ import type { CodexRateLimitAccountsState } from '../../../../shared/types' import type { ProviderRateLimits, RateLimitWindow } from '../../../../shared/rate-limit-types' import { ProviderIcon, ProviderPanel } from './tooltip' import { markLiveCodexSessionsForRestart } from '@/lib/codex-session-restart' +import { SshStatusSegment } from './SshStatusSegment' function getCodexAccountLabel( state: CodexRateLimitAccountsState, @@ -118,13 +119,6 @@ function ProviderSegment({ // Has data (ok, fetching with stale data, or error with stale data) const isStale = p.status === 'error' - const isFetching = p.status === 'fetching' - const trailingStatusIcon = isFetching ? ( - - ) : isStale ? ( - - ) : null - return ( @@ -132,12 +126,7 @@ function ProviderSegment({ {p.session && } {p.session && p.weekly && ·} {p.weekly && } - - {/* Why: reserve the trailing provider-status slot even when no icon is - visible so Claude/Codex do not shift horizontally as fetching/error - adornments appear and disappear. */} - {trailingStatusIcon} - + {isStale && } ) } @@ -463,7 +452,9 @@ function StatusBarInner(): React.JSX.Element | null { // configured but currently unavailable. const showClaude = claude && statusBarItems.includes('claude') const showCodex = codex && statusBarItems.includes('codex') + const showSsh = statusBarItems.includes('ssh') const anyVisible = showClaude || showCodex + const anyFetching = claude?.status === 'fetching' || codex?.status === 'fetching' const compact = containerWidth < 900 const iconOnly = containerWidth < 500 @@ -475,52 +466,41 @@ function StatusBarInner(): React.JSX.Element | null { ref={containerRefCallback} className="flex items-center h-6 min-h-[24px] px-3 gap-4 border-t border-border bg-[var(--bg-titlebar,var(--card))] text-xs select-none shrink-0" > - {iconOnly ? ( - <> - {showClaude && ( - - )} - {showCodex && } - - ) : ( - <> - {showClaude && ( - - )} - - {showCodex && } - - )} +
+ {showClaude && ( + + )} + {showCodex && } + {anyVisible && ( + + + + + + Refresh usage data + + + )} +
- {anyVisible && ( - - - - - - Refresh usage data - - - )} + {showSsh && }
@@ -537,6 +517,12 @@ function StatusBarInner(): React.JSX.Element | null { > Codex Usage + toggleStatusBarItem('ssh')} + > + SSH Status + ) diff --git a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx index 62446635e..ab397e728 100644 --- a/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalErrorToast.tsx @@ -1,3 +1,9 @@ +const SSH_PREFIX = 'SSH connection is not active' + +function isSshError(error: string): boolean { + return error.startsWith(SSH_PREFIX) +} + export function TerminalErrorToast({ error, onDismiss @@ -5,6 +11,8 @@ export function TerminalErrorToast({ error: string onDismiss: () => void }): React.JSX.Element { + const ssh = isSshError(error) + return (
{error} - {'\n'} - If this persists, please{' '} - - file an issue - - . + {!ssh && ( + <> + {'\n'} + If this persists, please{' '} + + file an issue + + . + + )}