From ffec74c7bab14a31a666ed26272f77fa5b288c65 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 23 May 2026 01:56:15 -0700 Subject: [PATCH] Fix SSH agent and identity file auth ordering (#2681) * fix: address review findings * fix: address CI failures --- .../ssh/ssh-agent-identity-filter.test.ts | 126 ++++++++++ src/main/ssh/ssh-agent-identity-filter.ts | 114 +++++++++ src/main/ssh/ssh-auth-resolution.ts | 175 ++++++++++++++ src/main/ssh/ssh-config-parser.test.ts | 66 ++++- src/main/ssh/ssh-config-parser.ts | 20 ++ src/main/ssh/ssh-connection-utils.test.ts | 168 ++++++++++++- src/main/ssh/ssh-connection-utils.ts | 89 +++---- src/main/ssh/ssh-connection.test.ts | 225 +++++++++++++++++- src/main/ssh/ssh-connection.ts | 85 ++++++- src/main/ssh/ssh-system-fallback.test.ts | 18 ++ src/main/ssh/ssh-system-fallback.ts | 8 + src/shared/ssh-types.ts | 4 + 12 files changed, 1020 insertions(+), 78 deletions(-) create mode 100644 src/main/ssh/ssh-agent-identity-filter.test.ts create mode 100644 src/main/ssh/ssh-agent-identity-filter.ts create mode 100644 src/main/ssh/ssh-auth-resolution.ts diff --git a/src/main/ssh/ssh-agent-identity-filter.test.ts b/src/main/ssh/ssh-agent-identity-filter.test.ts new file mode 100644 index 000000000..536ba3e7c --- /dev/null +++ b/src/main/ssh/ssh-agent-identity-filter.test.ts @@ -0,0 +1,126 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + createAgent: vi.fn(), + parseKey: vi.fn(), + readFileSync: vi.fn() +})) + +vi.mock('fs', () => ({ + readFileSync: (...args: unknown[]) => mocks.readFileSync(...args) +})) + +vi.mock('os', () => ({ + homedir: () => '/home/testuser' +})) + +vi.mock('ssh2', () => { + class MockBaseAgent {} + return { + BaseAgent: MockBaseAgent, + createAgent: (...args: unknown[]) => mocks.createAgent(...args), + utils: { + parseKey: (...args: unknown[]) => mocks.parseKey(...args) + } + } +}) + +import { createIdentityFilteredAgent } from './ssh-agent-identity-filter' + +type TestKey = { + id: string + equals: ReturnType +} + +function makeKey(id: string): TestKey { + return { + id, + equals: vi.fn((candidate: unknown) => (candidate as { id?: string }).id === id) + } +} + +describe('createIdentityFilteredAgent', () => { + beforeEach(() => { + mocks.createAgent.mockReset() + mocks.parseKey.mockReset() + mocks.readFileSync.mockReset() + }) + + it('offers only agent identities matching configured identity files', async () => { + const allowedKey = makeKey('allowed') + const otherKey = makeKey('other') + mocks.readFileSync.mockReturnValue('ssh-ed25519 AAAA allowed') + mocks.parseKey.mockReturnValue(allowedKey) + mocks.createAgent.mockReturnValue({ + getIdentities: vi.fn((callback) => callback(undefined, [allowedKey, otherKey])), + sign: vi.fn() + }) + + const agent = createIdentityFilteredAgent('/tmp/agent.sock', ['~/.ssh/work_key']) + const identities = await new Promise((resolve, reject) => { + agent?.getIdentities((error, keys) => { + if (error) { + reject(error) + return + } + resolve(keys ?? []) + }) + }) + + expect(identities).toEqual([allowedKey]) + expect(mocks.readFileSync).toHaveBeenCalledWith('/home/testuser/.ssh/work_key.pub') + }) + + it('filters nested public key entries returned by ssh2 agents', async () => { + const allowedKey = makeKey('allowed') + const otherKey = makeKey('other') + const allowedEntry = { pubKey: { pubKey: allowedKey, comment: 'allowed' } } + const otherEntry = { pubKey: { pubKey: otherKey, comment: 'other' } } + mocks.readFileSync.mockReturnValue('ssh-ed25519 AAAA allowed') + mocks.parseKey.mockReturnValue(allowedKey) + mocks.createAgent.mockReturnValue({ + getIdentities: vi.fn((callback) => callback(undefined, [allowedEntry, otherEntry])), + sign: vi.fn() + }) + + const agent = createIdentityFilteredAgent('/tmp/agent.sock', ['/home/testuser/.ssh/work_key']) + const identities = await new Promise((resolve, reject) => { + agent?.getIdentities((error, keys) => { + if (error) { + reject(error) + return + } + resolve(keys ?? []) + }) + }) + + expect(identities).toEqual([allowedEntry]) + }) + + it('does not create a broad agent when configured identity keys cannot be parsed', () => { + mocks.readFileSync.mockReturnValue('not-a-key') + mocks.parseKey.mockReturnValue(new Error('parse failed')) + + expect(createIdentityFilteredAgent('/tmp/agent.sock', ['~/.ssh/work_key'])).toBeUndefined() + expect(mocks.createAgent).not.toHaveBeenCalled() + }) + + it('delegates signing to the underlying agent', () => { + const allowedKey = makeKey('allowed') + const sign = vi.fn() + const options = { hash: 'sha256' as const } + const callback = vi.fn() + mocks.readFileSync.mockReturnValue('ssh-ed25519 AAAA allowed') + mocks.parseKey.mockReturnValue(allowedKey) + mocks.createAgent.mockReturnValue({ + getIdentities: vi.fn(), + sign + }) + + const agent = createIdentityFilteredAgent('/tmp/agent.sock', ['~/.ssh/work_key']) + const data = Buffer.from('payload') + agent?.sign(allowedKey as never, data, options, callback) + + expect(sign).toHaveBeenCalledWith(allowedKey, data, options, callback) + }) +}) diff --git a/src/main/ssh/ssh-agent-identity-filter.ts b/src/main/ssh/ssh-agent-identity-filter.ts new file mode 100644 index 000000000..3e821188f --- /dev/null +++ b/src/main/ssh/ssh-agent-identity-filter.ts @@ -0,0 +1,114 @@ +import { readFileSync } from 'fs' +import { homedir } from 'os' +import { join } from 'path' +import { + BaseAgent, + createAgent, + utils, + type IdentityCallback, + type ParsedKey, + type PublicKeyEntry, + type SignCallback, + type SigningRequestOptions +} from 'ssh2' + +type AgentPublicKey = ParsedKey | Buffer | string | PublicKeyEntry + +function resolveHomePath(filepath: string): string { + if (filepath.startsWith('~/') || filepath === '~') { + return join(homedir(), filepath.slice(1)) + } + return filepath +} + +function comparablePublicKey(key: AgentPublicKey): ParsedKey | Buffer | string { + if (typeof key === 'object' && 'pubKey' in key) { + const pubKey = key.pubKey + if (typeof pubKey === 'object' && 'pubKey' in pubKey) { + return pubKey.pubKey + } + return pubKey + } + return key +} + +class IdentityFilteredAgent extends BaseAgent { + readonly kind = 'identity-filtered-agent' + declare getStream?: BaseAgent['getStream'] + + constructor( + readonly socketPath: string, + private readonly agent: BaseAgent, + private readonly allowedKeys: ParsedKey[] + ) { + super() + if (agent.getStream) { + this.getStream = agent.getStream.bind(agent) + } + } + + getIdentities(callback: IdentityCallback): void { + this.agent.getIdentities((error, keys) => { + if (error) { + callback(error) + return + } + callback( + undefined, + keys?.filter((key) => + this.allowedKeys.some((allowedKey) => allowedKey.equals(comparablePublicKey(key))) + ) ?? [] + ) + }) + } + + sign( + pubKey: ParsedKey | Buffer | string, + data: Buffer, + optionsOrCallback?: SigningRequestOptions | SignCallback, + callback?: SignCallback + ): void { + if (typeof optionsOrCallback === 'function') { + this.agent.sign(pubKey, data, optionsOrCallback) + return + } + this.agent.sign(pubKey, data, optionsOrCallback ?? {}, callback) + } +} + +function parseIdentityKeyFile(filePath: string): ParsedKey | undefined { + try { + const parsed = utils.parseKey(readFileSync(filePath)) as ParsedKey | ParsedKey[] | Error + if (parsed instanceof Error) { + return undefined + } + return Array.isArray(parsed) ? parsed[0] : parsed + } catch { + return undefined + } +} + +function readIdentityKeys(paths: string[]): ParsedKey[] { + const keys: ParsedKey[] = [] + for (const path of paths) { + const identityPath = resolveHomePath(path) + const key = parseIdentityKeyFile(`${identityPath}.pub`) ?? parseIdentityKeyFile(identityPath) + if (key) { + keys.push(key) + } + } + return keys +} + +export function createIdentityFilteredAgent( + agentSocket: string, + identityFilePaths: string[] +): BaseAgent | undefined { + const identityKeys = readIdentityKeys(identityFilePaths) + if (identityKeys.length === 0) { + return undefined + } + // Why: IdentitiesOnly must not offer every key loaded in the agent. ssh2 has + // no built-in equivalent, so wrap the agent and expose only IdentityFile keys. + return new IdentityFilteredAgent(agentSocket, createAgent(agentSocket), identityKeys) +} diff --git a/src/main/ssh/ssh-auth-resolution.ts b/src/main/ssh/ssh-auth-resolution.ts new file mode 100644 index 000000000..07f7f4cf5 --- /dev/null +++ b/src/main/ssh/ssh-auth-resolution.ts @@ -0,0 +1,175 @@ +import { existsSync, readFileSync } from 'fs' +import { homedir } from 'os' +import { join } from 'path' +import { utils, type BaseAgent, type ParsedKey } from 'ssh2' +import type { SshTarget } from '../../shared/ssh-types' +import type { SshResolvedConfig } from './ssh-config-parser' +import { createIdentityFilteredAgent } from './ssh-agent-identity-filter' + +// 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'] + +const DEFAULT_KEY_PATHS = DEFAULT_KEY_NAMES.map((name) => `~/.ssh/${name}`) +const WINDOWS_OPENSSH_AGENT_PIPE = '\\\\.\\pipe\\openssh-ssh-agent' + +// Why: parseSshGOutput expands ~ to homedir(), so resolved identityFile +// paths won't match the ~/... form in DEFAULT_KEY_PATHS. +const EXPANDED_DEFAULT_KEY_PATHS = DEFAULT_KEY_NAMES.map((name) => join(homedir(), '.ssh', name)) + +export type PrivateKeyFile = { path: string; contents: Buffer } + +export function findDefaultKeyFile(): PrivateKeyFile | undefined { + for (const keyPath of DEFAULT_KEY_PATHS) { + const resolved = keyPath.replace(/^~/, homedir()) + try { + if (!existsSync(resolved)) { + continue + } + const contents = readFileSync(resolved) + return { path: keyPath, contents } + } catch { + continue + } + } + return undefined +} + +function resolveHomePath(filepath: string): string { + if (filepath.startsWith('~/') || filepath === '~') { + return join(homedir(), filepath.slice(1)) + } + return filepath +} + +function expandIdentityAgentEnv(value: string): string | undefined { + if (value === 'SSH_AUTH_SOCK') { + return process.env.SSH_AUTH_SOCK || undefined + } + + let missingEnv = false + const expanded = value.replace(/\$(\w+)|\$\{([^}]+)\}/g, (_match, bare, braced) => { + const envName = String(bare || braced) + const envValue = process.env[envName] + if (envValue === undefined) { + missingEnv = true + return '' + } + return envValue + }) + + return missingEnv ? undefined : expanded +} + +function resolveDefaultAgentSocket(): string | undefined { + return ( + process.env.SSH_AUTH_SOCK || + (process.platform === 'win32' ? WINDOWS_OPENSSH_AGENT_PIPE : undefined) + ) +} + +export function resolveAgentSocket( + target: Pick, + resolved: Pick | null +): string | undefined { + // Why: imported config-host targets may contain raw OpenSSH tokens like %d. + // ssh -G resolves those tokens, so its value must win when available. + const configuredIdentityAgent = target.configHost + ? (resolved?.identityAgent ?? target.identityAgent) + : (target.identityAgent ?? resolved?.identityAgent) + if (configuredIdentityAgent != null) { + const trimmed = configuredIdentityAgent.trim() + if (!trimmed || trimmed.toLowerCase() === 'none') { + return undefined + } + return expandIdentityAgentEnv(resolveHomePath(trimmed)) + } + return resolveDefaultAgentSocket() +} + +function resolveExplicitPrivateKeyPath( + target: SshTarget, + resolved: SshResolvedConfig | null +): string | undefined { + const resolvedIdentity = resolved?.identityFile?.[0] + return ( + target.identityFile || + (resolvedIdentity && !EXPANDED_DEFAULT_KEY_PATHS.includes(resolvedIdentity) + ? resolvedIdentity + : undefined) + ) +} + +function readPrivateKey(keyPath: string): PrivateKeyFile | undefined { + try { + const resolvedPath = resolveHomePath(keyPath) + return { path: keyPath, contents: readFileSync(resolvedPath) } + } catch { + return undefined + } +} + +function resolveExplicitPrivateKey( + target: SshTarget, + resolved: SshResolvedConfig | null +): PrivateKeyFile | undefined { + const explicitKey = resolveExplicitPrivateKeyPath(target, resolved) + if (explicitKey) { + return readPrivateKey(explicitKey) + } + return undefined +} + +export function resolvePrivateKey( + target: SshTarget, + resolved: SshResolvedConfig | null +): PrivateKeyFile | undefined { + if (resolveExplicitPrivateKeyPath(target, resolved)) { + return resolveExplicitPrivateKey(target, resolved) + } + return findDefaultKeyFile() +} + +function isUnencryptedPrivateKey(contents: Buffer): boolean { + const parsed = utils.parseKey(contents) as ParsedKey | ParsedKey[] | Error + if (parsed instanceof Error) { + return false + } + const keys = Array.isArray(parsed) ? parsed : [parsed] + return keys.some((key) => key && typeof key.isPrivateKey === 'function' && key.isPrivateKey()) +} + +export function resolveUnencryptedExplicitPrivateKey( + target: SshTarget, + resolved: SshResolvedConfig | null +): PrivateKeyFile | undefined { + const key = resolveExplicitPrivateKey(target, resolved) + if (!key) { + return undefined + } + return isUnencryptedPrivateKey(key.contents) ? key : undefined +} + +function resolveIdentityFilePaths(target: SshTarget, resolved: SshResolvedConfig | null): string[] { + if (target.configHost && resolved?.identityFile?.length) { + return resolved.identityFile + } + if (target.identityFile) { + return [target.identityFile] + } + return resolved?.identityFile ?? [] +} + +export function resolveAgentConfigValue( + agentSocket: string, + target: SshTarget, + resolved: SshResolvedConfig | null +): BaseAgent | string | undefined { + const identitiesOnly = resolved?.identitiesOnly ?? target.identitiesOnly ?? false + if (!identitiesOnly) { + return agentSocket + } + + return createIdentityFilteredAgent(agentSocket, resolveIdentityFilePaths(target, resolved)) +} diff --git a/src/main/ssh/ssh-config-parser.test.ts b/src/main/ssh/ssh-config-parser.test.ts index 72fe263c2..2b5cf6af3 100644 --- a/src/main/ssh/ssh-config-parser.test.ts +++ b/src/main/ssh/ssh-config-parser.test.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-lines -- Why: SSH config parsing fixtures cover OpenSSH file parsing and ssh -G output together so import and connection resolution stay aligned. */ import { describe, expect, it, vi } from 'vitest' import { parseSshConfig, sshConfigHostsToTargets, parseSshGOutput } from './ssh-config-parser' @@ -77,6 +78,26 @@ Host myserver expect(hosts[0].identityFile).toBe('/home/testuser/.ssh/id_ed25519') }) + it('parses IdentityAgent with ~ expansion', () => { + const config = ` +Host myserver + HostName example.com + IdentityAgent ~/.1password/agent.sock +` + const hosts = parseSshConfig(config) + expect(hosts[0].identityAgent).toBe('/home/testuser/.1password/agent.sock') + }) + + it('parses IdentitiesOnly', () => { + const config = ` +Host myserver + HostName example.com + IdentitiesOnly yes +` + const hosts = parseSshConfig(config) + expect(hosts[0].identitiesOnly).toBe(true) + }) + it('parses ProxyCommand, ProxyUseFdpass, and ProxyJump', () => { const config = ` Host internal @@ -151,6 +172,27 @@ Host staging stage *.example.com ]) }) + it('applies identity agent settings to every concrete alias on a multi-pattern Host line', () => { + const config = ` +Host staging stage + IdentityAgent ~/.1password/agent.sock + IdentitiesOnly yes +` + const hosts = parseSshConfig(config) + expect(hosts).toEqual([ + { + host: 'staging', + identityAgent: '/home/testuser/.1password/agent.sock', + identitiesOnly: true + }, + { + host: 'stage', + identityAgent: '/home/testuser/.1password/agent.sock', + identitiesOnly: true + } + ]) + }) + it('defaults port to 22 for invalid port values', () => { const config = ` Host myserver @@ -197,12 +239,14 @@ describe('sshConfigHostsToTargets', () => { expect(targets[0].username).toBe('') }) - it('carries through identityFile, proxyCommand, and jumpHost', () => { + it('carries through identityFile, identityAgent, identitiesOnly, proxyCommand, and jumpHost', () => { const hosts = [ { host: 'internal', hostname: '10.0.0.5', identityFile: '/home/user/.ssh/id_rsa', + identityAgent: '/home/user/.1password/agent.sock', + identitiesOnly: true, proxyCommand: 'ssh -W %h:%p bastion', proxyUseFdpass: true, proxyJump: 'bastion.example.com' @@ -210,6 +254,8 @@ describe('sshConfigHostsToTargets', () => { ] const targets = sshConfigHostsToTargets(hosts, new Set()) expect(targets[0].identityFile).toBe('/home/user/.ssh/id_rsa') + expect(targets[0].identityAgent).toBe('/home/user/.1password/agent.sock') + expect(targets[0].identitiesOnly).toBe(true) expect(targets[0].proxyCommand).toBe('ssh -W %h:%p bastion') expect(targets[0].jumpHost).toBe('bastion.example.com') }) @@ -338,4 +384,22 @@ describe('parseSshGOutput', () => { const result = parseSshGOutput(output) expect(result.identityFile).toEqual(['/home/testuser/custom_key']) }) + + it('parses identityagent with ~ expansion', () => { + const output = 'hostname example.com\nidentityagent ~/.1password/agent.sock\nport 22' + const result = parseSshGOutput(output) + expect(result.identityAgent).toBe('/home/testuser/.1password/agent.sock') + }) + + it('preserves identityagent none so auth can disable agent fallback', () => { + const output = 'hostname example.com\nidentityagent none\nport 22' + const result = parseSshGOutput(output) + expect(result.identityAgent).toBe('none') + }) + + it('parses identitiesonly yes', () => { + const output = 'hostname example.com\nidentitiesonly yes\nport 22' + const result = parseSshGOutput(output) + expect(result.identitiesOnly).toBe(true) + }) }) diff --git a/src/main/ssh/ssh-config-parser.ts b/src/main/ssh/ssh-config-parser.ts index 048c6a9c5..70b0cacdb 100644 --- a/src/main/ssh/ssh-config-parser.ts +++ b/src/main/ssh/ssh-config-parser.ts @@ -11,6 +11,8 @@ export type SshConfigHost = { port?: number user?: string identityFile?: string + identityAgent?: string + identitiesOnly?: boolean proxyCommand?: string proxyUseFdpass?: boolean proxyJump?: string @@ -91,6 +93,16 @@ export function parseSshConfig(content: string): SshConfigHost[] { host.identityFile = resolveHomePath(value) } break + case 'identityagent': + for (const host of current) { + host.identityAgent = resolveHomePath(value) + } + break + case 'identitiesonly': + for (const host of current) { + host.identitiesOnly = value.toLowerCase() === 'yes' + } + break case 'proxycommand': for (const host of current) { host.proxyCommand = value @@ -209,6 +221,8 @@ export function sshConfigHostsToTargets( port: entry.port ?? 22, username: entry.user ?? '', identityFile: entry.identityFile, + identityAgent: entry.identityAgent, + identitiesOnly: entry.identitiesOnly, proxyCommand: entry.proxyCommand, jumpHost: entry.proxyJump }) @@ -223,6 +237,8 @@ export type SshResolvedConfig = { user?: string port: number identityFile: string[] + identityAgent?: string + identitiesOnly: boolean forwardAgent: boolean proxyCommand?: string proxyUseFdpass: boolean @@ -273,12 +289,16 @@ export function parseSshGOutput(stdout: string): SshResolvedConfig { const proxyCommand = rawProxy && rawProxy !== 'none' ? rawProxy : undefined const rawJump = map.get('proxyjump') const proxyJump = rawJump && rawJump !== 'none' ? rawJump : undefined + const rawIdentityAgent = map.get('identityagent') + const identityAgent = rawIdentityAgent ? resolveHomePath(rawIdentityAgent) : undefined return { hostname: map.get('hostname') ?? '', user: map.get('user') || undefined, port: parseInt(map.get('port') ?? '22', 10), identityFile: identityFiles, + identityAgent, + identitiesOnly: map.get('identitiesonly') === 'yes', forwardAgent: map.get('forwardagent') === 'yes', proxyCommand, proxyUseFdpass: map.get('proxyusefdpass') === 'yes', diff --git a/src/main/ssh/ssh-connection-utils.test.ts b/src/main/ssh/ssh-connection-utils.test.ts index a335df31c..f075c9532 100644 --- a/src/main/ssh/ssh-connection-utils.test.ts +++ b/src/main/ssh/ssh-connection-utils.test.ts @@ -1,4 +1,6 @@ +/* eslint-disable max-lines -- Why: SSH connection utility tests share mocked filesystem and environment setup across auth, proxy, and retry helpers. */ import { afterEach, describe, expect, it, vi, beforeEach } from 'vitest' +import { BaseAgent, utils, type ParsedKey } from 'ssh2' vi.mock('os', () => ({ homedir: () => '/home/testuser' @@ -15,10 +17,12 @@ vi.mock('fs', () => ({ import { isTransientError, isAuthError, + isAgentFallbackError, sleep, shellEscape, findDefaultKeyFile, buildConnectConfig, + resolveAgentSocket, resolveEffectiveProxy, CONNECT_TIMEOUT_MS, INITIAL_RETRY_ATTEMPTS, @@ -125,11 +129,37 @@ describe('isAuthError', () => { expect(isAuthError(err)).toBe(true) }) + it('returns true for server auth-attempt exhaustion', () => { + expect(isAuthError(new Error('Received disconnect: Too many authentication failures'))).toBe( + true + ) + }) + it('returns false for transient errors', () => { expect(isAuthError(new Error('connect ETIMEDOUT'))).toBe(false) }) }) +// ── isAgentFallbackError ──────────────────────────────────────────── + +describe('isAgentFallbackError', () => { + it('returns true for ssh2 agent-level failures', () => { + const err = new Error('Failed to connect to agent') as Error & { level: string } + err.level = 'agent' + expect(isAgentFallbackError(err)).toBe(true) + }) + + it('returns true when agent auth exhausts the server auth attempt limit', () => { + expect( + isAgentFallbackError(new Error('Received disconnect: Too many authentication failures')) + ).toBe(true) + }) + + it('keeps unrelated transport errors out of agent fallback handling', () => { + expect(isAgentFallbackError(new Error('connect ECONNRESET'))).toBe(false) + }) +}) + // ── sleep ──────────────────────────────────────────────────────────── describe('sleep', () => { @@ -164,6 +194,7 @@ describe('shellEscape', () => { describe('findDefaultKeyFile', () => { beforeEach(() => { + mockExistsSync.mockReset() mockExistsSync.mockReturnValue(false) mockReadFileSync.mockReset() }) @@ -238,6 +269,7 @@ function makeResolved(overrides?: Partial): SshResolvedConfig port: 22, identityFile: [], forwardAgent: false, + identitiesOnly: false, proxyUseFdpass: false, ...overrides } @@ -247,12 +279,14 @@ describe('buildConnectConfig', () => { const originalEnv = process.env.SSH_AUTH_SOCK beforeEach(() => { + mockExistsSync.mockReset() mockExistsSync.mockReturnValue(false) mockReadFileSync.mockReset() process.env.SSH_AUTH_SOCK = '/tmp/agent.sock' }) afterEach(() => { + vi.restoreAllMocks() if (originalEnv !== undefined) { process.env.SSH_AUTH_SOCK = originalEnv } else { @@ -292,40 +326,160 @@ describe('buildConnectConfig', () => { expect(config.agent).toBe('/tmp/agent.sock') }) - it('uses keyFile auth when target.identityFile is set', () => { + it('uses configured IdentityAgent before SSH_AUTH_SOCK', () => { + const config = buildConnectConfig( + makeTarget(), + makeResolved({ identityAgent: '/tmp/one-password.sock' }) + ) + expect(config.agent).toBe('/tmp/one-password.sock') + }) + + it('prefers ssh -G resolved IdentityAgent for config-host targets', () => { + const config = buildConnectConfig( + makeTarget({ configHost: 'work', identityAgent: '%d/.1password/agent.sock' }), + makeResolved({ identityAgent: '/home/testuser/.1password/agent.sock' }) + ) + expect(config.agent).toBe('/home/testuser/.1password/agent.sock') + }) + + it('allows IdentityAgent none to disable agent auth', () => { + const config = buildConnectConfig(makeTarget(), makeResolved({ identityAgent: 'none' })) + expect(config.agent).toBeUndefined() + }) + + it('resolves IdentityAgent SSH_AUTH_SOCK from the environment', () => { + expect(resolveAgentSocket(makeTarget(), makeResolved({ identityAgent: 'SSH_AUTH_SOCK' }))).toBe( + '/tmp/agent.sock' + ) + expect( + resolveAgentSocket(makeTarget(), makeResolved({ identityAgent: '$SSH_AUTH_SOCK' })) + ).toBe('/tmp/agent.sock') + }) + + it('uses the Windows OpenSSH agent pipe when no environment socket is available on Windows', () => { + const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + delete process.env.SSH_AUTH_SOCK + + try { + expect(resolveAgentSocket(makeTarget(), null)).toBe('\\\\.\\pipe\\openssh-ssh-agent') + } finally { + platformSpy.mockRestore() + } + }) + + it('wraps agent auth with IdentityFile filtering when IdentitiesOnly is enabled', () => { + mockReadFileSync.mockImplementation((path: unknown) => { + if (String(path) === '/home/user/.ssh/work_key.pub') { + return 'ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAILI4wa2zRZoB26D015dsafYmu3jDCI7rh26bFXZrUiAp test-key' + } + throw new Error('unexpected read') + }) + const config = buildConnectConfig( + makeTarget(), + makeResolved({ identityFile: ['/home/user/.ssh/work_key'], identitiesOnly: true }) + ) + + expect(config.agent).toMatchObject({ kind: 'identity-filtered-agent' }) + expect(config.agent).toBeInstanceOf(BaseAgent) + expect(config.privateKey).toBeUndefined() + expect(mockReadFileSync).toHaveBeenCalledWith('/home/user/.ssh/work_key.pub') + }) + + it('does not offer broad agent auth when IdentitiesOnly keys cannot be parsed', () => { + mockReadFileSync.mockReturnValue(Buffer.from('not-a-key')) + const config = buildConnectConfig( + makeTarget(), + makeResolved({ identityFile: ['/home/user/.ssh/work_key'], identitiesOnly: true }) + ) + + expect(config.agent).toBeUndefined() + expect(config.privateKey).toEqual(Buffer.from('not-a-key')) + }) + + it('includes unencrypted target.identityFile auth when an agent is available', () => { + vi.spyOn(utils, 'parseKey').mockReturnValue({ + isPrivateKey: () => true + } as ParsedKey) + mockReadFileSync.mockReturnValue(Buffer.from('key')) + const config = buildConnectConfig(makeTarget({ identityFile: '/home/user/.ssh/custom' }), null) + expect(config.agent).toBe('/tmp/agent.sock') + expect(config.privateKey).toEqual(Buffer.from('key')) + expect(mockReadFileSync).toHaveBeenCalledWith('/home/user/.ssh/custom') + }) + + it('defers encrypted target.identityFile auth when an agent is available', () => { + vi.spyOn(utils, 'parseKey').mockReturnValue( + new Error('Encrypted private OpenSSH key detected, but no passphrase given') + ) + mockReadFileSync.mockReturnValue(Buffer.from('encrypted-key')) + const config = buildConnectConfig(makeTarget({ identityFile: '/home/user/.ssh/custom' }), null) + expect(config.agent).toBe('/tmp/agent.sock') + expect(config.privateKey).toBeUndefined() + expect(mockReadFileSync).toHaveBeenCalledWith('/home/user/.ssh/custom') + }) + + it('uses keyFile auth when target.identityFile is set and no agent is available', () => { + delete process.env.SSH_AUTH_SOCK 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') + expect(config.agent).toBeUndefined() }) - it('uses keyFile auth when resolved identityFile is non-default', () => { + it('includes unencrypted resolved identityFile auth when an agent is available', () => { + vi.spyOn(utils, 'parseKey').mockReturnValue({ + isPrivateKey: () => true + } as ParsedKey) 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') + expect(config.privateKey).toEqual(Buffer.from('custom-key')) }) - it('uses agent auth when resolved identityFile is a default path (expanded)', () => { + it('uses agent auth without probing 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') + expect(config.privateKey).toBeUndefined() + expect(mockReadFileSync).not.toHaveBeenCalled() }) - it('provides fallback key in agent auth mode', () => { + it('does not probe default key files before agent auth', () => { + mockExistsSync.mockImplementation( + (p: unknown) => String(p) === '/home/testuser/.ssh/id_ed25519' + ) + const config = buildConnectConfig(makeTarget(), null) + expect(config.agent).toBe('/tmp/agent.sock') + expect(config.privateKey).toBeUndefined() + expect(mockExistsSync).not.toHaveBeenCalled() + }) + + it('provides fallback key when no agent is available', () => { + delete process.env.SSH_AUTH_SOCK 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.agent).toBeUndefined() expect(config.privateKey).toEqual(Buffer.from('fallback')) }) + + it('can force private key inclusion for the post-agent fallback path', () => { + mockReadFileSync.mockReturnValue(Buffer.from('key')) + const config = buildConnectConfig( + makeTarget({ identityFile: '/home/user/.ssh/custom' }), + null, + { includeAgent: false, includePrivateKey: true } + ) + expect(config.agent).toBeUndefined() + expect(config.privateKey).toEqual(Buffer.from('key')) + }) }) // ── resolveEffectiveProxy ─────────────────────────────────────────── diff --git a/src/main/ssh/ssh-connection-utils.ts b/src/main/ssh/ssh-connection-utils.ts index 5b2a40334..013b456cb 100644 --- a/src/main/ssh/ssh-connection-utils.ts +++ b/src/main/ssh/ssh-connection-utils.ts @@ -1,12 +1,17 @@ -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' +import { + resolveAgentConfigValue, + resolveAgentSocket, + resolvePrivateKey, + resolveUnencryptedExplicitPrivateKey +} from './ssh-auth-resolution' + +export { findDefaultKeyFile, resolveAgentSocket } from './ssh-auth-resolution' export type SshCredentialKind = 'passphrase' | 'password' @@ -43,10 +48,15 @@ export function isAuthError(err: Error): boolean { return ( msg.includes('all configured authentication methods failed') || msg.includes('authentication failed') || + msg.includes('too many authentication failures') || (err as { level?: string }).level === 'client-authentication' ) } +export function isAgentFallbackError(err: Error): boolean { + return isAuthError(err) || (err as { level?: string }).level === 'agent' +} + export function isTransientError(err: Error): boolean { const code = (err as NodeJS.ErrnoException).code if (code && TRANSIENT_ERROR_CODES.has(code)) { @@ -82,39 +92,18 @@ function cmdEscape(s: string): string { return `"${s.replace(/"/g, '""')}"` } -// 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'] - -const DEFAULT_KEY_PATHS = DEFAULT_KEY_NAMES.map((name) => `~/.ssh/${name}`) - -// 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)) - -export function findDefaultKeyFile(): { path: string; contents: Buffer } | undefined { - for (const keyPath of DEFAULT_KEY_PATHS) { - const resolved = keyPath.replace(/^~/, homedir()) - try { - if (!existsSync(resolved)) { - continue - } - const contents = readFileSync(resolved) - return { path: keyPath, contents } - } catch { - continue - } - } - return undefined +type BuildConnectConfigOptions = { + includeAgent?: boolean + includePrivateKey?: boolean } -// Why: matches VS Code's _connectSSH auth method selection (lines 606-611, 727-758). -// ssh2 handles the auth negotiation natively — no custom authHandler needed. +// Why: ssh2 tries privateKey before agent, but parses encrypted privateKey +// values before any agent auth can run. Keep unencrypted explicit keys first +// while deferring encrypted keys until the post-agent passphrase path. export function buildConnectConfig( target: SshTarget, - resolved: SshResolvedConfig | null + resolved: SshResolvedConfig | null, + options: BuildConnectConfigOptions = {} ): ConnectConfig { const effectiveHost = target.host || resolved?.hostname || target.label const effectivePort = target.port || resolved?.port || 22 @@ -128,32 +117,20 @@ export function buildConnectConfig( 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 + const shouldIncludeAgent = options.includeAgent ?? true + const agentSocket = shouldIncludeAgent ? resolveAgentSocket(target, resolved) : undefined + const agent = agentSocket ? resolveAgentConfigValue(agentSocket, target, resolved) : undefined + + if (agent) { + config.agent = agent } - 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 - } + const key = + (options.includePrivateKey ?? !agent) + ? resolvePrivateKey(target, resolved) + : resolveUnencryptedExplicitPrivateKey(target, resolved) + if (key) { + config.privateKey = key.contents } return config as ConnectConfig diff --git a/src/main/ssh/ssh-connection.test.ts b/src/main/ssh/ssh-connection.test.ts index 4a342e417..55e100953 100644 --- a/src/main/ssh/ssh-connection.test.ts +++ b/src/main/ssh/ssh-connection.test.ts @@ -1,19 +1,26 @@ +/* eslint-disable max-lines -- Why: SSH connection lifecycle tests share one ssh2 mock so auth, reconnect, and system-transport behavior stay consistent. */ import { describe, expect, it, vi, beforeEach } from 'vitest' import { Socket } from 'net' import { EventEmitter } from 'events' +import { mkdtempSync, rmSync, writeFileSync } from 'fs' +import { tmpdir } from 'os' +import { join } from 'path' let eventHandlers: Map void> let connectBehavior: 'ready' | 'error' = 'ready' let connectErrorMessage = '' +let connectSequence: ('ready' | Error)[] = [] type MockSshClient = { setNoDelay: ReturnType _sock: Socket | undefined lastExecCommand?: string + lastConnectConfig?: unknown } let clientInstances: MockSshClient[] = [] vi.mock('ssh2', () => { + class MockBaseAgent {} class MockSshClient { setNoDelay = vi.fn() // Why: production code reads `client._sock` and checks `instanceof net.Socket` @@ -21,14 +28,25 @@ vi.mock('ssh2', () => { // exercise the "enabled" branch instead of the "skipped (proxy socket)" branch. _sock: Socket | undefined = new Socket() lastExecCommand?: string + lastConnectConfig?: unknown constructor() { clientInstances.push(this) } on(event: string, handler: (...args: unknown[]) => void) { eventHandlers?.set(event, handler) } - connect() { + connect(config?: unknown) { + this.lastConnectConfig = config setTimeout(() => { + const next = connectSequence.shift() + if (next instanceof Error) { + eventHandlers?.get('error')?.(next) + return + } + if (next === 'ready') { + eventHandlers?.get('ready')?.() + return + } if (connectBehavior === 'error') { eventHandlers?.get('error')?.(new Error(connectErrorMessage)) } else { @@ -44,7 +62,14 @@ vi.mock('ssh2', () => { } sftp() {} } - return { Client: MockSshClient } + return { + BaseAgent: MockBaseAgent, + Client: MockSshClient, + createAgent: vi.fn(), + utils: { + parseKey: vi.fn() + } + } }) const { spawnSystemSshCommandMock } = vi.hoisted(() => ({ @@ -121,6 +146,7 @@ describe('SshConnection', () => { eventHandlers = new Map() connectBehavior = 'ready' connectErrorMessage = '' + connectSequence = [] clientInstances = [] spawnSystemSshCommandMock.mockReset() spawnSystemSshCommandMock.mockImplementation(() => createSystemCommandChannel()) @@ -249,6 +275,199 @@ describe('SshConnection', () => { expect(resolveWithSshG).toHaveBeenCalledWith('ssh-alias') }) + it('tries ssh-agent before reading an explicit private key', async () => { + vi.stubEnv('SSH_AUTH_SOCK', '/tmp/agent.sock') + const callbacks = createCallbacks({ + onCredentialRequest: vi.fn() + }) + const conn = new SshConnection( + createTarget({ + identityFile: '/tmp/encrypted-key' + }), + callbacks + ) + + await conn.connect() + + const initialConfig = clientInstances[0].lastConnectConfig as { + agent?: unknown + privateKey?: unknown + } + expect(initialConfig.agent).toBe('/tmp/agent.sock') + expect(initialConfig.privateKey).toBeUndefined() + expect(callbacks.onCredentialRequest).not.toHaveBeenCalled() + }) + + it('falls back to direct private key auth when agent auth fails', async () => { + vi.stubEnv('SSH_AUTH_SOCK', '/tmp/agent.sock') + const tempDir = mkdtempSync(join(tmpdir(), 'orca-ssh-key-')) + const keyPath = join(tempDir, 'id_ed25519') + writeFileSync(keyPath, 'test-key') + connectSequence = [new Error('All configured authentication methods failed'), 'ready'] + + try { + const conn = new SshConnection(createTarget({ identityFile: keyPath }), createCallbacks()) + + await conn.connect() + + expect(clientInstances).toHaveLength(2) + const initialConfig = clientInstances[0].lastConnectConfig as { + agent?: unknown + privateKey?: unknown + } + const fallbackConfig = clientInstances[1].lastConnectConfig as { + agent?: unknown + privateKey?: Buffer + } + expect(initialConfig.agent).toBe('/tmp/agent.sock') + expect(initialConfig.privateKey).toBeUndefined() + expect(fallbackConfig.agent).toBeUndefined() + expect(fallbackConfig.privateKey).toEqual(Buffer.from('test-key')) + } finally { + rmSync(tempDir, { recursive: true, force: true }) + } + }) + + it('falls back to direct private key auth when the agent socket is unavailable', async () => { + vi.stubEnv('SSH_AUTH_SOCK', '/tmp/stale-agent.sock') + const tempDir = mkdtempSync(join(tmpdir(), 'orca-ssh-key-')) + const keyPath = join(tempDir, 'id_ed25519') + writeFileSync(keyPath, 'test-key') + const agentError = new Error('Failed to connect to agent') as Error & { level: string } + agentError.level = 'agent' + connectSequence = [agentError, 'ready'] + + try { + const conn = new SshConnection(createTarget({ identityFile: keyPath }), createCallbacks()) + + await conn.connect() + + expect(clientInstances).toHaveLength(2) + const fallbackConfig = clientInstances[1].lastConnectConfig as { + agent?: unknown + privateKey?: Buffer + } + expect(fallbackConfig.agent).toBeUndefined() + expect(fallbackConfig.privateKey).toEqual(Buffer.from('test-key')) + } finally { + rmSync(tempDir, { recursive: true, force: true }) + } + }) + + it('falls back to direct private key auth after too many agent authentication failures', async () => { + vi.stubEnv('SSH_AUTH_SOCK', '/tmp/agent.sock') + const tempDir = mkdtempSync(join(tmpdir(), 'orca-ssh-key-')) + const keyPath = join(tempDir, 'id_ed25519') + writeFileSync(keyPath, 'test-key') + connectSequence = [new Error('Received disconnect: Too many authentication failures'), 'ready'] + + try { + const conn = new SshConnection(createTarget({ identityFile: keyPath }), createCallbacks()) + + await conn.connect() + + expect(clientInstances).toHaveLength(2) + const fallbackConfig = clientInstances[1].lastConnectConfig as { + agent?: unknown + privateKey?: Buffer + } + expect(fallbackConfig.agent).toBeUndefined() + expect(fallbackConfig.privateKey).toEqual(Buffer.from('test-key')) + } finally { + rmSync(tempDir, { recursive: true, force: true }) + } + }) + + it('retries password auth without a stale agent when no private key fallback exists', async () => { + vi.stubEnv('SSH_AUTH_SOCK', '/tmp/stale-agent.sock') + const agentError = new Error('Failed to connect to agent') as Error & { level: string } + agentError.level = 'agent' + connectSequence = [agentError, 'ready'] + const onCredentialRequest = vi.fn(async () => 'password-123') + const conn = new SshConnection( + createTarget({ identityFile: join(tmpdir(), 'missing-key') }), + createCallbacks({ onCredentialRequest }) + ) + + await conn.connect() + + expect(clientInstances).toHaveLength(2) + const retryConfig = clientInstances[1].lastConnectConfig as { + agent?: unknown + password?: string + privateKey?: unknown + } + expect(retryConfig.agent).toBeUndefined() + expect(retryConfig.password).toBe('password-123') + expect(retryConfig.privateKey).toBeUndefined() + expect(onCredentialRequest).toHaveBeenCalledWith('target-1', 'password', 'example.com') + }) + + it('retries password auth with the no-agent key config after direct key fallback fails', async () => { + vi.stubEnv('SSH_AUTH_SOCK', '/tmp/agent.sock') + const tempDir = mkdtempSync(join(tmpdir(), 'orca-ssh-key-')) + const keyPath = join(tempDir, 'id_ed25519') + writeFileSync(keyPath, 'test-key') + connectSequence = [ + new Error('All configured authentication methods failed'), + new Error('All configured authentication methods failed'), + 'ready' + ] + const onCredentialRequest = vi.fn(async () => 'password-123') + + try { + const conn = new SshConnection( + createTarget({ identityFile: keyPath }), + createCallbacks({ onCredentialRequest }) + ) + + await conn.connect() + + expect(clientInstances).toHaveLength(3) + const keyRetryConfig = clientInstances[1].lastConnectConfig as { + agent?: unknown + privateKey?: Buffer + } + const passwordRetryConfig = clientInstances[2].lastConnectConfig as { + agent?: unknown + password?: string + privateKey?: Buffer + } + expect(keyRetryConfig.agent).toBeUndefined() + expect(keyRetryConfig.privateKey).toEqual(Buffer.from('test-key')) + expect(passwordRetryConfig.agent).toBeUndefined() + expect(passwordRetryConfig.privateKey).toEqual(Buffer.from('test-key')) + expect(passwordRetryConfig.password).toBe('password-123') + } finally { + rmSync(tempDir, { recursive: true, force: true }) + } + }) + + it('does not prompt twice when post-agent private key passphrase is cancelled', async () => { + vi.stubEnv('SSH_AUTH_SOCK', '/tmp/agent.sock') + const tempDir = mkdtempSync(join(tmpdir(), 'orca-ssh-key-')) + const keyPath = join(tempDir, 'id_ed25519') + writeFileSync(keyPath, 'test-key') + connectSequence = [ + new Error('All configured authentication methods failed'), + new Error('Encrypted private OpenSSH key detected, but no passphrase given') + ] + const onCredentialRequest = vi.fn(async () => null) + + try { + const conn = new SshConnection( + createTarget({ identityFile: keyPath }), + createCallbacks({ onCredentialRequest }) + ) + + await expect(conn.connect()).rejects.toThrow('Encrypted private OpenSSH key detected') + expect(onCredentialRequest).toHaveBeenCalledTimes(1) + expect(onCredentialRequest).toHaveBeenCalledWith('target-1', 'passphrase', keyPath) + } finally { + rmSync(tempDir, { recursive: true, force: true }) + } + }) + it('wraps exec commands in /bin/sh so non-POSIX login shells do not parse relay snippets', async () => { const conn = new SshConnection(createTarget(), createCallbacks()) await conn.connect() @@ -266,6 +485,7 @@ describe('SshConnection', () => { port: 22, identityFile: [], forwardAgent: false, + identitiesOnly: false, proxyUseFdpass: true }) const conn = new SshConnection(createTarget({ configHost: 'fdpass-host' }), createCallbacks()) @@ -299,6 +519,7 @@ describe('SshConnectionManager', () => { eventHandlers = new Map() connectBehavior = 'ready' connectErrorMessage = '' + connectSequence = [] clientInstances = [] }) diff --git a/src/main/ssh/ssh-connection.ts b/src/main/ssh/ssh-connection.ts index 39fa7b518..422a1f2b7 100644 --- a/src/main/ssh/ssh-connection.ts +++ b/src/main/ssh/ssh-connection.ts @@ -19,6 +19,7 @@ import { CONNECT_TIMEOUT_MS, isTransientError, isAuthError, + isAgentFallbackError, isPassphraseError, sleep, buildConnectConfig, @@ -238,27 +239,87 @@ export class SshConnection { try { await this.doSsh2Connect(config, connectGeneration) } catch (err) { - if (!(err instanceof Error) || !this.callbacks.onCredentialRequest) { + if (!(err instanceof Error)) { this.proxyProcess?.kill() this.proxyProcess = null throw err } + + let authError = err + let passphrasePromptHandled = false + let credentialRetryConfig = config + + // Why: ssh2 parses encrypted privateKey values before it tries agent + // auth. When an agent is available, give it the first attempt and only + // fall back to direct key parsing after agent auth fails. + if (isAgentFallbackError(authError) && config.agent && !config.privateKey) { + const keyConfig = buildConnectConfig(this.target, resolved, { + includeAgent: false, + includePrivateKey: true + }) + // Why: if the agent path failed, password/passphrase retries should not + // go back through the same agent-only config. + credentialRetryConfig = keyConfig + if (this.cachedPassphrase) { + keyConfig.passphrase = this.cachedPassphrase + } + if (this.cachedPassword) { + keyConfig.password = this.cachedPassword + } + if (keyConfig.privateKey || keyConfig.password) { + this.respawnProxy(keyConfig, effectiveProxy) + try { + await this.doSsh2Connect(keyConfig, connectGeneration) + return + } catch (keyErr) { + if (!(keyErr instanceof Error)) { + this.proxyProcess?.kill() + this.proxyProcess = null + throw keyErr + } + authError = keyErr + if (isPassphraseError(authError) && !this.cachedPassphrase) { + passphrasePromptHandled = true + 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 + keyConfig.passphrase = val + this.respawnProxy(keyConfig, effectiveProxy) + await this.doSsh2Connect(keyConfig, connectGeneration) + return + } + } + } + } + } + + if (!this.callbacks.onCredentialRequest) { + this.proxyProcess?.kill() + this.proxyProcess = null + throw authError + } + // 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) { + if (isPassphraseError(authError) && !this.cachedPassphrase && !passphrasePromptHandled) { 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) + credentialRetryConfig.passphrase = val + this.respawnProxy(credentialRetryConfig, effectiveProxy) + await this.doSsh2Connect(credentialRetryConfig, 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) { + // Why: an agent socket failure can still be recovered by password auth, + // but the retry must use the no-agent config selected above. + if (isAgentFallbackError(authError) && !this.cachedPassword) { const val = await this.callbacks.onCredentialRequest( this.target.id, 'password', @@ -266,15 +327,15 @@ export class SshConnection { ) if (val) { this.cachedPassword = val - config.password = val - this.respawnProxy(config, effectiveProxy) - await this.doSsh2Connect(config, connectGeneration) + credentialRetryConfig.password = val + this.respawnProxy(credentialRetryConfig, effectiveProxy) + await this.doSsh2Connect(credentialRetryConfig, connectGeneration) return } } this.proxyProcess?.kill() this.proxyProcess = null - throw err + throw authError } } diff --git a/src/main/ssh/ssh-system-fallback.test.ts b/src/main/ssh/ssh-system-fallback.test.ts index a7e7b7f5b..9d88fdb3d 100644 --- a/src/main/ssh/ssh-system-fallback.test.ts +++ b/src/main/ssh/ssh-system-fallback.test.ts @@ -111,6 +111,22 @@ describe('spawnSystemSsh', () => { expect(args).toContain('/home/user/.ssh/id_ed25519') }) + it('includes identity agent option', () => { + spawnSystemSsh(createTarget({ identityAgent: '/home/user/.1password/agent.sock' })) + + const args = spawnMock.mock.calls[0][1] as string[] + expect(args).toContain('-o') + expect(args).toContain('IdentityAgent=/home/user/.1password/agent.sock') + }) + + it('includes identities only option', () => { + spawnSystemSsh(createTarget({ identitiesOnly: true })) + + const args = spawnMock.mock.calls[0][1] as string[] + expect(args).toContain('-o') + expect(args).toContain('IdentitiesOnly=yes') + }) + it('includes jump host flag', () => { spawnSystemSsh(createTarget({ jumpHost: 'bastion.example.com' })) @@ -135,6 +151,7 @@ describe('spawnSystemSsh', () => { port: 2222, username: 'deploy', identityFile: '/tmp/key', + identityAgent: '/tmp/agent.sock', proxyCommand: 'ignored' }) ) @@ -143,6 +160,7 @@ describe('spawnSystemSsh', () => { expect(args).not.toContain('resolved.example.com') expect(args).not.toContain('-p') expect(args).not.toContain('-i') + expect(args).not.toContain('IdentityAgent=/tmp/agent.sock') expect(args).not.toContain('ProxyCommand=ignored') }) diff --git a/src/main/ssh/ssh-system-fallback.ts b/src/main/ssh/ssh-system-fallback.ts index b7761d3ba..cd5b244e0 100644 --- a/src/main/ssh/ssh-system-fallback.ts +++ b/src/main/ssh/ssh-system-fallback.ts @@ -177,6 +177,14 @@ export function buildSshArgs(target: SshTarget): string[] { args.push('-i', target.identityFile) } + if (!useConfigHost && target.identityAgent) { + args.push('-o', `IdentityAgent=${target.identityAgent}`) + } + + if (!useConfigHost && target.identitiesOnly) { + args.push('-o', 'IdentitiesOnly=yes') + } + if (!useConfigHost && target.jumpHost) { args.push('-J', target.jumpHost) } diff --git a/src/shared/ssh-types.ts b/src/shared/ssh-types.ts index 58c8b02d9..dd23298c2 100644 --- a/src/shared/ssh-types.ts +++ b/src/shared/ssh-types.ts @@ -14,6 +14,10 @@ export type SshTarget = { username: string /** Path to private key file, if using key-based auth. */ identityFile?: string + /** SSH agent socket path from IdentityAgent, if configured. */ + identityAgent?: string + /** Whether OpenSSH IdentitiesOnly should limit public-key auth attempts. */ + identitiesOnly?: boolean /** ProxyCommand from SSH config, if any. */ proxyCommand?: string /** Jump host (ProxyJump), if any. */