From e2b887923b62be75b1bf722bc96a0ddd51dd391d Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sun, 30 Aug 2026 08:02:54 +0000 Subject: [PATCH] ssh: find the agent and the default keys, the way ssh does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported as "I can't connect to any of my live servers", and reproducible against the real ~/.ssh/config: every host that declares no IdentityFile became `agent` authentication, and agent authentication demanded that SSH_AUTH_SOCK be exported. A desktop app is launched from a session that exports far less than a login shell, so those hosts failed with "No SSH agent is available" — in a terminal, on the same machine, all of them worked. Agent authentication now does both halves of what ssh(1) does. It finds a socket: SSH_AUTH_SOCK when set, because that is intent rather than a guess, then the sockets the common agents leave in XDG_RUNTIME_DIR. And it offers the default identities — id_ed25519, id_ecdsa, id_rsa, id_dsa, in ssh's own order — so a host with no configured key is not a host with no key. Only when neither turns anything up is it an error, and the message now says what was looked for. Two smaller things that make a path unusable in the same way: `~` in a key path was expanded in exactly one place in the tree, the ssh_config reader. A path typed into the New server dialog, or stored by either import command, kept its tilde and failed as `ENOENT ... open '~/.ssh/id_ed25519'` — a path that reads as plainly correct. It is now expanded where the key is read, so every source is covered, and both imports store it expanded too. `IdentityFile "/path with spaces/key"` is valid ssh_config. The parser kept the quotes, making the path a file that cannot exist. Verified against the real config with SSH_AUTH_SOCK unset — the exact condition that broke it. Before: "No SSH agent is available" on every agent host. After: the agent socket and ~/.ssh/id_ed25519 are both found and those hosts reach the network, where what is left is ordinary reachability from this box. `dev` still lists its home directory, 225 entries. 18 new tests. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GTQ3RzTAey9nT6r1kbGBCd --- apps/cli/src/commands/connections.ts | 4 +- apps/desktop/electron/main/ipc.ts | 5 +- packages/ssh-core/src/identity.test.ts | 93 ++++++++++++++++++++++++ packages/ssh-core/src/identity.ts | 78 ++++++++++++++++++++ packages/ssh-core/src/index.ts | 1 + packages/ssh-core/src/session.ts | 33 +++++++-- packages/ssh-core/src/ssh-config.test.ts | 22 ++++++ packages/ssh-core/src/ssh-config.ts | 17 ++++- 8 files changed, 242 insertions(+), 11 deletions(-) create mode 100644 packages/ssh-core/src/identity.test.ts create mode 100644 packages/ssh-core/src/identity.ts diff --git a/apps/cli/src/commands/connections.ts b/apps/cli/src/commands/connections.ts index 7443855..c15e549 100644 --- a/apps/cli/src/commands/connections.ts +++ b/apps/cli/src/commands/connections.ts @@ -4,7 +4,7 @@ import { join } from 'node:path' import { createInterface } from 'node:readline/promises' import type { DiskPushStore } from '@diskpush/database' import { knownHostsPath } from '@diskpush/database' -import { parseSshConfig, probeConnection, SshSession } from '@diskpush/ssh-core' +import { expandTilde, parseSshConfig, probeConnection, SshSession } from '@diskpush/ssh-core' import { EXIT } from '../exit-codes.js' import { table } from '../format.js' import { failure, type Output } from '../output.js' @@ -171,7 +171,7 @@ async function importConnections(parsed: ParsedArgv, store: DiskPushStore, outpu port: host.port ?? 22, username: host.user ?? process.env.USER ?? 'root', authType: host.identityFile ? 'key' : 'agent', - keyPath: host.identityFile ?? null, + keyPath: host.identityFile ? expandTilde(host.identityFile) : null, defaultLocalPath: null, defaultRemotePath: null, jumpHost: host.proxyJump ?? null, diff --git a/apps/desktop/electron/main/ipc.ts b/apps/desktop/electron/main/ipc.ts index 1265160..2db69b8 100644 --- a/apps/desktop/electron/main/ipc.ts +++ b/apps/desktop/electron/main/ipc.ts @@ -2,7 +2,7 @@ import { lstat, mkdir, open, readdir, readFile, rename, rm, stat, unlink } from import { homedir } from 'node:os' import { isAbsolute, join, posix, resolve } from 'node:path' import { ipcMain, shell, type IpcMainInvokeEvent } from 'electron' -import { probeConnection, parseSshConfig, sshConfigConnections, type SftpBrowser } from '@diskpush/ssh-core' +import { expandTilde, probeConnection, parseSshConfig, sshConfigConnections, type SftpBrowser } from '@diskpush/ssh-core' import { z } from 'zod' import { ConnectionInputSchema, @@ -122,7 +122,8 @@ export function registerIpc(): void { port: host.port ?? 22, username: host.user ?? process.env.USER ?? 'root', authType: host.identityFile ? 'key' : 'agent', - keyPath: host.identityFile ?? null, + // Expanded on the way in, so a stored key path is always usable. + keyPath: host.identityFile ? expandTilde(host.identityFile) : null, defaultLocalPath: null, defaultRemotePath: null, jumpHost: host.proxyJump ?? null, diff --git a/packages/ssh-core/src/identity.test.ts b/packages/ssh-core/src/identity.test.ts new file mode 100644 index 0000000..88dd2af --- /dev/null +++ b/packages/ssh-core/src/identity.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, it } from 'vitest' +import { + agentSocketCandidates, + defaultIdentityPaths, + expandTilde, + findAgentSocket, + findDefaultIdentity, +} from './identity.js' + +const HOME = '/home/you' + +describe('expandTilde', () => { + it('expands a leading ~/', () => { + expect(expandTilde('~/.ssh/id_ed25519', HOME)).toBe('/home/you/.ssh/id_ed25519') + }) + + it('expands a bare ~', () => { + expect(expandTilde('~', HOME)).toBe(HOME) + }) + + it('leaves an absolute path alone', () => { + expect(expandTilde('/etc/ssh/key', HOME)).toBe('/etc/ssh/key') + }) + + it('leaves a ~ that is not at the front alone', () => { + expect(expandTilde('/tmp/back~up', HOME)).toBe('/tmp/back~up') + }) + + it('leaves ~user alone rather than guessing another home', () => { + // Resolving it means reading passwd, and /home/ is wrong often enough + // that a wrong path is worse than an unexpanded one. + expect(expandTilde('~someone/.ssh/id_rsa', HOME)).toBe('~someone/.ssh/id_rsa') + }) +}) + +describe('findDefaultIdentity', () => { + it('tries the keys ssh tries, in ssh order', () => { + expect(defaultIdentityPaths(HOME)).toEqual([ + '/home/you/.ssh/id_ed25519', + '/home/you/.ssh/id_ecdsa', + '/home/you/.ssh/id_rsa', + '/home/you/.ssh/id_dsa', + ]) + }) + + it('prefers ed25519 when several exist', () => { + expect(findDefaultIdentity(() => true, HOME)).toBe('/home/you/.ssh/id_ed25519') + }) + + it('falls through to the one that is actually there', () => { + const only = '/home/you/.ssh/id_rsa' + expect(findDefaultIdentity((path) => path === only, HOME)).toBe(only) + }) + + it('is null when the user has no keys at all', () => { + expect(findDefaultIdentity(() => false, HOME)).toBeNull() + }) +}) + +describe('findAgentSocket', () => { + /** + * The bug this exists to prevent: agent auth required SSH_AUTH_SOCK to be + * exported. A desktop app is launched from a session that exports far less + * than a login shell, so every host with no IdentityFile failed with "no SSH + * agent is available" — in a terminal, on the same machine, they all worked. + */ + it('honours SSH_AUTH_SOCK above every guess', () => { + const env = { SSH_AUTH_SOCK: '/tmp/explicit.sock', XDG_RUNTIME_DIR: '/run/user/1000' } + expect(findAgentSocket(() => true, env, 1000)).toBe('/tmp/explicit.sock') + }) + + it('finds the systemd agent socket when the variable is missing', () => { + const socket = '/run/user/1000/ssh-agent.socket' + expect(findAgentSocket((path) => path === socket, { XDG_RUNTIME_DIR: '/run/user/1000' }, 1000)).toBe(socket) + }) + + it('finds a gnome-keyring socket too', () => { + const socket = '/run/user/1000/keyring/ssh' + expect(findAgentSocket((path) => path === socket, { XDG_RUNTIME_DIR: '/run/user/1000' }, 1000)).toBe(socket) + }) + + it('derives the runtime directory from the uid when XDG_RUNTIME_DIR is unset', () => { + expect(agentSocketCandidates({}, 1000)).toContain('/run/user/1000/ssh-agent.socket') + }) + + it('is null when there is no socket anywhere, rather than a path to nothing', () => { + expect(findAgentSocket(() => false, { XDG_RUNTIME_DIR: '/run/user/1000' }, 1000)).toBeNull() + }) + + it('offers nothing to guess at when there is no runtime directory and no uid', () => { + expect(agentSocketCandidates({}, null)).toEqual([]) + }) +}) diff --git a/packages/ssh-core/src/identity.ts b/packages/ssh-core/src/identity.ts new file mode 100644 index 0000000..e82ea97 --- /dev/null +++ b/packages/ssh-core/src/identity.ts @@ -0,0 +1,78 @@ +import { homedir } from 'node:os' +import { join } from 'node:path' + +/** + * Finding the things OpenSSH finds on its own. + * + * DiskPush used to require both halves of this to be spelled out: agent + * authentication meant `SSH_AUTH_SOCK` had to be exported, and a host with no + * `IdentityFile` in ssh_config had no key at all. Both hold in a terminal and + * neither holds in a desktop app, which is launched from a session that + * exports far less than a login shell — so every server that worked in the + * terminal failed in the window with "no SSH agent is available". + */ + +/** `~` at the front of a path, expanded. Anywhere else it is an ordinary character. */ +export function expandTilde(path: string, home: string = homedir()): string { + if (path === '~') return home + if (path.startsWith('~/')) return join(home, path.slice(2)) + // `~user/...` is deliberately left alone: resolving another account's home + // means reading passwd, and guessing `/home/` is wrong often enough. + return path +} + +/** + * The identity files OpenSSH tries when a host names none, in its order. + * + * ssh(1) reads these from ~/.ssh by default; DSA is included because ssh still + * lists it, and omitting a key someone actually uses is the failure this whole + * module exists to prevent. + */ +export const DEFAULT_IDENTITY_FILES = ['id_ed25519', 'id_ecdsa', 'id_rsa', 'id_dsa'] as const + +export function defaultIdentityPaths(home: string = homedir()): string[] { + return DEFAULT_IDENTITY_FILES.map((name) => join(home, '.ssh', name)) +} + +/** The first default identity that exists, or null when the user has no keys. */ +export function findDefaultIdentity(exists: (path: string) => boolean, home: string = homedir()): string | null { + return defaultIdentityPaths(home).find((path) => exists(path)) ?? null +} + +/** + * Where an agent socket is likely to be when `SSH_AUTH_SOCK` is not set. + * + * `SSH_AUTH_SOCK` wins whenever it is present — it is the only one of these + * that is a statement of intent rather than a guess. The rest are the sockets + * the common agents put in the runtime directory: systemd's ssh-agent unit, + * then gnome-keyring, then the KDE/ssh-agent unit name. + */ +export function agentSocketCandidates( + env: NodeJS.ProcessEnv = process.env, + // `null` rather than `undefined` for "no uid": a default parameter fires on + // undefined, so undefined could not have meant anything else. + uid: number | null = process.getuid?.() ?? null, +): string[] { + const candidates: string[] = [] + if (env.SSH_AUTH_SOCK) candidates.push(env.SSH_AUTH_SOCK) + + const runtime = env.XDG_RUNTIME_DIR ?? (uid === null ? null : `/run/user/${uid}`) + if (runtime) { + candidates.push( + join(runtime, 'ssh-agent.socket'), + join(runtime, 'keyring', 'ssh'), + join(runtime, 'gcr', 'ssh'), + join(runtime, 'openssh_agent'), + ) + } + return candidates +} + +/** The first agent socket that exists, or null. */ +export function findAgentSocket( + exists: (path: string) => boolean, + env: NodeJS.ProcessEnv = process.env, + uid: number | null = process.getuid?.() ?? null, +): string | null { + return agentSocketCandidates(env, uid).find((path) => exists(path)) ?? null +} diff --git a/packages/ssh-core/src/index.ts b/packages/ssh-core/src/index.ts index a04d0a9..2cc2246 100644 --- a/packages/ssh-core/src/index.ts +++ b/packages/ssh-core/src/index.ts @@ -1,6 +1,7 @@ export * from './browser.js' export * from './capabilities.js' export * from './fingerprint.js' +export * from './identity.js' export * from './known-hosts.js' export * from './preflight.js' export * from './session.js' diff --git a/packages/ssh-core/src/session.ts b/packages/ssh-core/src/session.ts index 9219682..4dea102 100644 --- a/packages/ssh-core/src/session.ts +++ b/packages/ssh-core/src/session.ts @@ -1,7 +1,8 @@ -import { readFileSync } from 'node:fs' +import { existsSync, readFileSync } from 'node:fs' import { Client, type ConnectConfig, type SFTPWrapper } from 'ssh2' import type { Connection } from '@diskpush/schemas' import { keyTypeOf, sha256Fingerprint } from './fingerprint.js' +import { expandTilde, findAgentSocket, findDefaultIdentity } from './identity.js' import { appendKnownHost, readKnownHosts, verifyHostKey, type HostKeyVerdict } from './known-hosts.js' export class SshError extends Error { @@ -59,12 +60,34 @@ export class SshSession { } if (connection.authType === 'agent') { - const agent = options.agentSocket ?? process.env.SSH_AUTH_SOCK - if (!agent) throw new SshError('No SSH agent is available (SSH_AUTH_SOCK is unset).', 'auth') - config.agent = agent + // Both halves, the way ssh(1) does it: an agent if one can be found, and + // the default identity files regardless. Requiring SSH_AUTH_SOCK to be + // exported meant every agent host failed in the desktop app, which is + // launched from a session that exports far less than a login shell. + const agent = options.agentSocket ?? findAgentSocket(existsSync) + if (agent) config.agent = agent + + const identity = findDefaultIdentity(existsSync) + if (identity) { + config.privateKey = readFileSync(identity) + if (options.passphrase) config.passphrase = options.passphrase + } + + if (!agent && !identity) { + throw new SshError( + 'No SSH agent and no default key. Looked for an agent socket, then for ' + + '~/.ssh/id_ed25519, id_ecdsa, id_rsa and id_dsa. Set a key file on this connection, ' + + 'or start an agent and add one.', + 'auth', + ) + } } else if (connection.authType === 'key' || connection.authType === 'key-passphrase') { if (!connection.keyPath) throw new SshError('This connection is set to key authentication but has no key path.', 'auth') - config.privateKey = readFileSync(connection.keyPath) + // `~` is expanded here rather than trusted to have been expanded by + // whoever stored the path: it can come from ssh_config, from an import, + // or from someone typing it into the New server dialog, and only one of + // those three used to expand it. + config.privateKey = readFileSync(expandTilde(connection.keyPath)) if (options.passphrase) config.passphrase = options.passphrase } else if (connection.authType === 'password') { if (!options.password) throw new SshError('This connection needs a password, which was not supplied.', 'auth') diff --git a/packages/ssh-core/src/ssh-config.test.ts b/packages/ssh-core/src/ssh-config.test.ts index 6c4ca24..28ee212 100644 --- a/packages/ssh-core/src/ssh-config.test.ts +++ b/packages/ssh-core/src/ssh-config.test.ts @@ -69,3 +69,25 @@ describe('parseSshConfig', () => { expect(parseSshConfig('')).toEqual([]) }) }) + +describe('quoted values', () => { + /** + * `IdentityFile "/path with spaces/key"` is valid ssh_config. Keeping the + * quotes makes the path a file that cannot exist, and the ENOENT names a + * path that looks plainly correct — quotes and all. + */ + it('drops double quotes around a value', () => { + const [host] = parseSshConfig('Host a\n IdentityFile "/home/you/.ssh/my key"\n') + expect(host?.identityFile).toBe('/home/you/.ssh/my key') + }) + + it('drops single quotes too', () => { + const [host] = parseSshConfig("Host a\n HostName 'example.com'\n") + expect(host?.hostName).toBe('example.com') + }) + + it('leaves an unquoted value untouched', () => { + const [host] = parseSshConfig('Host a\n IdentityFile ~/.ssh/id_rsa\n') + expect(host?.identityFile).toBe('~/.ssh/id_rsa') + }) +}) diff --git a/packages/ssh-core/src/ssh-config.ts b/packages/ssh-core/src/ssh-config.ts index ab8b31a..2d42bda 100644 --- a/packages/ssh-core/src/ssh-config.ts +++ b/packages/ssh-core/src/ssh-config.ts @@ -7,6 +7,7 @@ */ import { homedir } from 'node:os' +import { expandTilde } from './identity.js' import { join } from 'node:path' import { readFileSync } from 'node:fs' import type { Connection } from '@diskpush/schemas' @@ -36,7 +37,7 @@ export function parseSshConfig(contents: string): SshConfigHost[] { const match = /^(\S+)[\s=]+(.*)$/.exec(line) if (!match) continue const key = match[1]!.toLowerCase() - const value = match[2]!.trim() + const value = unquote(match[2]!.trim()) if (key === 'host') { // A wildcard block sets defaults across many hosts. Modelling that @@ -64,6 +65,18 @@ export function parseSshConfig(contents: string): SshConfigHost[] { return hosts } +/** + * Drops the quotes OpenSSH allows around a value. + * + * `IdentityFile "/path with spaces/key"` is valid ssh_config, and keeping the + * quotes makes the path a file that cannot exist — an ENOENT naming a path + * that is plainly right, quotes and all. + */ +function unquote(value: string): string { + const quoted = /^"(.*)"$/.exec(value) ?? /^'(.*)'$/.exec(value) + return quoted ? quoted[1]! : value +} + function applySetting(host: SshConfigHost, key: string, value: string): void { switch (key) { case 'hostname': @@ -119,7 +132,7 @@ export function sshConfigConnections(env: NodeJS.ProcessEnv = process.env): Conn return true }) .map((host): Connection => { - const identity = host.identityFile ? host.identityFile.replace(/^~/, homedir()) : null + const identity = host.identityFile ? expandTilde(host.identityFile) : null return { id: `ssh-config:${host.alias}`, name: host.alias,