fix(ssh): offer the held-back challenge when a strict MaxAuthTries host drops the key retry (#29541)

When the agent-first attempt skips the host's keyboard-interactive challenge
so the deferred ~/.ssh key can go first, a host with MaxAuthTries=1
disconnects after rejecting that key, so the key retry never reaches the
challenge. Record when an attempt held the challenge back and when an
attempt actually offered it; if the key retry fails with an auth error
without offering it, run one attempt with no agent and no key that offers
only the challenge.

Fixes #29443

src/main/ssh/ssh-connection-auth-fallback.test.ts

@@ -1,4 +1,4 @@
-import { describe, expect, it, vi, beforeEach } from 'vitest'
+import { afterEach, describe, expect, it, vi, beforeEach } from 'vitest'
 import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
@@ -143,6 +143,96 @@ describe('SshConnection', () => {
     }
   })
 
+  describe('when the agent-first attempt held the challenge back for ~/.ssh/id_ed25519', () => {
+    let tempDir: string
+    const hostMethods = ['publickey' as const, 'keyboard-interactive' as const]
+    const settle = (): Promise<void> => new Promise((resolve) => setTimeout(resolve, 0))
+
+    beforeEach(async () => {
+      vi.stubEnv('SSH_AUTH_SOCK', '/tmp/agent.sock')
+      tempDir = mkdtempSync(join(tmpdir(), 'orca-ssh-home-'))
+      mkdirSync(join(tempDir, '.ssh'))
+      // Real key bytes and parser: deferral requires the waiting key to parse as unencrypted.
+      const realSsh2 = await vi.importActual<{ utils: typeof utils }>('ssh2')
+      writeFileSync(
+        join(tempDir, '.ssh', 'id_ed25519'),
+        realSsh2.utils.generateKeyPairSync('ecdsa', { bits: 256 }).private
+      )
+      vi.mocked(utils.parseKey).mockImplementation(realSsh2.utils.parseKey)
+      vi.stubEnv('HOME', tempDir)
+      vi.stubEnv('USERPROFILE', tempDir)
+    })
+
+    afterEach(() => {
+      vi.mocked(utils.parseKey).mockReset()
+      rmSync(tempDir, { recursive: true, force: true })
+    })
+
+    /** Runs the agent-first attempt to its skipped challenge and fails it, leaving the key retry. */
+    async function failAgentFirstAttempt(
+      conn: SshConnection
+    ): Promise<{ connected: Promise<void> }> {
+      const created = nextSshClientCreation()
+      const connected = conn.connect()
+      await created
+      await settle()
+      expect(walkInitialAuthLadder(clientInstances[0].lastConnectConfig, hostMethods)).toEqual([
+        'none',
+        'agent'
+      ])
+      const keyRetryCreated = nextSshClientCreation()
+      emitSshEvent('error', new Error('All configured authentication methods failed'))
+      await keyRetryCreated
+      await settle()
+      // Wrapped: an async function returning the promise itself would wait for the whole connect.
+      return { connected }
+    }
+
+    it('offers the challenge alone when a MaxAuthTries=1 host drops the key retry first', async () => {
+      ssh2Mock.connectSequence = ['silent', 'silent', 'ready']
+      const onCredentialRequest = vi.fn()
+      const conn = new SshConnection(createTarget(), createCallbacks({ onCredentialRequest }))
+
+      const { connected } = await failAgentFirstAttempt(conn)
+      // The host disconnects after the rejected key, before the retry reaches keyboard-interactive.
+      expect(walkInitialAuthLadder(clientInstances[1].lastConnectConfig, hostMethods, 2)).toEqual([
+        'none',
+        'publickey'
+      ])
+      const challengeCreated = nextSshClientCreation()
+      emitSshEvent('error', new Error('Received disconnect: Too many authentication failures'))
+      await challengeCreated
+      await connected
+
+      expect(clientInstances).toHaveLength(3)
+      const challengeConfig = clientInstances[2].lastConnectConfig
+      expect(challengeConfig).not.toHaveProperty('agent')
+      expect(challengeConfig).not.toHaveProperty('privateKey')
+      expect(walkInitialAuthLadder(challengeConfig, hostMethods)).toEqual([
+        'none',
+        'keyboard-interactive'
+      ])
+      expect(onCredentialRequest).not.toHaveBeenCalled()
+    })
+
+    it('does not repeat the challenge when the key retry already offered it', async () => {
+      ssh2Mock.connectSequence = ['silent', 'silent']
+      const onCredentialRequest = vi.fn(async () => null)
+      const conn = new SshConnection(createTarget(), createCallbacks({ onCredentialRequest }))
+
+      const { connected } = await failAgentFirstAttempt(conn)
+      expect(walkInitialAuthLadder(clientInstances[1].lastConnectConfig, hostMethods)).toEqual([
+        'none',
+        'publickey',
+        'keyboard-interactive'
+      ])
+      emitSshEvent('error', new Error('All configured authentication methods failed'))
+
+      await expect(connected).rejects.toThrow('All configured authentication methods failed')
+      expect(clientInstances).toHaveLength(2)
+    })
+  })
+
   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-'))

