diff --git a/src/main/ssh/ssh-connection.test.ts b/src/main/ssh/ssh-connection.test.ts index d53e2684062..8910e3ac9ed 100644 --- a/src/main/ssh/ssh-connection.test.ts +++ b/src/main/ssh/ssh-connection.test.ts @@ -461,6 +461,25 @@ describe('SshConnection', () => { expect(conn.getState().error).toMatch(/host key verification failed/i) }) + // Everyone's first connection happens before ~/.ssh/known_hosts exists — ssh creates it on its + // own first connect. Treating a file that was never there as a source we failed to read would + // refuse every connection a new profile ever makes, which is how a fail-closed rule turns into a + // product that does not work. + it('connects on first contact when no known_hosts file exists yet', async () => { + const emptyHome = mkdtempSync(join(tmpdir(), 'orca-ssh-home-')) + vi.stubEnv('HOME', emptyHome) + + try { + const conn = new SshConnection(createTarget(), createCallbacks()) + await conn.connect() + + expect(lastHostKeyAccepted).toBe(true) + expect(conn.getState().status).toBe('connected') + } finally { + rmSync(emptyHome, { recursive: true, force: true }) + } + }) + // Retrying re-derives the same decision, so a ladder that treated this as transient would back off // against a host it has already refused until it gave up — burying the reason. it('does not retry a refused host key', async () => { diff --git a/src/main/ssh/ssh-connection.ts b/src/main/ssh/ssh-connection.ts index 2302f1ad705..42d64714154 100644 --- a/src/main/ssh/ssh-connection.ts +++ b/src/main/ssh/ssh-connection.ts @@ -1265,14 +1265,12 @@ export class SshConnection { displayHost: hostKeyLookupHost, strictHostKeyChecking: hostKeyResolved?.strictHostKeyChecking ?? 'ask', isEphemeralRuntimeTarget: this.target.owner?.type === 'on-demand-runtime', - // No readable source at all is not evidence of a new host — it is the absence of evidence, - // and recording trust from it would let a changed key be accepted the one time we could not - // check. Reuses the strict path rather than adding a fifth outcome. - siteConfigSuppressed: - siteConfigSuppressed || - (knownHostsEvidence.configuredFileCount > 0 && - knownHostsEvidence.readableFileCount === 0 && - trustedHostKeys.length === 0), + // A source that exists but will not open is the absence of evidence, not evidence of a new + // host: the entry that would have said "this key changed" may be in the file we could not + // read, so recording trust would accept a changed key the one time we could not check. + // Reuses the strict path rather than adding a fifth outcome. An ABSENT file is not this — + // that is the normal state for a fresh profile and genuinely means nothing is known. + siteConfigSuppressed: siteConfigSuppressed || knownHostsEvidence.unreadableFileCount > 0, entries: knownHostsEntries, isTrusted: ({ host, port, keyType, key }) => { const forHost = trustedHostKeys.filter( diff --git a/src/main/ssh/ssh-known-hosts-source.test.ts b/src/main/ssh/ssh-known-hosts-source.test.ts index 68da1217b52..857cdc60b43 100644 --- a/src/main/ssh/ssh-known-hosts-source.test.ts +++ b/src/main/ssh/ssh-known-hosts-source.test.ts @@ -7,6 +7,7 @@ import { matchKnownHosts, readHostKeyType, type KnownHostsEntry } from './ssh-kn import { defaultKnownHostsFiles, loadKnownHostsEntries, + loadKnownHostsEvidence, resolveKnownHostsFiles, resolveKnownHostsLookupHost } from './ssh-known-hosts-source' @@ -215,6 +216,71 @@ describe('loadKnownHostsEntries', () => { }) }) +// The caller turns this count into a refusal, so the line between the two cases is the line between +// "every connection works" and "no connection works". +describe('loadKnownHostsEvidence', () => { + // A profile that has never connected has no known_hosts — ssh writes one on its own first + // connect. Counting that as a source we failed to read would refuse every first connection every + // new user ever makes. + it('does not count an absent file as unreadable', async () => { + const root = await createRoot() + + const evidence = await loadKnownHostsEvidence([ + join(root, 'known_hosts'), + join(root, 'known_hosts2') + ]) + + expect(evidence.unreadableFileCount).toBe(0) + expect(evidence.entries).toEqual([]) + }) + + it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( + 'counts a file that exists but will not open', + async () => { + const root = await createRoot() + const unreadable = join(root, 'known_hosts') + await writeFile(unreadable, hostLine('secret.example', ED_B)) + await chmod(unreadable, 0o000) + + const evidence = await loadKnownHostsEvidence([unreadable]) + + // The entry that would have said "this key changed" could be in here; we cannot tell. + expect(evidence.unreadableFileCount).toBe(1) + } + ) + + it('counts a path that is a directory', async () => { + const root = await createRoot() + await mkdir(join(root, 'a_directory')) + + const evidence = await loadKnownHostsEvidence([join(root, 'a_directory')]) + + expect(evidence.unreadableFileCount).toBe(1) + }) + + it('reports nothing unreadable when every file parses', async () => { + const root = await createRoot() + const present = join(root, 'known_hosts') + await writeFile(present, hostLine('alpha.example', ED_A)) + + const evidence = await loadKnownHostsEvidence([present]) + + expect(evidence.unreadableFileCount).toBe(0) + expect(evidence.entries).toHaveLength(1) + }) + + // An empty file is a file we read successfully. Nothing is withheld, so nothing is suppressed. + it('does not count an empty file as unreadable', async () => { + const root = await createRoot() + const empty = join(root, 'known_hosts') + await writeFile(empty, '') + + const evidence = await loadKnownHostsEvidence([empty]) + + expect(evidence.unreadableFileCount).toBe(0) + }) +}) + describe('resolveKnownHostsLookupHost', () => { it('prefers HostKeyAlias over the resolved hostname', () => { // A bastion tunnelled through localhost:2200 would otherwise mismatch on every target. diff --git a/src/main/ssh/ssh-known-hosts-source.ts b/src/main/ssh/ssh-known-hosts-source.ts index 9bc1c58ca3b..d693d6d7ee8 100644 --- a/src/main/ssh/ssh-known-hosts-source.ts +++ b/src/main/ssh/ssh-known-hosts-source.ts @@ -43,12 +43,15 @@ export async function loadKnownHostsEntries(files: readonly string[]): Promise { try { - return { entries: parseKnownHosts(await readFile(path, 'utf8')), readable: true } - } catch { - // Missing, unreadable or a directory. A file we cannot read contributes no trust; the - // remaining files still decide, and the count tells the caller whether any spoke at all. - return { entries: [] as KnownHostsEntry[], readable: false } + return { entries: parseKnownHosts(await readFile(path, 'utf8')), unreadable: false } + } catch (err) { + const absent = (err as NodeJS.ErrnoException).code === 'ENOENT' + return { entries: [] as KnownHostsEntry[], unreadable: !absent } } }) ) return { entries: perFile.flatMap((file) => file.entries), - readableFileCount: perFile.filter((file) => file.readable).length, - configuredFileCount: files.length + unreadableFileCount: perFile.filter((file) => file.unreadable).length } }