fix: expand backslash SSH home paths (#3861)

* fix: tighten GitHub submit shortcut handling

* fix: expand backslash ssh home paths
This commit is contained in:
Jinwoo Hong
2026-05-30 12:32:11 -07:00
committed by GitHub
parent bcfbd91d48
commit 55a33c5b68
7 changed files with 141 additions and 63 deletions
+32 -1
View File
@@ -1,4 +1,5 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { join } from 'path'
const mocks = vi.hoisted(() => ({
createAgent: vi.fn(),
@@ -27,6 +28,12 @@ vi.mock('ssh2', () => {
import { createIdentityFilteredAgent } from './ssh-agent-identity-filter'
const TEST_HOME = '/home/testuser'
function testHomePath(...parts: string[]): string {
return join(TEST_HOME, ...parts)
}
type TestKey = {
id: string
equals: ReturnType<typeof vi.fn>
@@ -68,7 +75,31 @@ describe('createIdentityFilteredAgent', () => {
})
expect(identities).toEqual([allowedKey])
expect(mocks.readFileSync).toHaveBeenCalledWith('/home/testuser/.ssh/work_key.pub')
expect(mocks.readFileSync).toHaveBeenCalledWith(testHomePath('.ssh', 'work_key.pub'))
})
it('expands Windows-style configured identity file paths before filtering', async () => {
const allowedKey = makeKey('allowed')
mocks.readFileSync.mockReturnValue('ssh-ed25519 AAAA allowed')
mocks.parseKey.mockReturnValue(allowedKey)
mocks.createAgent.mockReturnValue({
getIdentities: vi.fn((callback) => callback(undefined, [allowedKey])),
sign: vi.fn()
})
const agent = createIdentityFilteredAgent('/tmp/agent.sock', ['~\\.ssh\\work_key'])
const identities = await new Promise<unknown[]>((resolve, reject) => {
agent?.getIdentities((error, keys) => {
if (error) {
reject(error)
return
}
resolve(keys ?? [])
})
})
expect(identities).toEqual([allowedKey])
expect(mocks.readFileSync).toHaveBeenCalledWith(testHomePath('.ssh', 'work_key.pub'))
})
it('filters nested public key entries returned by ssh2 agents', async () => {
+2 -10
View File
@@ -1,6 +1,4 @@
import { readFileSync } from 'fs'
import { homedir } from 'os'
import { join } from 'path'
import {
BaseAgent,
createAgent,
@@ -11,16 +9,10 @@ import {
type SignCallback,
type SigningRequestOptions
} from 'ssh2'
import { resolveSshConfigHomePath } from './ssh-config-path-expansion'
type AgentPublicKey = ParsedKey | Buffer | string | PublicKeyEntry
function resolveHomePath(filepath: string): string {
if (filepath.startsWith('~/') || filepath === '~') {
return join(homedir(), filepath.slice(1))
}
return filepath
}
function comparablePublicKey(key: AgentPublicKey): ParsedKey | Buffer | string {
if (typeof key === 'object' && 'pubKey' in key) {
const pubKey = key.pubKey
@@ -91,7 +83,7 @@ function parseIdentityKeyFile(filePath: string): ParsedKey | undefined {
function readIdentityKeys(paths: string[]): ParsedKey[] {
const keys: ParsedKey[] = []
for (const path of paths) {
const identityPath = resolveHomePath(path)
const identityPath = resolveSshConfigHomePath(path)
const key = parseIdentityKeyFile(`${identityPath}.pub`) ?? parseIdentityKeyFile(identityPath)
if (key) {
keys.push(key)
+7 -15
View File
@@ -1,10 +1,9 @@
import { existsSync, readFileSync } from 'fs'
import { homedir } from 'os'
import { join } from 'path'
import { utils, type BaseAgent, type ParsedKey } from 'ssh2'
import type { SshTarget } from '../../shared/ssh-types'
import type { SshResolvedConfig } from './ssh-config-parser'
import { createIdentityFilteredAgent } from './ssh-agent-identity-filter'
import { resolveSshConfigHomePath } from './ssh-config-path-expansion'
// Why: ssh2 only tries keys that are explicitly provided. Users with keys in
// standard locations (e.g. ~/.ssh/id_ed25519) but no SSH agent running would
@@ -14,15 +13,15 @@ const DEFAULT_KEY_NAMES = ['id_ed25519', 'id_rsa', 'id_ecdsa', 'id_dsa', 'id_xms
const DEFAULT_KEY_PATHS = DEFAULT_KEY_NAMES.map((name) => `~/.ssh/${name}`)
const WINDOWS_OPENSSH_AGENT_PIPE = '\\\\.\\pipe\\openssh-ssh-agent'
// Why: parseSshGOutput expands ~ to homedir(), so resolved identityFile
// paths won't match the ~/... form in DEFAULT_KEY_PATHS.
const EXPANDED_DEFAULT_KEY_PATHS = DEFAULT_KEY_NAMES.map((name) => join(homedir(), '.ssh', name))
// Why: resolved IdentityFile paths are expanded before auth resolution, so they
// won't match the ~/... form in DEFAULT_KEY_PATHS.
const EXPANDED_DEFAULT_KEY_PATHS = DEFAULT_KEY_PATHS.map(resolveSshConfigHomePath)
export type PrivateKeyFile = { path: string; contents: Buffer }
export function findDefaultKeyFile(): PrivateKeyFile | undefined {
for (const keyPath of DEFAULT_KEY_PATHS) {
const resolved = keyPath.replace(/^~/, homedir())
const resolved = resolveSshConfigHomePath(keyPath)
try {
if (!existsSync(resolved)) {
continue
@@ -36,13 +35,6 @@ export function findDefaultKeyFile(): PrivateKeyFile | undefined {
return undefined
}
function resolveHomePath(filepath: string): string {
if (filepath.startsWith('~/') || filepath === '~') {
return join(homedir(), filepath.slice(1))
}
return filepath
}
function expandIdentityAgentEnv(value: string): string | undefined {
if (value === 'SSH_AUTH_SOCK') {
return process.env.SSH_AUTH_SOCK || undefined
@@ -83,7 +75,7 @@ export function resolveAgentSocket(
if (!trimmed || trimmed.toLowerCase() === 'none') {
return undefined
}
return expandIdentityAgentEnv(resolveHomePath(trimmed))
return expandIdentityAgentEnv(resolveSshConfigHomePath(trimmed))
}
return resolveDefaultAgentSocket()
}
@@ -103,7 +95,7 @@ function resolveExplicitPrivateKeyPath(
function readPrivateKey(keyPath: string): PrivateKeyFile | undefined {
try {
const resolvedPath = resolveHomePath(keyPath)
const resolvedPath = resolveSshConfigHomePath(keyPath)
return { path: keyPath, contents: readFileSync(resolvedPath) }
} catch {
return undefined
+30 -8
View File
@@ -1,5 +1,6 @@
/* eslint-disable max-lines -- Why: SSH config parsing fixtures cover OpenSSH file parsing and ssh -G output together so import and connection resolution stay aligned. */
import { describe, expect, it, vi } from 'vitest'
import { join } from 'path'
import { parseSshConfig, sshConfigHostsToTargets, parseSshGOutput } from './ssh-config-parser'
vi.mock('os', () => ({
@@ -7,6 +8,11 @@ vi.mock('os', () => ({
}))
const LARGE_HOST_ALIAS_COUNT = 150_000
const TEST_HOME = '/home/testuser'
function testHomePath(...parts: string[]): string {
return join(TEST_HOME, ...parts)
}
function buildHostAliases(count: number): string {
const aliases: string[] = []
@@ -85,7 +91,17 @@ Host myserver
IdentityFile ~/.ssh/id_ed25519
`
const hosts = parseSshConfig(config)
expect(hosts[0].identityFile).toBe('/home/testuser/.ssh/id_ed25519')
expect(hosts[0].identityFile).toBe(testHomePath('.ssh', 'id_ed25519'))
})
it('parses Windows-style IdentityFile with ~ expansion', () => {
const config = `
Host myserver
HostName example.com
IdentityFile ~\\.ssh\\id_ed25519
`
const hosts = parseSshConfig(config)
expect(hosts[0].identityFile).toBe(testHomePath('.ssh', 'id_ed25519'))
})
it('parses IdentityAgent with ~ expansion', () => {
@@ -95,7 +111,7 @@ Host myserver
IdentityAgent ~/.1password/agent.sock
`
const hosts = parseSshConfig(config)
expect(hosts[0].identityAgent).toBe('/home/testuser/.1password/agent.sock')
expect(hosts[0].identityAgent).toBe(testHomePath('.1password', 'agent.sock'))
})
it('parses IdentitiesOnly', () => {
@@ -217,12 +233,12 @@ Host staging stage
expect(hosts).toEqual([
{
host: 'staging',
identityAgent: '/home/testuser/.1password/agent.sock',
identityAgent: testHomePath('.1password', 'agent.sock'),
identitiesOnly: true
},
{
host: 'stage',
identityAgent: '/home/testuser/.1password/agent.sock',
identityAgent: testHomePath('.1password', 'agent.sock'),
identitiesOnly: true
}
])
@@ -341,8 +357,8 @@ describe('parseSshGOutput', () => {
const result = parseSshGOutput(output)
expect(result.identityFile).toEqual([
'/home/testuser/.ssh/id_ed25519',
'/home/testuser/.ssh/id_rsa'
testHomePath('.ssh', 'id_ed25519'),
testHomePath('.ssh', 'id_rsa')
])
})
@@ -417,13 +433,19 @@ describe('parseSshGOutput', () => {
it('handles ~ expansion in identity file paths', () => {
const output = 'hostname example.com\nidentityfile ~/custom_key\nport 22'
const result = parseSshGOutput(output)
expect(result.identityFile).toEqual(['/home/testuser/custom_key'])
expect(result.identityFile).toEqual([testHomePath('custom_key')])
})
it('handles Windows-style ~ expansion in identity file paths', () => {
const output = 'hostname example.com\nidentityfile ~\\.ssh\\custom_key\nport 22'
const result = parseSshGOutput(output)
expect(result.identityFile).toEqual([testHomePath('.ssh', 'custom_key')])
})
it('parses identityagent with ~ expansion', () => {
const output = 'hostname example.com\nidentityagent ~/.1password/agent.sock\nport 22'
const result = parseSshGOutput(output)
expect(result.identityAgent).toBe('/home/testuser/.1password/agent.sock')
expect(result.identityAgent).toBe(testHomePath('.1password', 'agent.sock'))
})
it('preserves identityagent none so auth can disable agent fallback', () => {
+5 -11
View File
@@ -4,6 +4,7 @@ import { join } from 'path'
import { homedir } from 'os'
import type { SshTarget } from '../../shared/ssh-types'
import { expandSshConfigIncludes } from './ssh-config-include-expander'
import { resolveSshConfigHomePath } from './ssh-config-path-expansion'
export type SshConfigHost = {
host: string
@@ -90,12 +91,12 @@ export function parseSshConfig(content: string): SshConfigHost[] {
break
case 'identityfile':
for (const host of current) {
host.identityFile = resolveHomePath(value)
host.identityFile = resolveSshConfigHomePath(value)
}
break
case 'identityagent':
for (const host of current) {
host.identityAgent = resolveHomePath(value)
host.identityAgent = resolveSshConfigHomePath(value)
}
break
case 'identitiesonly':
@@ -181,13 +182,6 @@ function splitHostPatterns(input: string): string[] {
return patterns
}
function resolveHomePath(filepath: string): string {
if (filepath.startsWith('~/') || filepath === '~') {
return join(homedir(), filepath.slice(1))
}
return filepath
}
/** Read and parse the user's ~/.ssh/config file. Returns empty array if not found. */
export function loadUserSshConfig(): SshConfigHost[] {
const configPath = join(homedir(), '.ssh', 'config')
@@ -311,7 +305,7 @@ export function parseSshGOutput(stdout: string): SshResolvedConfig {
const key = line.substring(0, spaceIdx).toLowerCase()
const value = line.substring(spaceIdx + 1).trim()
if (key === 'identityfile') {
identityFiles.push(resolveHomePath(value))
identityFiles.push(resolveSshConfigHomePath(value))
} else {
map.set(key, value)
}
@@ -324,7 +318,7 @@ export function parseSshGOutput(stdout: string): SshResolvedConfig {
const rawJump = map.get('proxyjump')
const proxyJump = rawJump && rawJump !== 'none' ? rawJump : undefined
const rawIdentityAgent = map.get('identityagent')
const identityAgent = rawIdentityAgent ? resolveHomePath(rawIdentityAgent) : undefined
const identityAgent = rawIdentityAgent ? resolveSshConfigHomePath(rawIdentityAgent) : undefined
return {
hostname: map.get('hostname') ?? '',
+18
View File
@@ -0,0 +1,18 @@
import { homedir } from 'os'
import { join } from 'path'
export function resolveSshConfigHomePath(filepath: string): string {
if (filepath === '~') {
return homedir()
}
if (filepath.startsWith('~/') || filepath.startsWith('~\\')) {
return join(
homedir(),
...filepath
.slice(2)
.split(/[\\/]+/)
.filter(Boolean)
)
}
return filepath
}
+47 -18
View File
@@ -1,5 +1,6 @@
/* eslint-disable max-lines -- Why: SSH connection utility tests share mocked filesystem and environment setup across auth, proxy, and retry helpers. */
import { afterEach, describe, expect, it, vi, beforeEach } from 'vitest'
import { join } from 'path'
import { BaseAgent, utils, type ParsedKey } from 'ssh2'
vi.mock('os', () => ({
@@ -8,6 +9,11 @@ vi.mock('os', () => ({
const mockExistsSync = vi.fn().mockReturnValue(false)
const mockReadFileSync = vi.fn()
const TEST_HOME = '/home/testuser'
function testHomePath(...parts: string[]): string {
return join(TEST_HOME, ...parts)
}
vi.mock('fs', () => ({
existsSync: (...args: unknown[]) => mockExistsSync(...args),
@@ -205,7 +211,7 @@ describe('findDefaultKeyFile', () => {
it('returns the first existing key file', () => {
mockExistsSync.mockImplementation((path: unknown) => {
return path === '/home/testuser/.ssh/id_ed25519'
return path === testHomePath('.ssh', 'id_ed25519')
})
mockReadFileSync.mockReturnValue(Buffer.from('key-contents'))
@@ -225,20 +231,20 @@ describe('findDefaultKeyFile', () => {
findDefaultKeyFile()
expect(checkedPaths).toEqual([
'/home/testuser/.ssh/id_ed25519',
'/home/testuser/.ssh/id_rsa',
'/home/testuser/.ssh/id_ecdsa',
'/home/testuser/.ssh/id_dsa',
'/home/testuser/.ssh/id_xmss'
testHomePath('.ssh', 'id_ed25519'),
testHomePath('.ssh', 'id_rsa'),
testHomePath('.ssh', 'id_ecdsa'),
testHomePath('.ssh', 'id_dsa'),
testHomePath('.ssh', 'id_xmss')
])
})
it('skips unreadable key files and tries next', () => {
mockExistsSync.mockImplementation((path: unknown) => {
return path === '/home/testuser/.ssh/id_ed25519' || path === '/home/testuser/.ssh/id_rsa'
return path === testHomePath('.ssh', 'id_ed25519') || path === testHomePath('.ssh', 'id_rsa')
})
mockReadFileSync.mockImplementation((path: unknown) => {
if (String(path) === '/home/testuser/.ssh/id_ed25519') {
if (String(path) === testHomePath('.ssh', 'id_ed25519')) {
throw new Error('permission denied')
}
return Buffer.from('rsa-key')
@@ -337,9 +343,9 @@ describe('buildConnectConfig', () => {
it('prefers ssh -G resolved IdentityAgent for config-host targets', () => {
const config = buildConnectConfig(
makeTarget({ configHost: 'work', identityAgent: '%d/.1password/agent.sock' }),
makeResolved({ identityAgent: '/home/testuser/.1password/agent.sock' })
makeResolved({ identityAgent: testHomePath('.1password', 'agent.sock') })
)
expect(config.agent).toBe('/home/testuser/.1password/agent.sock')
expect(config.agent).toBe(testHomePath('.1password', 'agent.sock'))
})
it('allows IdentityAgent none to disable agent auth', () => {
@@ -419,11 +425,29 @@ describe('buildConnectConfig', () => {
})
it('uses keyFile auth when target.identityFile is set and no agent is available', () => {
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('linux')
delete process.env.SSH_AUTH_SOCK
mockReadFileSync.mockReturnValue(Buffer.from('key'))
const config = buildConnectConfig(makeTarget({ identityFile: '/home/user/.ssh/custom' }), null)
try {
const config = buildConnectConfig(
makeTarget({ identityFile: '/home/user/.ssh/custom' }),
null
)
expect(config.privateKey).toEqual(Buffer.from('key'))
expect(config.agent).toBeUndefined()
} finally {
platformSpy.mockRestore()
}
})
it('expands Windows-style target.identityFile before reading private key', () => {
mockReadFileSync.mockReturnValue(Buffer.from('key'))
const config = buildConnectConfig(makeTarget({ identityFile: '~\\.ssh\\custom' }), null, {
includeAgent: false,
includePrivateKey: true
})
expect(config.privateKey).toEqual(Buffer.from('key'))
expect(config.agent).toBeUndefined()
expect(mockReadFileSync).toHaveBeenCalledWith(testHomePath('.ssh', 'custom'))
})
it('includes unencrypted resolved identityFile auth when an agent is available', () => {
@@ -442,7 +466,7 @@ describe('buildConnectConfig', () => {
it('uses agent auth without probing when resolved identityFile is a default path (expanded)', () => {
const config = buildConnectConfig(
makeTarget(),
makeResolved({ identityFile: ['/home/testuser/.ssh/id_ed25519'] })
makeResolved({ identityFile: [testHomePath('.ssh', 'id_ed25519')] })
)
expect(config.agent).toBe('/tmp/agent.sock')
expect(config.privateKey).toBeUndefined()
@@ -451,7 +475,7 @@ describe('buildConnectConfig', () => {
it('does not probe default key files before agent auth', () => {
mockExistsSync.mockImplementation(
(p: unknown) => String(p) === '/home/testuser/.ssh/id_ed25519'
(p: unknown) => String(p) === testHomePath('.ssh', 'id_ed25519')
)
const config = buildConnectConfig(makeTarget(), null)
expect(config.agent).toBe('/tmp/agent.sock')
@@ -460,14 +484,19 @@ describe('buildConnectConfig', () => {
})
it('provides fallback key when no agent is available', () => {
const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('linux')
delete process.env.SSH_AUTH_SOCK
mockExistsSync.mockImplementation(
(p: unknown) => String(p) === '/home/testuser/.ssh/id_ed25519'
(p: unknown) => String(p) === testHomePath('.ssh', 'id_ed25519')
)
mockReadFileSync.mockReturnValue(Buffer.from('fallback'))
const config = buildConnectConfig(makeTarget(), null)
expect(config.agent).toBeUndefined()
expect(config.privateKey).toEqual(Buffer.from('fallback'))
try {
const config = buildConnectConfig(makeTarget(), null)
expect(config.agent).toBeUndefined()
expect(config.privateKey).toEqual(Buffer.from('fallback'))
} finally {
platformSpy.mockRestore()
}
})
it('can force private key inclusion for the post-agent fallback path', () => {