mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
fix: flag a config host whose machine is already registered under another name
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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(
|
||||
[
|
||||
|
||||
@@ -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<SshTarget, 'configHost' | 'label'>[],
|
||||
// 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<SshTarget, 'configHost' | 'label'> &
|
||||
Partial<Pick<SshTarget, 'host' | 'port' | 'username'>>)[],
|
||||
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<SshTarget, 'configHost' | 'label'>[],
|
||||
// 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<SshTarget, 'configHost' | 'label'> &
|
||||
Partial<Pick<SshTarget, 'host' | 'port' | 'username'>>)[],
|
||||
query?: string,
|
||||
suppressedAliases?: readonly string[],
|
||||
options?: { refresh?: boolean }
|
||||
|
||||
@@ -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' })
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -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() ?? ''}`
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user