From afde3a374101894efed27dad64c8d69b197bccfe Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 18 Jun 2026 15:52:55 -0700 Subject: [PATCH] Honor configured SSH for remote-base worktrees (#5739) --- src/main/git/runner-command-exec.test.ts | 184 +++++++++++++++++ src/main/git/runner.ts | 211 ++++++++++++++++++-- src/main/runtime/fetch-remote-cache.test.ts | 16 +- src/main/runtime/orca-runtime.test.ts | 6 +- src/main/runtime/orca-runtime.ts | 9 +- 5 files changed, 404 insertions(+), 22 deletions(-) diff --git a/src/main/git/runner-command-exec.test.ts b/src/main/git/runner-command-exec.test.ts index 5c8844a31..2d31c2555 100644 --- a/src/main/git/runner-command-exec.test.ts +++ b/src/main/git/runner-command-exec.test.ts @@ -288,6 +288,190 @@ describe('runner execFile timeout handling', () => { expect(capturedEnv?.GIT_SSH_COMMAND).toContain('BatchMode=yes') }) + it('probes core.sshCommand for opted-in network git calls', async () => { + const child = createMockChildProcess(1234) + const calls: { args: string[]; env: NodeJS.ProcessEnv }[] = [] + execFileMock.mockImplementation((_cmd, args, opts, cb) => { + calls.push({ args, env: opts.env }) + cb(null, args[0] === 'config' ? 'ssh -F ~/.ssh/github-work -i ~/.ssh/work_key\n' : '', '') + return child + }) + + await gitExecFileAsync(['fetch', 'origin'], { + cwd: '/repo', + env: {}, + useConfiguredSshCommandForNetwork: true + }) + + expect(calls[0]?.args).toEqual(['config', '--get', 'core.sshCommand']) + expect(calls[0]?.env.GIT_TERMINAL_PROMPT).toBe('0') + expect(calls[0]?.env.GIT_SSH_COMMAND).toBeUndefined() + expect(calls[1]?.args).toEqual(['fetch', 'origin']) + expect(calls[1]?.env.GIT_SSH_COMMAND).toBe( + 'ssh -F ~/.ssh/github-work -i ~/.ssh/work_key -o BatchMode=yes' + ) + }) + + it('replaces configured BatchMode for opted-in mergeable OpenSSH commands', async () => { + const child = createMockChildProcess(1234) + let capturedEnv: NodeJS.ProcessEnv | undefined + execFileMock.mockImplementation((_cmd, args, opts, cb) => { + if (args[0] === 'config') { + cb(null, 'ssh -o BatchMode=no -i ~/.ssh/personal\n', '') + } else { + capturedEnv = opts.env + cb(null, '', '') + } + return child + }) + + await gitExecFileAsync(['fetch', 'origin'], { + cwd: '/repo', + env: {}, + useConfiguredSshCommandForNetwork: true + }) + + expect(capturedEnv?.GIT_SSH_COMMAND).toBe('ssh -i ~/.ssh/personal -o BatchMode=yes') + }) + + it('merges quoted ssh.exe command shapes for opted-in network calls', async () => { + const child = createMockChildProcess(1234) + let capturedEnv: NodeJS.ProcessEnv | undefined + execFileMock.mockImplementation((_cmd, args, opts, cb) => { + if (args[0] === 'config') { + cb(null, '"C:/Program Files/Git/usr/bin/ssh.exe" -F ~/.ssh/config\n', '') + } else { + capturedEnv = opts.env + cb(null, '', '') + } + return child + }) + + await gitExecFileAsync(['fetch', 'origin'], { + cwd: '/repo', + env: {}, + useConfiguredSshCommandForNetwork: true + }) + + expect(capturedEnv?.GIT_SSH_COMMAND).toBe( + "'C:/Program Files/Git/usr/bin/ssh.exe' -F ~/.ssh/config -o BatchMode=yes" + ) + }) + + it('merges unquoted Windows ssh.exe paths for opted-in network calls', async () => { + const child = createMockChildProcess(1234) + let capturedEnv: NodeJS.ProcessEnv | undefined + execFileMock.mockImplementation((_cmd, args, opts, cb) => { + if (args[0] === 'config') { + cb(null, `${String.raw`C:\Git\usr\bin\ssh.exe -i C:\Users\me\.ssh\work_key`}\n`, '') + } else { + capturedEnv = opts.env + cb(null, '', '') + } + return child + }) + + await gitExecFileAsync(['fetch', 'origin'], { + cwd: '/repo', + env: {}, + useConfiguredSshCommandForNetwork: true + }) + + expect(capturedEnv?.GIT_SSH_COMMAND).toBe( + String.raw`'C:\Git\usr\bin\ssh.exe' -i 'C:\Users\me\.ssh\work_key' -o BatchMode=yes` + ) + }) + + it('passes through unmergeable core.sshCommand wrappers without generic fallback', async () => { + const child = createMockChildProcess(1234) + let capturedEnv: NodeJS.ProcessEnv | undefined + execFileMock.mockImplementation((_cmd, args, opts, cb) => { + if (args[0] === 'config') { + cb(null, '/usr/local/bin/work-ssh-wrapper --account work\n', '') + } else { + capturedEnv = opts.env + cb(null, '', '') + } + return child + }) + + await gitExecFileAsync(['fetch', 'origin'], { + cwd: '/repo', + env: {}, + useConfiguredSshCommandForNetwork: true + }) + + expect(capturedEnv?.GIT_TERMINAL_PROMPT).toBe('0') + expect(capturedEnv?.GIT_ASKPASS).toBe('') + expect(capturedEnv?.SSH_ASKPASS).toBe('') + expect(capturedEnv?.GIT_SSH_COMMAND).toBeUndefined() + }) + + it('passes through shell-expanding OpenSSH configs without changing expansion semantics', async () => { + const child = createMockChildProcess(1234) + let capturedEnv: NodeJS.ProcessEnv | undefined + execFileMock.mockImplementation((_cmd, args, opts, cb) => { + if (args[0] === 'config') { + cb(null, 'ssh -i "$HOME/.ssh/work_key"\n', '') + } else { + capturedEnv = opts.env + cb(null, '', '') + } + return child + }) + + await gitExecFileAsync(['fetch', 'origin'], { + cwd: '/repo', + env: {}, + useConfiguredSshCommandForNetwork: true + }) + + expect(capturedEnv?.GIT_TERMINAL_PROMPT).toBe('0') + expect(capturedEnv?.GIT_SSH_COMMAND).toBeUndefined() + }) + + it('falls back to generic batch-mode SSH when opted-in config is unset', async () => { + const child = createMockChildProcess(1234) + let capturedEnv: NodeJS.ProcessEnv | undefined + execFileMock.mockImplementation((_cmd, args, opts, cb) => { + if (args[0] === 'config') { + cb(Object.assign(new Error('missing'), { code: 1 }), '', '') + } else { + capturedEnv = opts.env + cb(null, '', '') + } + return child + }) + + await gitExecFileAsync(['fetch', 'origin'], { + cwd: '/repo', + env: {}, + useConfiguredSshCommandForNetwork: true + }) + + expect(capturedEnv?.GIT_SSH_COMMAND).toBe('ssh -o BatchMode=yes') + }) + + it('preserves explicit GIT_SSH_COMMAND and skips the opted-in config probe', async () => { + const child = createMockChildProcess(1234) + let capturedEnv: NodeJS.ProcessEnv | undefined + execFileMock.mockImplementation((_cmd, _args, opts, cb) => { + capturedEnv = opts.env + cb(null, '', '') + return child + }) + + await gitExecFileAsync(['fetch', 'origin'], { + cwd: '/repo', + env: { GIT_SSH_COMMAND: 'custom-ssh -o IdentityAgent=none' }, + useConfiguredSshCommandForNetwork: true + }) + + expect(execFileMock).toHaveBeenCalledTimes(1) + expect(capturedEnv?.GIT_SSH_COMMAND).toBe('custom-ssh -o IdentityAgent=none') + expect(capturedEnv?.GIT_TERMINAL_PROMPT).toBe('0') + }) + it('routes git through the selected WSL distro login shell when requested', async () => { await withPlatform('win32', async () => { const child = createMockChildProcess(1234) diff --git a/src/main/git/runner.ts b/src/main/git/runner.ts index 576ba99c6..a66bdba08 100644 --- a/src/main/git/runner.ts +++ b/src/main/git/runner.ts @@ -253,6 +253,7 @@ type GitExecOptions = { env?: NodeJS.ProcessEnv signal?: AbortSignal wslDistro?: string + useConfiguredSshCommandForNetwork?: boolean } type CommandExecOptions = { @@ -517,6 +518,15 @@ export function gitOptionalLocksDisabledEnv( } } +function promptGuardGitEnv(env: NodeJS.ProcessEnv = process.env): NodeJS.ProcessEnv { + return { + ...env, + GIT_TERMINAL_PROMPT: '0', + GIT_ASKPASS: env.GIT_ASKPASS ?? '', + SSH_ASKPASS: env.SSH_ASKPASS ?? '' + } +} + /** * Force git to be non-interactive so it fails fast instead of blocking forever * on a prompt. Without this, a git read-path call (status, worktree list, …) @@ -533,18 +543,175 @@ export function gitOptionalLocksDisabledEnv( * caller hasn't set its own GIT_SSH_COMMAND. */ export function nonInteractiveGitEnv(env: NodeJS.ProcessEnv = process.env): NodeJS.ProcessEnv { - const next: NodeJS.ProcessEnv = { - ...env, - GIT_TERMINAL_PROMPT: '0', - GIT_ASKPASS: env.GIT_ASKPASS ?? '', - SSH_ASKPASS: env.SSH_ASKPASS ?? '' - } + const next = promptGuardGitEnv(env) if (!next.GIT_SSH_COMMAND) { next.GIT_SSH_COMMAND = 'ssh -o BatchMode=yes' } return next } +type GitSshPolicyMode = + | 'default' + | 'explicit-env' + | 'fallback' + | 'configured-openssh' + | 'configured-wrapper-passthrough' + +const CORE_SSH_COMMAND_PROBE_TIMEOUT_MS = 2500 + +function commandBasename(command: string): string { + const pieces = command.split(/[\\/]+/) + return pieces.at(-1)?.toLowerCase() ?? command.toLowerCase() +} + +function isMergeableOpenSshCommand(command: string): boolean { + const basename = commandBasename(command) + return basename === 'ssh' || basename === 'ssh.exe' +} + +function shellTokenize(command: string): string[] | null { + const tokens: string[] = [] + let current = '' + let quote: "'" | '"' | null = null + let escaped = false + + for (let i = 0; i < command.length; i++) { + const char = command[i] + if (escaped) { + current += char + escaped = false + continue + } + if (char === '\\') { + const next = command[i + 1] + if (next && /[\s'"\\]/.test(next)) { + escaped = true + } else { + current += char + } + continue + } + if (quote) { + if (char === quote) { + quote = null + } else { + current += char + } + continue + } + if (char === "'" || char === '"') { + quote = char + continue + } + if (/\s/.test(char)) { + if (current) { + tokens.push(current) + current = '' + } + continue + } + if (';&|<>()`'.includes(char)) { + return null + } + current += char + } + + if (escaped || quote) { + return null + } + if (current) { + tokens.push(current) + } + return tokens +} + +function shellQuoteToken(token: string): string { + return /^[A-Za-z0-9_@%+=:,./~-]+$/.test(token) ? token : quotePosixShell(token) +} + +function containsShellExpansionSyntax(command: string): boolean { + return command.includes('$') +} + +function withoutBatchModeOptions(tokens: string[]): string[] { + const next: string[] = [] + for (let i = 0; i < tokens.length; i++) { + const token = tokens[i] + const lower = token.toLowerCase() + if (lower === '-o') { + const option = tokens[i + 1]?.toLowerCase() + if (option?.startsWith('batchmode')) { + i += 1 + continue + } + } + if (lower.startsWith('-obatchmode')) { + continue + } + next.push(token) + } + return next +} + +function buildOpenSshBatchModeCommand(configuredCommand: string): string | null { + if (containsShellExpansionSyntax(configuredCommand)) { + return null + } + const tokens = shellTokenize(configuredCommand) + if (!tokens || tokens.length === 0 || !isMergeableOpenSshCommand(tokens[0])) { + return null + } + return [...withoutBatchModeOptions(tokens), '-o', 'BatchMode=yes'].map(shellQuoteToken).join(' ') +} + +async function buildNetworkSshPolicyEnv(options: GitExecOptions): Promise<{ + env: NodeJS.ProcessEnv + mode: GitSshPolicyMode +}> { + const promptEnv = promptGuardGitEnv(options.env) + if (promptEnv.GIT_SSH_COMMAND) { + return { env: promptEnv, mode: 'explicit-env' } + } + + const resolved = resolveCommand( + 'git', + ['config', '--get', 'core.sshCommand'], + options.cwd, + options.wslDistro, + { useWslLoginShell: Boolean(options.wslDistro) } + ) + let configuredCommand = '' + try { + const { stdout } = await execFileCapture(resolved.binary, resolved.args, { + cwd: resolved.cwd, + encoding: 'utf-8', + maxBuffer: DEFAULT_GIT_MAX_BUFFER, + timeout: CORE_SSH_COMMAND_PROBE_TIMEOUT_MS, + env: promptEnv, + signal: options.signal + }) + configuredCommand = String(stdout).trim() + } catch { + configuredCommand = '' + } + + if (!configuredCommand) { + return { env: { ...promptEnv, GIT_SSH_COMMAND: 'ssh -o BatchMode=yes' }, mode: 'fallback' } + } + + const batchModeCommand = buildOpenSshBatchModeCommand(configuredCommand) + if (!batchModeCommand) { + // Why: custom wrappers are executable user policy; rewriting their argv is + // riskier than relying on prompt guards plus the caller's target timeout. + return { env: promptEnv, mode: 'configured-wrapper-passthrough' } + } + + return { + env: { ...promptEnv, GIT_SSH_COMMAND: batchModeCommand }, + mode: 'configured-openssh' + } +} + /** * Async git command execution. Drop-in replacement for * `execFileAsync('git', args, { cwd, encoding, ... })`. @@ -563,16 +730,28 @@ export async function gitExecFileAsync( const resolved = resolveCommand('git', args, options.cwd, options.wslDistro, { useWslLoginShell: Boolean(options.wslDistro) }) - const { stdout, stderr } = await execFileCapture(resolved.binary, resolved.args, { - cwd: resolved.cwd, - encoding: (options.encoding ?? 'utf-8') as BufferEncoding, - maxBuffer: options.maxBuffer, - timeout: options.timeout, - // Why: never let a git read-path call block on an interactive prompt - // (issue #5308) — fail fast instead of hanging the runtime. - env: nonInteractiveGitEnv(options.env), - signal: options.signal - }) + const policy = options.useConfiguredSshCommandForNetwork + ? await buildNetworkSshPolicyEnv(options) + : { env: nonInteractiveGitEnv(options.env), mode: 'default' as const } + let result: { stdout: string | Buffer; stderr: string | Buffer } + try { + result = await execFileCapture(resolved.binary, resolved.args, { + cwd: resolved.cwd, + encoding: (options.encoding ?? 'utf-8') as BufferEncoding, + maxBuffer: options.maxBuffer, + timeout: options.timeout, + // Why: never let a git read-path call block on an interactive prompt + // (issue #5308) — fail fast instead of hanging the runtime. + env: policy.env, + signal: options.signal + }) + } catch (error) { + if (options.useConfiguredSshCommandForNetwork && error && typeof error === 'object') { + Object.assign(error, { gitSshPolicyMode: policy.mode }) + } + throw error + } + const { stdout, stderr } = result return { stdout: stdout as string, stderr: stderr as string } } ) diff --git a/src/main/runtime/fetch-remote-cache.test.ts b/src/main/runtime/fetch-remote-cache.test.ts index 848465d49..514594eb6 100644 --- a/src/main/runtime/fetch-remote-cache.test.ts +++ b/src/main/runtime/fetch-remote-cache.test.ts @@ -28,6 +28,14 @@ function fetchCallCount(): number { ).length } +function exactBaseRefreshOptions(cwd: string): { + cwd: string + timeout: number + useConfiguredSshCommandForNetwork: boolean +} { + return { cwd, timeout: 60_000, useConfiguredSshCommandForNetwork: true } +} + function mockFetchResults(results: (Promise | unknown)[]): void { let fetchIndex = 0 gitExecFileAsyncMock.mockImplementation((argv: string[]) => { @@ -175,7 +183,7 @@ describe('OrcaRuntimeService.fetchRemoteWithCache', () => { expect(gitExecFileAsyncMock).toHaveBeenCalledWith( ['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'], - { cwd: '/repo/f' } + exactBaseRefreshOptions('/repo/f') ) }) @@ -289,7 +297,7 @@ describe('OrcaRuntimeService.fetchRemoteWithCache', () => { expect(fetchCalls).toEqual([ [ ['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'], - { cwd: '/repo/h' } + exactBaseRefreshOptions('/repo/h') ], [['fetch', 'origin'], { cwd: '/repo/h' }] ]) @@ -336,7 +344,7 @@ describe('OrcaRuntimeService.fetchRemoteWithCache', () => { [['fetch', 'origin'], { cwd: '/repo/i' }], [ ['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'], - { cwd: '/repo/i' } + exactBaseRefreshOptions('/repo/i') ] ]) }) @@ -382,7 +390,7 @@ describe('OrcaRuntimeService.fetchRemoteWithCache', () => { [['fetch', 'origin'], { cwd: '/repo/i-fail' }], [ ['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'], - { cwd: '/repo/i-fail' } + exactBaseRefreshOptions('/repo/i-fail') ] ]) }) diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 20f030526..756f72449 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -1915,7 +1915,11 @@ describe('OrcaRuntimeService', () => { await vi.waitFor(() => { expect(gitSpy).toHaveBeenCalledWith( ['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'], - { cwd: TEST_REPO_PATH } + { + cwd: TEST_REPO_PATH, + useConfiguredSshCommandForNetwork: true, + timeout: 60_000 + } ) }) expect(addWorktree).not.toHaveBeenCalled() diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 3b8c8dca5..d20b86a7c 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -11558,7 +11558,14 @@ export class OrcaRuntimeService { } return gitExecFileAsync( ['fetch', '--no-tags', base.remote, `+refs/heads/${base.branch}:${base.ref}`], - { cwd: repoPath, ...gitOptions } + { + cwd: repoPath, + ...gitOptions, + // Why: exact remote-base refresh is the network gate for worktree + // creation, so honor repo SSH routing and bound custom wrappers. + useConfiguredSshCommandForNetwork: true, + timeout: 60_000 + } ) .then((): RemoteFetchResult => { this.rememberFreshFetchCompletedAt(key)