mirror of
https://github.com/stablyai/orca.git
synced 2026-11-13 09:27:29 +00:00
fix(git): keep the agent-terminal credential guard scalar-only so env scrubs can't break git (#38716)
* fix(git): keep the agent-terminal credential guard scalar-only so env scrubs can't break git Agents that scrub env names matching KEY/TOKEN/... (DeepSeek Harness) dropped GIT_CONFIG_KEY_n but kept GIT_CONFIG_COUNT, so every git command in those terminals failed with "missing config key GIT_CONFIG_KEY_0". GCM_INTERACTIVE already carries credential.interactive and suppresses GCM's GUI, so the guard now emits only scalar variables. Tracked in #26594. Co-authored-by: yu.xia <yuxianice@163.com> * chore(git): drop stale indexed-config comments and a dead WSL forward branch --------- Co-authored-by: yu.xia <yuxianice@163.com>
This commit is contained in:
@@ -56,7 +56,7 @@ function composeGuardedDaemonGitConfigEnv(
|
||||
if (policy !== 'guard' && launchAgent === undefined) {
|
||||
return
|
||||
}
|
||||
// Preserve the long-lived daemon’s inherited Git config while appending the guard.
|
||||
// Apply the guard after the inherited-env merge so it reflects the pane's final askpass values.
|
||||
Object.assign(env, gitCredentialPromptGuardEnv(env, process.platform))
|
||||
}
|
||||
|
||||
|
||||
@@ -11,8 +11,6 @@ export function gitOptionalLocksDisabledEnv(
|
||||
}
|
||||
}
|
||||
|
||||
/** Append Git >= 2.31 config-env entries without overwriting the caller's existing entries. */
|
||||
|
||||
/** Pin Git diagnostics to English so parsers work under any user locale. */
|
||||
export function untranslatedGitOutputEnv(env: NodeJS.ProcessEnv = process.env): NodeJS.ProcessEnv {
|
||||
return { ...env, ...UNTRANSLATED_GIT_OUTPUT_ENV }
|
||||
|
||||
+14
-68
@@ -1,8 +1,5 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
appendGitConfigEnv,
|
||||
mergeGitConfigEnvProtocol
|
||||
} from '../../shared/git/commands/git-credential-prompt-env'
|
||||
import { mergeGitConfigEnvProtocol } from '../../shared/git/commands/git-credential-prompt-env'
|
||||
import {
|
||||
extractExecError,
|
||||
isTransientGhError,
|
||||
@@ -14,19 +11,6 @@ import {
|
||||
untranslatedGitOutputEnv
|
||||
} from './runner'
|
||||
|
||||
function readGitConfigEnv(env: NodeJS.ProcessEnv): Record<string, string> {
|
||||
const count = Number.parseInt(env.GIT_CONFIG_COUNT ?? '0', 10)
|
||||
const config: Record<string, string> = {}
|
||||
for (let i = 0; i < count; i++) {
|
||||
const key = env[`GIT_CONFIG_KEY_${i}`]
|
||||
const value = env[`GIT_CONFIG_VALUE_${i}`]
|
||||
if (key !== undefined && value !== undefined) {
|
||||
config[key] = value
|
||||
}
|
||||
}
|
||||
return config
|
||||
}
|
||||
|
||||
describe('redirectPortedHostnameToEnv', () => {
|
||||
it('moves a ported --hostname into GITLAB_HOST and strips the flag', () => {
|
||||
const { args, options } = redirectPortedHostnameToEnv(
|
||||
@@ -163,45 +147,6 @@ describe('extractExecError', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('appendGitConfigEnv', () => {
|
||||
it('injects entries starting at count 0 when none exist', () => {
|
||||
const env = appendGitConfigEnv({ PATH: '/usr/bin' }, [['credential.interactive', 'false']])
|
||||
expect(env.GIT_CONFIG_COUNT).toBe('1')
|
||||
expect(env.GIT_CONFIG_KEY_0).toBe('credential.interactive')
|
||||
expect(env.GIT_CONFIG_VALUE_0).toBe('false')
|
||||
expect(env.PATH).toBe('/usr/bin')
|
||||
})
|
||||
|
||||
it('composes with an existing count instead of clobbering caller config', () => {
|
||||
const env = appendGitConfigEnv(
|
||||
{ GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'core.quotePath', GIT_CONFIG_VALUE_0: 'false' },
|
||||
[['credential.guiPrompt', 'false']]
|
||||
)
|
||||
expect(env.GIT_CONFIG_COUNT).toBe('2')
|
||||
expect(env.GIT_CONFIG_KEY_0).toBe('core.quotePath')
|
||||
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')
|
||||
})
|
||||
|
||||
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(
|
||||
@@ -230,11 +175,8 @@ describe('promptGuardGitEnv credential-interactivity disable (STA-1292)', () =>
|
||||
it('disables the GCM GUI prompt without nuking the credential helper', () => {
|
||||
const env = promptGuardGitEnv({ PATH: '/usr/bin' })
|
||||
expect(env.GCM_INTERACTIVE).toBe('never')
|
||||
const config = readGitConfigEnv(env)
|
||||
expect(config['credential.interactive']).toBe('false')
|
||||
expect(config['credential.guiPrompt']).toBe('false')
|
||||
// Preserve credential helpers so private repos can still use cached authentication.
|
||||
expect(config['credential.helper']).toBeUndefined()
|
||||
// Scalar guards only: env scrubs can split an indexed config protocol (#26594).
|
||||
expect(env.GIT_CONFIG_COUNT).toBeUndefined()
|
||||
expect(env.GIT_TERMINAL_PROMPT).toBe('0')
|
||||
})
|
||||
})
|
||||
@@ -243,25 +185,29 @@ describe('nonInteractiveGitEnv credential-interactivity disable (STA-1292)', ()
|
||||
it('carries the credential-interactivity disable through from promptGuardGitEnv', () => {
|
||||
const env = nonInteractiveGitEnv({ PATH: '/usr/bin' })
|
||||
expect(env.GCM_INTERACTIVE).toBe('never')
|
||||
const config = readGitConfigEnv(env)
|
||||
expect(config['credential.interactive']).toBe('false')
|
||||
expect(config['credential.guiPrompt']).toBe('false')
|
||||
expect(config['credential.helper']).toBeUndefined()
|
||||
expect(env.GIT_CONFIG_COUNT).toBeUndefined()
|
||||
expect(env.GIT_SSH_COMMAND).toBe('ssh -o BatchMode=yes')
|
||||
})
|
||||
})
|
||||
|
||||
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 env = promptGuardGitEnv(
|
||||
{
|
||||
PATH: '/usr/bin',
|
||||
GIT_CONFIG_COUNT: '1',
|
||||
GIT_CONFIG_KEY_0: 'http.proxy',
|
||||
GIT_CONFIG_VALUE_0: 'http://proxy.invalid'
|
||||
},
|
||||
'win32'
|
||||
)
|
||||
const keys = (env.WSLENV ?? '').split(':')
|
||||
expect(keys).toContain('GIT_TERMINAL_PROMPT')
|
||||
expect(keys).toContain('GCM_INTERACTIVE')
|
||||
// A caller's indexed config is forwarded as a complete set.
|
||||
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')
|
||||
|
||||
@@ -32,7 +32,7 @@ export type BuildPtyHostEnvOptions = {
|
||||
networkProxySettings?: NetworkProxySettings
|
||||
/** Headless paired runtimes hand browser launches to the client-hosted Orca browser. */
|
||||
routeBrowserOpensToClient?: boolean
|
||||
/** Keep indexed Git config off the sparse daemon wire; the daemon appends guard entries after merging its inherited env. */
|
||||
/** The daemon reapplies the Git credential guard after merging its inherited env, so askpass reflects the final pane env. */
|
||||
deferGitConfigGuardToDaemon?: boolean
|
||||
}
|
||||
|
||||
|
||||
@@ -244,9 +244,9 @@ describe('registerPtyHandlers', () => {
|
||||
|
||||
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')
|
||||
expect(env.GIT_CONFIG_COUNT).toBe('1')
|
||||
expect(env.GIT_CONFIG_KEY_0).toBe('http.proxy')
|
||||
expect(env.GIT_CONFIG_KEY_1).toBeUndefined()
|
||||
})
|
||||
it('passes the minted sessionId through to provider.spawn and host env setup', async () => {
|
||||
const daemonSpawn = setupDaemonAdapter()
|
||||
|
||||
@@ -550,12 +550,12 @@ describe('registerPtyHandlers', () => {
|
||||
const env = await spawnAndGetEnv()
|
||||
expect(env.ORCA_IMAGE_PROTOCOL).toBe('kitty')
|
||||
})
|
||||
it('keeps indexed Git prompt guards in a local agent terminal env', async () => {
|
||||
it('guards a local agent terminal with scalar Git prompt variables only', 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')
|
||||
// Agents that scrub KEY-named env would split an indexed protocol and break git (#26594).
|
||||
expect(Object.keys(env).filter((key) => key.startsWith('GIT_CONFIG_'))).toEqual([])
|
||||
})
|
||||
it('guards a trusted local agent when its command uses a custom wrapper', async () => {
|
||||
const env = await spawnAndGetEnv(
|
||||
|
||||
@@ -205,16 +205,11 @@ export function runHook(
|
||||
for (const [key, value] of Object.entries(envVars)) {
|
||||
wslEnv[key] = toLinuxPath(value)
|
||||
}
|
||||
// Forward unattended Git flags and indexed config to WSL; keep host askpass out of the guest.
|
||||
// Forward unattended Git flags to WSL; keep host askpass out of the guest.
|
||||
const guardedEnv = promptGuardShellEnv(wslEnv)
|
||||
const guestEnv: Record<string, string> = { ...wslEnv }
|
||||
for (const [key, value] of Object.entries(guardedEnv)) {
|
||||
if (
|
||||
value !== undefined &&
|
||||
(key === 'GIT_TERMINAL_PROMPT' ||
|
||||
key === 'GCM_INTERACTIVE' ||
|
||||
key.startsWith('GIT_CONFIG_'))
|
||||
) {
|
||||
if (value !== undefined && (key === 'GIT_TERMINAL_PROMPT' || key === 'GCM_INTERACTIVE')) {
|
||||
guestEnv[key] = value
|
||||
}
|
||||
}
|
||||
|
||||
@@ -57,25 +57,6 @@ export function readValidGitConfigEnvCount(env: NodeJS.ProcessEnv): number | nul
|
||||
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) {
|
||||
// Ambiguous indices may hold caller data; use scalar guards instead of overwriting them.
|
||||
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.
|
||||
@@ -84,23 +65,19 @@ 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'
|
||||
},
|
||||
// Retain cached credentials while disabling only interactive helper fallback.
|
||||
[
|
||||
['credential.interactive', 'false'],
|
||||
['credential.guiPrompt', 'false']
|
||||
]
|
||||
)
|
||||
// Why: scalar guards only. Agents scrub env names matching KEY/TOKEN/..., which drops
|
||||
// GIT_CONFIG_KEY_n but keeps GIT_CONFIG_COUNT, and Git then refuses every command (#26594).
|
||||
// GCM_INTERACTIVE is GCM's env form of credential.interactive and also suppresses its GUI.
|
||||
const next: NodeJS.ProcessEnv = {
|
||||
...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'
|
||||
}
|
||||
if (platform === 'win32') {
|
||||
// WSLENV must register the complete indexed config protocol or Git rejects its count.
|
||||
// WSLENV must forward a caller's indexed config as a complete set or Git rejects its count.
|
||||
const configKeys =
|
||||
readValidGitConfigEnvCount(next) === null
|
||||
? []
|
||||
|
||||
@@ -747,19 +747,17 @@ describeBinaryCompatibility('real Git binary compatibility', () => {
|
||||
await runGit(['-C', worktree, 'update-ref', '-d', backupRef, backupOid])
|
||||
})
|
||||
|
||||
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)
|
||||
}
|
||||
it('keeps git usable when an agent scrubs credential-shaped env names (#26594)', async () => {
|
||||
// The DeepSeek Harness subprocess filter, recorded in #26594.
|
||||
const sensitiveEnvPattern = /KEY|PASSWORD|SECRET|TOKEN/i
|
||||
const scrubbed = Object.fromEntries(
|
||||
Object.entries(gitCredentialPromptGuardEnv({}, 'linux')).filter(
|
||||
([key]) => !sensitiveEnvPattern.test(key)
|
||||
)
|
||||
)
|
||||
expect(scrubbed.GIT_TERMINAL_PROMPT).toBe('0')
|
||||
expect(scrubbed.GCM_INTERACTIVE).toBe('never')
|
||||
await expect(runGit(['status', '--short'], scrubbed)).resolves.toBeDefined()
|
||||
})
|
||||
|
||||
// Git 2.25–2.49 echoes --end-of-options with --symbolic-full-name but consumes it with --verify; git-history skips the echoed line.
|
||||
|
||||
@@ -1,8 +1,6 @@
|
||||
import { recognizeAgentLaunchCommand } from '../../agent-launch/startup/agent-command-line-intent'
|
||||
import { gitCredentialPromptGuardEnv } from '../../git/commands/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'
|
||||
|
||||
@@ -13,7 +11,7 @@ export function applyTerminalGitCredentialPromptGuard(
|
||||
launchCommand?: string | null
|
||||
isUnattended?: boolean
|
||||
platform?: NodeJS.Platform
|
||||
/** A detached host appends indexed config after its authoritative env merge. */
|
||||
/** A detached host reapplies the guard after its authoritative env merge. */
|
||||
deferGitConfigGuardToHost?: boolean
|
||||
}
|
||||
): boolean {
|
||||
@@ -37,10 +35,10 @@ export function applyTerminalGitCredentialPromptGuard(
|
||||
return true
|
||||
}
|
||||
|
||||
// The daemon must append indexed config after merging inherited env; send only the policy and scalar guards.
|
||||
// The daemon reapplies the guard after merging inherited env; send the policy and scalar guards.
|
||||
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)) {
|
||||
if (typeof value !== 'string') {
|
||||
continue
|
||||
}
|
||||
if (key === 'WSLENV') {
|
||||
|
||||
@@ -68,11 +68,7 @@ describe('buildRelayUnattendedGitEnv', () => {
|
||||
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_CONFIG_COUNT).toBeUndefined()
|
||||
expect(env.GIT_SSH_COMMAND).toBeUndefined()
|
||||
expect(env.LC_ALL).toBe('en_US.UTF-8')
|
||||
expect(env.PATH?.split(':')).toEqual(expect.arrayContaining(['/custom/bin', '/usr/bin']))
|
||||
|
||||
Reference in New Issue
Block a user