src/main/ssh/ssh-connection-test-fixtures.ts

@@ -17,10 +17,14 @@ export function createTarget(overrides?: Partial<SshTarget>): SshTarget {
   }
 }
 
-/** Walks an attempt's initial auth ladder against a host that keeps listing `hostMethods`. */
+/**
+ * Walks an attempt's initial auth ladder against a host that keeps listing `hostMethods`, stopping
+ * after `maxAttempts` methods as a host that disconnects at its MaxAuthTries would.
+ */
 export function walkInitialAuthLadder(
   config: unknown,
-  hostMethods: AuthenticationType[]
+  hostMethods: AuthenticationType[],
+  maxAttempts = Number.POSITIVE_INFINITY
 ): string[] {
   const authHandler =
     typeof config === 'object' && config !== null && 'authHandler' in config
@@ -38,7 +42,7 @@ export function walkInitialAuthLadder(
   const walked: (AuthenticationType | AnyAuthMethod)[] = []
   let exhausted = false
   let firstCall = true
-  while (!exhausted) {
+  while (!exhausted && walked.length < maxAttempts) {
     handler(firstCall ? null : hostMethods, false, (attempt) => {
       if (attempt === false) {
         exhausted = true

src/main/ssh/ssh-connection-utils.ts

@@ -183,6 +183,8 @@ export function createSshOperationAbortError(): Error & { name: string } {
 export type BuildConnectConfigOptions = {
   includeAgent?: boolean
   includePrivateKey?: boolean
+  /** No agent and no key: only the host's keyboard-interactive challenge (and a cached password). */
+  keyboardInteractiveOnly?: boolean
 }
 
 // Why: ssh2 tries privateKey before agent, but parses encrypted privateKey
@@ -212,7 +214,7 @@ export function buildConnectConfig(
     tryKeyboard: true
   }
 
-  const shouldIncludeAgent = options.includeAgent ?? true
+  const shouldIncludeAgent = !options.keyboardInteractiveOnly && (options.includeAgent ?? true)
   const agentSocket = shouldIncludeAgent ? resolveAgentSocket(target, resolved) : undefined
   const agent = agentSocket ? resolveAgentConfigValue(agentSocket, target, resolved) : undefined
 
@@ -224,8 +226,9 @@ export function buildConnectConfig(
     config.agentForward = true
   }
 
-  const keys =
-    (options.includePrivateKey ?? !agent)
+  const keys = options.keyboardInteractiveOnly
+    ? []
+    : (options.includePrivateKey ?? !agent)
       ? resolvePrivateKeys(target, resolved)
       : resolveUnencryptedExplicitPrivateKeys(target, resolved)
   let deferralDecision: boolean | undefined
@@ -243,7 +246,7 @@ export function buildConnectConfig(
       // Decided lazily so the deferred files are read only once the agent has failed, as the retry
       // would read them anyway, never on an agent login that needs no file.
       deferKeyboardInteractive:
-        keys.length === 0
+        keys.length === 0 && !options.keyboardInteractiveOnly
           ? () => {
               if (deferralDecision === undefined) {
                 const deferredKeys = resolvePrivateKeys(target, resolved)

src/main/ssh/ssh-connection.ts

@@ -80,7 +80,11 @@ import {
 } from './ssh-reconnect-error-classification'
 import { SshReconnectLadder } from './ssh-reconnect-ladder'
 import { mayUserSshConfigClaimAlias } from './ssh-config-alias-claim'
-import { getPassphrasePrivateKeyPath } from './ssh-private-key-authentication'
+import {
+  didDeferKeyboardInteractive,
+  didOfferKeyboardInteractive,
+  getPassphrasePrivateKeyPath
+} from './ssh-private-key-authentication'
 import {
   requiresSystemSshForSecurityKey,
   shouldUseSystemSshTransport
@@ -735,6 +739,32 @@ export class SshConnection {
                 return
               }
             }
+            // Why: a strict MaxAuthTries host can drop the key retry before it reaches the challenge
+            // the agent-first attempt held back for that key, so offer the challenge on its own.
+            if (
+              isAgentFallbackError(authError) &&
+              didDeferKeyboardInteractive(config) &&
+              !didOfferKeyboardInteractive(keyConfig)
+            ) {
+              const challengeConfig = buildConnectConfig(this.target, resolved, {
+                keyboardInteractiveOnly: true
+              })
+              this.respawnProxy(challengeConfig, effectiveProxy, connectGeneration)
+              try {
+                await this.doSsh2Connect(challengeConfig, connectGeneration)
+                return
+              } catch (challengeErr) {
+                if (
+                  !(challengeErr instanceof Error) ||
+                  isHostKeyVerificationError(challengeErr) ||
+                  this.keyboardInteractiveCancelled
+                ) {
+                  this.killProxyIfCurrent(connectGeneration)
+                  throw challengeErr
+                }
+                authError = challengeErr
+              }
+            }
           }
         }
       }

src/main/ssh/ssh-private-key-authentication.ts

@@ -3,6 +3,8 @@ import type { PrivateKeyFile } from './ssh-auth-resolution'
 import { observeAgentSigning } from './ssh-agent-sign-observer'
 
 const passphraseKeyPaths = new WeakMap<ConnectConfig, string>()
+const deferredKeyboardInteractive = new WeakSet<ConnectConfig>()
+const offeredKeyboardInteractive = new WeakSet<ConnectConfig>()
 
 // Bounds an `AuthenticationMethods a,b,c` ladder so a host that keeps replying
 // "partial success" cannot keep the client prompting forever.
@@ -99,12 +101,26 @@ export function configurePrivateKeyAuthentication(
       authsLeft.includes('publickey') &&
       options.deferKeyboardInteractive?.()
     ) {
+      deferredKeyboardInteractive.add(config)
       attempt = queue.shift()
     }
+    if (attempt === 'keyboard-interactive') {
+      offeredKeyboardInteractive.add(config)
+    }
     next((attempt ?? false) as Parameters<NextAuthHandler>[0])
   }
 }
 
+/** True once this config's handler held the host's first-factor challenge back for a deferred key. */
+export function didDeferKeyboardInteractive(config: ConnectConfig): boolean {
+  return deferredKeyboardInteractive.has(config)
+}
+
+/** True once this config's handler sent keyboard-interactive to the host. */
+export function didOfferKeyboardInteractive(config: ConnectConfig): boolean {
+  return offeredKeyboardInteractive.has(config)
+}
+
 export function getPassphrasePrivateKeyPath(config: ConnectConfig): string | undefined {
   return passphraseKeyPaths.get(config)
 }