From f2070df6dbd367b62e211f8dc4692274bea54df2 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 14 Aug 2026 03:47:03 -0700 Subject: [PATCH] test(ssh): pin the parser against real ssh -G output, and accept-new against ask MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps the audit named. The ssh -G fixtures were all hand-written, which means they encode what I expect ssh to print. This one is verbatim OpenSSH_10.2p1 output for a Host block using HostName, HostKeyAlias, StrictHostKeyChecking accept-new, two UserKnownHostsFile paths and a non-default port. The format detail that matters: each file list arrives space-separated on ONE line, so reading it as a single path would consult nothing for anyone with more than one file configured. And accept-new was untested despite being a real OpenSSH value. It currently behaves identically to ask, which is exactly right while no dialog exists and is what lets the defence ship without a modal — so the equivalence is now pinned, and Phase 2 has to break it deliberately rather than discover it. Plus a guard that accept-new never falls into the strict branch. --- src/main/ssh/ssh-host-key-decision.test.ts | 17 +++++++ src/main/ssh/ssh-known-hosts-source.test.ts | 49 +++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/src/main/ssh/ssh-host-key-decision.test.ts b/src/main/ssh/ssh-host-key-decision.test.ts index ce6bc350d68..3d0e6d788ee 100644 --- a/src/main/ssh/ssh-host-key-decision.test.ts +++ b/src/main/ssh/ssh-host-key-decision.test.ts @@ -104,6 +104,23 @@ describe('deciding what to do with a presented host key', () => { expect(decideHostKey(input({ strictHostKeyChecking: value })).action).toBe('accept') }) + // Phase 1 has no dialog, so `ask` behaves as `accept-new` — deliberate, and the reason the + // whole defence can ship without a modal. This pins the equivalence so Phase 2 has to break it + // ON PURPOSE: `accept-new` must still not prompt, `ask` must. + it('treats accept-new and ask alike while no dialog exists', () => { + const acceptNew = decideHostKey(input({ strictHostKeyChecking: 'accept-new' })) + const ask = decideHostKey(input({ strictHostKeyChecking: 'ask' })) + expect(acceptNew.action).toBe('accept-and-remember') + expect(ask.action).toBe(acceptNew.action) + }) + + // accept-new is a real OpenSSH value, not a typo — it must never fall into the strict branch. + it('does not treat accept-new as strict', () => { + expect(decideHostKey(input({ strictHostKeyChecking: 'accept-new' })).action).not.toBe( + 'reject' + ) + }) + it('treats an unrecognised value as ask', () => { expect(decideHostKey(input({ strictHostKeyChecking: 'banana' })).action).toBe( 'accept-and-remember' diff --git a/src/main/ssh/ssh-known-hosts-source.test.ts b/src/main/ssh/ssh-known-hosts-source.test.ts index 857cdc60b43..a282d3787c2 100644 --- a/src/main/ssh/ssh-known-hosts-source.test.ts +++ b/src/main/ssh/ssh-known-hosts-source.test.ts @@ -141,6 +141,55 @@ describe('resolveKnownHostsFiles', () => { }) }) +// Verbatim `ssh -F -G probe` output from OpenSSH_10.2p1 for a Host block setting HostName, +// HostKeyAlias, StrictHostKeyChecking accept-new, two UserKnownHostsFile paths and Port 2222. +// Hand-written fixtures elsewhere encode what we EXPECT ssh to print; this one is what it does +// print, which is the only version that catches us guessing the format wrong. +const REAL_SSH_G_OUTPUT = [ + 'hostname real.example.com', + 'port 2222', + 'checkhostip no', + 'hashknownhosts no', + 'stricthostkeychecking accept-new', + 'updatehostkeys false', + 'hostkeyalias alias-host', + 'globalknownhostsfile /etc/ssh/ssh_known_hosts /etc/ssh/ssh_known_hosts2', + 'userknownhostsfile /Users/nwparker/.ssh/kh_one /Users/nwparker/.ssh/kh_two' +].join('\n') + +describe('parsing real ssh -G output', () => { + const resolved = parseSshGOutput(REAL_SSH_G_OUTPUT) + + // Every file list arrives space-separated on ONE line, not repeated per line. Reading it as a + // single path would silently consult nothing for anyone with more than one file configured. + it('splits both known_hosts lists on one line each', () => { + expect(resolved.userKnownHostsFiles).toEqual([ + '/Users/nwparker/.ssh/kh_one', + '/Users/nwparker/.ssh/kh_two' + ]) + expect(resolved.globalKnownHostsFiles).toEqual([ + '/etc/ssh/ssh_known_hosts', + '/etc/ssh/ssh_known_hosts2' + ]) + }) + + // Drives the whole strict branch; a miss defaults it to 'ask' and quietly loses the user's policy. + it('reads StrictHostKeyChecking, including accept-new', () => { + expect(resolved.strictHostKeyChecking).toBe('accept-new') + }) + + // The one name that outranks the dialed host. A bastion tunnelled through localhost:port depends + // on it, and it is the only field allowed to override what we key the lookup on. + it('reads HostKeyAlias and prefers it for the lookup', () => { + expect(resolved.hostKeyAlias).toBe('alias-host') + expect(resolveKnownHostsLookupHost(resolved, '127.0.0.1')).toBe('alias-host') + }) + + it('reads the non-default port', () => { + expect(resolved.port).toBe(2222) + }) +}) + describe('loadKnownHostsEntries', () => { it('unions the entries of every file', async () => { const root = await createRoot()