From 1a6abc87d117bcdd87e5a7a804cee47761184dd4 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 14 Jul 2026 15:23:06 -0700 Subject: [PATCH] =?UTF-8?q?Suppress=20Git=20Credential=20Manager=20OAuth?= =?UTF-8?q?=20popup=20loop=20in=20Orca-run=20git=20=E2=80=94=20clone,=20te?= =?UTF-8?q?rminals/agents,=20setup=20hooks=20(fixes=20#7652)=20(#7986)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Suppress Git Credential Manager OAuth popup on git clone (fixes #7652) Orca's git runner disables the interactive credential prompt on every git call that goes through gitExecFileAsync/gitStreamStdout, but the two raw 'git clone' spawns (desktop repos:clone and the runtime clone path) passed no env, so they inherited process.env with no guard. On Windows a clone that needs GitHub auth then makes Git Credential Manager pop its 'Connect to GitHub' OAuth window, and in a network-restricted intranet the browser/device flow never completes while git's credential retry re-pops it. Apply nonInteractiveGitEnv() to both clone spawns so the prompt is suppressed (GCM_INTERACTIVE=never, credential.interactive=false, GIT_TERMINAL_PROMPT=0). The credential *helper* is kept, so cached-token clones for private repos still work; only the interactive fallback popup is disabled and the clone fails fast with a clear error instead. * Suppress GCM OAuth popup in agent terminals and setup hooks too (#7652) The clone-spawn fix stopped Orca's own managed git from popping Git Credential Manager, but git run in terminals and setup scripts inherited process.env with no guard. That is the more likely source of the reported loop: agents are told to run 'git pull --rebase'/'git fetch'/retry 'git push' (preamble + conflict/push-failure prompts), and each retry re-pops GCM's 'Connect to GitHub' window in a network-restricted intranet. Apply the credential-prompt guard to: - setup/archive/hook scripts (hooks.ts non-WSL exec env), which run unattended on worktree create/archive. - the shared PTY host env (buildPtyHostEnv), via a small applyTerminalGitCredentialPromptGuard helper. Agent terminals are guarded unconditionally (they cannot dismiss a GUI popup); user terminals are guarded by default via the new terminalSuppressGitCredentialPrompt setting so power users can opt out. The credential helper is kept, so cached gh auth still works; only the interactive fallback prompt is disabled. Verified end-to-end in a real Orca terminal (GIT_TERMINAL_PROMPT=0 + GCM_INTERACTIVE=never by default; absent when the opt-out is set). * Scope user-terminal credential guard to Windows, add settings toggle, forward guard into WSL (#7652) * Retrigger PR checks (Actions dropped the synchronize dispatch for 57e7ce249) * Keep shell locale out of the terminal/hook credential guard (#7652 review fix) * Fix Fable review findings: guard WSL hook branch, wire settings search, catalog keyword keys, sparse-env askpass, one-shot agent classification (#7652) * fix(terminal): harden Git credential popup guard * test(pty): cover SSH credential guard setting * fix(git): guard remote clones and setup runners * fix(git): scope credential guards to unattended work --------- Co-authored-by: Brennan Benson --- src/main/daemon/daemon-pty-adapter.ts | 5 + src/main/daemon/daemon-pty-router.test.ts | 17 +- src/main/daemon/daemon-pty-router.ts | 5 + src/main/daemon/pty-subprocess.test.ts | 143 ++++++++++++++ src/main/daemon/pty-subprocess.ts | 27 ++- src/main/daemon/terminal-host.test.ts | 8 +- src/main/daemon/terminal-host.ts | 1 + src/main/daemon/types.ts | 5 +- src/main/git/runner-command-exec.test.ts | 53 +++++ src/main/git/runner.test.ts | 97 ++++++++++ src/main/git/runner.ts | 92 +++++---- src/main/hooks.test.ts | 36 +++- src/main/hooks.ts | 27 ++- src/main/ipc/pty.test.ts | 83 +++++++- src/main/ipc/pty.ts | 28 ++- src/main/ipc/repos-remote.test.ts | 35 +++- src/main/ipc/repos.ts | 9 +- .../ipc/terminal-git-credential-guard.test.ts | 183 ++++++++++++++++++ src/main/ipc/terminal-git-credential-guard.ts | 4 + src/main/providers/local-pty-provider.test.ts | 42 ++++ src/main/providers/local-pty-provider.ts | 13 +- src/main/providers/ssh-pty-provider.test.ts | 19 ++ src/main/providers/ssh-pty-provider.ts | 1 + src/main/providers/types.ts | 2 + src/main/runtime/orca-runtime.test.ts | 2 + src/main/runtime/orca-runtime.ts | 10 +- src/main/wsl-env.ts | 18 +- src/relay/agent-exec-handler.test.ts | 88 ++++++++- src/relay/agent-exec-handler.ts | 13 +- src/relay/git-handler.ts | 12 +- src/relay/pty-handler.test.ts | 152 +++++++++++++++ src/relay/pty-handler.ts | 50 +++-- src/relay/relay-command-env.test.ts | 32 ++- src/relay/relay-command-env.ts | 13 ++ src/shared/agent-process-recognition.test.ts | 14 ++ src/shared/agent-process-recognition.ts | 37 ++-- src/shared/git-binary-compatibility.test.ts | 27 ++- src/shared/git-credential-prompt-env.ts | 115 +++++++++++ src/shared/terminal-git-credential-guard.ts | 54 ++++++ src/shared/wsl-env.ts | 17 ++ .../terminal-cold-park-resource-bench.mjs | 11 +- .../terminal-cold-park-reveal-bench.mjs | 18 +- 42 files changed, 1478 insertions(+), 140 deletions(-) create mode 100644 src/main/ipc/terminal-git-credential-guard.test.ts create mode 100644 src/main/ipc/terminal-git-credential-guard.ts create mode 100644 src/shared/git-credential-prompt-env.ts create mode 100644 src/shared/terminal-git-credential-guard.ts create mode 100644 src/shared/wsl-env.ts diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index 25a9be9754a..0fc07729383 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -11,6 +11,7 @@ import { mintPtySessionId, parsePtySessionId } from './pty-session-id' import { supportsPtyStartupBarrier } from './shell-ready' import { CODEX_SHELL_READY_TIMEOUT_MS } from './session' import { + GIT_CREDENTIAL_GUARD_HOST_PROTOCOL_VERSION, PROTOCOL_VERSION, type CreateOrAttachResult, type DaemonEvent, @@ -140,6 +141,10 @@ export class DaemonPtyAdapter implements IPtyProvider { private static FULL_CHECKPOINT_COOLDOWN_MS = 45_000 private lastFullCheckpointAt = new Map() + supportsGitCredentialGuardHost(): boolean { + return this.protocolVersion >= GIT_CREDENTIAL_GUARD_HOST_PROTOCOL_VERSION + } + constructor(opts: DaemonPtyAdapterOptions) { this.protocolVersion = opts.protocolVersion ?? PROTOCOL_VERSION this.socketPath = opts.socketPath diff --git a/src/main/daemon/daemon-pty-router.test.ts b/src/main/daemon/daemon-pty-router.test.ts index eb8acb86488..4a2cb52c9d6 100644 --- a/src/main/daemon/daemon-pty-router.test.ts +++ b/src/main/daemon/daemon-pty-router.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import { DaemonPtyRouter } from './daemon-pty-router' import type { DaemonPtyAdapter } from './daemon-pty-adapter' import type { PtyBackgroundStreamEvent, PtySpawnOptions, PtySpawnResult } from '../providers/types' +import { GIT_CREDENTIAL_GUARD_HOST_PROTOCOL_VERSION } from './types' type AdapterMock = DaemonPtyAdapter & { emitData: (id: string, data: string, sequenceChars?: number) => void @@ -22,7 +23,8 @@ function buildSessionIds(prefix: string, count: number): string[] { function createAdapter( label: string, sessions: string[] = [], - reconcileResult?: { alive: string[]; killed: string[] } + reconcileResult?: { alive: string[]; killed: string[] }, + protocolVersion = GIT_CREDENTIAL_GUARD_HOST_PROTOCOL_VERSION ): AdapterMock { const writes: { id: string; data: string }[] = [] const dataListeners: ((payload: { id: string; data: string; sequenceChars?: number }) => void)[] = @@ -30,6 +32,9 @@ function createAdapter( const backgroundListeners: ((payload: PtyBackgroundStreamEvent) => void)[] = [] const exitListeners: ((payload: { id: string; code: number }) => void)[] = [] return { + protocolVersion, + supportsGitCredentialGuardHost: () => + protocolVersion >= GIT_CREDENTIAL_GUARD_HOST_PROTOCOL_VERSION, spawn: vi.fn(async (opts: PtySpawnOptions): Promise => { const id = opts.sessionId ?? `${label}-new` sessions.push(id) @@ -122,6 +127,16 @@ function createAdapter( } describe('DaemonPtyRouter', () => { + it('reports guard-host support for the adapter that owns the session', async () => { + const current = createAdapter('current', [], undefined, 22) + const legacy = createAdapter('legacy', ['legacy-session'], undefined, 21) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + await router.discoverLegacySessions() + + expect(router.supportsGitCredentialGuardHost()).toBe(true) + expect(router.supportsGitCredentialGuardHost('legacy-session')).toBe(false) + }) + it('routes fresh foreground confirmation to the session-owning daemon', async () => { const current = createAdapter('current', ['current-session']) const legacy = createAdapter('legacy', ['legacy-session']) diff --git a/src/main/daemon/daemon-pty-router.ts b/src/main/daemon/daemon-pty-router.ts index 18bf0f926b5..6db11c42efb 100644 --- a/src/main/daemon/daemon-pty-router.ts +++ b/src/main/daemon/daemon-pty-router.ts @@ -62,6 +62,11 @@ export class DaemonPtyRouter implements IPtyProvider { return result } + supportsGitCredentialGuardHost(sessionId?: string): boolean { + const adapter = sessionId ? this.adapterFor(sessionId) : this.current + return adapter.supportsGitCredentialGuardHost() + } + async attach(id: string): Promise { await this.adapterFor(id).attach(id) } diff --git a/src/main/daemon/pty-subprocess.test.ts b/src/main/daemon/pty-subprocess.test.ts index 99a35047743..472c59a619f 100644 --- a/src/main/daemon/pty-subprocess.test.ts +++ b/src/main/daemon/pty-subprocess.test.ts @@ -68,6 +68,8 @@ vi.mock('../providers/agent-foreground-process', () => ({ })) import { createPtySubprocess, checkPtySpawnHealth } from './pty-subprocess' +import { PREVIOUS_DAEMON_PROTOCOL_VERSIONS, PROTOCOL_VERSION } from './types' +import { TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV } from '../../shared/terminal-git-credential-guard' const ORCA_SHELL_WRAPPER_ENV = [ 'ORCA_ATTRIBUTION_SHIM_DIR', @@ -186,6 +188,147 @@ describe('createPtySubprocess', () => { ) }) + it('appends Git prompt guards after the detached daemon inherited config', () => { + const proc = mockPtyProcess() + spawnMock.mockReturnValue(proc) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + const previousWslEnv = process.env.WSLENV + const savedGitConfigEnv = Object.fromEntries( + Object.entries(process.env).filter(([key]) => + /^GIT_CONFIG_(?:COUNT|KEY_\d+|VALUE_\d+)$/.test(key) + ) + ) + for (const key of Object.keys(process.env)) { + if (/^GIT_CONFIG_(?:COUNT|KEY_\d+|VALUE_\d+)$/.test(key)) { + delete process.env[key] + } + } + process.env.GIT_CONFIG_COUNT = '1' + process.env.GIT_CONFIG_KEY_0 = 'core.quotePath' + process.env.GIT_CONFIG_VALUE_0 = 'false' + process.env.WSLENV = 'DAEMON_ONLY/p' + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + + try { + createPtySubprocess({ + sessionId: 'guarded-git-config', + cols: 80, + rows: 24, + env: { + COMSPEC: CMD_ABS, + [TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV]: 'guard' + } + }) + + const spawnEnv = spawnMock.mock.calls.at(-1)?.[2]?.env as Record + expect(spawnEnv.GIT_TERMINAL_PROMPT).toBe('0') + expect(spawnEnv.GCM_INTERACTIVE).toBe('never') + expect(spawnEnv.GIT_CONFIG_COUNT).toBe('3') + expect(spawnEnv.GIT_CONFIG_KEY_0).toBe('core.quotePath') + expect(spawnEnv.GIT_CONFIG_VALUE_0).toBe('false') + expect(spawnEnv.GIT_CONFIG_KEY_1).toBe('credential.interactive') + expect(spawnEnv.GIT_CONFIG_KEY_2).toBe('credential.guiPrompt') + expect((spawnEnv.WSLENV ?? '').split(':')).toContain('DAEMON_ONLY/p') + expect((spawnEnv.WSLENV ?? '').split(':')).toContain('GIT_CONFIG_KEY_2') + } finally { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + for (const key of Object.keys(process.env)) { + if (/^GIT_CONFIG_(?:COUNT|KEY_\d+|VALUE_\d+)$/.test(key)) { + delete process.env[key] + } + } + Object.assign(process.env, savedGitConfigEnv) + if (previousWslEnv === undefined) { + delete process.env.WSLENV + } else { + process.env.WSLENV = previousWslEnv + } + } + }) + + it('does not infer a guard from caller-set prompt scalars', () => { + const proc = mockPtyProcess() + spawnMock.mockReturnValue(proc) + const savedGitConfigEnv = Object.fromEntries( + Object.entries(process.env).filter(([key]) => + /^GIT_CONFIG_(?:COUNT|KEY_\d+|VALUE_\d+)$/.test(key) + ) + ) + for (const key of Object.keys(process.env)) { + if (/^GIT_CONFIG_(?:COUNT|KEY_\d+|VALUE_\d+)$/.test(key)) { + delete process.env[key] + } + } + process.env.GIT_CONFIG_COUNT = '3' + process.env.GIT_CONFIG_KEY_0 = 'core.quotePath' + process.env.GIT_CONFIG_VALUE_0 = 'false' + process.env.GIT_CONFIG_KEY_1 = 'base.one' + process.env.GIT_CONFIG_VALUE_1 = 'one' + process.env.GIT_CONFIG_KEY_2 = 'base.two' + process.env.GIT_CONFIG_VALUE_2 = 'two' + + try { + createPtySubprocess({ + sessionId: 'explicit-guarded-git-config', + cols: 80, + rows: 24, + env: { + SHELL: '/bin/bash', + GIT_TERMINAL_PROMPT: '0', + GCM_INTERACTIVE: 'never', + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'http.proxy', + GIT_CONFIG_VALUE_0: 'http://proxy.invalid' + } + }) + + const spawnEnv = spawnMock.mock.calls.at(-1)?.[2]?.env as Record + expect(spawnEnv.GIT_TERMINAL_PROMPT).toBe('0') + expect(spawnEnv.GCM_INTERACTIVE).toBe('never') + expect(spawnEnv.GIT_CONFIG_COUNT).toBe('1') + expect(spawnEnv.GIT_CONFIG_KEY_0).toBe('http.proxy') + expect(spawnEnv.GIT_CONFIG_VALUE_0).toBe('http://proxy.invalid') + expect(Object.values(spawnEnv)).not.toContain('core.quotePath') + expect(Object.values(spawnEnv)).not.toContain('base.one') + expect(Object.values(spawnEnv)).not.toContain('base.two') + expect(spawnEnv.GIT_CONFIG_KEY_1).toBeUndefined() + } finally { + for (const key of Object.keys(process.env)) { + if (/^GIT_CONFIG_(?:COUNT|KEY_\d+|VALUE_\d+)$/.test(key)) { + delete process.env[key] + } + } + Object.assign(process.env, savedGitConfigEnv) + } + }) + + it('guards a trusted daemon agent whose launch command is wrapped', () => { + const proc = mockPtyProcess() + spawnMock.mockReturnValue(proc) + + createPtySubprocess({ + sessionId: 'trusted-wrapped-agent', + cols: 80, + rows: 24, + command: 'cd /repo && custom-agent-wrapper', + launchAgent: 'claude', + env: { SHELL: '/bin/bash' } + }) + + const spawnEnv = spawnMock.mock.calls.at(-1)?.[2]?.env as Record + expect(spawnEnv.GIT_TERMINAL_PROMPT).toBe('0') + expect(spawnEnv.GCM_INTERACTIVE).toBe('never') + expect(Object.values(spawnEnv)).toContain('credential.interactive') + expect(Object.values(spawnEnv)).toContain('credential.guiPrompt') + }) + + it('uses a new daemon protocol for post-merge Git guard behavior', () => { + expect(PROTOCOL_VERSION).toBeGreaterThan(21) + expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(21) + }) + it('resolves a missing Unix default before spawning node-pty', () => { const proc = mockPtyProcess() spawnMock.mockReturnValue(proc) diff --git a/src/main/daemon/pty-subprocess.ts b/src/main/daemon/pty-subprocess.ts index ce1b1faf35c..b0e69655a08 100644 --- a/src/main/daemon/pty-subprocess.ts +++ b/src/main/daemon/pty-subprocess.ts @@ -34,6 +34,11 @@ import { removeInheritedNoColor } from '../pty/terminal-color-env' import { removeAppImageRuntimeEnv } from '../pty/appimage-terminal-env' import { parseWslPath } from '../wsl' import { addWslEnvKeys } from '../wsl-env' +import { + gitCredentialPromptGuardEnv, + mergeGitConfigEnvProtocol +} from '../../shared/git-credential-prompt-env' +import { TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV } from '../../shared/terminal-git-credential-guard' import { getWslContextFromSessionId } from './wsl-session-context' import { addOrcaWslInteropEnv } from '../pty/wsl-orca-env' import { @@ -58,6 +63,7 @@ import { parsePtySessionId } from './pty-session-id' import { getAgentForegroundContextPaths } from '../providers/agent-foreground-context-paths' import { assertSafeAgentStartupCwd, resolveSafePtyDefaultCwd } from '../providers/pty-default-cwd' import { ORCA_HERMES_STARTUP_QUERY_ENV } from '../../shared/hermes-startup-query' +import type { TuiAgent } from '../../shared/types' const PANE_IDENTITY_ENV_KEYS = [ 'ORCA_PANE_KEY', @@ -82,6 +88,22 @@ const PTY_SPAWN_HEALTH_TIMEOUT_MS = 4_000 const PTY_SPAWN_HEALTH_RETRY_ATTEMPTS = 2 const PENDING_PRE_LISTENER_DATA_MAX_CHARS = 512 * 1024 +function composeGuardedDaemonGitConfigEnv( + env: Record, + explicitEnv: Record | undefined, + launchAgent: TuiAgent | undefined +): void { + const policy = explicitEnv?.[TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV] + delete env[TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV] + if (policy !== 'guard' && launchAgent === undefined) { + return + } + // Why: the daemon can outlive Electron, so only its process.env is the + // authoritative inherited config. The raw env merge already gives an + // explicit wire protocol normal override semantics; append only the guard. + Object.assign(env, gitCredentialPromptGuardEnv(env, process.platform)) +} + export type PtySubprocessOptions = { sessionId: string cols: number @@ -91,6 +113,7 @@ export type PtySubprocessOptions = { envToDelete?: string[] command?: string startupCommandDelivery?: StartupCommandDelivery + launchAgent?: TuiAgent /** Explicit shell executable path/basename the renderer asked for. * Overrides env.COMSPEC / env.SHELL resolution inside the daemon so a user * who picks "New WSL terminal" from the "+" menu actually gets WSL. */ @@ -543,8 +566,7 @@ function spawnDaemonPtyWithWindowsFallback(args: { export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandle { const size = normalizePtySize(opts.cols, opts.rows) const env: Record = { - ...process.env, - ...opts.env, + ...mergeGitConfigEnvProtocol(process.env, opts.env), TERM: 'xterm-256color', COLORTERM: 'truecolor', TERM_PROGRAM: 'Orca', @@ -561,6 +583,7 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl // restores clickable refs like `owner/repo#123` / `PR#123`. FORCE_HYPERLINK: '1' } as Record + composeGuardedDaemonGitConfigEnv(env, opts.env, opts.launchAgent) for (const key of opts.envToDelete ?? []) { delete env[key] } diff --git a/src/main/daemon/terminal-host.test.ts b/src/main/daemon/terminal-host.test.ts index 46e495e5f2e..2a12b5bf4ab 100644 --- a/src/main/daemon/terminal-host.test.ts +++ b/src/main/daemon/terminal-host.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { Session, type SubprocessHandle } from './session' import { TerminalHost } from './terminal-host' +import type { TuiAgent } from '../../shared/types' function createMockSubprocess( options: { startupCommandDeliveredInShellArgs?: boolean; shellPath?: string } = {} @@ -45,6 +46,7 @@ type MockSpawnFn = (opts: { cwd?: string env?: Record command?: string + launchAgent?: TuiAgent }) => SubprocessHandle describe('TerminalHost', () => { @@ -124,13 +126,14 @@ describe('TerminalHost', () => { expect(result.snapshot?.cols).toBe(80) }) - it('passes cwd and env to spawn', async () => { + it('passes cwd, env, and trusted agent identity to spawn', async () => { await host.createOrAttach({ sessionId: 'session-1', cols: 80, rows: 24, cwd: '/home/user', env: { FOO: 'bar' }, + launchAgent: 'claude', streamClient: { onData: vi.fn(), onExit: vi.fn() } }) @@ -138,7 +141,8 @@ describe('TerminalHost', () => { expect.objectContaining({ sessionId: 'session-1', cwd: '/home/user', - env: { FOO: 'bar' } + env: { FOO: 'bar' }, + launchAgent: 'claude' }) ) }) diff --git a/src/main/daemon/terminal-host.ts b/src/main/daemon/terminal-host.ts index b15420fed14..e9726742bc8 100644 --- a/src/main/daemon/terminal-host.ts +++ b/src/main/daemon/terminal-host.ts @@ -100,6 +100,7 @@ export class TerminalHost { envToDelete: opts.envToDelete, command: opts.command, startupCommandDelivery: opts.startupCommandDelivery, + ...(opts.launchAgent ? { launchAgent: opts.launchAgent } : {}), shellOverride: opts.shellOverride, terminalWindowsWslDistro: opts.terminalWindowsWslDistro, terminalWindowsPowerShellImplementation: opts.terminalWindowsPowerShellImplementation diff --git a/src/main/daemon/types.ts b/src/main/daemon/types.ts index 550b211aa74..e9e7fb3c7a6 100644 --- a/src/main/daemon/types.ts +++ b/src/main/daemon/types.ts @@ -16,9 +16,10 @@ import type { TuiAgent } from '../../shared/types' // when daemon-baked behavior cannot be delivered by on-disk wrapper refresh. // Why: bump when adding daemon wire behavior so same-version old daemons do // not silently accept the handshake and then reject new RPCs. -export const PROTOCOL_VERSION = 21 +export const PROTOCOL_VERSION = 22 +export const GIT_CREDENTIAL_GUARD_HOST_PROTOCOL_VERSION = 22 export const PREVIOUS_DAEMON_PROTOCOL_VERSIONS = [ - 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21 ] as const // ─── Session State Machine ────────────────────────────────────────── diff --git a/src/main/git/runner-command-exec.test.ts b/src/main/git/runner-command-exec.test.ts index 2d31c255528..62fbde608b3 100644 --- a/src/main/git/runner-command-exec.test.ts +++ b/src/main/git/runner-command-exec.test.ts @@ -501,6 +501,59 @@ describe('runner execFile timeout handling', () => { }) }) + it('forwards synthesized network SSH policy into the selected WSL distro', async () => { + await withPlatform('win32', async () => { + const child = createMockChildProcess(1234) + let capturedEnv: NodeJS.ProcessEnv | undefined + execFileMock.mockImplementation((_cmd, args, opts, cb) => { + const shellCommand = args[5] as string + if (shellCommand.includes("'config'")) { + cb(Object.assign(new Error('missing'), { code: 1 }), '', '') + } else { + capturedEnv = opts.env + cb(null, '', '') + } + return child + }) + + await gitExecFileAsync(['fetch', 'origin'], { + cwd: String.raw`C:\repo`, + env: {}, + wslDistro: 'Ubuntu', + useConfiguredSshCommandForNetwork: true + }) + + expect(capturedEnv?.GIT_SSH_COMMAND).toBe('ssh -o BatchMode=yes') + expect((capturedEnv?.WSLENV ?? '').split(':')).toContain('GIT_SSH_COMMAND') + }) + }) + + it('forwards synthesized network SSH policy when a UNC cwd selects WSL', async () => { + await withPlatform('win32', async () => { + const child = createMockChildProcess(1234) + let capturedEnv: NodeJS.ProcessEnv | undefined + execFileMock.mockImplementation((_cmd, args, opts, cb) => { + const shellCommand = args[5] as string + if (shellCommand.includes("'config'")) { + cb(Object.assign(new Error('missing'), { code: 1 }), '', '') + } else { + capturedEnv = opts.env + cb(null, '', '') + } + return child + }) + + await gitExecFileAsync(['fetch', 'origin'], { + cwd: String.raw`\\wsl.localhost\Ubuntu\home\me\repo`, + env: {}, + useConfiguredSshCommandForNetwork: true + }) + + expect(capturedEnv?.GIT_SSH_COMMAND).toBe('ssh -o BatchMode=yes') + expect((capturedEnv?.WSLENV ?? '').split(':')).toContain('GIT_SSH_COMMAND') + }) + }) + it('quotes WSL-routed executables before entering the shell', async () => { await withPlatform('win32', async () => { const child = createMockChildProcess(1234) diff --git a/src/main/git/runner.test.ts b/src/main/git/runner.test.ts index df0392c3c10..92832fc0f89 100644 --- a/src/main/git/runner.test.ts +++ b/src/main/git/runner.test.ts @@ -9,9 +9,11 @@ import { nonInteractiveGitEnv, parseRetryAfterMs, promptGuardGitEnv, + promptGuardShellEnv, redirectPortedHostnameToEnv, untranslatedGitOutputEnv } from './runner' +import { mergeGitConfigEnvProtocol } from '../../shared/git-credential-prompt-env' // Reads git config injected via the GIT_CONFIG_COUNT/KEY/VALUE env protocol // back into a plain key→value map so tests can assert on it directly. @@ -189,6 +191,47 @@ describe('appendGitConfigEnv', () => { expect(env.GIT_CONFIG_KEY_1).toBe('credential.guiPrompt') expect(env.GIT_CONFIG_VALUE_1).toBe('false') }) + + it.each(['bogus', '-1', '0', String(Number.MAX_SAFE_INTEGER)])( + 'does not overwrite dangling caller config when count is %s', + (count) => { + const original = { + GIT_CONFIG_COUNT: count, + GIT_CONFIG_KEY_0: 'user.key', + GIT_CONFIG_VALUE_0: 'caller-value' + } + expect(appendGitConfigEnv(original, [['credential.interactive', 'false']])).toEqual(original) + } + ) + + it('does not append to an incomplete indexed-config protocol', () => { + const original = { GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'user.key' } + expect(appendGitConfigEnv(original, [['credential.interactive', 'false']])).toEqual(original) + }) +}) + +describe('mergeGitConfigEnvProtocol', () => { + it('replaces inherited indexed config atomically when an override has a smaller count', () => { + const env = mergeGitConfigEnvProtocol( + { + GIT_CONFIG_COUNT: '2', + GIT_CONFIG_KEY_0: 'base.zero', + GIT_CONFIG_VALUE_0: 'zero', + GIT_CONFIG_KEY_1: 'base.one', + GIT_CONFIG_VALUE_1: 'one' + }, + { + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'override.zero', + GIT_CONFIG_VALUE_0: 'override' + } + ) + + expect(env.GIT_CONFIG_COUNT).toBe('1') + expect(env.GIT_CONFIG_KEY_0).toBe('override.zero') + expect(env.GIT_CONFIG_KEY_1).toBeUndefined() + expect(env.GIT_CONFIG_VALUE_1).toBeUndefined() + }) }) describe('promptGuardGitEnv credential-interactivity disable (STA-1292)', () => { @@ -220,6 +263,60 @@ describe('nonInteractiveGitEnv credential-interactivity disable (STA-1292)', () }) }) +describe('guard-env WSLENV forwarding (#7652)', () => { + it('registers the guard vars in WSLENV on Windows so WSL-routed git imports them', () => { + const env = promptGuardGitEnv({ PATH: '/usr/bin' }, 'win32') + const keys = (env.WSLENV ?? '').split(':') + expect(keys).toContain('GIT_TERMINAL_PROMPT') + expect(keys).toContain('GCM_INTERACTIVE') + expect(keys).toContain('GIT_CONFIG_COUNT') + expect(keys).toContain('GIT_CONFIG_KEY_0') + expect(keys).toContain('GIT_CONFIG_VALUE_0') + expect(keys).toContain('GIT_CONFIG_KEY_1') + expect(keys).toContain('GIT_CONFIG_VALUE_1') + // Windows askpass paths are meaningless inside a distro. + expect(keys).not.toContain('GIT_ASKPASS') + expect(keys).not.toContain('SSH_ASKPASS') + }) + + it('preserves a caller-set WSLENV instead of clobbering it', () => { + const env = promptGuardGitEnv({ PATH: '/usr/bin', WSLENV: 'MY_VAR/p' }, 'win32') + const keys = (env.WSLENV ?? '').split(':') + expect(keys[0]).toBe('MY_VAR/p') + expect(keys).toContain('GIT_TERMINAL_PROMPT') + }) + + it('does not touch WSLENV on non-Windows hosts', () => { + const env = promptGuardGitEnv({ PATH: '/usr/bin' }, 'darwin') + expect(env.WSLENV).toBeUndefined() + }) + + it('forwards GIT_SSH_COMMAND only when nonInteractiveGitEnv set the default itself', () => { + const defaulted = nonInteractiveGitEnv({ PATH: '/usr/bin' }, 'win32') + expect((defaulted.WSLENV ?? '').split(':')).toContain('GIT_SSH_COMMAND') + + // A caller's Windows-specific ssh command must not leak into the distro. + const callerSet = nonInteractiveGitEnv( + { PATH: '/usr/bin', GIT_SSH_COMMAND: 'C:\\ssh\\ssh.exe' }, + 'win32' + ) + expect((callerSet.WSLENV ?? '').split(':')).not.toContain('GIT_SSH_COMMAND') + }) +}) + +describe('promptGuardShellEnv keeps the shell locale (#7652 x #7808)', () => { + it('guards without pinning the locale — a terminal env is the whole shell, not just git', () => { + const env = promptGuardShellEnv({ PATH: '/usr/bin', LC_ALL: 'ja_JP.UTF-8' }, 'win32') + expect(env.GIT_TERMINAL_PROMPT).toBe('0') + expect(env.GCM_INTERACTIVE).toBe('never') + expect((env.WSLENV ?? '').split(':')).toContain('GIT_TERMINAL_PROMPT') + // The user's locale survives; no pins appear where none existed. + expect(env.LC_ALL).toBe('ja_JP.UTF-8') + expect(env.LANG).toBeUndefined() + expect(env.LANGUAGE).toBeUndefined() + }) +}) + describe('git env forces untranslated diagnostics (issue #7808)', () => { it('overrides an inherited non-English locale so stderr parsers keep working', () => { // A gettext-enabled git under de_DE translates even the `fatal:` prefix, diff --git a/src/main/git/runner.ts b/src/main/git/runner.ts index 29990ad9c64..27d1e5837b9 100644 --- a/src/main/git/runner.ts +++ b/src/main/git/runner.ts @@ -29,6 +29,11 @@ import { notifyGhPrimaryRateLimit } from './gh-rate-limit-breaker' import { getDefaultWslDistro, parseWslPath, toWindowsWslPath, type WslPathInfo } from '../wsl' +import { addWslEnvKeys } from '../wsl-env' +import { + appendGitConfigEnv, + gitCredentialPromptGuardEnv +} from '../../shared/git-credential-prompt-env' import { getSpawnArgsForWindows, isWindowsBatchScript, resolveWindowsCommand } from '../win32-utils' import { buildWslLoginShellCommand, @@ -558,20 +563,7 @@ export function gitOptionalLocksDisabledEnv( * already present in `env` so we never clobber config a caller injected the * same way. */ -export function appendGitConfigEnv( - env: NodeJS.ProcessEnv, - entries: readonly (readonly [key: string, value: string])[] -): NodeJS.ProcessEnv { - const parsed = Number.parseInt(env.GIT_CONFIG_COUNT ?? '', 10) - const base = Number.isInteger(parsed) && parsed > 0 ? parsed : 0 - const next = { ...env } - entries.forEach(([key, value], index) => { - next[`GIT_CONFIG_KEY_${base + index}`] = key - next[`GIT_CONFIG_VALUE_${base + index}`] = value - }) - next.GIT_CONFIG_COUNT = String(base + entries.length) - return next -} +export { appendGitConfigEnv } /** * Pin Orca-spawned git to untranslated English output so stderr/progress @@ -584,26 +576,25 @@ export function untranslatedGitOutputEnv(env: NodeJS.ProcessEnv = process.env): return { ...env, ...UNTRANSLATED_GIT_OUTPUT_ENV } } -export function promptGuardGitEnv(env: NodeJS.ProcessEnv = process.env): NodeJS.ProcessEnv { - return appendGitConfigEnv( - { - ...untranslatedGitOutputEnv(env), - GIT_TERMINAL_PROMPT: '0', - GIT_ASKPASS: env.GIT_ASKPASS ?? '', - SSH_ASKPASS: env.SSH_ASKPASS ?? '', - // Why: Git Credential Manager ignores GIT_TERMINAL_PROMPT / GIT_ASKPASS and - // pops a GUI on first auth — the Windows worktree-create hang (STA-1292). - // `never` suppresses the prompt while still serving cached credentials. - GCM_INTERACTIVE: 'never' - }, - // Why: disable only the *interactive* credential prompt, NOT the helper - // itself — an empty credential.helper would break cached-credential auth for - // private repos. Harmless on macOS/Linux (no GCM) and on the SSH path. - [ - ['credential.interactive', 'false'], - ['credential.guiPrompt', 'false'] - ] - ) +export function promptGuardGitEnv( + env: NodeJS.ProcessEnv = process.env, + platform: NodeJS.Platform = process.platform +): NodeJS.ProcessEnv { + return gitCredentialPromptGuardEnv(untranslatedGitOutputEnv(env), platform) +} + +/** + * Credential-prompt guard for a general-purpose shell environment (terminal + * PTYs, hook scripts): everything promptGuardGitEnv does EXCEPT the issue-7808 + * locale pins. Those exist so Orca can parse stderr of git it spawns itself; + * forcing LC_ALL/LANG/LANGUAGE onto a user's shell would change the locale of + * every child process, not just git's. + */ +export function promptGuardShellEnv( + env: NodeJS.ProcessEnv = process.env, + platform: NodeJS.Platform = process.platform +): NodeJS.ProcessEnv { + return gitCredentialPromptGuardEnv(env, platform) } /** @@ -614,17 +605,28 @@ export function promptGuardGitEnv(env: NodeJS.ProcessEnv = process.env): NodeJS. * stuck calls pile up and the runtime stops answering all clients (issue #5308). * * - GIT_TERMINAL_PROMPT=0: git refuses to prompt for credentials and errors out. - * - GIT_ASKPASS / SSH_ASKPASS='': disable any GUI/askpass credential helper that - * would otherwise pop a prompt and block. + * - GIT_ASKPASS / SSH_ASKPASS: emptied when unset so no GUI/askpass helper can + * pop a prompt and block. A caller-provided askpass is preserved on purpose — + * custom askpass setups commonly *serve* credentials non-interactively, and + * blanking them would break those fetches. * - GIT_SSH_COMMAND BatchMode=yes: SSH fails instead of waiting on an * interactive password/host-key prompt. BatchMode does NOT change host trust * (an unknown host still errors, it just won't hang). Only added when the * caller hasn't set its own GIT_SSH_COMMAND. */ -export function nonInteractiveGitEnv(env: NodeJS.ProcessEnv = process.env): NodeJS.ProcessEnv { - const next = promptGuardGitEnv(env) +export function nonInteractiveGitEnv( + env: NodeJS.ProcessEnv = process.env, + platform: NodeJS.Platform = process.platform +): NodeJS.ProcessEnv { + const next = promptGuardGitEnv(env, platform) if (!next.GIT_SSH_COMMAND) { next.GIT_SSH_COMMAND = 'ssh -o BatchMode=yes' + if (platform === 'win32') { + // Why: forward across the WSL boundary only when we set the value — + // plain `ssh` resolves inside the distro, whereas a caller's + // Windows-specific GIT_SSH_COMMAND must not leak into Linux git. + addWslEnvKeys(next, ['GIT_SSH_COMMAND']) + } } return next } @@ -775,7 +777,12 @@ async function buildNetworkSshPolicyEnv(options: GitExecOptions): Promise<{ } if (!configuredCommand) { - return { env: { ...promptEnv, GIT_SSH_COMMAND: 'ssh -o BatchMode=yes' }, mode: 'fallback' } + const env = { ...promptEnv, GIT_SSH_COMMAND: 'ssh -o BatchMode=yes' } + // Why: WSL routing can come from either an explicit distro or a UNC cwd. + if (resolved.wsl) { + addWslEnvKeys(env, ['GIT_SSH_COMMAND']) + } + return { env, mode: 'fallback' } } const batchModeCommand = buildOpenSshBatchModeCommand(configuredCommand) @@ -785,10 +792,11 @@ async function buildNetworkSshPolicyEnv(options: GitExecOptions): Promise<{ return { env: promptEnv, mode: 'configured-wrapper-passthrough' } } - return { - env: { ...promptEnv, GIT_SSH_COMMAND: batchModeCommand }, - mode: 'configured-openssh' + const env = { ...promptEnv, GIT_SSH_COMMAND: batchModeCommand } + if (resolved.wsl) { + addWslEnvKeys(env, ['GIT_SSH_COMMAND']) } + return { env, mode: 'configured-openssh' } } /** diff --git a/src/main/hooks.test.ts b/src/main/hooks.test.ts index 14a28fc46c4..9e945322fa4 100644 --- a/src/main/hooks.test.ts +++ b/src/main/hooks.test.ts @@ -1,5 +1,6 @@ /* eslint-disable max-lines -- Why: hook parsing, shell selection, and execution-path regressions are tightly coupled, so these cases stay in one file to preserve the behavior matrix across platforms. */ import type { Repo } from '../shared/types' +import type * as GitRunner from './git/runner' import { join } from 'node:path' import { describe, expect, it, vi } from 'vitest' @@ -29,7 +30,8 @@ vi.mock('child_process', () => ({ spawn: vi.fn() })) -vi.mock('./git/runner', () => ({ +vi.mock('./git/runner', async () => ({ + ...(await vi.importActual('./git/runner')), gitExecFileSync: gitExecFileSyncMock })) @@ -892,7 +894,13 @@ describe('runHook', () => { 'echo hello', expect.objectContaining({ cwd: '/repo/worktree', - shell: '/bin/bash' + shell: '/bin/bash', + // Setup hooks run unattended: git in them must not pop the OS + // credential helper's OAuth window and loop it (issue #7652). + env: expect.objectContaining({ + GIT_TERMINAL_PROMPT: '0', + GCM_INTERACTIVE: 'never' + }) }), expect.any(Function) ) @@ -948,7 +956,15 @@ describe('runHook', () => { expect(execFileMock).toHaveBeenCalledWith( 'wsl.exe', ['-d', 'Ubuntu', '--', 'bash', '-c', "cd '/home/jin/feature' && echo hello"], - expect.any(Object), + // #7652 regression: the unattended WSL hook branch must carry the + // credential guard, and WSLENV is what carries it into the distro. + expect.objectContaining({ + env: expect.objectContaining({ + GIT_TERMINAL_PROMPT: '0', + GCM_INTERACTIVE: 'never', + WSLENV: expect.stringContaining('GIT_TERMINAL_PROMPT') + }) + }), expect.any(Function) ) expect(execMock).not.toHaveBeenCalled() @@ -1153,6 +1169,20 @@ describe('createSetupRunnerScript', () => { .waitForAgentStartup ).toBe(true) }) + + it('marks setup-runner terminals for the always-on credential guard', async () => { + gitExecFileSyncMock.mockReset() + gitExecFileSyncMock.mockReturnValue('/test/repo/.git/orca/setup-runner.sh\n') + const { createSetupRunnerScript } = await import('./hooks') + + const setup = createSetupRunnerScript(makeRepo(), '/test/worktree', 'git fetch') + + expect(setup.envVars).toMatchObject({ + ORCA_ROOT_PATH: '/test/repo', + ORCA_WORKTREE_PATH: '/test/worktree', + ORCA_INTERNAL_TERMINAL_GIT_CREDENTIAL_GUARD_POLICY: 'guard' + }) + }) }) describe('shouldRunSetupForCreate', () => { diff --git a/src/main/hooks.ts b/src/main/hooks.ts index 28c2e32f913..e93fa2f033c 100644 --- a/src/main/hooks.ts +++ b/src/main/hooks.ts @@ -6,8 +6,9 @@ import { getDefaultRepoHookSettings } from '../shared/constants' import { getRuntimePathBasename } from '../shared/cross-platform-path' import { resolveHookCommandSourcePolicy } from '../shared/hook-command-source-policy' import { shouldWaitForSetupBeforeAgentStartup } from '../shared/setup-agent-startup-policy' +import { TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV } from '../shared/terminal-git-credential-guard' import { parseOrcaYaml } from '../shared/orca-yaml' -import { gitExecFileSync } from './git/runner' +import { gitExecFileSync, promptGuardShellEnv } from './git/runner' import { isWslPath, parseWslPath, toWindowsWslPath, toLinuxPath } from './wsl' import type { HookCommandSourcePolicy, @@ -455,7 +456,12 @@ export function createSetupRunnerScript( } export function getSetupRunnerEnvVars(repo: Repo, worktreePath: string): Record { - return getSetupEnvVars(repo, worktreePath) + return { + ...getSetupEnvVars(repo, worktreePath), + // Why: the visible Setup terminal is still unattended automation; user + // terminal opt-out must not let its git commands open credential UI. + [TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV]: 'guard' + } } export function buildPosixRunnerScript(script: string): string { @@ -511,7 +517,7 @@ function createWorktreeRunnerScript( runtimeTarget?: HookRuntimeTarget, waitForAgentStartup?: boolean ): WorktreeSetupLaunch { - const envVars = getSetupEnvVars(repo, worktreePath) + const envVars = getSetupRunnerEnvVars(repo, worktreePath) // Why: WSL worktrees run on a Linux filesystem even though process.platform // is 'win32'. Use bash scripts for WSL, .cmd for native Windows. const wslWorktree = isWslPath(worktreePath) || Boolean(runtimeTarget?.wslDistro) @@ -636,7 +642,11 @@ export function runHook( { timeout: HOOK_TIMEOUT, encoding: 'utf-8', - env: { ...process.env, ...wslEnv } + // Why: same unattended-git guard as the non-WSL branch below + // (issue #7652) — WSL repos are the likeliest to hit the GCM + // popup, and the guard's WSLENV registration is what carries it + // across the wsl.exe boundary into the distro. + env: promptGuardShellEnv({ ...process.env, ...wslEnv }) }, (error, stdout, stderr) => { finish(error ?? null, stdout, stderr) @@ -655,10 +665,15 @@ export function runHook( cwd, timeout: HOOK_TIMEOUT, shell: getHookShell(), - env: { + // Why: setup/archive hooks run unattended, so a `git fetch`/`submodule + // update` inside one must never make Git Credential Manager pop its + // "Connect to GitHub" OAuth window on Windows and loop when the network + // can't complete it (issue #7652). The guard keeps the credential + // helper, so cached auth still works; only the interactive prompt dies. + env: promptGuardShellEnv({ ...process.env, ...getSetupEnvVars(repo, cwd) - } + }) }, (error, stdout, stderr) => { if (error) { diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index d7c2b11ca71..40efbe69908 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -11,6 +11,7 @@ import { import { CLIPBOARD_TEXT_MEASURE_YIELD_CODE_UNITS } from '../../shared/clipboard-text' import { redactPtyIdForDiagnostics } from '../../shared/pty-delivery-diagnostics' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../shared/constants' +import type { TuiAgent } from '../../shared/types' const isWindowsHost = process.platform === 'win32' const posixOnlyIt = isWindowsHost ? it.skip : it @@ -759,7 +760,8 @@ describe('registerPtyHandlers', () => { // buildPtyHostEnv to piTitlebarExtensionService.buildPtyEnv was untested // for the OMP case because this helper never forwarded a command. Accept // an optional `command` so callers can exercise OMP target resolution. - command?: string + command?: string, + launchAgent?: TuiAgent ): Promise> { const savedEnv: Record = {} if (processEnvOverrides) { @@ -787,7 +789,8 @@ describe('registerPtyHandlers', () => { cols: 80, rows: 24, ...(argsEnv ? { env: argsEnv } : {}), - ...(command ? { command } : {}) + ...(command ? { command } : {}), + ...(launchAgent ? { launchAgent } : {}) }) const spawnCall = spawnMock.mock.calls.at(-1)! return spawnCall[2].env as Record @@ -876,6 +879,27 @@ describe('registerPtyHandlers', () => { expect(env.TERM_PROGRAM).toBe('Orca') }) + it('keeps indexed Git prompt guards in a local agent terminal env', async () => { + const env = await spawnAndGetEnv(undefined, undefined, undefined, undefined, 'claude') + expect(env.GIT_TERMINAL_PROMPT).toBe('0') + expect(env.GCM_INTERACTIVE).toBe('never') + expect(Object.values(env)).toContain('credential.interactive') + expect(Object.values(env)).toContain('credential.guiPrompt') + }) + + it('guards a trusted local agent when its command uses a custom wrapper', async () => { + const env = await spawnAndGetEnv( + undefined, + undefined, + undefined, + undefined, + 'cd /repo && custom-agent-wrapper', + 'claude' + ) + expect(env.GIT_TERMINAL_PROMPT).toBe('0') + expect(env.GCM_INTERACTIVE).toBe('never') + }) + it('advertises OSC 8 hyperlink support via FORCE_HYPERLINK', async () => { // Why: the supports-hyperlinks npm package hard-codes a TERM_PROGRAM // allowlist (iTerm.app / WezTerm / vscode) and reports false for @@ -1361,7 +1385,7 @@ describe('registerPtyHandlers', () => { // OpenCode plugin dir, Pi managed extension env, Codex home, and dev-mode CLI // overrides were silently missing for daemon users (the common case). - function setupDaemonAdapter() { + function setupDaemonAdapter(supportsGitCredentialGuardHost = true) { const daemonSpawn = vi.fn( async (options: { env: Record @@ -1373,6 +1397,7 @@ describe('registerPtyHandlers', () => { ) setLocalPtyProvider({ spawn: daemonSpawn, + supportsGitCredentialGuardHost: () => supportsGitCredentialGuardHost, write: vi.fn(), resize: vi.fn(), kill: vi.fn(), @@ -1449,9 +1474,10 @@ describe('registerPtyHandlers', () => { shellOverride?: string command?: string envToDelete?: string[] - } + }, + supportsGitCredentialGuardHost = true ): Promise { - const daemonSpawn = setupDaemonAdapter() + const daemonSpawn = setupDaemonAdapter(supportsGitCredentialGuardHost) const savedEnv: Record = {} if (processEnvOverrides) { for (const [k, v] of Object.entries(processEnvOverrides)) { @@ -1498,7 +1524,8 @@ describe('registerPtyHandlers', () => { httpProxyBypassRules?: string }, processEnvOverrides?: Record, - spawnArgs?: { cwd?: string; shellOverride?: string; command?: string } + spawnArgs?: { cwd?: string; shellOverride?: string; command?: string }, + supportsGitCredentialGuardHost = true ): Promise> { return ( await daemonSpawnAndGetOptions( @@ -1506,7 +1533,8 @@ describe('registerPtyHandlers', () => { getSelectedCodexHomePath, getSettings, processEnvOverrides, - spawnArgs + spawnArgs, + supportsGitCredentialGuardHost ) ).env } @@ -2065,6 +2093,47 @@ describe('registerPtyHandlers', () => { } }) + it('defers indexed Git prompt guards from the daemon wire environment', async () => { + const env = await daemonSpawnAndGetEnv( + { + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'http.proxy', + GIT_CONFIG_VALUE_0: 'http://proxy.invalid' + }, + undefined, + undefined, + undefined, + { command: 'claude' } + ) + + expect(env.GIT_TERMINAL_PROMPT).toBe('0') + expect(env.GCM_INTERACTIVE).toBe('never') + expect(env.GIT_CONFIG_COUNT).toBe('1') + expect(env.GIT_CONFIG_KEY_0).toBe('http.proxy') + expect(env.GIT_CONFIG_KEY_1).toBeUndefined() + }) + + it('materializes the full guard for a legacy daemon host', async () => { + const env = await daemonSpawnAndGetEnv( + { + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'http.proxy', + GIT_CONFIG_VALUE_0: 'http://proxy.invalid' + }, + undefined, + undefined, + undefined, + { command: 'claude' }, + false + ) + + expect(env.GIT_TERMINAL_PROMPT).toBe('0') + expect(env.GCM_INTERACTIVE).toBe('never') + expect(env.GIT_CONFIG_COUNT).toBe('3') + expect(env.GIT_CONFIG_KEY_1).toBe('credential.interactive') + expect(env.GIT_CONFIG_KEY_2).toBe('credential.guiPrompt') + }) + it('passes the minted sessionId through to provider.spawn and host env setup', async () => { const daemonSpawn = setupDaemonAdapter() handlers.clear() diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index d1c501a3230..67db1b690cd 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -41,6 +41,7 @@ import { isWslShellName, resolveLocalWindowsTerminalRuntimeOptions } from '../../shared/local-windows-terminal-runtime' +import { applyTerminalGitCredentialPromptGuard } from './terminal-git-credential-guard' import { openCodeHookService } from '../opencode/hook-service' import { mimoCodeHookService } from '../mimo/hook-service' import { @@ -553,6 +554,8 @@ export type BuildPtyHostEnvOptions = { * resolve to Pi for back-compat. NEVER infer from disk presence; that's * the bug this option fixes (cross-agent shadowing when both dirs exist). */ launchCommand?: string + /** Trusted agent identity for wrapped commands that cannot be recognized from text. */ + launchAgent?: TuiAgent shellPath?: string isWsl?: boolean /** Distro for WSL spawns (null = Windows default distro). Drives the WSL @@ -560,6 +563,9 @@ export type BuildPtyHostEnvOptions = { wslDistro?: string | null agentStatusHooksEnabled: boolean networkProxySettings?: NetworkProxySettings + /** Keep indexed Git config off the sparse daemon wire; the daemon appends + * guard entries after merging its authoritative inherited environment. */ + deferGitConfigGuardToDaemon?: boolean } function readInheritedPath(baseEnv: Record): string { @@ -849,6 +855,15 @@ export function buildPtyHostEnv( const piAgentKind = detectPiAgentKindFromCommand(launchCommandHint) const hasLaunchCommand = typeof launchCommandHint === 'string' && launchCommandHint.trim().length > 0 + + // Why: unattended agents must fail instead of opening OS credential UI and + // retrying auth in a loop; ordinary user terminals keep normal Git behavior. + applyTerminalGitCredentialPromptGuard(baseEnv, { + launchCommand: launchCommandHint, + isUnattended: opts.launchAgent !== undefined, + deferGitConfigGuardToHost: opts.deferGitConfigGuardToDaemon + }) + const shouldPrepareOmpShadow = piAgentKind === 'omp' || !hasLaunchCommand // Why: source shadows are agent-scoped. Trusting the other kind's source // would reintroduce the exact Pi/OMP extension-state shadowing this PR fixes. @@ -1548,6 +1563,7 @@ export function registerPtyHandlers( skipCodexHomeEnv: ctx?.isWsl === true && !selectedCodexHomePath, githubAttributionEnabled: getSettings?.()?.enableGitHubAttribution ?? false, launchCommand: ctx?.command, + launchAgent: ctx?.launchAgent, shellPath: ctx?.shellPath, isWsl: ctx?.isWsl, wslDistro: ctx?.wslDistro ?? null, @@ -3031,11 +3047,13 @@ export function registerPtyHandlers( skipCodexHomeEnv, githubAttributionEnabled: getSettings?.()?.enableGitHubAttribution ?? false, launchCommand: args.command, + launchAgent: isTuiAgent(args.launchAgent) ? args.launchAgent : undefined, shellPath: daemonShellOverride ?? process.env.COMSPEC, isWsl: shouldSkipCodexHomeEnvForWindowsShell(daemonShellOverride, cwd), wslDistro: codexSelectionTarget.runtime === 'wsl' ? codexSelectionTarget.wslDistro : null, agentStatusHooksEnabled: isAgentStatusHooksEnabled(getSettings?.()), - networkProxySettings: getSettings?.() + networkProxySettings: getSettings?.(), + deferGitConfigGuardToDaemon: provider.supportsGitCredentialGuardHost?.(sessionId) === true }) promoteAgentTeamsShimPath(env, requestedAgentTeamsPath) } @@ -3071,6 +3089,9 @@ export function registerPtyHandlers( if (args.startupCommandDelivery !== undefined) { spawnOptions.startupCommandDelivery = args.startupCommandDelivery } + if (isTuiAgent(args.launchAgent)) { + spawnOptions.launchAgent = args.launchAgent + } if (args.worktreeId !== undefined) { spawnOptions.worktreeId = args.worktreeId } @@ -3888,12 +3909,15 @@ export function registerPtyHandlers( skipCodexHomeEnv, githubAttributionEnabled: getSettings?.()?.enableGitHubAttribution ?? false, launchCommand: args.command, + launchAgent: isTuiAgent(args.launchAgent) ? args.launchAgent : undefined, shellPath: effectiveShellOverride ?? process.env.COMSPEC, isWsl: shouldSkipCodexHomeEnvForWindowsShell(effectiveShellOverride, cwd), wslDistro: codexSelectionTarget.runtime === 'wsl' ? codexSelectionTarget.wslDistro : null, agentStatusHooksEnabled: isAgentStatusHooksEnabled(getSettings?.()), - networkProxySettings: getSettings?.() + networkProxySettings: getSettings?.(), + deferGitConfigGuardToDaemon: + provider.supportsGitCredentialGuardHost?.(effectiveSessionId) === true }) promoteAgentTeamsShimPath(env, requestedAgentTeamsPath) } catch (err) { diff --git a/src/main/ipc/repos-remote.test.ts b/src/main/ipc/repos-remote.test.ts index 18f73ca6f8f..eee64eff2eb 100644 --- a/src/main/ipc/repos-remote.test.ts +++ b/src/main/ipc/repos-remote.test.ts @@ -9,6 +9,7 @@ import { existsSync } from 'node:fs' import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' +import type * as GitRunner from '../git/runner' import type * as RepoModule from '../git/repo' import { DEFAULT_REPO_BADGE_COLOR } from '../../shared/constants' import { getGitRepoRoot, isGitRepo } from '../git/repo' @@ -104,15 +105,15 @@ vi.mock('../git/repo', async () => { } }) -vi.mock('../git/runner', () => ({ +vi.mock('../git/runner', async () => ({ + // Why: keep the real env builders (nonInteractiveGitEnv, + // gitOptionalLocksDisabledEnv) so the clone regression test (#7652) asserts + // the actual guard's markers, not a mock echoing itself. + ...(await vi.importActual('../git/runner')), gitExecFileAsync: gitExecFileAsyncMock, gitExecFileAsyncBuffer: vi.fn(), gitStreamStdout: vi.fn(), - gitSpawn: gitSpawnMock, - gitOptionalLocksDisabledEnv: (env: NodeJS.ProcessEnv = process.env) => ({ - ...env, - GIT_OPTIONAL_LOCKS: '0' - }) + gitSpawn: gitSpawnMock })) vi.mock('../git/worktree', () => ({ @@ -2118,6 +2119,28 @@ describe('repos:add + repos:clone', () => { expect(result).toHaveProperty('path', join(destination, 'orca')) }) + it('clones with the non-interactive credential guard so Git Credential Manager cannot pop its OAuth window (#7652)', async () => { + const destination = await createTempRoot() + + await handlers.get('repos:clone')!(null, { + url: 'https://example.com/orca.git', + destination + }) + + // Without this env, a clone that needs GitHub auth makes Git Credential + // Manager pop its "Connect to GitHub" OAuth window on Windows and loop it + // when the network cannot complete the flow. + expect(gitSpawnMock).toHaveBeenCalledWith( + ['clone', '--progress', '--', 'https://example.com/orca.git', join(destination, 'orca')], + expect.objectContaining({ + env: expect.objectContaining({ + GIT_TERMINAL_PROMPT: '0', + GCM_INTERACTIVE: 'never' + }) + }) + ) + }) + it('treats cloneAbort with no active clone as a no-op', async () => { await expect(handlers.get('repos:cloneAbort')!(null, undefined)).resolves.toBeUndefined() }) diff --git a/src/main/ipc/repos.ts b/src/main/ipc/repos.ts index 0ccc0fcbd90..4c6fef6ef8a 100644 --- a/src/main/ipc/repos.ts +++ b/src/main/ipc/repos.ts @@ -42,7 +42,7 @@ import { isTuiAgent } from '../../shared/tui-agent-config' import { invalidateAuthorizedRootsCache } from './filesystem-auth' import type { ChildProcess } from 'node:child_process' import { access, mkdir, readdir, rm } from 'node:fs/promises' -import { gitExecFileAsync, gitSpawn } from '../git/runner' +import { gitExecFileAsync, gitSpawn, nonInteractiveGitEnv } from '../git/runner' import { isAbsolute, join, posix } from 'node:path' import { cleanupClaimedCloneTarget, @@ -2260,6 +2260,13 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v try { proc = gitSpawn(['clone', '--progress', '--', args.url, clonePath], { cwd: args.destination, + // Why: without the non-interactive guard, a clone that needs + // GitHub auth makes Git Credential Manager pop its "Connect to + // GitHub" OAuth window on Windows; in a network-restricted env the + // browser/device flow can never complete and git's credential + // retry re-pops it (issue #7652). Fail fast with a clear error and + // let Orca's non-intrusive GitHub state stand instead. + env: nonInteractiveGitEnv(), stdio: ['ignore', 'ignore', 'pipe'] }) } catch (err) { diff --git a/src/main/ipc/terminal-git-credential-guard.test.ts b/src/main/ipc/terminal-git-credential-guard.test.ts new file mode 100644 index 00000000000..5046fea569b --- /dev/null +++ b/src/main/ipc/terminal-git-credential-guard.test.ts @@ -0,0 +1,183 @@ +import { describe, expect, it } from 'vitest' +import { + applyTerminalGitCredentialPromptGuard, + TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV +} from './terminal-git-credential-guard' + +function expectGuarded(env: Record): void { + expect(env.GIT_TERMINAL_PROMPT).toBe('0') + expect(env.GCM_INTERACTIVE).toBe('never') + expect(Object.values(env)).toContain('credential.interactive') + expect(Object.values(env)).toContain('credential.guiPrompt') + expect(Object.values(env)).not.toContain('credential.helper') +} + +describe('applyTerminalGitCredentialPromptGuard', () => { + it('guards an agent terminal on every platform', () => { + for (const platform of ['win32', 'darwin', 'linux'] as const) { + const env: Record = { PATH: '/usr/bin' } + + expect( + applyTerminalGitCredentialPromptGuard(env, { + launchCommand: 'claude', + platform + }) + ).toBe(true) + expectGuarded(env) + } + }) + + it('guards a headless one-shot agent launch', () => { + const env: Record = { PATH: '/usr/bin' } + + expect( + applyTerminalGitCredentialPromptGuard(env, { + launchCommand: 'claude -p "fix the tests"', + platform: 'darwin' + }) + ).toBe(true) + expectGuarded(env) + }) + + it('guards a trusted agent whose wrapped command is not recognizable', () => { + const env: Record = { PATH: '/usr/bin' } + + expect( + applyTerminalGitCredentialPromptGuard(env, { + launchCommand: 'cd /repo && custom-agent-wrapper', + isUnattended: true, + platform: 'linux' + }) + ).toBe(true) + expectGuarded(env) + }) + + it('leaves ordinary user terminals unchanged on every platform', () => { + for (const platform of ['win32', 'darwin', 'linux'] as const) { + const original = { + PATH: '/usr/bin', + GIT_TERMINAL_PROMPT: '1', + GCM_INTERACTIVE: 'auto', + GIT_ASKPASS: '/usr/local/bin/user-askpass' + } + const env = { ...original } + + expect( + applyTerminalGitCredentialPromptGuard(env, { + launchCommand: '/bin/zsh', + platform + }) + ).toBe(false) + expect(env).toEqual(original) + } + }) + + it('does not treat a generic Orca CLI command as an agent', () => { + const env: Record = { PATH: '/usr/bin' } + + expect( + applyTerminalGitCredentialPromptGuard(env, { + launchCommand: 'orca status', + platform: 'linux' + }) + ).toBe(false) + expect(env).toEqual({ PATH: '/usr/bin' }) + }) + + it('guards explicitly marked automation and consumes its internal marker', () => { + const env: Record = { + PATH: '/usr/bin', + [TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV]: 'guard' + } + + expect( + applyTerminalGitCredentialPromptGuard(env, { + launchCommand: '/bin/zsh', + platform: 'linux' + }) + ).toBe(true) + expectGuarded(env) + expect(env[TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV]).toBeUndefined() + }) + + it('preserves caller askpass and indexed config when appending the guard', () => { + const env: Record = { + GIT_ASKPASS: '/usr/local/bin/user-askpass', + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'http.proxy', + GIT_CONFIG_VALUE_0: 'http://proxy.invalid' + } + + applyTerminalGitCredentialPromptGuard(env, { + launchCommand: 'claude', + platform: 'linux' + }) + + expect(env.GIT_ASKPASS).toBe('/usr/local/bin/user-askpass') + expect(env.GIT_CONFIG_COUNT).toBe('3') + expect(env.GIT_CONFIG_KEY_0).toBe('http.proxy') + expect(env.GIT_CONFIG_VALUE_0).toBe('http://proxy.invalid') + expect(env.GIT_CONFIG_KEY_1).toBe('credential.interactive') + expect(env.GIT_CONFIG_KEY_2).toBe('credential.guiPrompt') + }) + + it('registers a guarded Windows agent environment for WSL forwarding', () => { + const env: Record = { PATH: 'C:\\Windows\\System32' } + + applyTerminalGitCredentialPromptGuard(env, { + launchCommand: 'claude', + platform: 'win32' + }) + + const wslenvKeys = (env.WSLENV ?? '').split(':') + expect(wslenvKeys).toContain('GIT_TERMINAL_PROMPT') + expect(wslenvKeys).toContain('GCM_INTERACTIVE') + expect(wslenvKeys).toContain('GIT_CONFIG_COUNT') + expect(wslenvKeys).toContain('GIT_CONFIG_KEY_0') + expect(wslenvKeys).toContain('GIT_CONFIG_VALUE_0') + expect(wslenvKeys).not.toContain('GIT_ASKPASS') + expect(wslenvKeys).not.toContain('SSH_ASKPASS') + }) + + it('forwards only the guard decision and scalars to a detached host', () => { + const env: Record = { + PATH: '/usr/bin', + GIT_ASKPASS: '/usr/local/bin/user-askpass', + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'core.quotePath', + GIT_CONFIG_VALUE_0: 'false' + } + + expect( + applyTerminalGitCredentialPromptGuard(env, { + launchCommand: 'claude', + platform: 'win32', + deferGitConfigGuardToHost: true + }) + ).toBe(true) + + expect(env.GIT_TERMINAL_PROMPT).toBe('0') + expect(env.GCM_INTERACTIVE).toBe('never') + expect(env.GIT_ASKPASS).toBe('/usr/local/bin/user-askpass') + expect(env.SSH_ASKPASS).toBeUndefined() + expect(env.GIT_CONFIG_COUNT).toBe('1') + expect(env.GIT_CONFIG_KEY_0).toBe('core.quotePath') + expect(env.GIT_CONFIG_KEY_1).toBeUndefined() + expect(env.WSLENV).toBeUndefined() + expect(env[TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV]).toBe('guard') + }) + + it('does not add premature WSL forwarding entries to a detached-host wire env', () => { + const env: Record = { + WSLENV: 'CALLER_VALUE/p', + [TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV]: 'guard' + } + + applyTerminalGitCredentialPromptGuard(env, { + platform: 'win32', + deferGitConfigGuardToHost: true + }) + + expect(env.WSLENV).toBe('CALLER_VALUE/p') + }) +}) diff --git a/src/main/ipc/terminal-git-credential-guard.ts b/src/main/ipc/terminal-git-credential-guard.ts new file mode 100644 index 00000000000..0a4e45588c0 --- /dev/null +++ b/src/main/ipc/terminal-git-credential-guard.ts @@ -0,0 +1,4 @@ +export { + applyTerminalGitCredentialPromptGuard, + TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV +} from '../../shared/terminal-git-credential-guard' diff --git a/src/main/providers/local-pty-provider.test.ts b/src/main/providers/local-pty-provider.test.ts index 66f68d6e868..cdb7656b236 100644 --- a/src/main/providers/local-pty-provider.test.ts +++ b/src/main/providers/local-pty-provider.test.ts @@ -429,6 +429,48 @@ describe('LocalPtyProvider', () => { expect(spawnCall[2].env.ORCA_ATTRIBUTION_SHIM_DIR).toBeUndefined() }) + it('drops stale inherited Git config indices behind a smaller explicit count', async () => { + const keys = [ + 'GIT_CONFIG_COUNT', + 'GIT_CONFIG_KEY_0', + 'GIT_CONFIG_VALUE_0', + 'GIT_CONFIG_KEY_1', + 'GIT_CONFIG_VALUE_1' + ] as const + const saved = Object.fromEntries(keys.map((key) => [key, process.env[key]])) + process.env.GIT_CONFIG_COUNT = '2' + process.env.GIT_CONFIG_KEY_0 = 'base.zero' + process.env.GIT_CONFIG_VALUE_0 = 'zero' + process.env.GIT_CONFIG_KEY_1 = 'base.one' + process.env.GIT_CONFIG_VALUE_1 = 'one' + + try { + await provider.spawn({ + cols: 80, + rows: 24, + env: { + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'override.zero', + GIT_CONFIG_VALUE_0: 'override' + } + }) + + const spawnEnv = spawnMock.mock.calls.at(-1)?.[2]?.env as Record + expect(spawnEnv.GIT_CONFIG_COUNT).toBe('1') + expect(spawnEnv.GIT_CONFIG_KEY_0).toBe('override.zero') + expect(spawnEnv.GIT_CONFIG_KEY_1).toBeUndefined() + expect(spawnEnv.GIT_CONFIG_VALUE_1).toBeUndefined() + } finally { + for (const key of keys) { + if (saved[key] === undefined) { + delete process.env[key] + } else { + process.env[key] = saved[key] + } + } + } + }) + it('does not inherit AppImage runtime env into Linux PTY shells', async () => { const saved = { APPIMAGE: process.env.APPIMAGE, diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index 3d9962fcb48..03abbb1e54b 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -58,6 +58,7 @@ import { readWindowsConptyProcessIds } from './windows-conpty-process-membership import { shouldUseShellReadyStartupDelivery } from '../../shared/codex-startup-delivery' import { assertSafeAgentStartupCwd, resolveSafePtyDefaultCwd } from './pty-default-cwd' import { ORCA_HERMES_STARTUP_QUERY_ENV } from '../../shared/hermes-startup-query' +import { mergeGitConfigEnvProtocol } from '../../shared/git-credential-prompt-env' const PANE_IDENTITY_ENV_KEYS = [ 'ORCA_PANE_KEY', @@ -296,7 +297,13 @@ export type LocalPtyProviderOptions = { buildSpawnEnv?: ( id: string, baseEnv: Record, - ctx?: { command?: string; shellPath?: string; isWsl?: boolean; wslDistro?: string | null } + ctx?: { + command?: string + launchAgent?: PtySpawnOptions['launchAgent'] + shellPath?: string + isWsl?: boolean + wslDistro?: string | null + } ) => Record /** Whether worktree-scoped shell history is enabled. When true (or absent) * and a worktreeId is provided, HISTFILE is scoped per-worktree. */ @@ -469,8 +476,7 @@ export class LocalPtyProvider implements IPtyProvider { validateWorkingDirectory(validationCwd) const spawnEnv: Record = { - ...process.env, - ...args.env, + ...mergeGitConfigEnvProtocol(process.env, args.env), TERM: 'xterm-256color', COLORTERM: 'truecolor', TERM_PROGRAM: 'Orca', @@ -522,6 +528,7 @@ export class LocalPtyProvider implements IPtyProvider { const finalEnv = this.opts.buildSpawnEnv ? this.opts.buildSpawnEnv(id, spawnEnv, { command: args.command, + launchAgent: args.launchAgent, shellPath, isWsl: isWslShell, wslDistro: launchWslDistro diff --git a/src/main/providers/ssh-pty-provider.test.ts b/src/main/providers/ssh-pty-provider.test.ts index ca0a9fd8261..e2fbde7be26 100644 --- a/src/main/providers/ssh-pty-provider.test.ts +++ b/src/main/providers/ssh-pty-provider.test.ts @@ -67,6 +67,25 @@ describe('SshPtyProvider', () => { }) }) + it('forwards trusted agent identity for wrapped remote commands', async () => { + mux.request.mockResolvedValue({ id: 'pty-agent' }) + + await provider.spawn({ + cols: 120, + rows: 40, + command: 'cd /repo && custom-agent-wrapper', + launchAgent: 'claude' + }) + + expect(mux.request).toHaveBeenCalledWith( + 'pty.spawn', + expect.objectContaining({ + command: 'cd /repo && custom-agent-wrapper', + launchAgent: 'claude' + }) + ) + }) + it('forwards pane identity as relay metadata on fresh spawn', async () => { mux.request.mockResolvedValue({ id: 'pty-2' }) diff --git a/src/main/providers/ssh-pty-provider.ts b/src/main/providers/ssh-pty-provider.ts index e386e89dbba..bdc81787996 100644 --- a/src/main/providers/ssh-pty-provider.ts +++ b/src/main/providers/ssh-pty-provider.ts @@ -154,6 +154,7 @@ export class SshPtyProvider implements IPtyProvider { // Pi-compatible agent is being launched, while commandDelivery tells it // whether to submit the command itself for runtime-owned background PTYs. ...(opts.command ? { command: opts.command } : {}), + ...(opts.launchAgent ? { launchAgent: opts.launchAgent } : {}), ...(opts.shellOverride !== undefined ? { shellOverride: opts.shellOverride } : {}), ...(opts.terminalWindowsWslDistro !== undefined ? { terminalWindowsWslDistro: opts.terminalWindowsWslDistro } diff --git a/src/main/providers/types.ts b/src/main/providers/types.ts index 66642e38ad1..f6e4042f482 100644 --- a/src/main/providers/types.ts +++ b/src/main/providers/types.ts @@ -154,6 +154,8 @@ export type PtyProcessInfo = { export type IPtyProvider = { spawn(opts: PtySpawnOptions): Promise + /** Whether this spawn target can append the Git guard after its final env merge. */ + supportsGitCredentialGuardHost?: (sessionId?: string) => boolean attach(id: string): Promise hasPty?: (id: string) => boolean write(id: string, data: string): void diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index e7b11ef8bb2..2122e17d9a2 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -9792,6 +9792,8 @@ describe('OrcaRuntimeService', () => { } }) + expect(spawn).toHaveBeenCalledWith(expect.objectContaining({ launchAgent: 'claude' })) + const spawnCall = spawn.mock.calls[0]?.[0] as { env?: Record } | undefined const spawnedEnv = spawnCall?.env ?? {} const spawnedLeafId = spawnedEnv.ORCA_PANE_KEY.slice(`${spawnedEnv.ORCA_TAB_ID}:`.length) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 0458efbdc82..6a193a404e5 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -54,7 +54,7 @@ import { AGENT_PROMPT_SUBMIT_DELAY_MS, buildAgentPromptPasteBytes } from '../../shared/agent-prompt-injection' -import { gitExecFileAsync, gitSpawn } from '../git/runner' +import { gitExecFileAsync, gitSpawn, nonInteractiveGitEnv } from '../git/runner' import { runWithGitReadCacheInvalidation } from '../git/status' import { cleanupClaimedCloneTarget, @@ -1191,6 +1191,7 @@ type RuntimePtyController = { rows: number cwd?: string command?: string + launchAgent?: TuiAgent commandDelivery?: 'renderer' | 'provider' startupCommandDelivery?: WorktreeStartupLaunch['startupCommandDelivery'] env?: Record @@ -12268,6 +12269,12 @@ export class OrcaRuntimeService { try { proc = gitSpawn(['clone', '--progress', '--', trimmedUrl, clonePath], { cwd: trimmedDestination, + // Why: without the non-interactive guard, a clone that needs GitHub + // auth makes Git Credential Manager pop its "Connect to GitHub" OAuth + // window on Windows; in a network-restricted env the browser/device + // flow can never complete and git's credential retry re-pops it + // (issue #7652). Fail fast with a clear error instead. + env: nonInteractiveGitEnv(), stdio: ['ignore', 'ignore', 'pipe'] }) } catch (err) { @@ -17815,6 +17822,7 @@ export class OrcaRuntimeService { command: sequencedStartupCommand ? launchOpts.command : (agentTeamsPlan?.command ?? launchOpts.command), + launchAgent: launchOpts.launchAgent, commandDelivery: 'provider', startupCommandDelivery: launchOpts.startupCommandDelivery, env, diff --git a/src/main/wsl-env.ts b/src/main/wsl-env.ts index 71e836e945b..c77d15ebf17 100644 --- a/src/main/wsl-env.ts +++ b/src/main/wsl-env.ts @@ -1,17 +1 @@ -export function addWslEnvKeys( - env: Record, - keys: readonly string[] -): void { - const existing = env.WSLENV ?? process.env.WSLENV ?? '' - const tokens = existing.split(':').filter(Boolean) - const tokenNames = new Set(tokens.map((token) => token.split('/')[0])) - - for (const key of keys) { - if (!tokenNames.has(key)) { - tokens.push(key) - tokenNames.add(key) - } - } - - env.WSLENV = tokens.join(':') -} +export { addWslEnvKeys } from '../shared/wsl-env' diff --git a/src/relay/agent-exec-handler.test.ts b/src/relay/agent-exec-handler.test.ts index 96efa698479..0d6d73babba 100644 --- a/src/relay/agent-exec-handler.test.ts +++ b/src/relay/agent-exec-handler.test.ts @@ -2,6 +2,7 @@ import { exec, spawn } from 'node:child_process' import { beforeEach, describe, expect, it, vi } from 'vitest' import type * as ChildProcess from 'node:child_process' import { createFakeChild, createHandlers, requestContext } from './agent-exec-handler-test-harness' +import { TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV } from '../shared/terminal-git-credential-guard' vi.mock('child_process', async (importOriginal) => { const actual = await importOriginal() @@ -52,7 +53,11 @@ describe('AgentExecHandler', () => { }) expect(spawnMock).toHaveBeenCalledWith('agent', ['--flag', '42'], { cwd: '/repo', - env: process.env, + env: expect.objectContaining({ + ...process.env, + GIT_TERMINAL_PROMPT: '0', + GCM_INTERACTIVE: 'never' + }), stdio: ['pipe', 'pipe', 'pipe'], windowsHide: true }) @@ -97,6 +102,87 @@ describe('AgentExecHandler', () => { }) }) + it('consumes an unattended marker and applies the full Git guard on the relay host', async () => { + const child = createFakeChild() + spawnMock.mockReturnValue(child as never) + const handlers = createHandlers() + + const pending = handlers.get('agent.execNonInteractive')!( + { + binary: '/bin/bash', + args: ['-lc', 'git fetch'], + cwd: '/repo', + timeoutMs: 5_000, + env: { [TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV]: 'guard' } + }, + requestContext() + ) + + child.emit('close', 0) + await expect(pending).resolves.toMatchObject({ exitCode: 0 }) + + const env = spawnMock.mock.calls[0]?.[2]?.env as Record + expect(env[TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV]).toBeUndefined() + expect(env.GIT_TERMINAL_PROMPT).toBe('0') + expect(env.GCM_INTERACTIVE).toBe('never') + expect(Object.values(env)).toContain('credential.interactive') + expect(Object.values(env)).toContain('credential.guiPrompt') + }) + + it('guards wrapped agents after atomically replacing inherited indexed config', async () => { + const keys = [ + 'GIT_CONFIG_COUNT', + 'GIT_CONFIG_KEY_0', + 'GIT_CONFIG_VALUE_0', + 'GIT_CONFIG_KEY_1', + 'GIT_CONFIG_VALUE_1' + ] as const + const saved = Object.fromEntries(keys.map((key) => [key, process.env[key]])) + process.env.GIT_CONFIG_COUNT = '2' + process.env.GIT_CONFIG_KEY_0 = 'base.one' + process.env.GIT_CONFIG_VALUE_0 = 'one' + process.env.GIT_CONFIG_KEY_1 = 'base.two' + process.env.GIT_CONFIG_VALUE_1 = 'two' + + try { + const child = createFakeChild() + spawnMock.mockReturnValue(child as never) + const handlers = createHandlers() + const pending = handlers.get('agent.execNonInteractive')!( + { + binary: 'npx', + args: ['codex', 'exec'], + cwd: '/repo', + timeoutMs: 5_000, + env: { + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'http.proxy', + GIT_CONFIG_VALUE_0: 'http://proxy.invalid' + } + }, + requestContext() + ) + + child.emit('close', 0) + await expect(pending).resolves.toMatchObject({ exitCode: 0 }) + const env = spawnMock.mock.calls[0]?.[2]?.env as Record + expect(env.GIT_TERMINAL_PROMPT).toBe('0') + expect(env.GIT_CONFIG_COUNT).toBe('3') + expect(env.GIT_CONFIG_KEY_0).toBe('http.proxy') + expect(env.GIT_CONFIG_KEY_1).toBe('credential.interactive') + expect(env.GIT_CONFIG_KEY_2).toBe('credential.guiPrompt') + expect(Object.values(env)).not.toContain('base.two') + } finally { + for (const key of keys) { + if (saved[key] === undefined) { + delete process.env[key] + } else { + process.env[key] = saved[key] + } + } + } + }) + it('cancels the in-flight command for the requested cwd', async () => { const child = createFakeChild() spawnMock.mockReturnValue(child as never) diff --git a/src/relay/agent-exec-handler.ts b/src/relay/agent-exec-handler.ts index 61b0b118a44..2e3868be369 100644 --- a/src/relay/agent-exec-handler.ts +++ b/src/relay/agent-exec-handler.ts @@ -2,6 +2,8 @@ import { exec, spawn, type ChildProcess } from 'node:child_process' import { existsSync } from 'node:fs' import { delimiter, join } from 'node:path' import type { RelayDispatcher, RequestContext } from './dispatcher' +import { applyTerminalGitCredentialPromptGuard } from '../shared/terminal-git-credential-guard' +import { mergeGitConfigEnvProtocol } from '../shared/git-credential-prompt-env' const DEFAULT_TIMEOUT_MS = 60_000 const MAX_TIMEOUT_MS = 5 * 60 * 1000 @@ -168,7 +170,16 @@ export class AgentExecHandler { params.env && typeof params.env === 'object' && !Array.isArray(params.env) ? (params.env as Record) : null - const spawnEnv = extraEnv ? { ...process.env, ...extraEnv } : process.env + const spawnEnv = mergeGitConfigEnvProtocol(process.env, extraEnv ?? undefined) as Record< + string, + string + > + // Why: this RPC has no interactive terminal, regardless of which wrapper + // launches the agent or hook command. + applyTerminalGitCredentialPromptGuard(spawnEnv, { + isUnattended: true, + platform: process.platform + }) return new Promise((resolve) => { let child diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index 510dd60f84b..9a9a5d7cae6 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -59,7 +59,7 @@ import { resolveEffectiveGitUpstream } from '../shared/git-effective-upstream' import { loadGitHistoryFromExecutor } from '../shared/git-history' -import { buildRelayGitEnv } from './relay-command-env' +import { buildRelayGitEnv, buildRelayUnattendedGitEnv } from './relay-command-env' import { removeSafeUntrackedDiscardTarget, removeSafeUntrackedDiscardTargets @@ -307,16 +307,10 @@ export class GitHandler { timeout?: number } ): Promise<{ stdout: string; stderr: string }> { - const env = buildRelayGitEnv() + const env = opts?.nonInteractive ? buildRelayUnattendedGitEnv() : buildRelayGitEnv() if (opts?.disableOptionalLocks) { env.GIT_OPTIONAL_LOCKS = '0' } - if (opts?.nonInteractive) { - env.GIT_TERMINAL_PROMPT = '0' - env.GIT_ASKPASS = '' - env.SSH_ASKPASS = '' - env.GIT_SSH_COMMAND ??= 'ssh -o BatchMode=yes' - } const execOptions = { cwd: expandTilde(cwd), env, @@ -1182,7 +1176,7 @@ export class GitHandler { return await new Promise((resolve, reject) => { const child = spawn('git', args, { cwd: expandTilde(cwd), - env: buildRelayGitEnv(), + env: buildRelayUnattendedGitEnv(), stdio: ['ignore', 'pipe', 'pipe'] }) let stdout = '' diff --git a/src/relay/pty-handler.test.ts b/src/relay/pty-handler.test.ts index c90e874800e..289b05d6971 100644 --- a/src/relay/pty-handler.test.ts +++ b/src/relay/pty-handler.test.ts @@ -174,6 +174,97 @@ describe('PtyHandler', () => { expect(handler.activePtyCount).toBe(1) }) + it('guards SSH agent terminals after merging the relay inherited Git config', async () => { + const gitConfigKeys = [ + 'GIT_CONFIG_COUNT', + 'GIT_CONFIG_KEY_0', + 'GIT_CONFIG_VALUE_0', + 'GIT_CONFIG_KEY_1', + 'GIT_CONFIG_VALUE_1', + 'GIT_CONFIG_KEY_2', + 'GIT_CONFIG_VALUE_2' + ] as const + const saved = Object.fromEntries(gitConfigKeys.map((key) => [key, process.env[key]])) + process.env.GIT_CONFIG_COUNT = '3' + process.env.GIT_CONFIG_KEY_0 = 'core.quotePath' + process.env.GIT_CONFIG_VALUE_0 = 'false' + process.env.GIT_CONFIG_KEY_1 = 'base.one' + process.env.GIT_CONFIG_VALUE_1 = 'one' + process.env.GIT_CONFIG_KEY_2 = 'base.two' + process.env.GIT_CONFIG_VALUE_2 = 'two' + + try { + await dispatcher.callRequest('pty.spawn', { + command: 'claude', + env: { + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'http.proxy', + GIT_CONFIG_VALUE_0: 'http://proxy.invalid' + } + }) + + const spawnEnv = mockPtySpawn.mock.calls[0]?.[2]?.env as Record + expect(spawnEnv.GIT_TERMINAL_PROMPT).toBe('0') + expect(spawnEnv.GCM_INTERACTIVE).toBe('never') + expect(spawnEnv.GIT_CONFIG_COUNT).toBe('3') + expect(spawnEnv.GIT_CONFIG_KEY_0).toBe('http.proxy') + expect(spawnEnv.GIT_CONFIG_KEY_1).toBe('credential.interactive') + expect(spawnEnv.GIT_CONFIG_KEY_2).toBe('credential.guiPrompt') + expect(spawnEnv.GIT_CONFIG_KEY_3).toBeUndefined() + } finally { + for (const key of gitConfigKeys) { + if (saved[key] === undefined) { + delete process.env[key] + } else { + process.env[key] = saved[key] + } + } + } + }) + + it('guards a trusted SSH agent when its command uses a custom wrapper', async () => { + await dispatcher.callRequest('pty.spawn', { + command: 'cd /repo && custom-agent-wrapper', + launchAgent: 'claude' + }) + + const spawnEnv = mockPtySpawn.mock.calls[0]?.[2]?.env as Record + expect(spawnEnv.GIT_TERMINAL_PROMPT).toBe('0') + expect(spawnEnv.GCM_INTERACTIVE).toBe('never') + expect(Object.values(spawnEnv)).toContain('credential.interactive') + expect(Object.values(spawnEnv)).toContain('credential.guiPrompt') + }) + + it('leaves an ordinary Windows SSH user terminal unchanged', async () => { + const originalPlatform = process.platform + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + + try { + await dispatcher.callRequest('pty.spawn', { + env: { + GIT_TERMINAL_PROMPT: '1', + GCM_INTERACTIVE: 'auto', + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'core.quotePath', + GIT_CONFIG_VALUE_0: 'false' + } + }) + const userEnv = mockPtySpawn.mock.calls[0]?.[2]?.env as Record + expect(userEnv.GIT_TERMINAL_PROMPT).toBe('1') + expect(userEnv.GCM_INTERACTIVE).toBe('auto') + expect(userEnv.GIT_CONFIG_COUNT).toBe('1') + expect(userEnv.GIT_CONFIG_KEY_0).toBe('core.quotePath') + expect(userEnv.GIT_CONFIG_KEY_1).toBeUndefined() + const state = (await dispatcher.callRequest('pty.serialize', { ids: ['pty-1'] })) as string + expect(JSON.parse(state)[0]?.gitCredentialPromptGuarded).toBe(false) + } finally { + Object.defineProperty(process, 'platform', { + configurable: true, + value: originalPlatform + }) + } + }) + it('uses an explicit shell override and falls back to the default shell otherwise', async () => { const originalPlatform = process.platform Object.defineProperty(process, 'platform', { @@ -1555,6 +1646,67 @@ describe('PtyHandler', () => { expect(callArgs.env.TERM_PROGRAM).toBe('Orca') }) + it('revive preserves the credential guard chosen for an SSH agent terminal', async () => { + await dispatcher.callRequest('pty.spawn', { + command: 'claude' + }) + const state = (await dispatcher.callRequest('pty.serialize', { ids: ['pty-1'] })) as string + expect(JSON.parse(state)[0]?.gitCredentialPromptGuarded).toBe(true) + + handler.dispose() + mockPtySpawn.mockClear() + dispatcher = createMockDispatcher() + handler = new PtyHandler(dispatcher as unknown as RelayDispatcher) + const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => true) + try { + await dispatcher.callRequest('pty.revive', { state }) + } finally { + killSpy.mockRestore() + } + + const revivedEnv = mockPtySpawn.mock.calls[0]?.[2]?.env as Record + expect(revivedEnv.GIT_TERMINAL_PROMPT).toBe('0') + expect(revivedEnv.GCM_INTERACTIVE).toBe('never') + expect(Object.values(revivedEnv)).toContain('credential.interactive') + expect(Object.values(revivedEnv)).toContain('credential.guiPrompt') + }) + + it('revive treats legacy relay state as an ordinary unguarded terminal', async () => { + const savedTerminalPrompt = process.env.GIT_TERMINAL_PROMPT + const savedGcmInteractive = process.env.GCM_INTERACTIVE + delete process.env.GIT_TERMINAL_PROMPT + delete process.env.GCM_INTERACTIVE + const state = JSON.stringify([ + { + id: 'pty-legacy', + pid: process.pid, + cols: 80, + rows: 24, + cwd: process.cwd() + } + ]) + const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => true) + + try { + await dispatcher.callRequest('pty.revive', { state }) + const revivedEnv = mockPtySpawn.mock.calls[0]?.[2]?.env as Record + expect(revivedEnv.GIT_TERMINAL_PROMPT).toBeUndefined() + expect(revivedEnv.GCM_INTERACTIVE).toBeUndefined() + } finally { + killSpy.mockRestore() + if (savedTerminalPrompt === undefined) { + delete process.env.GIT_TERMINAL_PROMPT + } else { + process.env.GIT_TERMINAL_PROMPT = savedTerminalPrompt + } + if (savedGcmInteractive === undefined) { + delete process.env.GCM_INTERACTIVE + } else { + process.env.GCM_INTERACTIVE = savedGcmInteractive + } + } + }) + it('normalizes an explicit empty TERM and preserves sanitized env deletions on revive', async () => { await dispatcher.callRequest('pty.spawn', { env: { TERM: '' }, diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index d7dd995005a..09c62462fa9 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -24,6 +24,12 @@ import { scanForShellReady, type ShellReadyScanState } from '../main/shell-ready-marker-scanner' +import { applyTerminalGitCredentialPromptGuard } from '../shared/terminal-git-credential-guard' +import { + gitCredentialPromptGuardEnv, + mergeGitConfigEnvProtocol +} from '../shared/git-credential-prompt-env' +import { isTuiAgent } from '../shared/tui-agent-config' // Why: node-pty is a native addon that may not be installed on the remote. // Dynamic import keeps the require() lazy so loadPty() returns null gracefully @@ -70,6 +76,7 @@ type ManagedPty = { terminalHandle?: string explicitTerm?: string envToDelete: string[] + gitCredentialPromptGuarded: boolean startupCommand?: ManagedStartupCommand } @@ -196,6 +203,8 @@ type SerializedPtyEntry = { terminalHandle?: string explicitTerm?: string envToDelete?: string[] + /** Optional for state serialized by relays predating the credential guard. */ + gitCredentialPromptGuarded?: boolean } function sanitizeEnvToDelete(value: unknown): string[] { @@ -310,16 +319,18 @@ export class PtyHandler { ctx: { id: string; paneKey?: string; shell: string; command?: string }, envToDelete: readonly string[] = [] ): Record { - const baseEnv = { - ...process.env, - TERM: 'xterm-256color', - COLORTERM: 'truecolor', - TERM_PROGRAM: 'Orca', - TERM_PROGRAM_VERSION: - rendererEnv?.ORCA_APP_VERSION || process.env.ORCA_APP_VERSION || '0.0.0-dev', - FORCE_HYPERLINK: '1', - ...rendererEnv - } as Record + const baseEnv = mergeGitConfigEnvProtocol( + { + ...process.env, + TERM: 'xterm-256color', + COLORTERM: 'truecolor', + TERM_PROGRAM: 'Orca', + TERM_PROGRAM_VERSION: + rendererEnv?.ORCA_APP_VERSION || process.env.ORCA_APP_VERSION || '0.0.0-dev', + FORCE_HYPERLINK: '1' + }, + rendererEnv + ) as Record const augmented: Record = {} for (const augmenter of this.envAugmenters) { try { @@ -330,7 +341,7 @@ export class PtyHandler { ) } } - const result = { ...baseEnv, ...augmented } + const result = mergeGitConfigEnvProtocol(baseEnv, augmented) as Record // Why: match local/daemon precedence so relay defaults and augmenters // cannot resurrect attribution or identity values explicitly removed. for (const key of envToDelete) { @@ -648,6 +659,14 @@ export class PtyHandler { const shouldProviderDeliverCommand = commandDelivery === 'provider' && command !== undefined const spawnEnv = this.buildSpawnEnv(env, { id, paneKey, shell, command }, envToDelete) const launchCommandHint = resolveSetupAgentSequenceLaunchCommand(spawnEnv, command) + // Why: SSH PTYs bypass main's host-env builder. Apply the policy only + // after the relay merges its authoritative process environment so indexed + // Git config and remote Windows/WSL behavior remain intact. + const gitCredentialPromptGuarded = applyTerminalGitCredentialPromptGuard(spawnEnv, { + launchCommand: launchCommandHint, + isUnattended: isTuiAgent(params.launchAgent), + platform: process.platform + }) const shouldEmitShellReadyMarker = launchCommandHint !== undefined && shouldUseShellReadyStartupDelivery({ @@ -700,6 +719,7 @@ export class PtyHandler { worktreeId, ...(explicitTerm !== undefined ? { explicitTerm } : {}), envToDelete, + gitCredentialPromptGuarded, ...(terminalHandle ? { terminalHandle } : {}), ...(shouldProviderDeliverCommand ? { @@ -1002,6 +1022,7 @@ export class PtyHandler { worktreeId: managed.worktreeId, ...(managed.explicitTerm !== undefined ? { explicitTerm: managed.explicitTerm } : {}), envToDelete: managed.envToDelete, + gitCredentialPromptGuarded: managed.gitCredentialPromptGuarded, ...(managed.terminalHandle ? { terminalHandle: managed.terminalHandle } : {}) }) } @@ -1070,6 +1091,12 @@ export class PtyHandler { }, envToDelete ) + // Why: revive lacks the original launch command, so preserve the guard + // decision made at fresh spawn. Legacy state remains an ordinary shell. + const gitCredentialPromptGuarded = entry.gitCredentialPromptGuarded === true + if (gitCredentialPromptGuarded) { + Object.assign(spawnEnv, gitCredentialPromptGuardEnv(spawnEnv, process.platform)) + } const shellLaunch = getRelayShellLaunchConfig(shell, spawnEnv) const term = ptyMod.spawn(shell, shellLaunch.args, { // Why: revive must preserve the same terminal identity as fresh spawn. @@ -1092,6 +1119,7 @@ export class PtyHandler { worktreeId: entry.worktreeId, ...(explicitTerm !== undefined ? { explicitTerm } : {}), envToDelete, + gitCredentialPromptGuarded, ...(entry.terminalHandle ? { terminalHandle: entry.terminalHandle } : {}) }) diff --git a/src/relay/relay-command-env.test.ts b/src/relay/relay-command-env.test.ts index e9164fb86f7..dbdc99bf24f 100644 --- a/src/relay/relay-command-env.test.ts +++ b/src/relay/relay-command-env.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it, vi } from 'vitest' import { homedir } from 'node:os' -import { buildRelayCommandEnv, buildRelayGitEnv } from './relay-command-env' +import { + buildRelayCommandEnv, + buildRelayGitEnv, + buildRelayUnattendedGitEnv +} from './relay-command-env' // homedir() is the fallback when the relay env carries no HOME; mock it so the // fallback path is deterministic and the "no resolvable home" branch is reachable. @@ -245,3 +249,29 @@ describe('buildRelayGitEnv', () => { expect(env.PATH?.split(':')).toEqual(expect.arrayContaining(['/usr/bin', '/usr/local/bin'])) }) }) + +describe('buildRelayUnattendedGitEnv', () => { + it('disables credential UI while preserving relay PATH, locale, and caller askpass', () => { + const env = buildRelayUnattendedGitEnv( + { + HOME: '/home/me', + PATH: '/custom/bin', + LC_ALL: 'de_DE.UTF-8', + GIT_ASKPASS: '/opt/noninteractive-credential-feeder' + }, + 'linux' + ) + + expect(env.GIT_TERMINAL_PROMPT).toBe('0') + expect(env.GCM_INTERACTIVE).toBe('never') + expect(env.GIT_ASKPASS).toBe('/opt/noninteractive-credential-feeder') + expect(env.GIT_CONFIG_COUNT).toBe('2') + expect(env.GIT_CONFIG_KEY_0).toBe('credential.interactive') + expect(env.GIT_CONFIG_VALUE_0).toBe('false') + expect(env.GIT_CONFIG_KEY_1).toBe('credential.guiPrompt') + expect(env.GIT_CONFIG_VALUE_1).toBe('false') + expect(env.GIT_SSH_COMMAND).toBe('ssh -o BatchMode=yes') + expect(env.LC_ALL).toBe('en_US.UTF-8') + expect(env.PATH?.split(':')).toEqual(expect.arrayContaining(['/custom/bin', '/usr/bin'])) + }) +}) diff --git a/src/relay/relay-command-env.ts b/src/relay/relay-command-env.ts index 3d4c22f7f30..5f06650e17b 100644 --- a/src/relay/relay-command-env.ts +++ b/src/relay/relay-command-env.ts @@ -1,5 +1,6 @@ import { homedir } from 'node:os' import { posix, win32 } from 'node:path' +import { gitCredentialPromptGuardEnv } from '../shared/git-credential-prompt-env' import { UNTRANSLATED_GIT_OUTPUT_ENV } from '../shared/git-output-locale' const POSIX_RELAY_PATH_FALLBACKS = ['/usr/local/bin', '/opt/homebrew/bin', '/usr/bin', '/bin'] @@ -168,3 +169,15 @@ export function buildRelayGitEnv( ): NodeJS.ProcessEnv { return { ...buildRelayCommandEnv(baseEnv, platform), ...UNTRANSLATED_GIT_OUTPUT_ENV } } + +/** Env for unattended Git RPCs, which have no terminal that can answer auth UI. */ +export function buildRelayUnattendedGitEnv( + baseEnv: NodeJS.ProcessEnv = process.env, + platform: NodeJS.Platform = process.platform +): NodeJS.ProcessEnv { + // Why: SSH-host GCM can open its own OAuth window even though the clone's + // stdin is ignored, leaving the relay request hung with no way to answer it. + const env = gitCredentialPromptGuardEnv(buildRelayGitEnv(baseEnv, platform), platform) + env.GIT_SSH_COMMAND ??= 'ssh -o BatchMode=yes' + return env +} diff --git a/src/shared/agent-process-recognition.test.ts b/src/shared/agent-process-recognition.test.ts index 11e11fe2f53..c64e9c3e376 100644 --- a/src/shared/agent-process-recognition.test.ts +++ b/src/shared/agent-process-recognition.test.ts @@ -198,6 +198,20 @@ describe('agent process recognition', () => { ).toEqual({ agent: 'gemini', processName: 'gemini' }) }) + it('recognizes only the agent subcommand of the generic Orca CLI', () => { + expect(recognizeAgentProcessFromCommandLine('orca claude-teams')).toEqual({ + agent: 'claude-agent-teams', + processName: 'orca' + }) + expect(recognizeAgentProcessFromCommandLine('orca status')).toBeNull() + expect(recognizeAgentProcessFromCommandLine('orca-dev terminal list')).toBeNull() + expect(recognizeAgentProcessFromCommandLine('node /usr/local/bin/orca claude-teams')).toEqual({ + agent: 'claude-agent-teams', + processName: 'orca' + }) + expect(recognizeAgentProcessFromCommandLine('node /usr/local/bin/orca status')).toBeNull() + }) + it('does not classify prompt text as a wrapped agent command', () => { expect( recognizeAgentProcessFromCommandLine( diff --git a/src/shared/agent-process-recognition.ts b/src/shared/agent-process-recognition.ts index cbf54eb39e9..cbeaca6f7a1 100644 --- a/src/shared/agent-process-recognition.ts +++ b/src/shared/agent-process-recognition.ts @@ -155,14 +155,9 @@ function isInterpreterProcessName(normalized: string): boolean { return STATIC_INTERPRETER_PROCESS_NAMES.has(normalized) || PYTHON_PROCESS_RE.test(normalized) } -function isPythonProcessName(normalized: string): boolean { - return PYTHON_PROCESS_RE.test(normalized) -} +const isPythonProcessName = (normalized: string): boolean => PYTHON_PROCESS_RE.test(normalized) -function optionName(token: string): string { - const eq = token.indexOf('=') - return eq === -1 ? token : token.slice(0, eq) -} +const optionName = (token: string): string => token.split('=', 1)[0] ?? '' function findInterpreterEntrypointToken(tokens: string[], firstNormalized: string): string | null { if (!isInterpreterProcessName(firstNormalized)) { @@ -285,18 +280,26 @@ export function recognizeAgentProcess( } return { agent, processName: normalized } } + export function recognizeAgentProcessFromCommandLine( - commandLine: string | null | undefined + commandLine: string | null | undefined, + // Why: TUI consumers (status hooks, shell shadows) filter out headless + // one-shots (`claude -p …`); non-interactivity guards include them — a + // one-shot agent can't answer a prompt either. + options?: { includeHeadlessOneShot?: boolean } ): RecognizedAgentProcess | null { if (!commandLine) { return null } + const keep = options?.includeHeadlessOneShot === true const tokens = tokenizeCommandLine(commandLine) const firstNormalized = normalizeProcessName(tokens[0]) - const directRecognition = filterHeadlessOneShotAgentCommand( - recognizeAgentProcess(tokens[0]), - tokens - ) + let direct = recognizeAgentProcess(tokens[0]) + // Why: the generic Orca CLI is not an agent; only this subcommand launches its TUI mode. + if (direct?.agent === 'claude-agent-teams' && tokens[1]?.toLowerCase() !== 'claude-teams') { + direct = null + } + const directRecognition = keep ? direct : filterHeadlessOneShotAgentCommand(direct, tokens) if (directRecognition) { return directRecognition } @@ -304,10 +307,16 @@ export function recognizeAgentProcessFromCommandLine( if (!entrypoint) { return null } - const entrypointRecognition = isPythonProcessName(firstNormalized) + const viaEntrypoint = isPythonProcessName(firstNormalized) ? recognizePythonEntrypoint(tokens, entrypoint) : (recognizeAgentProcess(entrypoint) ?? recognizeNodeScriptEntrypoint(entrypoint)) - return filterHeadlessOneShotAgentCommand(entrypointRecognition, tokens) + if ( + viaEntrypoint?.agent === 'claude-agent-teams' && + tokens[tokens.indexOf(entrypoint, 1) + 1]?.toLowerCase() !== 'claude-teams' + ) { + return null + } + return keep ? viaEntrypoint : filterHeadlessOneShotAgentCommand(viaEntrypoint, tokens) } export function isAgentForegroundWrapperProcess(processName: string | null | undefined): boolean { const normalized = normalizeProcessName(processName) diff --git a/src/shared/git-binary-compatibility.test.ts b/src/shared/git-binary-compatibility.test.ts index fdaaa7f6937..47e1a8d7690 100644 --- a/src/shared/git-binary-compatibility.test.ts +++ b/src/shared/git-binary-compatibility.test.ts @@ -13,6 +13,7 @@ import { hasUnsupportedRevParsePathFormatEcho, isUnsupportedWorktreeListZError } from './git-worktree-command-capabilities' +import { gitCredentialPromptGuardEnv } from './git-credential-prompt-env' const execFileAsync = promisify(execFile) const image = process.env.ORCA_GIT_COMPAT_IMAGE @@ -26,7 +27,7 @@ describeBinaryCompatibility('real Git binary compatibility', () => { let repoPath = '' let version = { major: 0, minor: 0 } - async function runGit(args: string[]): Promise { + async function runGit(args: string[], env?: NodeJS.ProcessEnv): Promise { if (image) { const dockerUser = typeof process.getuid === 'function' && typeof process.getgid === 'function' @@ -39,6 +40,9 @@ describeBinaryCompatibility('real Git binary compatibility', () => { '--rm', '--network=none', ...dockerUser, + ...Object.entries(env ?? {}).flatMap(([key, value]) => + value === undefined ? [] : ['--env', `${key}=${value}`] + ), '-v', `${repoPath}:/repo`, '-w', @@ -51,7 +55,11 @@ describeBinaryCompatibility('real Git binary compatibility', () => { { maxBuffer: 2 * 1024 * 1024 } ) } - return execFileAsync(binary!, args, { cwd: repoPath, maxBuffer: 2 * 1024 * 1024 }) + return execFileAsync(binary!, args, { + cwd: repoPath, + env: env ? { ...process.env, ...env } : undefined, + maxBuffer: 2 * 1024 * 1024 + }) } function supports(major: number, minor: number): boolean { @@ -142,4 +150,19 @@ describeBinaryCompatibility('real Git binary compatibility', () => { await expect(runGit([...legacyArgs, head, head])).resolves.toBeDefined() } }) + + it('degrades indexed credential config safely at the Git 2.31 boundary', async () => { + const guardEnv = gitCredentialPromptGuardEnv({}, 'linux') + await expect(runGit(['status', '--short'], guardEnv)).resolves.toBeDefined() + + try { + const result = await runGit(['config', '--get', 'credential.interactive'], guardEnv) + expect(supports(2, 31)).toBe(true) + expect(result.stdout.trim()).toBe('false') + } catch { + // Git 2.25 ignores the indexed variables rather than rejecting commands; + // the scalar prompt guards still provide the baseline fail-fast behavior. + expect(supports(2, 31)).toBe(false) + } + }) }) diff --git a/src/shared/git-credential-prompt-env.ts b/src/shared/git-credential-prompt-env.ts new file mode 100644 index 00000000000..c17dfdc4281 --- /dev/null +++ b/src/shared/git-credential-prompt-env.ts @@ -0,0 +1,115 @@ +import { addWslEnvKeys } from './wsl-env' + +const GIT_CONFIG_WSLENV_KEY_RE = /^GIT_CONFIG_(?:COUNT|KEY_\d+|VALUE_\d+)$/ +const GIT_CONFIG_INDEXED_KEY_RE = /^GIT_CONFIG_(?:KEY|VALUE)_(\d+)$/ + +/** Merge an indexed-config protocol as one atomic environment value. */ +export function mergeGitConfigEnvProtocol( + baseEnv: NodeJS.ProcessEnv, + overrideEnv: NodeJS.ProcessEnv | undefined +): NodeJS.ProcessEnv { + const next = { ...baseEnv, ...overrideEnv } + if (!overrideEnv || !Object.keys(overrideEnv).some((key) => GIT_CONFIG_WSLENV_KEY_RE.test(key))) { + return next + } + + // Why: COUNT and its indexed pairs form one protocol; retaining lower-priority + // indices behind a smaller override count makes an otherwise valid env ambiguous. + for (const key of Object.keys(next)) { + if (GIT_CONFIG_WSLENV_KEY_RE.test(key)) { + delete next[key] + } + } + for (const [key, value] of Object.entries(overrideEnv)) { + if (GIT_CONFIG_WSLENV_KEY_RE.test(key)) { + next[key] = value + } + } + return next +} + +/** Return the safe append position for Git's indexed-config environment protocol. */ +export function readValidGitConfigEnvCount(env: NodeJS.ProcessEnv): number | null { + const rawCount = env.GIT_CONFIG_COUNT + const indexedKeys = Object.keys(env).filter((key) => GIT_CONFIG_INDEXED_KEY_RE.test(key)) + if (rawCount === undefined) { + return indexedKeys.length === 0 ? 0 : null + } + if (!/^(?:0|[1-9]\d*)$/.test(rawCount)) { + return null + } + + const count = Number(rawCount) + if (!Number.isSafeInteger(count) || indexedKeys.length !== count * 2) { + return null + } + for (let index = 0; index < count; index++) { + if ( + typeof env[`GIT_CONFIG_KEY_${index}`] !== 'string' || + typeof env[`GIT_CONFIG_VALUE_${index}`] !== 'string' + ) { + return null + } + } + const hasDanglingIndex = indexedKeys.some((key) => { + const match = key.match(GIT_CONFIG_INDEXED_KEY_RE) + return !match || String(Number(match[1])) !== match[1] || Number(match[1]) >= count + }) + return hasDanglingIndex ? null : count +} + +/** Compose indexed Git config without clobbering caller-provided entries. */ +export function appendGitConfigEnv( + env: NodeJS.ProcessEnv, + entries: readonly (readonly [key: string, value: string])[] +): NodeJS.ProcessEnv { + const next = { ...env } + const base = readValidGitConfigEnvCount(env) + if (base === null) { + // Why: ambiguous protocol state may contain caller data at any index, so + // scalar guards are safer than overwriting it with Orca-owned entries. + return next + } + entries.forEach(([key, value], index) => { + next[`GIT_CONFIG_KEY_${base + index}`] = key + next[`GIT_CONFIG_VALUE_${base + index}`] = value + }) + next.GIT_CONFIG_COUNT = String(base + entries.length) + return next +} + +/** + * Disable interactive Git credential UI while preserving cached credentials + * and caller-provided askpass programs. + */ +export function gitCredentialPromptGuardEnv( + env: NodeJS.ProcessEnv, + platform: NodeJS.Platform = process.platform +): NodeJS.ProcessEnv { + const next = appendGitConfigEnv( + { + ...env, + GIT_TERMINAL_PROMPT: '0', + GIT_ASKPASS: env.GIT_ASKPASS ?? '', + SSH_ASKPASS: env.SSH_ASKPASS ?? '', + // Why: GCM can ignore terminal/askpass guards and open its own GUI. + GCM_INTERACTIVE: 'never' + }, + // Why: keep the helper so cached credentials continue to work; disable + // only its interactive fallback. + [ + ['credential.interactive', 'false'], + ['credential.guiPrompt', 'false'] + ] + ) + if (platform === 'win32') { + // Why: wsl.exe imports only variables registered in WSLENV. Indexed Git + // config must cross as a complete set or Git rejects the count. + const configKeys = + readValidGitConfigEnvCount(next) === null + ? [] + : Object.keys(next).filter((key) => GIT_CONFIG_WSLENV_KEY_RE.test(key)) + addWslEnvKeys(next, ['GIT_TERMINAL_PROMPT', 'GCM_INTERACTIVE', ...configKeys]) + } + return next +} diff --git a/src/shared/terminal-git-credential-guard.ts b/src/shared/terminal-git-credential-guard.ts new file mode 100644 index 00000000000..340e2cdc7cf --- /dev/null +++ b/src/shared/terminal-git-credential-guard.ts @@ -0,0 +1,54 @@ +import { recognizeAgentProcessFromCommandLine } from './agent-process-recognition' +import { gitCredentialPromptGuardEnv } from './git-credential-prompt-env' + +const GIT_CONFIG_PROTOCOL_KEY_RE = /^GIT_CONFIG_(?:COUNT|KEY_\d+|VALUE_\d+)$/ + +export const TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV = + 'ORCA_INTERNAL_TERMINAL_GIT_CREDENTIAL_GUARD_POLICY' + +/** Disable credential UI only for recognized agents and marked automation. */ +export function applyTerminalGitCredentialPromptGuard( + env: Record, + opts: { + launchCommand?: string | null + isUnattended?: boolean + platform?: NodeJS.Platform + /** A detached host appends indexed config after its authoritative env merge. */ + deferGitConfigGuardToHost?: boolean + } +): boolean { + const explicitlyGuarded = env[TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV] === 'guard' + delete env[TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV] + + const shouldGuard = + opts.isUnattended === true || + Boolean( + recognizeAgentProcessFromCommandLine(opts.launchCommand, { includeHeadlessOneShot: true }) + ) + if (!explicitlyGuarded && !shouldGuard) { + return false + } + + const guarded = gitCredentialPromptGuardEnv(env, opts.platform ?? process.platform) + if (!opts.deferGitConfigGuardToHost) { + Object.assign(env, guarded) + return true + } + + // Why: the daemon must append indexed config after merging its own inherited + // environment; the sparse wire carries only the guard decision and scalars. + env[TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV] = 'guard' + for (const [key, value] of Object.entries(guarded)) { + if (typeof value !== 'string' || GIT_CONFIG_PROTOCOL_KEY_RE.test(key)) { + continue + } + if (key === 'WSLENV') { + continue + } + if ((key === 'GIT_ASKPASS' || key === 'SSH_ASKPASS') && !Object.hasOwn(env, key)) { + continue + } + env[key] = value + } + return true +} diff --git a/src/shared/wsl-env.ts b/src/shared/wsl-env.ts new file mode 100644 index 00000000000..71e836e945b --- /dev/null +++ b/src/shared/wsl-env.ts @@ -0,0 +1,17 @@ +export function addWslEnvKeys( + env: Record, + keys: readonly string[] +): void { + const existing = env.WSLENV ?? process.env.WSLENV ?? '' + const tokens = existing.split(':').filter(Boolean) + const tokenNames = new Set(tokens.map((token) => token.split('/')[0])) + + for (const key of keys) { + if (!tokenNames.has(key)) { + tokens.push(key) + tokenNames.add(key) + } + } + + env.WSLENV = tokens.join(':') +} diff --git a/tools/benchmarks/terminal-cold-park-resource-bench.mjs b/tools/benchmarks/terminal-cold-park-resource-bench.mjs index 5612fd4408f..8e134adce0b 100644 --- a/tools/benchmarks/terminal-cold-park-resource-bench.mjs +++ b/tools/benchmarks/terminal-cold-park-resource-bench.mjs @@ -413,7 +413,8 @@ async function main() { appTotalSavedMB: subMB(off.appMemory?.totalMB, on.appMemory?.totalMB), appRendererSavedMB: subMB(off.appMemory?.rendererMB, on.appMemory?.rendererMB), appMainSavedMB: subMB(off.appMemory?.mainMB, on.appMemory?.mainMB), - domNodesReleased: off.cdpNodes != null && on.cdpNodes != null ? off.cdpNodes - on.cdpNodes : null + domNodesReleased: + off.cdpNodes != null && on.cdpNodes != null ? off.cdpNodes - on.cdpNodes : null } report.finalDiagnostics = await collectRendererDiagnostics(page) } finally { @@ -451,7 +452,13 @@ async function main() { const stamp = new Date(startedAt).toISOString().replace(/[:.]/g, '-') const reportPath = path.resolve( args.reportPath ?? - path.join(rootDir, 'tools', 'benchmarks', 'results', `cold-park-res-${args.label}-${stamp}.json`) + path.join( + rootDir, + 'tools', + 'benchmarks', + 'results', + `cold-park-res-${args.label}-${stamp}.json` + ) ) mkdirSync(path.dirname(reportPath), { recursive: true }) writeFileSync(reportPath, `${JSON.stringify(report, null, 2)}\n`) diff --git a/tools/benchmarks/terminal-cold-park-reveal-bench.mjs b/tools/benchmarks/terminal-cold-park-reveal-bench.mjs index 3c57f7cbbb9..5e90b949f3c 100644 --- a/tools/benchmarks/terminal-cold-park-reveal-bench.mjs +++ b/tools/benchmarks/terminal-cold-park-reveal-bench.mjs @@ -422,7 +422,12 @@ function summarize(values) { return null } const at = (f) => sorted[Math.min(sorted.length - 1, Math.round(f * (sorted.length - 1)))] - return { count: sorted.length, median: Math.round(at(0.5)), p95: Math.round(at(0.95)), max: sorted.at(-1) } + return { + count: sorted.length, + median: Math.round(at(0.5)), + p95: Math.round(at(0.95)), + max: sorted.at(-1) + } } function summarizeArm(name, samples) { @@ -514,8 +519,7 @@ async function main() { // registers on first pane mount, so poll rather than read once. const debugReady = await pollUntil( 'parking debug handle', - () => - page.evaluate(() => typeof window.__terminalParkingDebug?.parkedTabIds === 'function'), + () => page.evaluate(() => typeof window.__terminalParkingDebug?.parkedTabIds === 'function'), Boolean, 10_000, 100 @@ -576,7 +580,13 @@ async function main() { const stamp = new Date(startedAt).toISOString().replace(/[:.]/g, '-') const reportPath = path.resolve( args.reportPath ?? - path.join(rootDir, 'tools', 'benchmarks', 'results', `cold-park-${args.label}-${stamp}.json`) + path.join( + rootDir, + 'tools', + 'benchmarks', + 'results', + `cold-park-${args.label}-${stamp}.json` + ) ) mkdirSync(path.dirname(reportPath), { recursive: true }) writeFileSync(reportPath, `${JSON.stringify(report, null, 2)}\n`)