mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 00:03:45 +00:00
fix(ssh): gate the remaining #11297 config-backed branches on host-block evidence
Why: `ssh -G <vanished-alias>` still exits 0 with defaults, so the GitHub restricted-shell probe read the local account instead of a stored `git` username, and a defaulted `gssapiauthentication no` discarded a stored Kerberos flag. A resolved `yes` still wins so distro-wide defaults keep working. Also treats an undeterminable local account as no evidence, and matches ssh2's first-entry rule for the eager private key.
This commit is contained in:
@@ -180,6 +180,15 @@ function createTarget(overrides?: Partial<SshTarget>): SshTarget {
|
||||
}
|
||||
}
|
||||
|
||||
const LOCAL_ACCOUNT = 'localdev'
|
||||
|
||||
// Why: `ssh -G` echoes the local account as `user` for an alias with no Host block.
|
||||
function stubLocalAccount(): void {
|
||||
vi.stubEnv('USER', LOCAL_ACCOUNT)
|
||||
vi.stubEnv('LOGNAME', LOCAL_ACCOUNT)
|
||||
vi.stubEnv('USERNAME', LOCAL_ACCOUNT)
|
||||
}
|
||||
|
||||
function createResolvedConfig(overrides?: Partial<SshResolvedConfig>): SshResolvedConfig {
|
||||
return {
|
||||
hostname: 'example.com',
|
||||
@@ -1188,6 +1197,30 @@ describe('SshConnection', () => {
|
||||
expect(conn.usesSystemSshTransport()).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps the stored git username when the GitHub alias no longer has a Host block', async () => {
|
||||
stubLocalAccount()
|
||||
vi.mocked(resolveWithSshG).mockResolvedValueOnce(
|
||||
createResolvedConfig({ hostname: 'github.com', user: LOCAL_ACCOUNT })
|
||||
)
|
||||
spawnSystemSshCommandMock.mockImplementation(() =>
|
||||
createFailingSystemCommandChannel(1, 'Invalid command: echo ORCA-SYSTEM-SSH-OK')
|
||||
)
|
||||
const conn = new SshConnection(
|
||||
createTarget({
|
||||
source: 'ssh-config',
|
||||
configHost: 'github.com',
|
||||
host: 'github.com',
|
||||
username: 'git'
|
||||
}),
|
||||
createCallbacks()
|
||||
)
|
||||
|
||||
await conn.connect()
|
||||
|
||||
expect(conn.getState().status).toBe('connected')
|
||||
expect(conn.usesSystemSshTransport()).toBe(true)
|
||||
})
|
||||
|
||||
it('accepts GitHub restricted-shell SSH probes with resolved host and target username', async () => {
|
||||
vi.mocked(resolveWithSshG).mockResolvedValueOnce(
|
||||
createResolvedConfig({ hostname: 'github.com', user: undefined })
|
||||
@@ -1573,6 +1606,62 @@ describe('SshConnection', () => {
|
||||
expect(spawnSystemSshCommandMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps the stored GSSAPI flag when the alias no longer has a Host block', async () => {
|
||||
stubLocalAccount()
|
||||
// `ssh -G krb-box` with no matching block: echoed alias, local user, port 22, GSSAPI off.
|
||||
vi.mocked(resolveWithSshG).mockResolvedValue(
|
||||
createResolvedConfig({
|
||||
hostname: 'krb-box',
|
||||
user: LOCAL_ACCOUNT,
|
||||
proxyUseFdpass: false,
|
||||
gssapiAuthentication: false
|
||||
})
|
||||
)
|
||||
const conn = new SshConnection(
|
||||
createTarget({
|
||||
source: 'ssh-config',
|
||||
configHost: 'krb-box',
|
||||
host: 'krb-box',
|
||||
gssapiAuthentication: true
|
||||
}),
|
||||
createCallbacks()
|
||||
)
|
||||
|
||||
await conn.connect()
|
||||
|
||||
expect(conn.usesSystemSshTransport()).toBe(true)
|
||||
expect(spawnSystemSshCommandMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ gssapiAuthentication: true }),
|
||||
'echo ORCA-SYSTEM-SSH-OK',
|
||||
expect.objectContaining({ gssapiOnly: true, wrapCommand: false })
|
||||
)
|
||||
})
|
||||
|
||||
it('still probes GSSAPI from a distro-wide default when the alias has no Host block', async () => {
|
||||
stubLocalAccount()
|
||||
vi.mocked(resolveWithSshG).mockResolvedValue(
|
||||
createResolvedConfig({
|
||||
hostname: 'krb-box',
|
||||
user: LOCAL_ACCOUNT,
|
||||
proxyUseFdpass: false,
|
||||
gssapiAuthentication: true
|
||||
})
|
||||
)
|
||||
const conn = new SshConnection(
|
||||
createTarget({ source: 'ssh-config', configHost: 'krb-box', host: 'krb-box' }),
|
||||
createCallbacks()
|
||||
)
|
||||
|
||||
await conn.connect()
|
||||
|
||||
expect(conn.usesSystemSshTransport()).toBe(true)
|
||||
expect(spawnSystemSshCommandMock).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
'echo ORCA-SYSTEM-SSH-OK',
|
||||
expect.objectContaining({ gssapiOnly: true, wrapCommand: false })
|
||||
)
|
||||
})
|
||||
|
||||
it('falls back to system SSH after an ssh2 auth failure when resolved config enables GSSAPI', async () => {
|
||||
connectBehavior = 'error'
|
||||
connectErrorMessage = 'All configured authentication methods failed'
|
||||
|
||||
@@ -19,6 +19,7 @@ import {
|
||||
} from './ssh-system-fallback'
|
||||
import { resolveWithSshG, type SshResolvedConfig } from './ssh-config-parser'
|
||||
import { removeControlSocketPath } from './ssh-control-socket'
|
||||
import { hasOpenSshHostBlockMatch } from './ssh-g-host-block-match'
|
||||
import { isOpenSshConfigBackedTarget } from './system-ssh-args'
|
||||
import {
|
||||
INITIAL_RETRY_ATTEMPTS,
|
||||
@@ -85,8 +86,8 @@ function isGitHubRestrictedShellProbeSuccess(
|
||||
}
|
||||
|
||||
const effectiveUser = (
|
||||
isOpenSshConfigBackedTarget(target) && resolvedConfig
|
||||
? resolvedConfig.user?.trim() || target.username?.trim()
|
||||
hasOpenSshHostBlockMatch(target, resolvedConfig)
|
||||
? resolvedConfig?.user?.trim() || target.username?.trim()
|
||||
: target.username?.trim() || resolvedConfig?.user?.trim()
|
||||
)?.toLowerCase()
|
||||
if (effectiveUser !== 'git') {
|
||||
@@ -637,10 +638,12 @@ export class SshConnection {
|
||||
return
|
||||
}
|
||||
// Why: ssh2 lacks gssapi-with-mic; GSSAPIAuthentication hosts try Kerberos SSO via system OpenSSH first, then fall through to key/credential auth.
|
||||
// Why: without Host-block evidence a resolved `no` is just the /etc/ssh default, so it must not discard the stored flag; a resolved `yes` still wins so distro-wide defaults keep working.
|
||||
if (
|
||||
isOpenSshConfigBackedTarget(this.target) && resolved
|
||||
? resolved.gssapiAuthentication === true
|
||||
: this.target.gssapiAuthentication === true
|
||||
hasOpenSshHostBlockMatch(this.target, resolved)
|
||||
? resolved?.gssapiAuthentication === true
|
||||
: this.target.gssapiAuthentication === true ||
|
||||
(isOpenSshConfigBackedTarget(this.target) && resolved?.gssapiAuthentication === true)
|
||||
) {
|
||||
try {
|
||||
await this.doSystemSshProbeWithControlMasterRetry(connectGeneration, resolved, true)
|
||||
|
||||
@@ -188,6 +188,23 @@ describe('ssh -G host-block matching', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// A launchd/systemd-spawned main process has no USER/LOGNAME/USERNAME, so the
|
||||
// resolved user proves nothing and the stored fields have to win.
|
||||
it('ignores the resolved user when the local account cannot be determined', () => {
|
||||
vi.stubEnv('USER', '')
|
||||
vi.stubEnv('LOGNAME', '')
|
||||
vi.stubEnv('USERNAME', '')
|
||||
|
||||
const config = buildConnectConfig(
|
||||
storedTarget({ jumpHost: undefined }),
|
||||
unmatchedDefaults({ user: 'whoever' }),
|
||||
{ includeAgent: false, includePrivateKey: true }
|
||||
)
|
||||
|
||||
expect(config.username).toBe('deploy')
|
||||
expect(config.host).toBe('10.0.0.5')
|
||||
})
|
||||
|
||||
it('keeps manual targets on their stored fields regardless of ssh -G output', () => {
|
||||
const config = buildConnectConfig(
|
||||
storedTarget({ source: 'manual', configHost: undefined }),
|
||||
|
||||
@@ -11,16 +11,17 @@ function normalize(value: string | undefined): string {
|
||||
return value?.trim().toLowerCase() ?? ''
|
||||
}
|
||||
|
||||
// Why: `ssh -G` fills `user` from the local account whenever no User directive matched.
|
||||
function localAccountName(): string {
|
||||
const fromEnv = process.env.USER || process.env.LOGNAME || process.env.USERNAME
|
||||
// Why: `ssh -G` fills `user` from the local account whenever no User directive
|
||||
// matched; null means the local account is unknown, so `user` carries no verdict.
|
||||
function localAccountName(): string | null {
|
||||
const fromEnv = normalize(process.env.USER || process.env.LOGNAME || process.env.USERNAME)
|
||||
if (fromEnv) {
|
||||
return normalize(fromEnv)
|
||||
return fromEnv
|
||||
}
|
||||
try {
|
||||
return normalize(userInfo().username)
|
||||
return normalize(userInfo().username) || null
|
||||
} catch {
|
||||
return ''
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
@@ -37,7 +38,10 @@ function localAccountName(): string {
|
||||
*
|
||||
* Known limitation: the signals below read the *effective* config, so a wildcard
|
||||
* `Host *` supplying User/Port/ProxyCommand — or `CanonicalizeHostname yes` —
|
||||
* still reads as a match for an alias whose own block is gone.
|
||||
* still reads as a match for an alias whose own block is gone. The reverse
|
||||
* verdict falls back to the stored snapshot, whose IdentityFile is used verbatim
|
||||
* (only `~` is expanded), so a stored path with OpenSSH tokens (%d/%h/%r) stays
|
||||
* unreadable — pre-existing behaviour, unchanged here.
|
||||
*/
|
||||
export function hasOpenSshHostBlockMatch(
|
||||
target: HostBlockMatchTarget,
|
||||
@@ -58,6 +62,7 @@ export function hasOpenSshHostBlockMatch(
|
||||
if (resolved.proxyCommand || resolved.proxyJump) {
|
||||
return true
|
||||
}
|
||||
const localAccount = localAccountName()
|
||||
const user = normalize(resolved.user)
|
||||
return user !== '' && user !== localAccountName()
|
||||
return localAccount !== null && user !== '' && user !== localAccount
|
||||
}
|
||||
|
||||
@@ -202,6 +202,20 @@ describe('ordered SSH private-key authentication', () => {
|
||||
expect(getPassphrasePrivateKeyPath(config)).toBe('/keys/encrypted-first')
|
||||
})
|
||||
|
||||
it('falls back to the first identity when every candidate is encrypted', () => {
|
||||
mockEncryptedKeyPaths(['/keys/encrypted-first', '/keys/encrypted-second'])
|
||||
const config = buildConnectConfig(
|
||||
makeTarget(),
|
||||
makeResolved({ identityFile: ['/keys/encrypted-first', '/keys/encrypted-second'] }),
|
||||
{ includeAgent: false, includePrivateKey: true }
|
||||
)
|
||||
|
||||
// Why: no parseable candidate means the eager parse must still throw, so the
|
||||
// passphrase prompt fires instead of silently dropping to agent-only auth.
|
||||
expect(config.privateKey).toEqual(Buffer.from('/keys/encrypted-first'))
|
||||
expect(getPassphrasePrivateKeyPath(config)).toBe('/keys/encrypted-first')
|
||||
})
|
||||
|
||||
it('still hands ssh2 an encrypted key when it is the only identity', () => {
|
||||
mockEncryptedKeyPaths(['/keys/encrypted-only'])
|
||||
const config = buildConnectConfig(
|
||||
|
||||
@@ -41,16 +41,17 @@ function isUsableAsEagerPrivateKey(key: PrivateKeyFile, passphrase?: string | Bu
|
||||
if (parsed instanceof Error) {
|
||||
return false
|
||||
}
|
||||
const parsedKeys = Array.isArray(parsed) ? parsed : [parsed]
|
||||
return parsedKeys.some(
|
||||
(entry) => typeof entry?.isPrivateKey === 'function' && entry.isPrivateKey()
|
||||
)
|
||||
// Why: ssh2 keeps only the first entry of a multi-key parse (client.js `privateKey[0]`).
|
||||
const eager = Array.isArray(parsed) ? parsed[0] : parsed
|
||||
return typeof eager?.isPrivateKey === 'function' && eager.isPrivateKey()
|
||||
}
|
||||
|
||||
// Why: ssh2's Client.connect parses config.privateKey before any auth runs and
|
||||
// throws on an encrypted/unparseable one, so seeding it with the first candidate
|
||||
// would abort the connect instead of letting authHandler try the other keys.
|
||||
// Falling back to keys[0] keeps the passphrase prompt for an encrypted-only list.
|
||||
// `config.passphrase` is still unset here on the first connect — callers only fill
|
||||
// it after a prompt — so an encrypted key can only be picked on a retry pass.
|
||||
function selectEagerPrivateKey(config: ConnectConfig, keys: PrivateKeyFile[]): PrivateKeyFile {
|
||||
return keys.find((key) => isUsableAsEagerPrivateKey(key, config.passphrase)) ?? keys[0]!
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user