From 3c3a3c689c2f34cf4a44233fbbf39e89a2577e4a Mon Sep 17 00:00:00 2001 From: vam Date: Fri, 21 Aug 2026 15:08:33 +0800 Subject: [PATCH] fix: flag a config host whose machine is already registered under another name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit searchSshConfigHosts decided "already in Orca" by comparing normalized alias strings — a target's configHost and label — against the config entry's Host pattern. Two names for one machine therefore both pass: manual { label: 'AI Box', configHost: '172.16.203.131', host: '172.16.203.131' } picker Host aimp / HostName 172.16.203.131 Nothing in either record is string-equal, so the picker lists `aimp` as new and the user gets a second target for a box they already added. Both connect to the same endpoint and attach to the same relay daemon, so the two entries list the same remote sessions and operating either one drives the same underlying ones. Add sshEndpointIdentity — host, port and user, normalized and NUL-separated — and treat a candidate as already registered when either its alias or its endpoint matches. Multiple pathspecs of identity, same as before, just no longer name-only. Only entries carrying HostName participate: without it the candidate host degrades to the alias itself, and matching on that would re-run the alias check while letting unrelated aliases collide. That also bounds what this fixes — a Host block that resolves through DNS rather than HostName still yields two targets, because neither the parser nor `ssh -G` resolves names. alreadyInOrca is a display flag, not a gate, so a deliberate second target for the same endpoint (different identity file, different jump host) still gets added — it just says so first. Fixes #15478 Co-Authored-By: Claude Opus 5 (1M context) --- src/main/ssh/ssh-config-host-picker.test.ts | 52 ++++++++++++++++++++ src/main/ssh/ssh-config-host-picker.ts | 23 +++++++-- src/shared/ssh-config-alias.test.ts | 53 +++++++++++++++++++++ src/shared/ssh-config-alias.ts | 15 ++++++ 4 files changed, 139 insertions(+), 4 deletions(-) create mode 100644 src/shared/ssh-config-alias.test.ts diff --git a/src/main/ssh/ssh-config-host-picker.test.ts b/src/main/ssh/ssh-config-host-picker.test.ts index 783eef679ca..3ac84902dbf 100644 --- a/src/main/ssh/ssh-config-host-picker.test.ts +++ b/src/main/ssh/ssh-config-host-picker.test.ts @@ -57,6 +57,58 @@ describe('SSH config host picker search', () => { expect(last.hasMore).toBe(false) }) + it('flags a config host whose machine is already registered under another label', () => { + // Why: the same box gets added twice — once manually by IP, once from the config + // picker — and alias comparison cannot see that because the names differ. + const result = searchSshConfigHosts( + [{ host: 'aimp', hostname: '172.16.203.131', user: 'chanmuzi' }], + [ + { + configHost: '172.16.203.131', + label: 'AI Box', + host: '172.16.203.131', + port: 22, + username: 'chanmuzi' + } + ] + ) + + expect(result.hosts[0]).toMatchObject({ alias: 'aimp', alreadyInOrca: true }) + expect(result.newHostCount).toBe(0) + }) + + it('leaves a different machine unflagged', () => { + const result = searchSshConfigHosts( + [{ host: 'aimp', hostname: '172.16.203.131', user: 'chanmuzi' }], + [ + { + configHost: '10.0.0.9', + label: 'Other Box', + host: '10.0.0.9', + port: 22, + username: 'chanmuzi' + } + ] + ) + + expect(result.hosts[0]).toMatchObject({ alias: 'aimp', alreadyInOrca: false }) + expect(result.newHostCount).toBe(1) + }) + + it('does not collide config hosts that carry no HostName', () => { + // Why: without HostName the candidate host degrades to the alias itself, so the + // endpoint check must not start matching unrelated aliases against each other. + const result = searchSshConfigHosts( + [{ host: 'alpha' }, { host: 'beta' }], + [{ configHost: 'gamma', label: 'Gamma', host: 'gamma', port: 22, username: '' }] + ) + + expect(result.hosts.map((host) => [host.alias, host.alreadyInOrca])).toEqual([ + ['alpha', false], + ['beta', false] + ]) + }) + it('counts de-duplicated and existing aliases while matching effective display fields', () => { const result = searchSshConfigHosts( [ diff --git a/src/main/ssh/ssh-config-host-picker.ts b/src/main/ssh/ssh-config-host-picker.ts index d1870ad1b69..3fcc98859c1 100644 --- a/src/main/ssh/ssh-config-host-picker.ts +++ b/src/main/ssh/ssh-config-host-picker.ts @@ -5,7 +5,7 @@ import type { SshTarget } from '../../shared/ssh-types' import { SSH_CONFIG_HOST_RESULT_LIMIT } from '../../shared/ssh-types' -import { normalizeSshConfigAlias } from '../../shared/ssh-config-alias' +import { normalizeSshConfigAlias, sshEndpointIdentity } from '../../shared/ssh-config-alias' import { loadUserSshConfig, type SshConfigHost } from './ssh-config-parser' import { resolveWithSshG, type SshResolvedConfig } from './ssh-g-config-resolution' @@ -27,7 +27,10 @@ function getUserSshConfigHosts(refresh: boolean): SshConfigHost[] { export function searchSshConfigHosts( hosts: SshConfigHost[], - existingTargets: readonly Pick[], + // Why partial: callers that only know the names still type-check, and an entry with + // no host yields an empty identity rather than a false match. + existingTargets: readonly (Pick & + Partial>)[], query = '', suppressedAliases: readonly string[] = [] ): SshConfigHostListResult { @@ -36,6 +39,9 @@ export function searchSshConfigHosts( [target.configHost, target.label].map(normalizeSshConfigAlias).filter(Boolean) ) ) + // Why: the same machine can be registered twice under different names — once by IP, + // once from this picker — and comparing names cannot see that. + const existingEndpoints = new Set(existingTargets.map(sshEndpointIdentity).filter(Boolean)) const normalizedQuery = query.trim().toLowerCase() const suppressedAliasSet = new Set(suppressedAliases.map(normalizeSshConfigAlias)) const summaries: SshConfigHostSummary[] = [] @@ -50,7 +56,13 @@ export function searchSshConfigHosts( continue } seenAliases.add(normalizedAlias) - const alreadyInOrca = existingAliases.has(normalizedAlias) + // Why `entry.hostname` only: without HostName the candidate host degrades to the + // alias itself, and matching on that would just re-run the alias check. + const endpoint = entry.hostname + ? sshEndpointIdentity({ host: entry.hostname, port: entry.port, username: entry.user }) + : '' + const alreadyInOrca = + existingAliases.has(normalizedAlias) || (endpoint !== '' && existingEndpoints.has(endpoint)) // Why: tombstones only block passive bulk import — the picker still lists the // Host so deleting one Orca target never looks like "~/.ssh/config is empty". const previouslyRemoved = !alreadyInOrca && suppressedAliasSet.has(normalizedAlias) @@ -76,7 +88,10 @@ export function searchSshConfigHosts( } export function listUserSshConfigHostSummaries( - existingTargets: readonly Pick[], + // Why partial: callers that only know the names still type-check, and an entry with + // no host yields an empty identity rather than a false match. + existingTargets: readonly (Pick & + Partial>)[], query?: string, suppressedAliases?: readonly string[], options?: { refresh?: boolean } diff --git a/src/shared/ssh-config-alias.test.ts b/src/shared/ssh-config-alias.test.ts new file mode 100644 index 00000000000..1e154b38bdb --- /dev/null +++ b/src/shared/ssh-config-alias.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from 'vitest' +import { sshEndpointIdentity } from './ssh-config-alias' + +describe('sshEndpointIdentity', () => { + it('treats the same machine reached under different names as one endpoint', () => { + const manual = sshEndpointIdentity({ + host: '172.16.203.131', + port: 22, + username: 'chanmuzi' + }) + const fromConfig = sshEndpointIdentity({ + host: '172.16.203.131', + port: 22, + username: 'chanmuzi' + }) + + expect(manual).toBe(fromConfig) + }) + + it('defaults a missing port to 22 so an explicit 22 matches an omitted one', () => { + expect(sshEndpointIdentity({ host: 'box', username: 'me' })).toBe( + sshEndpointIdentity({ host: 'box', port: 22, username: 'me' }) + ) + }) + + it('separates host, port and user so no concatenation can alias them', () => { + // Why: 'a' + '1:2' and 'a:1' + '2' must not collapse into the same key. + expect(sshEndpointIdentity({ host: 'a', port: 12, username: '' })).not.toBe( + sshEndpointIdentity({ host: 'a', port: 1, username: '2' }) + ) + }) + + it('folds host and user case but keeps a different user distinct', () => { + expect(sshEndpointIdentity({ host: 'BOX', port: 22, username: 'Me' })).toBe( + sshEndpointIdentity({ host: 'box', port: 22, username: 'me' }) + ) + expect(sshEndpointIdentity({ host: 'box', port: 22, username: 'me' })).not.toBe( + sshEndpointIdentity({ host: 'box', port: 22, username: 'other' }) + ) + }) + + it('is empty for an unknown host so unresolved entries never collide', () => { + expect(sshEndpointIdentity({ host: '', port: 22, username: 'me' })).toBe('') + expect(sshEndpointIdentity({ host: ' ', port: 22, username: 'me' })).toBe('') + expect(sshEndpointIdentity({ port: 22, username: 'me' })).toBe('') + }) + + it('ignores a non-finite port rather than keying on NaN', () => { + expect(sshEndpointIdentity({ host: 'box', port: Number.NaN, username: 'me' })).toBe( + sshEndpointIdentity({ host: 'box', port: 22, username: 'me' }) + ) + }) +}) diff --git a/src/shared/ssh-config-alias.ts b/src/shared/ssh-config-alias.ts index ea93dd75de4..b5800c08cab 100644 --- a/src/shared/ssh-config-alias.ts +++ b/src/shared/ssh-config-alias.ts @@ -6,3 +6,18 @@ export function normalizeSshConfigAlias(alias: string | null | undefined): string { return alias ? alias.trim().toLowerCase() : '' } + +/** The machine a target actually reaches: hostname, port and user, normalized. + * Empty when the host is unknown, so an unresolved alias never collides. */ +export function sshEndpointIdentity(target: { + host?: string | null + port?: number | null + username?: string | null +}): string { + const host = target.host?.trim().toLowerCase() ?? '' + if (!host) { + return '' + } + const port = typeof target.port === 'number' && Number.isFinite(target.port) ? target.port : 22 + return `${host}\u0000${port}\u0000${target.username?.trim().toLowerCase() ?? ''}` +}