tui: don't let a dead host-key prompt hold the keyboard

apps/cli/src/tui/app.ts

@@ -112,27 +112,42 @@ export class Tui {
     const existing = this.sessions.get(connection.id)
     if (existing) return existing
 
-    const session = await SshSession.connect(connection, {
-      knownHostsPath: knownHostsPath(),
-      // Without this the first connection to any host fails with "not in
-      // known_hosts and DiskPush was not given a way to ask about it" — true,
-      // and useless: the answer is a keystroke away.
-      onUnknownHostKey: (details) =>
-        new Promise<boolean>((resolve) => {
-          this.hostKey = {
-            host: details.host,
-            fingerprint: details.fingerprint,
-            keyType: details.keyType,
-            decide: (trust) => {
-              this.hostKey = null
-              resolve(trust)
-            },
-          }
-          this.render()
-        }),
-    })
-    this.sessions.set(connection.id, session)
-    return session
+    try {
+      const session = await SshSession.connect(connection, {
+        knownHostsPath: knownHostsPath(),
+        // Without this the first connection to any host fails with "not in
+        // known_hosts and DiskPush was not given a way to ask about it" — true,
+        // and useless: the answer is a keystroke away.
+        onUnknownHostKey: (details) =>
+          new Promise<boolean>((resolve) => {
+            this.hostKey = {
+              host: details.host,
+              fingerprint: details.fingerprint,
+              keyType: details.keyType,
+              decide: (trust) => {
+                this.hostKey = null
+                resolve(trust)
+              },
+            }
+            this.render()
+          }),
+      })
+      this.sessions.set(connection.id, session)
+      return session
+    } finally {
+      // A connect that fails leaves its question on screen with nobody behind
+      // it. That is not a cosmetic leftover: the prompt owns the keyboard while
+      // it is up, so every key goes to a `decide` whose promise no one awaits
+      // any more -- arrows do nothing, and `q` does not quit. It is reached by
+      // simply not answering: readyTimeout fires after connectTimeoutSeconds,
+      // the connect rejects with "Timed out while waiting for handshake", and
+      // the question outlives the asker. The pane shows an error and the app
+      // looks frozen. Whoever asked is gone, so the question goes with them.
+      if (this.hostKey) {
+        this.hostKey = null
+        this.render()
+      }
+    }
   }
 
   async load(side: Side): Promise<void> {
@@ -232,6 +247,11 @@ export class Tui {
     if (isChar(key, CTRL_C)) return false
 
     if (this.hostKey) {
+      // Quit stays reachable from inside the prompt. Every other key is
+      // deliberately swallowed here -- a fingerprint is not something to
+      // dismiss by mashing -- but a dialog that can trap you in the app is
+      // worse than one you can leave, and `q` is the quit key everywhere else.
+      if (isChar(key, 'q')) return false
       if (isChar(key, 'y') || isChar(key, 'Y')) this.hostKey.decide(true)
       else if (key === 'escape' || isChar(key, 'n') || isChar(key, 'N') || key === 'enter') this.hostKey.decide(false)
       return true

apps/cli/src/tui/host-key-prompt.test.ts

@@ -0,0 +1,105 @@
+// The host-key prompt owns the keyboard while it is up, which is right — a
+// fingerprint should not be dismissable by mashing keys. What made that
+// dangerous is that it could outlive the connection that asked.
+//
+// Reached without doing anything unusual: point a pane at a host DiskPush has
+// not seen, then take fifteen seconds to compare the fingerprint. ssh2's
+// readyTimeout fires, connect rejects with "Timed out while waiting for
+// handshake", the pane shows that error -- and the question stays on screen
+// with nobody behind it. Every key then goes to a `decide` whose promise no one
+// awaits: arrows do nothing, and because the branch only ever returned true,
+// `q` did not quit either. The app is frozen and the only way out is Ctrl-C.
+//
+// Found by driving the real TUI over a pty against a real sshd and watching it
+// stop responding after a sync to a remote endpoint.
+import { describe, expect, it, vi } from 'vitest'
+
+type HostKeyDetails = { host: string; fingerprint: string; keyType: string }
+type ConnectOptions = { onUnknownHostKey: (d: HostKeyDetails) => Promise<boolean> }
+
+// Reassigned per test, so the mock is declared once and the behaviour varies.
+let connectImpl: (connection: unknown, options: ConnectOptions) => Promise<unknown> = async () => ({})
+
+vi.mock('@diskpush/ssh-core', () => ({
+  SshSession: { connect: (c: unknown, o: ConnectOptions) => connectImpl(c, o) },
+  SftpBrowser: { open: async () => ({ list: async () => [], close: () => {} }) },
+}))
+vi.mock('@diskpush/database', () => ({ knownHostsPath: () => '/tmp/known_hosts.test' }))
+
+const { Tui, blankPane } = await import('./app.js')
+type Tui = InstanceType<typeof Tui>
+
+/** A Tui with its private host-key prompt set, as a failed connect would leave it. */
+function tuiWithPrompt(onDecide: (trust: boolean) => void = () => {}) {
+  const tui = new Tui(blankPane('Local', '/tmp/a'), blankPane('Local', '/tmp/b'))
+  // Writing to stdout during a test would scribble on the reporter.
+  ;(tui as unknown as { render: () => void }).render = () => {}
+  ;(tui as unknown as { hostKey: unknown }).hostKey = {
+    host: 'example.test',
+    fingerprint: 'SHA256:abc',
+    keyType: 'ssh-ed25519',
+    decide: onDecide,
+  }
+  return tui
+}
+
+const prompt = (tui: InstanceType<typeof Tui>) => (tui as unknown as { hostKey: unknown }).hostKey
+
+describe('the host-key prompt', () => {
+  it('lets q quit rather than trapping the app', async () => {
+    const tui = tuiWithPrompt()
+    // false means "stop the app" to the caller in commands/tui.ts.
+    await expect(tui.onKey({ char: 'q' })).resolves.toBe(false)
+  })
+
+  it('still answers y and n, and keeps swallowing everything else', async () => {
+    const answers: boolean[] = []
+    const yes = tuiWithPrompt((trust) => answers.push(trust))
+    await expect(yes.onKey({ char: 'y' })).resolves.toBe(true)
+    expect(answers).toEqual([true])
+
+    const no = tuiWithPrompt((trust) => answers.push(trust))
+    await expect(no.onKey({ char: 'n' })).resolves.toBe(true)
+    expect(answers).toEqual([true, false])
+
+    const esc = tuiWithPrompt((trust) => answers.push(trust))
+    await expect(esc.onKey('escape')).resolves.toBe(true)
+    expect(answers).toEqual([true, false, false])
+
+    // An arrow must not leak past the prompt into the file panes behind it.
+    const other = tuiWithPrompt((trust) => answers.push(trust))
+    await expect(other.onKey('down')).resolves.toBe(true)
+    await expect(other.onKey('tab')).resolves.toBe(true)
+    expect(answers).toEqual([true, false, false])
+    expect(prompt(other)).not.toBeNull()
+  })
+
+  it('is cleared when the connect that raised it fails', async () => {
+    // The real session() path, with only the network stubbed: connect raises
+    // the question exactly as ssh2 would and then rejects the way readyTimeout
+    // does, with nobody having answered.
+    connectImpl = async (_connection, options) => {
+      void options.onUnknownHostKey({
+        host: 'example.test',
+        fingerprint: 'SHA256:abc',
+        keyType: 'ssh-ed25519',
+      })
+      throw new Error('Timed out while waiting for handshake')
+    }
+
+    const tui = new Tui(blankPane('Local', '/tmp/a'), blankPane('Local', '/tmp/b'))
+    ;(tui as unknown as { render: () => void }).render = () => {}
+    const session = (tui as unknown as {
+      session: (c: unknown) => Promise<unknown>
+    }).session.bind(tui)
+
+    await expect(session({ id: 'c1', host: 'example.test' })).rejects.toThrow(
+      'Timed out while waiting for handshake',
+    )
+
+    // Before the fix this was still set, and the app was unusable from here on.
+    expect(prompt(tui)).toBeNull()
+    // With the question gone, the keyboard comes back to the panes.
+    await expect(tui.onKey({ char: 'q' })).resolves.toBe(false)
+  })
+})