diff --git a/config/scripts/pr-e2e-source-routing.mjs b/config/scripts/pr-e2e-source-routing.mjs index 9d97d3c61eb..72b61436f93 100644 --- a/config/scripts/pr-e2e-source-routing.mjs +++ b/config/scripts/pr-e2e-source-routing.mjs @@ -154,6 +154,17 @@ export const PR_E2E_SOURCE_ROUTES = [ file )) }, + { + id: 'ssh.orcad-identity-file-tokens', + specs: ['tests/e2e/ssh-orcad-identity-file-tokens.spec.ts'], + matches: (file) => + MANAGED_SSH_HARNESS.test(file) || + /^tests\/e2e\/helpers\/ssh-config-host-picker\.ts$/.test(file) || + (isProductSource(file) && + /^src\/main\/ssh\/(?:config\/(?:ssh-g-config-resolution|ssh-identity-file-tokens|ssh-config-path-expansion)|ssh-auth-resolution|ssh-identity-file-read)\.ts$/.test( + file + )) + }, { id: 'ssh.orcad-update-anyway', specs: ['tests/e2e/ssh-orcad-update-anyway.spec.ts'], diff --git a/src/main/ssh/config/ssh-g-config-resolution.test.ts b/src/main/ssh/config/ssh-g-config-resolution.test.ts index 770b5611fbd..cff7bc0fa14 100644 --- a/src/main/ssh/config/ssh-g-config-resolution.test.ts +++ b/src/main/ssh/config/ssh-g-config-resolution.test.ts @@ -9,7 +9,9 @@ type NodeOsModule = typeof NodeOs const { existsSyncMock, homedirMock, userInfoMock } = vi.hoisted(() => ({ existsSyncMock: vi.fn<(path: string) => boolean>(() => true), homedirMock: vi.fn<() => string>(() => '/home/env'), - userInfoMock: vi.fn<() => { homedir: string }>(() => ({ homedir: '/home/env' })) + userInfoMock: vi.fn<() => { homedir: string; username?: string }>(() => ({ + homedir: '/home/env' + })) })) vi.mock('node:fs', () => ({ existsSync: existsSyncMock })) @@ -21,6 +23,7 @@ vi.mock('node:os', async (importOriginal) => ({ })) import { + parseSshGOutput, siteConfigMayRestrictHostKeys, splitIncludeArguments, sshGArgsForHost @@ -75,6 +78,56 @@ describe('sshGArgsForHost', () => { }) /** -F excludes user and system config; read site policy separately and fail closed on uncertainty. */ +describe('parseSshGOutput IdentityFile tokens', () => { + beforeEach(() => { + homedirMock.mockReset().mockReturnValue('/home/env') + userInfoMock.mockReset().mockReturnValue({ homedir: '/home/passwd', username: 'alice' }) + }) + + const sshG = (...identityFiles: string[]): string => + [ + 'host work', + ...identityFiles.map((file) => `identityfile ${file}`), + 'user bob', + 'hostname example.com', + 'port 2200' + ].join('\n') + + it('expands the tokens OpenSSH 10.3 leaves raw in ssh -G output', () => { + expect(parseSshGOutput(sshG('%d/keys/%h-%r-%p-%n-%u-%%.pem')).identityFile).toEqual([ + '/home/passwd/keys/example.com-bob-2200-work-alice-%.pem' + ]) + }) + + it('still expands a leading tilde', () => { + expect(parseSshGOutput(sshG('~/.ssh/id_work')).identityFile).toEqual([ + join('/home/env', '.ssh', 'id_work') + ]) + }) + + it('expands ${NAME} from the environment, and keeps the path raw when NAME is unset', () => { + vi.stubEnv('ORCA_TEST_KEYS', '/srv/keys-%r') + try { + expect( + parseSshGOutput(sshG('${ORCA_TEST_KEYS}/%r', '${ORCA_TEST_UNSET_VAR}/k')).identityFile + ).toEqual(['/srv/keys-%r/bob', '${ORCA_TEST_UNSET_VAR}/k']) + } finally { + vi.unstubAllEnvs() + } + }) + + it('reads `IdentityFile none` as no key, not a missing file', () => { + expect(parseSshGOutput(sshG('none')).identityFile).toEqual([]) + }) + + it('keeps a path with a token it cannot resolve unchanged', () => { + expect(parseSshGOutput(sshG('%d/%C.pem', '/keys/100%')).identityFile).toEqual([ + '%d/%C.pem', + '/keys/100%' + ]) + }) +}) + describe('siteConfigMayRestrictHostKeys', () => { let dir: string diff --git a/src/main/ssh/config/ssh-g-config-resolution.ts b/src/main/ssh/config/ssh-g-config-resolution.ts index 17bfb660d6b..cac2ff225a7 100644 --- a/src/main/ssh/config/ssh-g-config-resolution.ts +++ b/src/main/ssh/config/ssh-g-config-resolution.ts @@ -4,6 +4,7 @@ import { readdir, readFile } from 'node:fs/promises' import { dirname, isAbsolute, join } from 'node:path' import { homedir, userInfo } from 'node:os' import { resolveSshConfigHomePath } from './ssh-config-path-expansion' +import { expandIdentityFileTokens } from './ssh-identity-file-tokens' export type SshResolvedConfig = { hostname: string @@ -204,7 +205,7 @@ export async function resolveWithSshG(host: string): Promise() - const identityFiles: string[] = [] + const rawIdentityFiles: string[] = [] for (const line of stdout.split('\n')) { const spaceIdx = line.indexOf(' ') @@ -214,12 +215,19 @@ export function parseSshGOutput(stdout: string): SshResolvedConfig { const key = line.substring(0, spaceIdx).toLowerCase() const value = line.substring(spaceIdx + 1).trim() if (key === 'identityfile') { - identityFiles.push(resolveSshConfigHomePath(value)) + // `IdentityFile none` loads no key; it is not a path. + if (value.toLowerCase() !== 'none') { + rawIdentityFiles.push(value) + } } else { map.set(key, value) } } + // Tokens need hostname/user/port, which ssh -G may print after identityfile lines. + const identityFiles = rawIdentityFiles.map((value) => + resolveSshConfigHomePath(expandIdentityFileTokens(value, map)) + ) return buildSshResolvedConfig(map, identityFiles) } diff --git a/src/main/ssh/config/ssh-identity-file-tokens.ts b/src/main/ssh/config/ssh-identity-file-tokens.ts new file mode 100644 index 00000000000..816478ca5a6 --- /dev/null +++ b/src/main/ssh/config/ssh-identity-file-tokens.ts @@ -0,0 +1,69 @@ +import { homedir, hostname, userInfo } from 'node:os' + +type TokenSource = (resolved: ReadonlyMap) => string | undefined + +// OpenSSH expands %d/%u from the passwd entry, which can differ from HOME. +function localUserInfo(): { homedir: string; username: string } | undefined { + try { + return userInfo() + } catch { + return undefined + } +} + +const TOKEN_SOURCES: Record = { + '%': () => '%', + d: () => localUserInfo()?.homedir || homedir(), + u: () => localUserInfo()?.username || process.env.USER || process.env.USERNAME || undefined, + i: () => { + const uid = process.getuid?.() + return uid === undefined ? undefined : String(uid) + }, + l: () => hostname(), + L: () => hostname().split('.')[0], + h: (resolved) => resolved.get('hostname'), + n: (resolved) => resolved.get('host'), + p: (resolved) => resolved.get('port'), + r: (resolved) => resolved.get('user') +} + +/** ssh -G prints IdentityFile `%x` and `${NAME}` raw (OpenSSH expands them at connect time), e.g. `%d/key.pem`. */ +export function expandIdentityFileTokens( + value: string, + resolved: ReadonlyMap +): string { + if (!value.includes('%') && !value.includes('${')) { + return value + } + // One pass, like OpenSSH: a replacement is never scanned for further tokens. + let expanded = '' + for (let index = 0; index < value.length; index += 1) { + const char = value[index] + if (char === '$' && value[index + 1] === '{') { + const end = value.indexOf('}', index + 2) + const envValue = end === -1 ? undefined : process.env[value.slice(index + 2, end)] + // OpenSSH refuses the path when NAME is unset; keep it raw so it reads as missing. + if (envValue === undefined) { + return value + } + expanded += envValue + index = end + continue + } + if (char !== '%') { + expanded += char + continue + } + const source = Object.hasOwn(TOKEN_SOURCES, value[index + 1] ?? '') + ? TOKEN_SOURCES[value[index + 1]] + : undefined + const replacement = source?.(resolved) + // Unknown tokens (%C, %j, %k) need OpenSSH internals; keep the raw path rather than guess. + if (replacement === undefined) { + return value + } + expanded += replacement + index += 1 + } + return expanded +} diff --git a/src/main/ssh/ssh-auth-resolution.ts b/src/main/ssh/ssh-auth-resolution.ts index 27f73897b1f..1f524f4e290 100644 --- a/src/main/ssh/ssh-auth-resolution.ts +++ b/src/main/ssh/ssh-auth-resolution.ts @@ -142,6 +142,9 @@ function readPrivateKey( if (read.problem) { skipped?.set(keyPath, read.problem) warnSkippedIdentityFileOnce(keyPath, read.problem) + } else if (!EXPANDED_DEFAULT_KEY_PATHS.includes(resolveSshConfigHomePath(keyPath))) { + // Absent defaults are normal; a configured key that is missing explains a later password prompt. + skipped?.set(keyPath, 'it does not exist') } return undefined } diff --git a/src/main/ssh/ssh-identity-file-read.test.ts b/src/main/ssh/ssh-identity-file-read.test.ts index 8cd654a6501..ad35bf6fc14 100644 --- a/src/main/ssh/ssh-identity-file-read.test.ts +++ b/src/main/ssh/ssh-identity-file-read.test.ts @@ -68,11 +68,15 @@ describe('SSH identity file reads', () => { ) }) - it('leaves non-auth errors and missing files unannotated', () => { + it('names a missing configured key', () => { const skipped: SkippedIdentityFiles = new Map() - expect(resolvePrivateKeys(targetFor(join(root, 'missing')), null, skipped)).toEqual([]) - expect(skipped.size).toBe(0) + const missing = join(root, 'missing') + expect(resolvePrivateKeys(targetFor(missing), null, skipped)).toEqual([]) + expect([...skipped]).toEqual([[missing, 'it does not exist']]) + }) + it('leaves non-auth errors unannotated', () => { + const skipped: SkippedIdentityFiles = new Map() resolvePrivateKeys(targetFor(join(root, 'directory')), null, skipped) const networkError = new Error('connect ECONNREFUSED') withSkippedIdentityFileDetail(networkError, skipped) diff --git a/tests/e2e/ssh-orcad-identity-file-tokens.spec.ts b/tests/e2e/ssh-orcad-identity-file-tokens.spec.ts new file mode 100644 index 00000000000..6cd2cc09cf8 --- /dev/null +++ b/tests/e2e/ssh-orcad-identity-file-tokens.spec.ts @@ -0,0 +1,84 @@ +// #33849: an ~/.ssh/config host whose IdentityFile uses OpenSSH tokens connects with that key, not a password prompt. +// ORCA_E2E_ORCAD_CONVERT_HOST=docker; ORCA_E2E_ORCAD_CONVERT_TEMPLATE must be built for the Docker host. +import { copyFileSync, mkdtempSync, rmSync } from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { expect, test } from './helpers/orca-app' +import { waitForSessionReady } from './helpers/store' +import { managedServer } from './helpers/orcad-convert-flow' +import { openSshHostSettings, seedIsolatedSshConfig } from './helpers/ssh-config-host-picker' +import { + ORCAD_CONVERT_HOST_ENV, + startOrcadConvertHost, + type OrcadConvertHost +} from './helpers/orcad-convert-host' + +const HOST = process.env[ORCAD_CONVERT_HOST_ENV] +const TEMPLATE_SOURCE = process.env.ORCA_E2E_ORCAD_CONVERT_TEMPLATE + +test.skip( + HOST !== 'docker' || !TEMPLATE_SOURCE, + `Set ${ORCAD_CONVERT_HOST_ENV}=docker and ORCA_E2E_ORCAD_CONVERT_TEMPLATE` +) +test.use({ + seedTestRepo: false, + orcaAppExtraEnv: { ORCA_ORCAD_TEMPLATE_PATH: TEMPLATE_SOURCE ?? '', SSH_AUTH_SOCK: '' } +}) + +let host: OrcadConvertHost | null = null +let keyDir: string | null = null +test.afterEach(() => { + host?.cleanup() + host = null + if (keyDir) { + rmSync(keyDir, { recursive: true, force: true }) + keyDir = null + } +}) + +test('an IdentityFile path with %r and %p tokens is the key Connect uses', async ({ + electronApp, + orcaPage +}, testInfo) => { + // Starts a Docker host and installs Orca's server there. + test.setTimeout(6 * 60_000) + host = startOrcadConvertHost('docker', testInfo) + keyDir = mkdtempSync(path.join(os.tmpdir(), 'orca-identity-tokens-')) + // %r and %p expand to root and the mapped port; ssh -G prints them unexpanded. + copyFileSync(host.input.identityFile!, path.join(keyDir, `id-root-${host.input.port}`)) + const alias = `orca-tokens-${host.input.port}` + await seedIsolatedSshConfig( + electronApp, + [ + `Host ${alias}`, + ` HostName ${host.input.host}`, + ` Port ${host.input.port}`, + ' User root', + ` IdentityFile ${keyDir}/id-%r-%p`, + ' IdentitiesOnly yes', + '' + ].join('\n') + ) + await waitForSessionReady(orcaPage) + const targetId = await orcaPage.evaluate(async (configHost) => { + const { target } = await window.api.ssh.addTarget({ + target: { + label: configHost, + configHost, + host: configHost, + port: 22, + username: '', + source: 'ssh-config' + } + }) + return target.id + }, alias) + const section = await openSshHostSettings(orcaPage) + const card = section.locator(`[data-ssh-target-card][data-ssh-target-label="${alias}"]`) + await card.hover() + await card.getByRole('button', { name: 'Connect', exact: true }).click() + await expect + .poll(() => managedServer(orcaPage, targetId), { timeout: 4 * 60_000 }) + .toMatchObject({ kind: 'managed' }) + await expect(orcaPage.getByRole('dialog', { name: 'SSH Password' })).toHaveCount(0) +})