diff --git a/src/main/daemon/pty-subprocess-git-credential-guard.test.ts b/src/main/daemon/pty-subprocess-git-credential-guard.test.ts index b145e22202b..4e6f19c8a0c 100644 --- a/src/main/daemon/pty-subprocess-git-credential-guard.test.ts +++ b/src/main/daemon/pty-subprocess-git-credential-guard.test.ts @@ -308,4 +308,62 @@ describe('createPtySubprocess', () => { restoreGitCredentialGuardEnv(saved) } }) + it('does not double the guard when the daemon inherited one and the request carries its own', async () => { + const proc = mockPtyProcess() + spawnMock.mockReturnValue(proc) + // Orca was launched from a guarded agent pane, so the daemon it forked + // inherits that pane's guard AND that pane's provenance marker. + const inherited: Record = { + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'core.quotePath', + GIT_CONFIG_VALUE_0: 'false', + GIT_TERMINAL_PROMPT: '1', + GCM_INTERACTIVE: 'auto' + } + applyTerminalGitCredentialPromptGuard(inherited, { + launchCommand: 'claude', + platform: process.platform + }) + const saved = takeGitCredentialGuardEnv(inherited) + + try { + // Exactly what main puts on the wire for a guarded pane it defers to the daemon. + const requested: Record = {} + applyTerminalGitCredentialPromptGuard(requested, { + launchCommand: 'claude', + platform: process.platform, + deferGitConfigGuardToHost: true + }) + await createPtySubprocess({ + sessionId: 'inherited-and-requested-guard', + cols: 80, + rows: 24, + env: { SHELL: '/bin/bash', ...requested } + }) + + const paneEnv = spawnMock.mock.calls.at(-1)?.[2]?.env as Record + // One caller entry plus one guard pair — not two guard pairs. + expect(paneEnv.GIT_CONFIG_COUNT).toBe('3') + expect(paneEnv.GIT_CONFIG_KEY_1).toBe('credential.interactive') + expect(paneEnv.GIT_CONFIG_KEY_2).toBe('credential.guiPrompt') + expect(paneEnv.GIT_CONFIG_KEY_3).toBeUndefined() + + // And the pane's own marker still describes the user's real pre-guard env, + // so a plain shell launched inside it lands exactly back on those values. + const child = { ...paneEnv } + expect( + applyTerminalGitCredentialPromptGuard(child, { + launchCommand: '/bin/zsh', + platform: process.platform + }) + ).toBe(false) + expect(child.GIT_TERMINAL_PROMPT).toBe('1') + expect(child.GCM_INTERACTIVE).toBe('auto') + expect(child.GIT_CONFIG_COUNT).toBe('1') + expect(child.GIT_CONFIG_KEY_0).toBe('core.quotePath') + expect(Object.values(child)).not.toContain('credential.interactive') + } finally { + restoreGitCredentialGuardEnv(saved) + } + }) }) diff --git a/src/main/daemon/pty-subprocess/spawn-environment.ts b/src/main/daemon/pty-subprocess/spawn-environment.ts index 50145d7fdb5..7ab997c70ff 100644 --- a/src/main/daemon/pty-subprocess/spawn-environment.ts +++ b/src/main/daemon/pty-subprocess/spawn-environment.ts @@ -39,9 +39,6 @@ function composeGuardedDaemonGitConfigEnv( ): void { const policy = explicitEnv?.[TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV] delete env[TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV] - // Why: the daemon forks from Electron and can be launched from a guarded pane, so - // its inherited env may already carry the guard; undo it before deciding afresh. - restoreUnguardedGitCredentialEnv(env) if (policy !== 'guard' && launchAgent === undefined) { return } @@ -145,8 +142,19 @@ function removeInheritedDevAgentHookEndpoint( } export function createDaemonPtyEnvironment(opts: PtySubprocessOptions): Record { + // Why: a marker only describes the environment it was stamped into. The daemon + // forks from Electron and can be launched from a guarded pane, and the request + // carries its own marker for the scalars main wrote on the wire — undo them on + // their own side, or the request's marker lands on top of the inherited one and + // the guard it was meant to explain survives the merge unexplained. + const inheritedEnv = stripInheritedBuildModeEnv(process.env) as Record + restoreUnguardedGitCredentialEnv(inheritedEnv) + const requestedEnv = opts.env ? { ...opts.env } : undefined + if (requestedEnv) { + restoreUnguardedGitCredentialEnv(requestedEnv) + } const env: Record = { - ...mergeGitConfigEnvProtocol(stripInheritedBuildModeEnv(process.env), opts.env), + ...mergeGitConfigEnvProtocol(inheritedEnv, requestedEnv), TERM: 'xterm-256color', COLORTERM: 'truecolor', TERM_PROGRAM: 'Orca', diff --git a/src/shared/git-credential-guard-provenance.test.ts b/src/shared/git-credential-guard-provenance.test.ts new file mode 100644 index 00000000000..78dc613520d --- /dev/null +++ b/src/shared/git-credential-guard-provenance.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it } from 'vitest' +import { + captureGitCredentialGuardPreGuardState, + recordGitCredentialGuardProvenance, + restoreUnguardedGitCredentialEnv, + TERMINAL_GIT_CREDENTIAL_GUARD_RESTORE_ENV as MARKER +} from './git-credential-guard-provenance' + +/** An env carrying the guard, marked with the provenance `raw` describes. */ +function guardedEnv(raw: string): Record { + return { + GIT_TERMINAL_PROMPT: '0', + GCM_INTERACTIVE: 'never', + GIT_ASKPASS: '', + SSH_ASKPASS: '', + GIT_CONFIG_COUNT: '2', + GIT_CONFIG_KEY_0: 'credential.interactive', + GIT_CONFIG_VALUE_0: 'false', + GIT_CONFIG_KEY_1: 'credential.guiPrompt', + GIT_CONFIG_VALUE_1: 'false', + [MARKER]: raw + } +} + +const INTACT_GUARD = { + GIT_TERMINAL_PROMPT: '0', + GCM_INTERACTIVE: 'never', + GIT_ASKPASS: '', + SSH_ASKPASS: '', + GIT_CONFIG_COUNT: '2', + GIT_CONFIG_KEY_0: 'credential.interactive', + GIT_CONFIG_VALUE_0: 'false', + GIT_CONFIG_KEY_1: 'credential.guiPrompt', + GIT_CONFIG_VALUE_1: 'false' +} + +describe('restoreUnguardedGitCredentialEnv marker handling', () => { + it('leaves everything alone when there is no marker at all', () => { + const env = { ...INTACT_GUARD } + expect(restoreUnguardedGitCredentialEnv(env)).toBe(false) + expect(env).toEqual(INTACT_GUARD) + }) + + // A marker Orca cannot read cannot tell it which values are Orca's, so the + // arm it falls to must be "change nothing" — never "clear the user's Git env". + it.each([ + ['truncated json', '{"v":1,"env":{"GIT_TERMINAL_PROM'], + ['not json at all', 'guard'], + ['a json non-object', '"guard"'], + ['a future marker version', '{"v":2,"env":{"GIT_TERMINAL_PROMPT":"1"},"configBase":0}'], + ['a missing version', '{"env":{"GIT_TERMINAL_PROMPT":"1"},"configBase":0}'], + ['a non-object env', '{"v":1,"env":"GIT_TERMINAL_PROMPT","configBase":0}'] + ])('leaves the guard intact for %s', (_label, raw) => { + const env = guardedEnv(raw) + expect(restoreUnguardedGitCredentialEnv(env)).toBe(false) + expect(env).toEqual(INTACT_GUARD) + }) + + it('drops an unreadable marker so it cannot be mistaken for provenance later', () => { + const env = guardedEnv('{"v":1,"env":{') + restoreUnguardedGitCredentialEnv(env) + expect(Object.hasOwn(env, MARKER)).toBe(false) + }) + + it('ignores a marker whose recorded values are not strings or null', () => { + const env = guardedEnv( + '{"v":1,"env":{"GIT_TERMINAL_PROMPT":7,"GCM_INTERACTIVE":null},"configBase":null}' + ) + expect(restoreUnguardedGitCredentialEnv(env)).toBe(true) + // 7 is not a recorded value, so the key is not Orca's to restore. + expect(env.GIT_TERMINAL_PROMPT).toBe('0') + expect(env.GCM_INTERACTIVE).toBeUndefined() + }) + + it('ignores a configBase that is not a safe integer', () => { + for (const base of ['"0"', '0.5', '1e400', 'true']) { + const env = guardedEnv(`{"v":1,"env":{},"configBase":${base}}`) + restoreUnguardedGitCredentialEnv(env) + expect(env.GIT_CONFIG_COUNT).toBe('2') + expect(env.GIT_CONFIG_KEY_0).toBe('credential.interactive') + } + }) + + // A marker that names no config base did not append config, so nothing indexed + // in this environment is Orca's — including a pair that looks exactly like ours. + it('never removes indexed config when the marker claims no config base', () => { + const env = guardedEnv('{"v":1,"env":{"GIT_TERMINAL_PROMPT":null},"configBase":null}') + expect(restoreUnguardedGitCredentialEnv(env)).toBe(true) + expect(env.GIT_TERMINAL_PROMPT).toBeUndefined() + 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') + }) + + it('round-trips a capture through a record and back to the captured values', () => { + const original = { + GIT_TERMINAL_PROMPT: '1', + GIT_ASKPASS: '/usr/local/bin/ask', + GIT_CONFIG_COUNT: '1', + GIT_CONFIG_KEY_0: 'http.proxy', + GIT_CONFIG_VALUE_0: 'http://proxy.invalid' + } + const env: Record = { ...original } + const pre = captureGitCredentialGuardPreGuardState(env) + Object.assign(env, { + GIT_TERMINAL_PROMPT: '0', + GCM_INTERACTIVE: 'never', + SSH_ASKPASS: '', + GIT_CONFIG_COUNT: '3', + GIT_CONFIG_KEY_1: 'credential.interactive', + GIT_CONFIG_VALUE_1: 'false', + GIT_CONFIG_KEY_2: 'credential.guiPrompt', + GIT_CONFIG_VALUE_2: 'false' + }) + recordGitCredentialGuardProvenance(env, pre, { appendedConfig: true, forwardToWsl: false }) + + expect(restoreUnguardedGitCredentialEnv(env)).toBe(true) + expect(env).toEqual(original) + }) +}) diff --git a/src/shared/terminal-git-credential-guard-inheritance.test.ts b/src/shared/terminal-git-credential-guard-inheritance.test.ts index bfaf697608a..a01cd44c945 100644 --- a/src/shared/terminal-git-credential-guard-inheritance.test.ts +++ b/src/shared/terminal-git-credential-guard-inheritance.test.ts @@ -1,4 +1,8 @@ import { describe, expect, it } from 'vitest' +import { + restoreGitCredentialGuardEnv, + takeGitCredentialGuardEnv +} from './git-credential-guard-env-test-harness' import { TERMINAL_GIT_CREDENTIAL_GUARD_RESTORE_ENV } from './git-credential-guard-provenance' import { applyTerminalGitCredentialPromptGuard, @@ -213,4 +217,87 @@ describe('guard env inherited by a terminal the guard declines to guard', () => expect(env.GIT_CONFIG_COUNT).toBe('2') }) + it('keeps a guard variable the pane itself re-exported', () => { + const parent = guardedParentEnv('linux') + // The pane's own shell rc re-enabled prompting after Orca guarded the pane. + parent.GIT_TERMINAL_PROMPT = '1' + parent.GCM_INTERACTIVE = 'auto' + + const { env } = spawnChild(parent, { launchCommand: '/bin/zsh', platform: 'linux' }) + + expect(env.GIT_TERMINAL_PROMPT).toBe('1') + expect(env.GCM_INTERACTIVE).toBe('auto') + }) + + it('keeps a guard config slot whose value the pane overrode', () => { + const parent = guardedParentEnv('linux') + // Someone inside the pane flipped Orca's own entry back on. + parent.GIT_CONFIG_VALUE_0 = 'true' + + const { env } = spawnChild(parent, { launchCommand: '/bin/zsh', platform: 'linux' }) + + expect(env.GIT_CONFIG_COUNT).toBe('1') + expect(env.GIT_CONFIG_KEY_0).toBe('credential.interactive') + expect(env.GIT_CONFIG_VALUE_0).toBe('true') + }) + + it("keeps the caller's own WSLENV token for a guard variable, flags and all", () => { + const saved = takeGitCredentialGuardEnv() + try { + const parent = guardedParentEnv('win32', { WSLENV: 'GIT_ASKPASS/p' }) + + const { env } = spawnChild(parent, { launchCommand: '/bin/zsh', platform: 'win32' }) + + expect(env.WSLENV).toBe('GIT_ASKPASS/p') + } finally { + restoreGitCredentialGuardEnv(saved) + } + }) + + it('removes WSLENV rather than leaving an empty one behind', () => { + const saved = takeGitCredentialGuardEnv() + try { + const parent = guardedParentEnv('win32') + expect(parent.WSLENV).toBeTruthy() + + const { env } = spawnChild(parent, { launchCommand: '/bin/zsh', platform: 'win32' }) + + expect(Object.hasOwn(env, 'WSLENV')).toBe(false) + } finally { + restoreGitCredentialGuardEnv(saved) + } + }) + + it('never lets a detached-host marker remove indexed config the host owns', () => { + const wire: Record = { PATH: '/usr/bin' } + expect( + applyTerminalGitCredentialPromptGuard(wire, { + launchCommand: 'claude', + platform: 'linux', + deferGitConfigGuardToHost: true + }) + ).toBe(true) + + // The host's own environment already hardens Git the same way Orca does. + const host: Record = { + ...wire, + GIT_CONFIG_COUNT: '2', + GIT_CONFIG_KEY_0: 'credential.interactive', + GIT_CONFIG_VALUE_0: 'false', + GIT_CONFIG_KEY_1: 'credential.guiPrompt', + GIT_CONFIG_VALUE_1: 'false' + } + delete host[TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV] + + expect( + applyTerminalGitCredentialPromptGuard(host, { + launchCommand: '/bin/zsh', + platform: 'linux' + }) + ).toBe(false) + expect(host.GIT_CONFIG_COUNT).toBe('2') + expect(host.GIT_CONFIG_KEY_0).toBe('credential.interactive') + expect(host.GIT_CONFIG_VALUE_0).toBe('false') + expect(host.GIT_CONFIG_KEY_1).toBe('credential.guiPrompt') + }) })