diff --git a/src/main/ipc/repos-remote.test.ts b/src/main/ipc/repos-remote.test.ts index 54ca5b5be..54af38629 100644 --- a/src/main/ipc/repos-remote.test.ts +++ b/src/main/ipc/repos-remote.test.ts @@ -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') + }) }) diff --git a/src/main/ipc/repos.ts b/src/main/ipc/repos.ts index d84539a10..565ad13f7 100644 --- a/src/main/ipc/repos.ts +++ b/src/main/ipc/repos.ts @@ -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, diff --git a/src/relay/context.ts b/src/relay/context.ts index 42b856d5b..e63968902 100644 --- a/src/relay/context.ts +++ b/src/relay/context.ts @@ -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)) { diff --git a/src/relay/fs-handler.ts b/src/relay/fs-handler.ts index 4a92251b0..fd65c5e0c 100644 --- a/src/relay/fs-handler.ts +++ b/src/relay/fs-handler.ts @@ -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) { - 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) { - 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) { - 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) { - 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) { - 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) { - 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) { - 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) { - 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) { - 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) { - 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) { 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): Promise { - 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) { - 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): void { - const rootPath = params.rootPath as string + const rootPath = expandTilde(params.rootPath as string) const state = this.watches.get(rootPath) if (state) { state.unwatchFn?.() diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index d5e9f280e..0875fb2d3 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -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 }) diff --git a/src/relay/relay.ts b/src/relay/relay.ts index f7fb890e2..ecc2d0d84 100644 --- a/src/relay/relay.ts +++ b/src/relay/relay.ts @@ -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, diff --git a/src/relay/subprocess.test.ts b/src/relay/subprocess.test.ts index d17ecfb46..51120255a 100644 --- a/src/relay/subprocess.test.ts +++ b/src/relay/subprocess.test.ts @@ -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) })