Fix Fable review findings: guard WSL hook branch, wire settings search, catalog keyword keys, sparse-env askpass, one-shot agent classification (#7652)

This commit is contained in:
Brennan Benson
2026-07-10 20:22:12 -07:00
parent 4a5941adc4
commit 968e951abc
14 changed files with 134 additions and 30 deletions
+4 -1
View File
@@ -614,7 +614,10 @@ export function promptGuardGitEnv(
// would never see the guard — and WSL setups commonly use the Windows
// credential manager as git's helper, which pops the same OAuth window
// (issue #7652). Askpass vars are deliberately not forwarded: a Windows
// askpass path is meaningless inside the distro.
// askpass path is meaningless inside the distro. GIT_CONFIG_* must be
// forwarded all-or-nothing (including caller-preexisting entries): a
// GIT_CONFIG_COUNT that crosses without every KEY_n/VALUE_n below it makes
// git error out on "bogus count".
addWslEnvKeys(next, [
'GIT_TERMINAL_PROMPT',
'GCM_INTERACTIVE',
+9 -1
View File
@@ -956,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()
+5 -1
View File
@@ -636,7 +636,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)
+6 -9
View File
@@ -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'
@@ -102,16 +103,12 @@ vi.mock('../git/repo', async () => {
}
})
vi.mock('../git/runner', () => ({
vi.mock('../git/runner', async () => ({
// Why: keep the real nonInteractiveGitEnv so the clone regression test
// (#7652) asserts the actual guard's markers, not a mock echoing itself.
...(await vi.importActual<typeof GitRunner>('../git/runner')),
gitExecFileAsync: vi.fn(),
gitSpawn: gitSpawnMock,
// Mirrors the real guard's credential-prompt markers so the clone regression
// test (#7652) can assert Git Credential Manager's OAuth popup is disabled.
nonInteractiveGitEnv: (env: NodeJS.ProcessEnv = process.env) => ({
...env,
GIT_TERMINAL_PROMPT: '0',
GCM_INTERACTIVE: 'never'
})
gitSpawn: gitSpawnMock
}))
vi.mock('../git/worktree', () => ({
@@ -50,6 +50,35 @@ describe('applyTerminalGitCredentialPromptGuard', () => {
expect(wslenvKeys).not.toContain('SSH_ASKPASS')
})
it('guards a headless one-shot agent launch — it can answer a prompt even less than a TUI', () => {
const env: Record<string, string> = { PATH: '/usr/bin' }
applyTerminalGitCredentialPromptGuard(env, {
launchCommand: 'claude -p "fix the tests"',
suppressUserTerminalPrompt: false,
platform: 'darwin'
})
expect(isGuarded(env)).toBe(true)
})
it('never materializes an empty askpass into a sparse daemon wire env, but keeps a caller-set one', () => {
const sparse: Record<string, string> = { PATH: '/usr/bin' }
applyTerminalGitCredentialPromptGuard(sparse, {
launchCommand: 'claude',
suppressUserTerminalPrompt: true,
platform: 'win32'
})
expect(sparse.GIT_ASKPASS).toBeUndefined()
expect(sparse.SSH_ASKPASS).toBeUndefined()
const withAskpass: Record<string, string> = { PATH: '/usr/bin', GIT_ASKPASS: 'C:\\feeder.exe' }
applyTerminalGitCredentialPromptGuard(withAskpass, {
launchCommand: 'claude',
suppressUserTerminalPrompt: true,
platform: 'win32'
})
expect(withAskpass.GIT_ASKPASS).toBe('C:\\feeder.exe')
})
it('never rewrites the terminal locale — the git-runner locale pins must not leak into a shell', () => {
const env: Record<string, string> = { PATH: '/usr/bin', LC_ALL: 'ja_JP.UTF-8' }
applyTerminalGitCredentialPromptGuard(env, {
+15 -3
View File
@@ -27,7 +27,11 @@ export function applyTerminalGitCredentialPromptGuard(
platform?: NodeJS.Platform
}
): void {
const isAgentTerminal = Boolean(recognizeAgentProcessFromCommandLine(opts.launchCommand))
// Why: include headless one-shots (`claude -p …`) — those can answer a
// credential prompt even less than a TUI agent can.
const isAgentTerminal = Boolean(
recognizeAgentProcessFromCommandLine(opts.launchCommand, { includeHeadlessOneShot: true })
)
const platform = opts.platform ?? process.platform
if (!isAgentTerminal && (!opts.suppressUserTerminalPrompt || platform !== 'win32')) {
return
@@ -35,8 +39,16 @@ export function applyTerminalGitCredentialPromptGuard(
// Why: the shell variant — a terminal env is the user's whole environment,
// so the git-runner locale pins (issue #7808) must not leak into it.
for (const [key, value] of Object.entries(promptGuardShellEnv(env, platform))) {
if (typeof value === 'string') {
env[key] = value
if (typeof value !== 'string') {
continue
}
// Why: the guard materializes GIT_ASKPASS/SSH_ASKPASS='' when absent.
// Daemon spawns pass a sparse wire env here and merge process.env later —
// a materialized '' would clobber a daemon-inherited askpass that the
// preserve-when-set design (see nonInteractiveGitEnv docs) means to keep.
if ((key === 'GIT_ASKPASS' || key === 'SSH_ASKPASS') && !(key in env)) {
continue
}
env[key] = value
}
}
@@ -22,6 +22,7 @@ import {
getTerminalTypographySearchEntries
} from './terminal-typography-search'
import {
getTerminalGitCredentialPopupSearchEntry,
getTerminalRightClickToPasteSearchEntry,
getTerminalWindowsPowershellImplementationSearchEntry,
getTerminalWindowsShellSearchEntry
@@ -112,7 +113,8 @@ export function getTerminalPaneSearchEntries(platform: {
...(isWindowsTerminalHost
? [
...getTerminalWindowsShellSearchEntry(),
...getTerminalWindowsPowershellImplementationSearchEntry()
...getTerminalWindowsPowershellImplementationSearchEntry(),
...getTerminalGitCredentialPopupSearchEntry()
]
: []),
...(platform.isWindows ? getTerminalRightClickToPasteSearchEntry() : []),
+10 -1
View File
@@ -8515,7 +8515,16 @@
"713c4a2f92": "Choose the default shell for new terminal panes on Windows.",
"13715f9d23": "Default Shell",
"8630676830": "Block Git Credential Popups",
"9d8b09bc09": "Stop git in terminals from opening Git Credential Manager sign-in windows."
"9d8b09bc09": "Stop git in terminals from opening Git Credential Manager sign-in windows.",
"ba9f11ecc3": "git",
"7c7c74ef60": "credential",
"8eff361700": "popup",
"7c82e855b0": "oauth",
"bf215181b5": "github",
"b19a25c6a0": "sign in",
"ef5acdb939": "gcm",
"4ae35dbb42": "prompt",
"27e4a4878d": "credential manager"
}
}
},
+10 -1
View File
@@ -8478,7 +8478,16 @@
"713c4a2f92": "Elija el shell predeterminado para los nuevos paneles de terminal en Windows.",
"13715f9d23": "Shell predeterminado",
"8630676830": "Block Git Credential Popups",
"9d8b09bc09": "Stop git in terminals from opening Git Credential Manager sign-in windows."
"9d8b09bc09": "Stop git in terminals from opening Git Credential Manager sign-in windows.",
"ba9f11ecc3": "git",
"7c7c74ef60": "credential",
"8eff361700": "popup",
"7c82e855b0": "oauth",
"bf215181b5": "github",
"b19a25c6a0": "sign in",
"ef5acdb939": "gcm",
"4ae35dbb42": "prompt",
"27e4a4878d": "credential manager"
}
}
},
+10 -1
View File
@@ -8500,7 +8500,16 @@
"713c4a2f92": "Windows 上の新規 terminal ペインのデフォルト シェルを選択します。",
"13715f9d23": "デフォルトのシェル",
"8630676830": "Block Git Credential Popups",
"9d8b09bc09": "Stop git in terminals from opening Git Credential Manager sign-in windows."
"9d8b09bc09": "Stop git in terminals from opening Git Credential Manager sign-in windows.",
"ba9f11ecc3": "git",
"7c7c74ef60": "credential",
"8eff361700": "popup",
"7c82e855b0": "oauth",
"bf215181b5": "github",
"b19a25c6a0": "sign in",
"ef5acdb939": "gcm",
"4ae35dbb42": "prompt",
"27e4a4878d": "credential manager"
}
}
},
+10 -1
View File
@@ -8463,7 +8463,16 @@
"713c4a2f92": "Windows의 새 terminal 패널에 대한 기본 셸을 선택합니다.",
"13715f9d23": "기본 쉘",
"8630676830": "Block Git Credential Popups",
"9d8b09bc09": "Stop git in terminals from opening Git Credential Manager sign-in windows."
"9d8b09bc09": "Stop git in terminals from opening Git Credential Manager sign-in windows.",
"ba9f11ecc3": "git",
"7c7c74ef60": "credential",
"8eff361700": "popup",
"7c82e855b0": "oauth",
"bf215181b5": "github",
"b19a25c6a0": "sign in",
"ef5acdb939": "gcm",
"4ae35dbb42": "prompt",
"27e4a4878d": "credential manager"
}
}
},
+10 -1
View File
@@ -8463,7 +8463,16 @@
"713c4a2f92": "为 Windows 上的新终端窗格选择默认 Shell。",
"13715f9d23": "默认 Shell",
"8630676830": "Block Git Credential Popups",
"9d8b09bc09": "Stop git in terminals from opening Git Credential Manager sign-in windows."
"9d8b09bc09": "Stop git in terminals from opening Git Credential Manager sign-in windows.",
"ba9f11ecc3": "git",
"7c7c74ef60": "credential",
"8eff361700": "popup",
"7c82e855b0": "oauth",
"bf215181b5": "github",
"b19a25c6a0": "sign in",
"ef5acdb939": "gcm",
"4ae35dbb42": "prompt",
"27e4a4878d": "credential manager"
}
}
},
+10 -7
View File
@@ -286,17 +286,20 @@ 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
)
const direct = recognizeAgentProcess(tokens[0])
const directRecognition = keep ? direct : filterHeadlessOneShotAgentCommand(direct, tokens)
if (directRecognition) {
return directRecognition
}
@@ -304,10 +307,10 @@ 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)
return keep ? viaEntrypoint : filterHeadlessOneShotAgentCommand(viaEntrypoint, tokens)
}
export function isAgentForegroundWrapperProcess(processName: string | null | undefined): boolean {
const normalized = normalizeProcessName(processName)
+3 -2
View File
@@ -2563,8 +2563,9 @@ export type GlobalSettings = {
* network-restricted environments (issue #7652). Only applies on Windows
* hosts: the popup is a Windows credential-manager behavior, and elsewhere
* the guard would only remove working tty prompts. Agent terminals and
* Orca-run setup scripts are always guarded on every platform regardless of
* this setting. Turn off to let your terminal prompt via the OS helper. */
* Orca-run setup scripts are always guarded regardless of this setting, on
* whichever host spawns them (an SSH host applies its own guard). Turn off
* to let your terminal prompt via the OS helper. */
terminalSuppressGitCredentialPrompt?: boolean
terminalWordSeparator?: string
terminalCursorOpacity?: number