diff --git a/src/main/agent-hooks/installer-utils.test.ts b/src/main/agent-hooks/installer-utils.test.ts index 215979206e9..1f5ff12e5cd 100644 --- a/src/main/agent-hooks/installer-utils.test.ts +++ b/src/main/agent-hooks/installer-utils.test.ts @@ -617,11 +617,44 @@ function expectedDecodedWindowsHookCommand(scriptPath: string): string { const quoted = `'${scriptPath.replaceAll("'", "''")}'` // Why: the execution-policy bypass rides in the payload, not on the command // line, so the launcher cannot spell the AV-blocked flag triple (#16003). + // Why only for a .ps1: the cmdlet consumes redirected stdin on PowerShell 5.1 + // (STA-6357), and execution policy does not govern the .cmd every other hook ships. + const bypass = scriptPath.toLowerCase().endsWith('.ps1') + ? 'try { Set-ExecutionPolicy -Scope Process -ExecutionPolicy Bypass -Force -ErrorAction SilentlyContinue } catch {}; ' + : '' // Why: PowerShell progress CLIXML corrupts consumers that merge stderr into JSON stdout. - return `$ProgressPreference='SilentlyContinue'; try { Set-ExecutionPolicy -Scope Process -ExecutionPolicy Bypass -Force -ErrorAction SilentlyContinue } catch {}; if (Test-Path -LiteralPath ${quoted} -PathType Leaf) { & ${quoted}; exit $LASTEXITCODE }; [Console]::In.ReadToEnd() | Out-Null; exit 0` + return `$ProgressPreference='SilentlyContinue'; ${bypass}if (Test-Path -LiteralPath ${quoted} -PathType Leaf) { & ${quoted}; exit $LASTEXITCODE }; [Console]::In.ReadToEnd() | Out-Null; exit 0` } describe('wrapWindowsHookCommand', () => { + it('spares a .cmd hook the stdin-consuming execution-policy cmdlet', () => { + // STA-6357: on PowerShell 5.1 Set-ExecutionPolicy takes a host confirmation + // path even with -Force, and with the agent's hook JSON piped in that prompt + // consumes the payload and echoes it to stdout — so the agent rejects the + // hook output and the .cmd behind it reads empty stdin. Execution policy + // never governed a batch file, so this costs the .cmd hooks nothing. + const command = wrapWindowsHookCommand('C:\\hooks\\cursor-hook.cmd') + + expect(decodeWindowsHookCommand(command)).not.toContain('Set-ExecutionPolicy') + }) + + it('keeps the bypass for the one managed hook that is a .ps1', () => { + // Copilot ships copilot-hook.ps1, which a Restricted or AllSigned machine + // policy refuses to run without it. + const command = wrapWindowsHookCommand('C:\\hooks\\copilot-hook.ps1') + + expect(decodeWindowsHookCommand(command)).toContain('Set-ExecutionPolicy -Scope Process') + }) + + it('launches both with the identical AV-measured command shape', () => { + // The payloads differ; the command line must not. See the launcher's headline test. + const shapeOf = (command: string): string => command.replace(/ -EncodedCommand \S+$/, '') + + expect(shapeOf(wrapWindowsHookCommand('C:\\hooks\\cursor-hook.cmd'))).toBe( + shapeOf(wrapWindowsHookCommand('C:\\hooks\\copilot-hook.ps1')) + ) + }) + it('invokes the .cmd through an encoded PowerShell command', () => { const command = wrapWindowsHookCommand('C:\\Users\\alice\\.orca\\agent-hooks\\codex-hook.cmd') expect(command).toMatch(qualifiedWindowsPowerShellCommand) diff --git a/src/main/agent-hooks/installer-utils.ts b/src/main/agent-hooks/installer-utils.ts index 8667c418492..f380f732271 100644 --- a/src/main/agent-hooks/installer-utils.ts +++ b/src/main/agent-hooks/installer-utils.ts @@ -15,7 +15,10 @@ import type { AgentHookSource } from '../../shared/agent-hook-relay' import { grantDirAcl, isPermissionError } from '../win32-utils' import { resolveHooksJsonWritePath } from './hook-config-write-path' import { writeRollingFileBackup } from '../rolling-file-backup' -import { wrapWindowsPowerShellEncodedCommand } from './windows-powershell-hook-launcher' +import { + needsHookExecutionPolicyBypass, + wrapWindowsPowerShellEncodedCommand +} from './windows-powershell-hook-launcher' export type HookCommandConfig = { type: 'command' @@ -112,6 +115,7 @@ export function quotePowerShellString(value: string): string { } export { + needsHookExecutionPolicyBypass, wrapWindowsPowerShellEncodedCommand, WINDOWS_POWERSHELL_HOOK_SWITCHES } from './windows-powershell-hook-launcher' @@ -132,7 +136,11 @@ export function wrapWindowsHookCommand( ? '' : `Write-Output ${quotePowerShellString(options.fallbackStdout)}; ` const command = `${envPrefix}if (Test-Path -LiteralPath ${quoted} -PathType Leaf) { & ${quoted}; exit $LASTEXITCODE }; [Console]::In.ReadToEnd() | Out-Null; ${fallback}exit 0` - return wrapWindowsPowerShellEncodedCommand(command) + // Why keyed on the script we are about to run: only Copilot ships a `.ps1`, and + // every other managed hook is a `.cmd` that execution policy does not govern. + return wrapWindowsPowerShellEncodedCommand(command, { + executionPolicyBypass: needsHookExecutionPolicyBypass(scriptPath) + }) } export const WINDOWS_CMD_SAFE_PATH = /^[A-Za-z0-9_.:\\~-]+$/ diff --git a/src/main/agent-hooks/windows-powershell-hook-launcher.test.ts b/src/main/agent-hooks/windows-powershell-hook-launcher.test.ts index df307d87035..57a3c5d059a 100644 --- a/src/main/agent-hooks/windows-powershell-hook-launcher.test.ts +++ b/src/main/agent-hooks/windows-powershell-hook-launcher.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest' import { encodeWindowsPowerShellHookCommand, getWindowsPowerShellExecutablePath, + needsHookExecutionPolicyBypass, WINDOWS_POWERSHELL_HOOK_SWITCHES, wrapWindowsPowerShellEncodedCommand } from './windows-powershell-hook-launcher' @@ -54,7 +55,9 @@ describe('windows PowerShell hook launcher', () => { // Why it must survive somewhere: Copilot's managed hook is a .ps1, which a // Restricted or AllSigned machine policy refuses to run without a bypass. // Process scope is exactly what the switch used to set. - expect(decodePayload(wrapWindowsPowerShellEncodedCommand('exit 0'))).toContain( + expect( + decodePayload(wrapWindowsPowerShellEncodedCommand('exit 0', { executionPolicyBypass: true })) + ).toContain( 'Set-ExecutionPolicy -Scope Process -ExecutionPolicy Bypass -Force -ErrorAction SilentlyContinue' ) }) @@ -64,14 +67,16 @@ describe('windows PowerShell hook launcher', () => { // process scope did not take. -ErrorAction covers only the non-terminating // half; the switch this replaced printed nothing either way, and an // ErrorRecord on stderr corrupts consumers that merge our streams into JSON. - const decoded = decodePayload(wrapWindowsPowerShellEncodedCommand('exit 0')) + const decoded = decodePayload( + wrapWindowsPowerShellEncodedCommand('exit 0', { executionPolicyBypass: true }) + ) expect(decoded).toMatch(/try \{[^}]*Set-ExecutionPolicy[^}]*\} catch \{\}/) }) it('applies the bypass before the caller command and keeps progress silenced', () => { const decoded = Buffer.from( - encodeWindowsPowerShellHookCommand('& $scriptPath'), + encodeWindowsPowerShellHookCommand('& $scriptPath', { executionPolicyBypass: true }), 'base64' ).toString('utf16le') @@ -88,7 +93,7 @@ describe('windows PowerShell hook launcher', () => { // first merged line -- the exact corruption HOOK_PROGRESS_SILENCER exists to // stop. Silencer-first measured 0 bytes. const decoded = Buffer.from( - encodeWindowsPowerShellHookCommand('& $scriptPath'), + encodeWindowsPowerShellHookCommand('& $scriptPath', { executionPolicyBypass: true }), 'base64' ).toString('utf16le') @@ -97,4 +102,60 @@ describe('windows PowerShell hook launcher', () => { decoded.indexOf('Set-ExecutionPolicy') ) }) + + /* + * HEADLINE INVARIANT. The launch shape was chosen by measurement on an AV host, + * not by reasoning: #16003 measured that the denial happens at CreateProcess and + * matches on the FLAG COMBINATION, independently of what the payload decodes to. + * So the payload may change freely, and the command line may not. + * + * This is what stops a later "tidy-up" — folding `-NonInteractive` into the + * switches constant is the tempting one (STA-6357 proposes exactly that) — from + * silently re-opening a resolved AV defect. Any change to the shape needs a new + * measurement on a Kaspersky host, not an argument that it ought to be fine. + */ + it('keeps the launch shape byte-identical whichever payload it carries (AV-measured; re-measure before changing)', () => { + const shapeOf = (command: string): string => command.replace(/ -EncodedCommand \S+$/, '') + + const withBypass = wrapWindowsPowerShellEncodedCommand('exit 0', { + executionPolicyBypass: true + }) + const withoutBypass = wrapWindowsPowerShellEncodedCommand('exit 0') + + expect(shapeOf(withBypass)).toBe(shapeOf(withoutBypass)) + expect(shapeOf(withoutBypass)).toBe(`${getWindowsPowerShellExecutablePath()} -NoProfile`) + // The payloads must actually differ, or the assertion above proves nothing. + expect(decodePayload(withBypass)).not.toBe(decodePayload(withoutBypass)) + }) + + it('omits the execution-policy cmdlet for a payload that only runs a .cmd', () => { + // Why it must go: on Windows PowerShell 5.1 the cmdlet takes a host + // confirmation path even with -Force, and with stdin redirected that prompt + // consumes the hook payload and echoes it to stdout (STA-6357). Execution + // policy does not govern a batch file, so a .cmd payload gains nothing by it. + const decoded = decodePayload(wrapWindowsPowerShellEncodedCommand('& $scriptPath')) + + expect(decoded).not.toContain('Set-ExecutionPolicy') + // The progress silencer is unrelated to the policy bypass and must survive. + expect(decoded).toBe("$ProgressPreference='SilentlyContinue'; & $scriptPath") + }) +}) + +describe('needsHookExecutionPolicyBypass', () => { + it('asks for the bypass only for a .ps1, whatever its casing', () => { + expect( + needsHookExecutionPolicyBypass('C:\\Users\\me\\.orca\\agent-hooks\\copilot-hook.ps1') + ).toBe(true) + expect( + needsHookExecutionPolicyBypass('C:\\Users\\me\\.orca\\agent-hooks\\COPILOT-HOOK.PS1') + ).toBe(true) + }) + + it('does not ask for it for the .cmd every other managed hook ships', () => { + for (const script of ['claude-hook.cmd', 'cursor-hook.cmd', 'gemini-hook.cmd']) { + expect(needsHookExecutionPolicyBypass(`C:\\Users\\me\\.orca\\agent-hooks\\${script}`)).toBe( + false + ) + } + }) }) diff --git a/src/main/agent-hooks/windows-powershell-hook-launcher.ts b/src/main/agent-hooks/windows-powershell-hook-launcher.ts index 2cdb8c0f3fa..8aaa2cf419d 100644 --- a/src/main/agent-hooks/windows-powershell-hook-launcher.ts +++ b/src/main/agent-hooks/windows-powershell-hook-launcher.ts @@ -70,18 +70,48 @@ const HOOK_PROGRESS_SILENCER = "$ProgressPreference='SilentlyContinue'; " * stderr is a live corruption risk for the consumers that merge our streams into * JSON stdout (see the progress silencer above). A hook must still answer its * agent when the policy is locked down. + * + * Only a payload that runs a `.ps1` needs it, and it is not free: on Windows + * PowerShell 5.1 the cmdlet takes a host confirmation path even with `-Force + * -ErrorAction SilentlyContinue`, and with stdin redirected — which is how every + * agent delivers its hook JSON — that prompt consumes the payload and echoes it + * back onto stdout. The agent then rejects the hook output as malformed JSON and + * the script it fronts receives empty stdin (STA-6357). Execution policy does not + * govern a batch file, so a `.cmd` payload pays that cost for nothing. */ const HOOK_EXECUTION_POLICY_BYPASS = 'try { Set-ExecutionPolicy -Scope Process -ExecutionPolicy Bypass -Force -ErrorAction SilentlyContinue } catch {}; ' +/** Whether PowerShell will execute a script file this policy actually gates. */ +export function needsHookExecutionPolicyBypass(scriptPath: string): boolean { + return scriptPath.toLowerCase().endsWith('.ps1') +} + +export type WindowsPowerShellHookOptions = { + /** + * Default false: the payload runs a `.cmd`, which execution policy does not + * govern. A caller that forgets this on a `.ps1` gets a loud policy failure + * under a locked-down GPO; the reverse mistake silently corrupts every hook's + * stdin, so the quiet failure is the one kept off the default. + */ + executionPolicyBypass?: boolean +} + // Why: encoding shields paths and switches from cmd.exe and MSYS rewriting (#6078, #14815). -export function encodeWindowsPowerShellHookCommand(command: string): string { +export function encodeWindowsPowerShellHookCommand( + command: string, + options: WindowsPowerShellHookOptions = {} +): string { + const executionPolicyBypass = options.executionPolicyBypass ? HOOK_EXECUTION_POLICY_BYPASS : '' return Buffer.from( - `${HOOK_PROGRESS_SILENCER}${HOOK_EXECUTION_POLICY_BYPASS}${command}`, + `${HOOK_PROGRESS_SILENCER}${executionPolicyBypass}${command}`, 'utf16le' ).toString('base64') } -export function wrapWindowsPowerShellEncodedCommand(command: string): string { - return `${getWindowsPowerShellExecutablePath()} ${WINDOWS_POWERSHELL_HOOK_SWITCHES} -EncodedCommand ${encodeWindowsPowerShellHookCommand(command)}` +export function wrapWindowsPowerShellEncodedCommand( + command: string, + options: WindowsPowerShellHookOptions = {} +): string { + return `${getWindowsPowerShellExecutablePath()} ${WINDOWS_POWERSHELL_HOOK_SWITCHES} -EncodedCommand ${encodeWindowsPowerShellHookCommand(command, options)}` }