xiaoxue_admin
/
orca
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ssh): use SSH target label for home directory remote repos (#1031)

This commit is contained in:
Jinwoo Hong 2026-04-24 16:58:48 -04:00 committed by GitHub
parent bcce74e110
commit 68e658e082
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 200 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

91
src/main/ipc/repos-remote.test.ts
View File

@ -1,17 +1,22 @@
import { describe, expect, it, vi, beforeEach } from 'vitest'
const { handleMock, mockStore, mockGitProvider } = vi.hoisted(() => ({
const { handleMock, mockStore, mockGitProvider, mockMultiplexer } = vi.hoisted(() => ({
handleMock: vi.fn(),
mockStore: {
getRepos: vi.fn().mockReturnValue([]),
addRepo: vi.fn(),
removeRepo: vi.fn(),
getRepo: vi.fn(),
updateRepo: vi.fn()
updateRepo: vi.fn(),
getSshTarget: vi.fn()
},
mockGitProvider: {
isGitRepo: vi.fn().mockReturnValue(true),
isGitRepoAsync: vi.fn().mockResolvedValue({ isRepo: true, rootPath: null })
},
mockMultiplexer: {
request: vi.fn(),
notify: vi.fn()
}
}))
@ -44,6 +49,15 @@ vi.mock('../providers/ssh-git-dispatch', () => ({
})
}))
vi.mock('./ssh', () => ({
getActiveMultiplexer: vi.fn().mockImplementation((id: string) => {
if (id === 'conn-1') {
return mockMultiplexer
}
return undefined
})
}))
import { registerRepoHandlers } from './repos'
describe('repos:addRemote', () => {
@ -61,6 +75,9 @@ describe('repos:addRemote', () => {
})
mockStore.getRepos.mockReset().mockReturnValue([])
mockStore.addRepo.mockReset()
mockStore.getSshTarget.mockReset()
mockMultiplexer.request.mockReset()
mockMultiplexer.notify.mockReset()
mockWindow.webContents.send.mockReset()
registerRepoHandlers(mockWindow as never, mockStore as never)
@ -88,7 +105,7 @@ describe('repos:addRemote', () => {
expect(result).toHaveProperty('repo.connectionId', 'conn-1')
})
it('uses custom displayName when provided', async () => {
it('uses custom displayName when provided', async () => {
const result = await handlers.get('repos:addRemote')!(null, {
connectionId: 'conn-1',
remotePath: '/home/user/project',
@ -188,4 +205,72 @@ describe('repos:addRemote', () => {
expect(mockWindow.webContents.send).toHaveBeenCalledWith('repos:changed')
})
it('resolves ~ to absolute path via relay and uses SSH target label', async () => {
mockMultiplexer.request.mockResolvedValueOnce({ resolvedPath: '/home/ubuntu' })
mockStore.getSshTarget.mockReturnValueOnce({
id: 'conn-1',
label: 'ubuntu-box',
host: '192.168.1.100',
port: 22,
username: 'user'
})
const result = await handlers.get('repos:addRemote')!(null, {
connectionId: 'conn-1',
remotePath: '~'
})
expect(mockMultiplexer.request).toHaveBeenCalledWith('session.resolveHome', { path: '~' })
expect(mockStore.addRepo).toHaveBeenCalledWith(
expect.objectContaining({
displayName: 'ubuntu-box',
path: '/home/ubuntu'
})
)
expect(result).toHaveProperty('repo.displayName', 'ubuntu-box')
expect(result).toHaveProperty('repo.path', '/home/ubuntu')
})
it('resolves ~/subdir to absolute path via relay', async () => {
mockMultiplexer.request.mockResolvedValueOnce({ resolvedPath: '/home/ubuntu/subdir' })
const result = await handlers.get('repos:addRemote')!(null, {
connectionId: 'conn-1',
remotePath: '~/subdir'
})
expect(mockStore.addRepo).toHaveBeenCalledWith(
expect.objectContaining({
path: '/home/ubuntu/subdir',
displayName: 'subdir'
})
)
expect(result).toHaveProperty('repo.path', '/home/ubuntu/subdir')
})
it('ignores SSH target label when custom displayName is provided', async () => {
mockMultiplexer.request.mockResolvedValueOnce({ resolvedPath: '/home/ubuntu' })
mockStore.getSshTarget.mockReturnValueOnce({
id: 'conn-1',
label: 'ubuntu-box',
host: '192.168.1.100',
port: 22,
username: 'user'
})
const result = await handlers.get('repos:addRemote')!(null, {
connectionId: 'conn-1',
remotePath: '~',
displayName: 'My Home'
})
expect(mockStore.addRepo).toHaveBeenCalledWith(
expect.objectContaining({
displayName: 'My Home',
path: '/home/ubuntu'
})
)
expect(result).toHaveProperty('repo.displayName', 'My Home')
})
})

45
src/main/ipc/repos.ts
View File

@ -98,18 +98,37 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v
return { error: `SSH connection "${args.connectionId}" not found or not connected` }
}
let repoKind: 'git' | 'folder' = args.kind ?? 'git'
let resolvedPath = args.remotePath
// Why: `~` is a shell expansion that Node's fs APIs don't understand.
// Resolve tilde paths to absolute paths via the relay before storing,
// so all downstream fs operations (readDir, stat, etc.) work correctly.
if (resolvedPath === '~' || resolvedPath === '~/' || resolvedPath.startsWith('~/')) {
const mux = getActiveMultiplexer(args.connectionId)
if (mux) {
try {
const result = (await mux.request('session.resolveHome', {
path: resolvedPath
})) as { resolvedPath: string }
resolvedPath = result.resolvedPath
} catch {
// Relay may not support resolveHome yet — fall through to raw path
}
}
}
// Why: check for duplicates after tilde resolution so that adding `~/`
// when `/home/ubuntu` is already stored correctly detects the duplicate.
const existing = store
.getRepos()
.find((r) => r.connectionId === args.connectionId && r.path === args.remotePath)
.find((r) => r.connectionId === args.connectionId && r.path === resolvedPath)
if (existing) {
return { repo: existing }
}
const pathSegments = args.remotePath.replace(/\/+$/, '').split('/')
const folderName = pathSegments.at(-1) || args.remotePath
let repoKind: 'git' | 'folder' = args.kind ?? 'git'
let resolvedPath = args.remotePath
const pathSegments = resolvedPath.replace(/\/+$/, '').split('/')
let folderName = pathSegments.at(-1) || resolvedPath
if (args.kind !== 'folder') {
// Why: when kind is not explicitly 'folder', verify the remote path is
@ -117,7 +136,7 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v
// Folder" confirmation dialog — matching the local add-repo behavior
// where non-git directories require explicit user consent.
try {
const check = await gitProvider.isGitRepoAsync(args.remotePath)
const check = await gitProvider.isGitRepoAsync(resolvedPath)
if (check.isRepo) {
repoKind = 'git'
if (check.rootPath) {
@ -134,10 +153,20 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v
}
}
// When folderName is the home directory basename (e.g. 'ubuntu'),
// use SSH target label for a more descriptive name
let displayName = args.displayName || folderName
if (!args.displayName && (args.remotePath === '~' || args.remotePath === '~/')) {
const sshTarget = store.getSshTarget(args.connectionId)
if (sshTarget) {
displayName = sshTarget.label
}
}
const repo: Repo = {
id: randomUUID(),
path: resolvedPath,
displayName: args.displayName || folderName,
displayName,
badgeColor: REPO_COLORS[store.getRepos().length % REPO_COLORS.length],
addedAt: Date.now(),
kind: repoKind,

18
src/relay/context.ts
View File

@ -1,7 +1,21 @@
import { resolve, relative, isAbsolute } from 'path'
import { homedir } from 'os'
import { realpathSync } from 'fs'
import { realpath } from 'fs/promises'
// Why: Node's fs APIs don't understand shell tilde expansion. Old repos may
// have been stored with `~` or `~/…` paths before the client-side fix, so the
// relay must expand them to absolute paths as a safety net.
export function expandTilde(p: string): string {
if (p === '~' || p === '~/') {
return homedir()
}
if (p.startsWith('~/')) {
return resolve(homedir(), p.slice(2))
}
return p
}
// Why: mutating FS operations on the remote must be scoped to workspace roots
// registered by the main process. Without this, a compromised or buggy client
// could delete arbitrary files on the remote host.
@ -16,7 +30,7 @@ export class RelayContext {
private rootsRegistered = false
registerRoot(rootPath: string): void {
const resolved = resolve(rootPath)
const resolved = resolve(expandTilde(rootPath))
this.authorizedRoots.add(resolved)
// Why: on macOS, /tmp is a symlink to /private/tmp. If a root is registered
// as /tmp/workspace, validatePathResolved would resolve it to /private/tmp/
@ -38,7 +52,7 @@ export class RelayContext {
throw new Error('No workspace roots registered yet — path validation denied')
}
const resolved = resolve(targetPath)
const resolved = resolve(expandTilde(targetPath))
for (const root of this.authorizedRoots) {
const rel = relative(root, resolved)
if (!rel.startsWith('..') && !isAbsolute(rel)) {

33
src/relay/fs-handler.ts
View File

@ -13,6 +13,7 @@ import {
import { extname } from 'path'
import type { RelayDispatcher } from './dispatcher'
import type { RelayContext } from './context'
import { expandTilde } from './context'
import {
MAX_FILE_SIZE,
DEFAULT_MAX_RESULTS,
@ -58,7 +59,7 @@ export class FsHandler {
}
private async readDir(params: Record<string, unknown>) {
const dirPath = params.dirPath as string
const dirPath = expandTilde(params.dirPath as string)
await this.context.validatePathResolved(dirPath)
const entries = await readdir(dirPath, { withFileTypes: true })
return entries
@ -76,7 +77,7 @@ export class FsHandler {
}
private async readFile(params: Record<string, unknown>) {
const filePath = params.filePath as string
const filePath = expandTilde(params.filePath as string)
await this.context.validatePathResolved(filePath)
const stats = await stat(filePath)
if (stats.size > MAX_FILE_SIZE) {
@ -97,7 +98,7 @@ export class FsHandler {
}
private async writeFile(params: Record<string, unknown>) {
const filePath = params.filePath as string
const filePath = expandTilde(params.filePath as string)
await this.context.validatePathResolved(filePath)
const content = params.content as string
try {
@ -114,7 +115,7 @@ export class FsHandler {
}
private async stat(params: Record<string, unknown>) {
const filePath = params.filePath as string
const filePath = expandTilde(params.filePath as string)
await this.context.validatePathResolved(filePath)
// Why: lstat is used instead of stat so that symlinks are reported as
// symlinks rather than being silently followed. stat() follows symlinks,
@ -130,7 +131,7 @@ export class FsHandler {
}
private async deletePath(params: Record<string, unknown>) {
const targetPath = params.targetPath as string
const targetPath = expandTilde(params.targetPath as string)
await this.context.validatePathResolved(targetPath)
const recursive = params.recursive as boolean | undefined
const stats = await stat(targetPath)
@ -141,7 +142,7 @@ export class FsHandler {
}
private async createFile(params: Record<string, unknown>) {
const filePath = params.filePath as string
const filePath = expandTilde(params.filePath as string)
// Why: symlinks in parent directories can redirect creation outside the
// workspace. validatePathResolved follows symlinks before checking roots.
await this.context.validatePathResolved(filePath)
@ -151,22 +152,22 @@ export class FsHandler {
}
private async createDir(params: Record<string, unknown>) {
const dirPath = params.dirPath as string
const dirPath = expandTilde(params.dirPath as string)
await this.context.validatePathResolved(dirPath)
await mkdir(dirPath, { recursive: true })
}
private async rename(params: Record<string, unknown>) {
const oldPath = params.oldPath as string
const newPath = params.newPath as string
const oldPath = expandTilde(params.oldPath as string)
const newPath = expandTilde(params.newPath as string)
await this.context.validatePathResolved(oldPath)
await this.context.validatePathResolved(newPath)
await rename(oldPath, newPath)
}
private async copy(params: Record<string, unknown>) {
const source = params.source as string
const destination = params.destination as string
const source = expandTilde(params.source as string)
const destination = expandTilde(params.destination as string)
// Why: cp follows symlinks — a symlink inside the workspace pointing to
// /etc would copy sensitive files into the workspace where readFile can
// exfiltrate them.
@ -176,7 +177,7 @@ export class FsHandler {
}
private async realpath(params: Record<string, unknown>) {
const filePath = params.filePath as string
const filePath = expandTilde(params.filePath as string)
this.context.validatePath(filePath)
const resolved = await realpath(filePath)
// Why: a symlink inside the workspace may resolve to a path outside it.
@ -187,7 +188,7 @@ export class FsHandler {
private async search(params: Record<string, unknown>) {
const query = params.query as string
const rootPath = params.rootPath as string
const rootPath = expandTilde(params.rootPath as string)
// Why: a symlink inside the workspace pointing to a directory outside it
// would let rg search (and return content from) files beyond the workspace.
await this.context.validatePathResolved(rootPath)
@ -224,7 +225,7 @@ export class FsHandler {
}
private async listFiles(params: Record<string, unknown>): Promise<string[]> {
const rootPath = params.rootPath as string
const rootPath = expandTilde(params.rootPath as string)
await this.context.validatePathResolved(rootPath)
const rgAvailable = await checkRgAvailable()
if (!rgAvailable) {
@ -234,7 +235,7 @@ export class FsHandler {
}
private async watch(params: Record<string, unknown>) {
const rootPath = params.rootPath as string
const rootPath = expandTilde(params.rootPath as string)
this.context.validatePath(rootPath)
if (this.watches.size >= 20) {
@ -277,7 +278,7 @@ export class FsHandler {
}
private unwatch(params: Record<string, unknown>): void {
const rootPath = params.rootPath as string
const rootPath = expandTilde(params.rootPath as string)
const state = this.watches.get(rootPath)
if (state) {
state.unwatchFn?.()

3
src/relay/git-handler.ts
View File

@ -5,6 +5,7 @@ import { readFile, rm } from 'fs/promises'
import * as path from 'path'
import type { RelayDispatcher } from './dispatcher'
import type { RelayContext } from './context'
import { expandTilde } from './context'
import {
parseStatusOutput,
parseUnmergedEntry,
@ -56,7 +57,7 @@ export class GitHandler {
opts?: { maxBuffer?: number }
): Promise<{ stdout: string; stderr: string }> {
return execFileAsync('git', args, {
cwd,
cwd: expandTilde(cwd),
encoding: 'utf-8',
maxBuffer: opts?.maxBuffer ?? MAX_GIT_BUFFER
})

17
src/relay/relay.ts
View File

@ -5,6 +5,8 @@
// The Electron app (client) deploys this script via SCP and launches
// it via an SSH exec channel.
import { homedir } from 'os'
import { resolve } from 'path'
import { RELAY_SENTINEL } from './protocol'
import { RelayDispatcher } from './dispatcher'
import { RelayContext } from './context'
@ -55,6 +57,21 @@ function main(): void {
}
})
// Why: the client stores repo paths as-is from user input, but `~` is a
// shell expansion — Node's fs APIs don't understand it. This handler lets
// the client resolve tilde paths to absolute paths on the remote host
// before persisting them, so all downstream fs operations work correctly.
dispatcher.onRequest('session.resolveHome', async (params) => {
const inputPath = params.path as string
if (inputPath === '~' || inputPath === '~/') {
return { resolvedPath: homedir() }
}
if (inputPath.startsWith('~/')) {
return { resolvedPath: resolve(homedir(), inputPath.slice(2)) }
}
return { resolvedPath: inputPath }
})
const ptyHandler = new PtyHandler(dispatcher, graceTimeMs)
const fsHandler = new FsHandler(dispatcher, context)
// Why: GitHandler registers its own request handlers on construction,

23
src/relay/subprocess.test.ts
View File

@ -199,4 +199,27 @@ describe('Subprocess: Relay entry point', () => {
await relay.waitForExit(3000)
expect(relay.proc.exitCode).toBe(0)
}, 10_000)
it('resolves ~ to home directory via session.resolveHome', async () => {
relay = spawn()
await relay.sentinelReceived
const homeDir = require('os').homedir()
const id1 = relay.send('session.resolveHome', { path: '~' })
const id2 = relay.send('session.resolveHome', { path: '~/projects' })
const id3 = relay.send('session.resolveHome', { path: '/absolute/path' })
const [r1, r2, r3] = await Promise.all([
relay.waitForResponse(id1),
relay.waitForResponse(id2),
relay.waitForResponse(id3)
])
expect((r1.result as { resolvedPath: string }).resolvedPath).toBe(homeDir)
expect((r2.result as { resolvedPath: string }).resolvedPath).toBe(
path.join(homeDir, 'projects')
)
expect((r3.result as { resolvedPath: string }).resolvedPath).toBe('/absolute/path')
}, 10_000)
})