fix(ssh): gate FIDO2 system-transport on an OpenSSH binary

`ssh -G` echoes OpenSSH's built-in default identity list for every host, so `usesDefaultPaths` was almost never true and the security-key gate returned `!usesDefaultPaths || findSystemSsh() !== null` — forcing system transport without checking that an `ssh` binary exists. `spawnSystemSsh()` then throws `No system ssh binary found`, hard-failing connections that worked on ssh2. The same flag also stopped the default scan at the first existing normal private key, so a host that only accepts a FIDO2 key never reached system OpenSSH when `~/.ssh/id_rsa` happened to exist. Both decisions are independent of where an identity path came from: always require `findSystemSsh() !== null` before forcing system transport, and scan every candidate identity instead of stopping on the first normal key. `shouldUseSystemSshTransport()` is untouched, so ProxyCommand / ProxyJump / ProxyUseFdpass keep their intentional system transport.
2026-10-20 16:03:20 +00:00 · 2026-08-01 18:18:25 -07:00
parent 8fc892dd02
commit b337db0936
2 changed files with 92 additions and 55 deletions
@@ -53,6 +53,29 @@ async function writeKey(contents: Buffer, filename = 'security key'): Promise<st
  return keyPath
 }

+async function createDefaultKeyHome(files: Record<string, Buffer>): Promise<string> {
+  const directory = await mkdtemp(join(tmpdir(), 'orca-default-key-home-'))
+  tempDirs.push(directory)
+  await mkdir(join(directory, '.ssh'))
+  for (const [name, contents] of Object.entries(files)) {
+    await writeFile(join(directory, '.ssh', name), contents)
+  }
+  return directory
+}
+
+// Why: `ssh -G` echoes this list, already home-expanded, for every host — configured or not.
+function listBuiltInDefaultIdentityFiles(home: string): string[] {
+  return [
+    'id_rsa',
+    'id_ecdsa',
+    'id_ecdsa_sk',
+    'id_ed25519',
+    'id_ed25519_sk',
+    'id_xmss',
+    'id_dsa'
+  ].map((name) => join(home, '.ssh', name))
+}
+
 afterEach(async () => {
  vi.unstubAllEnvs()
  await Promise.all(tempDirs.splice(0).map((directory) => rm(directory, { recursive: true })))
@@ -155,13 +178,9 @@ describe('isOpenSshSecurityKeyPrivateKey', () => {

 describe('requiresSystemSshForSecurityKey', () => {
  it('uses default FIDO2 identities only when config resolution is unavailable', async () => {
-    const directory = await mkdtemp(join(tmpdir(), 'orca-security-key-home-'))
-    tempDirs.push(directory)
-    await mkdir(join(directory, '.ssh'))
-    await writeFile(
-      join(directory, '.ssh', 'id_ed25519_sk'),
-      createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
-    )
+    const directory = await createDefaultKeyHome({
+      id_ed25519_sk: createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
+    })
    vi.stubEnv('ORCA_TEST_SSH_HOME', directory)

    await expect(requiresSystemSshForSecurityKey(createTarget(), null)).resolves.toBe(true)
@@ -170,28 +189,58 @@ describe('requiresSystemSshForSecurityKey', () => {
    ).resolves.toBe(false)
  })

-  it('keeps a regular default ahead of a dormant FIDO2 identity', async () => {
-    const directory = await mkdtemp(join(tmpdir(), 'orca-regular-key-home-'))
-    tempDirs.push(directory)
-    await mkdir(join(directory, '.ssh'))
-    await writeFile(join(directory, '.ssh', 'id_rsa'), createOpenSshPrivateKeyFixture(['ssh-rsa']))
-    await writeFile(
-      join(directory, '.ssh', 'id_ed25519_sk'),
-      createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
-    )
+  it('reaches a default FIDO2 identity that a regular default key precedes', async () => {
+    const directory = await createDefaultKeyHome({
+      id_rsa: createOpenSshPrivateKeyFixture(['ssh-rsa']),
+      id_ed25519_sk: createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
+    })
+    vi.stubEnv('ORCA_TEST_SSH_HOME', directory)
+
+    await expect(requiresSystemSshForSecurityKey(createTarget(), null)).resolves.toBe(true)
+
+    findSystemSshMock.mockReturnValue(null)
+    await expect(requiresSystemSshForSecurityKey(createTarget(), null)).resolves.toBe(false)
+  })
+
+  it('leaves regular-only defaults on ssh2', async () => {
+    const directory = await createDefaultKeyHome({
+      id_rsa: createOpenSshPrivateKeyFixture(['ssh-rsa'])
+    })
    vi.stubEnv('ORCA_TEST_SSH_HOME', directory)

    await expect(requiresSystemSshForSecurityKey(createTarget(), null)).resolves.toBe(false)
  })

-  it('keeps password and agent fallback when default FIDO2 needs unavailable OpenSSH', async () => {
-    const directory = await mkdtemp(join(tmpdir(), 'orca-no-system-ssh-home-'))
-    tempDirs.push(directory)
-    await mkdir(join(directory, '.ssh'))
-    await writeFile(
-      join(directory, '.ssh', 'id_ed25519_sk'),
-      createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
+  it('treats resolved built-in default identities as unconfigured, not as forced transport', async () => {
+    const directory = await createDefaultKeyHome({
+      id_rsa: createOpenSshPrivateKeyFixture(['ssh-rsa']),
+      id_ed25519_sk: createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
+    })
+    const identityFile = listBuiltInDefaultIdentityFiles(directory)
+
+    await expect(requiresSystemSshForSecurityKey(createTarget(), { identityFile })).resolves.toBe(
+      true
    )
+
+    findSystemSshMock.mockReturnValue(null)
+    await expect(requiresSystemSshForSecurityKey(createTarget(), { identityFile })).resolves.toBe(
+      false
+    )
+  })
+
+  it('keeps password and agent fallback when a configured FIDO2 identity has no OpenSSH', async () => {
+    const keyPath = await writeKey(createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY]))
+    findSystemSshMock.mockReturnValue(null)
+
+    await expect(
+      requiresSystemSshForSecurityKey(createTarget({ identityFile: keyPath }), null)
+    ).resolves.toBe(false)
+  })
+
+  it('keeps password and agent fallback when default FIDO2 needs unavailable OpenSSH', async () => {
+    const directory = await createDefaultKeyHome({
+      id_ed25519_sk: createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
+    })
    vi.stubEnv('ORCA_TEST_SSH_HOME', directory)
    findSystemSshMock.mockReturnValue(null)

@@ -199,17 +248,10 @@ describe('requiresSystemSshForSecurityKey', () => {
  })

  it('ignores an orphan regular sidecar before a valid default FIDO2 identity', async () => {
-    const directory = await mkdtemp(join(tmpdir(), 'orca-orphan-sidecar-home-'))
-    tempDirs.push(directory)
-    await mkdir(join(directory, '.ssh'))
-    await writeFile(
-      join(directory, '.ssh', 'id_ed25519.pub'),
-      createOpenSshPublicKeyFixture('ssh-ed25519')
-    )
-    await writeFile(
-      join(directory, '.ssh', 'id_ed25519_sk'),
-      createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
-    )
+    const directory = await createDefaultKeyHome({
+      'id_ed25519.pub': createOpenSshPublicKeyFixture('ssh-ed25519'),
+      id_ed25519_sk: createOpenSshPrivateKeyFixture([ED25519_SECURITY_KEY])
+    })
    vi.stubEnv('ORCA_TEST_SSH_HOME', directory)

    await expect(requiresSystemSshForSecurityKey(createTarget(), null)).resolves.toBe(true)
@@ -21,8 +21,6 @@ const READ_CHUNK_BYTES = 64 * 1024
 const READ_OPEN_FLAGS =
  constants.O_RDONLY | (process.platform === 'win32' ? 0 : constants.O_NONBLOCK)

-type IdentityInspection = { privateIdentityExists: boolean; requiresSystemSsh: boolean }
-
 async function readBoundedKeyFile(path: string): Promise<Buffer | null> {
  let handle: Awaited<ReturnType<typeof open>> | undefined
  try {
@@ -59,21 +57,15 @@ async function readBoundedKeyFile(path: string): Promise<Buffer | null> {
  }
 }

-async function inspectIdentityPath(keyPath: string): Promise<IdentityInspection> {
+async function identityRequiresSystemSsh(keyPath: string): Promise<boolean> {
  const resolvedPath = resolveSshConfigHomePath(keyPath)
  const identity = await readBoundedKeyFile(resolvedPath)
+  // Why: a present private key wins; a `.pub` beside it may describe a key already replaced.
  if (identity !== null) {
-    return {
-      privateIdentityExists: true,
-      requiresSystemSsh:
-        isOpenSshSecurityKeyPublicKey(identity) || isOpenSshSecurityKeyPrivateKey(identity)
-    }
+    return isOpenSshSecurityKeyPublicKey(identity) || isOpenSshSecurityKeyPrivateKey(identity)
  }
  const publicIdentity = await readBoundedKeyFile(`${resolvedPath}.pub`)
-  return {
-    privateIdentityExists: false,
-    requiresSystemSsh: publicIdentity !== null && isOpenSshSecurityKeyPublicKey(publicIdentity)
-  }
+  return publicIdentity !== null && isOpenSshSecurityKeyPublicKey(publicIdentity)
 }

 export function shouldUseSystemSshTransport(
@@ -102,16 +94,19 @@ export async function requiresSystemSshForSecurityKey(
  target: SshTarget,
  resolved: Pick<SshResolvedConfig, 'identityFile'> | null
 ): Promise<boolean> {
-  const configuredPaths = resolveIdentityFilePaths(target, resolved)
-  const usesDefaultPaths = configuredPaths.length === 0 && !resolved && !target.identityFile
-  const identityPaths = usesDefaultPaths ? listDefaultIdentityFilePaths() : configuredPaths
+  const resolvedPaths = resolveIdentityFilePaths(target, resolved)
+  // Why: `ssh -G` already echoes OpenSSH's built-in defaults, so its list is the real candidate
+  // set; guess at the defaults only when config resolution failed outright.
+  const identityPaths =
+    resolvedPaths.length === 0 && !resolved && !target.identityFile
+      ? listDefaultIdentityFilePaths()
+      : resolvedPaths
  for (const keyPath of identityPaths) {
-    const inspection = await inspectIdentityPath(keyPath)
-    if (inspection.requiresSystemSsh) {
-      return !usesDefaultPaths || findSystemSsh() !== null
-    }
-    if (usesDefaultPaths && inspection.privateIdentityExists) {
-      return false
+    if (await identityRequiresSystemSsh(keyPath)) {
+      // Why: scan every candidate — an earlier normal key never rules out a security key the host
+      // requires — but forcing system transport with no binary hard-fails a connection ssh2 could
+      // still have served over agent or password auth.
+      return findSystemSsh() !== null
    }
  }
  return false