diff --git a/src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts b/src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts index e0d8897a1b3..9b6726a4377 100644 --- a/src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts +++ b/src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts @@ -5,8 +5,11 @@ import { existsSync, mkdtempSync, readdirSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { removeTreeSync } from '../../shared/windows-transient-lock-removal' -import { WINDOWS_CMD_SAFE_PATH } from './installer-utils' -import { wrapWindowsDirectCmdHookCommand } from './windows-direct-cmd-hook-command' +import { WINDOWS_CMD_SAFE_PATH, wrapWindowsHookCommand } from './installer-utils' +import { + wrapWindowsBatchHookCommand, + wrapWindowsDirectCmdHookCommand +} from './windows-direct-cmd-hook-command' import { getWindowsClaudeHookEntry } from '../claude/windows-hook-files' import { getManagedScript } from '../claude/hook-script' import { getWindowsPowerShellExecutablePath } from './windows-powershell-hook-launcher' @@ -54,6 +57,22 @@ describe('wrapWindowsDirectCmdHookCommand', () => { }) }) +describe('wrapWindowsBatchHookCommand', () => { + it('registers a cmd-safe .cmd hook by its bare path', () => { + expect(wrapWindowsBatchHookCommand(SAFE_PATH)).toBe(wrapWindowsDirectCmdHookCommand(SAFE_PATH)) + }) + + it('keeps the encoded launcher for a path the shells cannot carry bare', () => { + const spaced = 'C:\\Users\\Bob Smith\\.orca\\agent-hooks\\gemini-hook.cmd' + expect(wrapWindowsBatchHookCommand(spaced)).toBe(wrapWindowsHookCommand(spaced)) + // Guarded Unicode .cmd paths take the cheaper policy switch, as wrapWindowsCmdHookCommand does. + const unicode = 'C:\\Users\\rené\\.orca\\agent-hooks\\gemini-hook.cmd' + expect(wrapWindowsBatchHookCommand(unicode)).toBe( + wrapWindowsHookCommand(unicode, {}, { useProcessPolicyEnvironment: true }) + ) + }) +}) + describe.skipIf(process.platform !== 'win32')( 'direct hook command, run by Windows hook hosts', () => { diff --git a/src/main/agent-hooks/windows-direct-cmd-hook-command.ts b/src/main/agent-hooks/windows-direct-cmd-hook-command.ts index e4fbc4d2ffc..dce62ec40a8 100644 --- a/src/main/agent-hooks/windows-direct-cmd-hook-command.ts +++ b/src/main/agent-hooks/windows-direct-cmd-hook-command.ts @@ -1,9 +1,13 @@ -import { WINDOWS_CMD_SAFE_PATH } from './installer-utils' +import { + isSafeUnicodeWindowsBatchHookPath, + WINDOWS_CMD_SAFE_PATH, + wrapWindowsHookCommand +} from './windows-hook-command' // UNC paths cannot be started consistently by all Windows hook hosts. const WINDOWS_DRIVE_LETTER_PATH = /^[A-Za-z]:\\/ -/** A bare path works without guessing whether Claude selected Bash or PowerShell. */ +/** A bare path runs the same under cmd.exe, PowerShell and Git Bash, so no host shell is guessed. */ export function wrapWindowsDirectCmdHookCommand(scriptPath: string): string | null { if (!WINDOWS_CMD_SAFE_PATH.test(scriptPath) || !WINDOWS_DRIVE_LETTER_PATH.test(scriptPath)) { return null @@ -11,3 +15,20 @@ export function wrapWindowsDirectCmdHookCommand(scriptPath: string): string | nu // The script owns failure handling; PowerShell 5.1 cannot parse a shell-level `||`. return scriptPath.replaceAll('\\', '/') } + +/** + * Registration for a managed `.cmd` hook: the bare path skips a PowerShell start-up per event + * (#18875); paths the shells cannot carry bare keep the encoded launcher. A missing script then + * prints "not recognized" until Orca reinstalls it instead of exiting silently. + */ +export function wrapWindowsBatchHookCommand(scriptPath: string): string { + return ( + wrapWindowsDirectCmdHookCommand(scriptPath) ?? + wrapWindowsHookCommand( + scriptPath, + {}, + // Same policy shortcut as wrapWindowsCmdHookCommand for guarded Unicode .cmd paths (#26382). + { useProcessPolicyEnvironment: isSafeUnicodeWindowsBatchHookPath(scriptPath) } + ) + ) +} diff --git a/src/main/agent-hooks/windows-hook-payload-delivery.test.ts b/src/main/agent-hooks/windows-hook-payload-delivery.test.ts index f242e8815ca..47eb53e26e4 100644 --- a/src/main/agent-hooks/windows-hook-payload-delivery.test.ts +++ b/src/main/agent-hooks/windows-hook-payload-delivery.test.ts @@ -4,13 +4,14 @@ // curl POST carries no payload. Shape, exit-code and stdout assertions all stayed green // through it, because none of them ever piped a payload. This suite pipes one and follows // it to the listener, so a launcher that takes stdin away from the hook fails here (#14818). +// Every agent that registers a managed .cmd runs here under the shell its vendor uses. import { afterEach, describe, expect, it, vi } from 'vitest' import { runProcess } from '@orca/process-host' import { createServer, type Server } from 'node:http' -import { mkdtempSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs' import { removeTreeSync } from '../../shared/windows-transient-lock-removal' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { dirname, join } from 'node:path' import type * as osModule from 'node:os' const { homedirMock } = vi.hoisted(() => ({ @@ -32,9 +33,14 @@ vi.mock('os', async (importOriginal) => { }) import { ClaudeHookService } from '../claude/hook-service' +import { CommandCodeHookService } from '../command-code/hook-service' +import { DroidHookService } from '../droid/hook-service' +import { DshHookService } from '../dsh/hook-service' +import { GeminiHookService } from '../gemini/hook-service' +import { KiroHookService } from '../kiro/hook-service' import { MuseHookService } from '../muse/hook-service' import { getMuseManagedHooksPath } from '../muse/hook-settings' -import { readHooksJson, WINDOWS_CMD_SAFE_PATH } from './installer-utils' +import { readHooksJson, WINDOWS_CMD_SAFE_PATH, wrapWindowsHookCommand } from './installer-utils' import { getConfigPath, getWindowsManagedLifecycleHook } from '../claude/hook-settings' import { findGitBash } from './windows-git-bash-path.test-fixture' @@ -66,10 +72,11 @@ async function startHookListener(): Promise<{ }) req.on('end', () => { const form = new URLSearchParams(body) + const token = req.headers['x-orca-agent-hook-token'] posts.push({ payload: form.get('payload'), paneKey: form.get('paneKey'), - token: req.headers['x-orca-agent-hook-token'] as string | null + token: typeof token === 'string' ? token : null }) res.writeHead(200, { 'content-type': 'application/json' }) res.end('{}') @@ -84,18 +91,177 @@ async function startHookListener(): Promise<{ return { server, port, posts } } -type HookRun = { exitCode: number | null; stdout: string; stderr: string; timedOut: boolean } - -function runHookCommand( - executable: string, - args: string[], - env: NodeJS.ProcessEnv -): Promise { - return runProcess({ program: executable, args, env, input: PAYLOAD, timeoutMs: 15_000 }).then( - (result) => ({ ...result, exitCode: result.code }) - ) +type Runner = { + name: string + program: () => string + args: (command: string) => string[] + verbatim?: boolean } +const SYSTEM32 = join(process.env.SystemRoot ?? 'C:\\Windows', 'System32') +const CMD = join(SYSTEM32, 'cmd.exe') +const POWERSHELL = join(SYSTEM32, 'WindowsPowerShell', 'v1.0', 'powershell.exe') + +const CMD_PLAIN: Runner = { + name: 'cmd.exe', + program: () => 'cmd.exe', + args: (command) => ['/d', '/c', command] +} +const GIT_BASH: Runner = { name: 'Git Bash', program: findGitBash, args: (c) => ['-c', c] } +// Vendor argv below; runners that prefer pwsh fall back to Windows PowerShell 5.1. +const CMD_NODE_SHELL: Runner = { + name: 'cmd.exe (Node shell:true)', + program: () => CMD, + args: (command) => ['/d', '/s', '/c', `"${command}"`], + verbatim: true +} +const CMD_FACTORY: Runner = { + name: 'cmd.exe (Factory)', + program: () => CMD, + args: (command) => ['/d', '/s', '/c', command], + verbatim: true +} +const POWERSHELL_GEMINI: Runner = { + name: 'Windows PowerShell (Gemini)', + program: () => POWERSHELL, + args: (command) => [ + '-NoProfile', + '-NonInteractive', + '-Command', + `${command}; if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }` + ] +} +const POWERSHELL_DSH: Runner = { + name: 'Windows PowerShell (DSH)', + program: () => POWERSHELL, + args: (command) => [ + '-NoLogo', + '-NoProfile', + '-NonInteractive', + '-Command', + `[Console]::OutputEncoding=[System.Text.Encoding]::UTF8;$OutputEncoding=[System.Text.Encoding]::UTF8;${command}` + ] +} +const POWERSHELL_PLAIN: Runner = { + name: 'Windows PowerShell', + program: () => POWERSHELL, + args: (command) => ['-NoProfile', '-Command', command] +} + +type Entry = { command?: string; hooks?: { command?: string }[] } +const commandsOf = (entries: Entry[] | undefined): (string | undefined)[] => + (entries ?? []).flatMap((entry) => + entry.hooks ? entry.hooks.map((h) => h.command) : entry.command + ) + +function readEvent(path: string, event: string): (string | undefined)[] { + return commandsOf(JSON.parse(readFileSync(path, 'utf8')).hooks?.[event]) +} + +function writeHooks( + path: string, + hooks: Record, + extra: { name?: string } = {} +): void { + mkdirSync(dirname(path), { recursive: true }) + writeFileSync(path, JSON.stringify({ ...extra, hooks })) +} + +type Provider = { + name: string + script: string + stdout: string + install: () => { state: string } + /** Managed commands registered for the event the row runs; exactly one is expected. */ + commands: (home: string) => (string | undefined)[] + /** Seeds the pre-#27717 encoded registration so install must replace it, not add to it. */ + seedStale?: (home: string, stale: string) => void + runners?: Runner[] +} + +const PROVIDERS: Provider[] = [ + { + name: 'Claude', + script: 'claude-hook.cmd', + stdout: '{}', + install: () => new ClaudeHookService().install(), + commands: () => commandsOf(readHooksJson(getConfigPath())?.hooks?.PreToolUse) + }, + { + name: 'Muse', + script: 'muse-hook.cmd', + stdout: '', + install: () => new MuseHookService().install(), + commands: () => commandsOf(readHooksJson(getMuseManagedHooksPath())?.hooks?.PreToolUse) + }, + { + name: 'Droid', + script: 'droid-hook.cmd', + stdout: '', + install: () => new DroidHookService().install(), + commands: (home) => readEvent(join(home, '.factory', 'settings.json'), 'UserPromptSubmit'), + seedStale: (home, stale) => + writeHooks(join(home, '.factory', 'settings.json'), { + UserPromptSubmit: [{ hooks: [{ command: stale }] }] + }), + runners: [CMD_FACTORY] + }, + { + name: 'Command Code', + script: 'command-code-hook.cmd', + stdout: '', + install: () => new CommandCodeHookService().install(), + commands: (home) => readEvent(join(home, '.commandcode', 'settings.json'), 'Stop'), + seedStale: (home, stale) => + writeHooks(join(home, '.commandcode', 'settings.json'), { + Stop: [{ hooks: [{ command: stale }] }] + }), + runners: [CMD_NODE_SHELL] + }, + { + name: 'Gemini', + script: 'gemini-hook.cmd', + stdout: '{}', + install: () => new GeminiHookService().install(), + commands: (home) => readEvent(join(home, '.gemini', 'settings.json'), 'BeforeAgent'), + seedStale: (home, stale) => + writeHooks(join(home, '.gemini', 'settings.json'), { + BeforeAgent: [{ hooks: [{ command: stale }] }] + }), + runners: [POWERSHELL_GEMINI] + }, + { + // dsh-hooks.json is Orca-owned and rewritten whole, so there is no stale entry to sweep. + name: 'DSH', + script: 'dsh-hook.cmd', + stdout: '', + install: () => new DshHookService().install(), + commands: (home) => + readEvent(join(home, '.orca', 'agent-hooks', 'dsh-hooks.json'), 'UserPromptSubmit'), + runners: [POWERSHELL_DSH] + }, + { + // Kiro is closed source; cover both shells it could use. + name: 'Kiro', + script: 'kiro-hook.cmd', + stdout: '', + install: () => new KiroHookService().install(), + commands: (home) => readEvent(join(home, '.kiro', 'agents', 'mine.json'), 'userPromptSubmit'), + seedStale: (home, stale) => + writeHooks( + join(home, '.kiro', 'agents', 'mine.json'), + { userPromptSubmit: [{ command: stale }] }, + { name: 'mine' } + ), + runners: [POWERSHELL_PLAIN, CMD_NODE_SHELL] + } +] + +// A space keeps the bare path off the command line, so that profile takes the encoded launcher. +const CASES = PROVIDERS.flatMap((provider) => + ['user', 'user name'].map((homeName) => ({ ...provider, homeName })) +) + // Why: a developer box may set HKCU\...\Command Processor\AutoRun, which cmd.exe runs before // any .cmd — and MSYS spawns a .cmd without `/d`, so it fires on the Git Bash leg. Redirecting // USERPROFILE to a temp home makes the usual `%USERPROFILE%\.cmd_aliases.cmd` target vanish, and @@ -105,9 +271,11 @@ function seedCmdAutoRunTarget(home: string): void { writeFileSync(join(home, '.cmd_aliases.cmd'), '@echo off\r\n', 'utf8') } +// Why: strip every inherited ORCA_* so an endpoint file from a live Orca can never redirect the +// hook's POST away from this listener. function hookEnvironment(extra: NodeJS.ProcessEnv): NodeJS.ProcessEnv { const base = Object.fromEntries( - Object.entries(process.env).filter(([key]) => !key.startsWith('ORCA_')) + Object.entries(process.env).filter(([key]) => !key.toUpperCase().startsWith('ORCA_')) ) return { ...base, ORCA_BACKGROUND_LAUNCH: '1', ...extra } } @@ -125,7 +293,7 @@ describe('Windows managed hook launcher', () => { }) describe.skipIf(process.platform !== 'win32')('Windows managed hook payload delivery', () => { - let home = '' + let root = '' let server: Server | null = null afterEach(() => { @@ -133,47 +301,44 @@ describe.skipIf(process.platform !== 'win32')('Windows managed hook payload deli server?.close() server = null homedirMock.mockImplementation(() => process.env.HOME ?? tmpdir()) - if (home) { - removeTreeSync(home) - home = '' + if (root) { + removeTreeSync(root) + root = '' } }) - it.each([ - { - name: 'Claude', - install: () => new ClaudeHookService().install(), - configPath: getConfigPath, - script: 'claude-hook.cmd', - stdout: '{}' - }, - { - name: 'Muse', - install: () => new MuseHookService().install(), - configPath: getMuseManagedHooksPath, - script: 'muse-hook.cmd', - stdout: '' - } - ])( - '$name delivers the piped payload through cmd.exe and Git Bash', + it.each(CASES)( + '$name delivers the piped payload through its runners (home "$homeName")', async (provider) => { - home = mkdtempSync(join(tmpdir(), 'orca-hook-payload-')) - vi.stubEnv('XDG_CONFIG_HOME', undefined) + root = mkdtempSync(join(tmpdir(), 'orca-hook-payload-')) + const home = join(root, provider.homeName) + mkdirSync(home) homedirMock.mockReturnValue(home) + for (const name of ['USERPROFILE', 'HOME']) { + vi.stubEnv(name, home) + } + for (const name of ['XDG_CONFIG_HOME', 'KIRO_HOME', 'DSH_HOME']) { + vi.stubEnv(name, undefined) + } seedCmdAutoRunTarget(home) + const scriptPath = join(home, '.orca', 'agent-hooks', provider.script) + provider.seedStale?.(home, wrapWindowsHookCommand(scriptPath)) expect(provider.install().state).toBe('installed') - const settings = readHooksJson(provider.configPath()) // Why: assert nothing about the launcher's shape here — this test's whole value is // that it fails for any launcher that loses the payload, named conhost or not. - const registeredCommand = settings?.hooks?.PreToolUse[0]?.hooks?.[0]?.command + const commands = provider.commands(home) + expect(commands, 'exactly one managed registration').toHaveLength(1) + const registeredCommand = commands[0] if (!registeredCommand) { - throw new Error('Missing managed PreToolUse command') + throw new Error(`Missing managed ${provider.name} command`) } // ...with one exception: a cmd-safe profile must reach the script with no interpreter in // front of it, or #18875's per-event PowerShell start-up has quietly come back. - if (WINDOWS_CMD_SAFE_PATH.test(join(home, '.orca', 'agent-hooks', provider.script))) { + if (WINDOWS_CMD_SAFE_PATH.test(scriptPath)) { expect(registeredCommand).not.toMatch(/powershell|-EncodedCommand/i) + } else { + expect(registeredCommand).toMatch(/-EncodedCommand/) } const listener = await startHookListener() @@ -185,41 +350,40 @@ describe.skipIf(process.platform !== 'win32')('Windows managed hook payload deli ORCA_AGENT_HOOK_TOKEN: HOOK_TOKEN, ORCA_PANE_KEY: PANE_KEY }) + const run = (runner: Runner, runEnv: NodeJS.ProcessEnv) => + runProcess({ + program: runner.program(), + args: runner.args(registeredCommand), + windowsVerbatimArguments: runner.verbatim, + env: runEnv, + input: PAYLOAD, + timeoutMs: 15_000 + }) - const shells = [ - { name: 'cmd.exe', executable: 'cmd.exe', args: ['/d', '/c', registeredCommand] }, - { name: 'Git Bash', executable: findGitBash(), args: ['-c', registeredCommand] } - ] - for (const shell of shells) { + for (const runner of provider.runners ?? [CMD_PLAIN, GIT_BASH]) { const before = listener.posts.length - const result = await runHookCommand(shell.executable, shell.args, env) - expect(result.timedOut, `${shell.name} timed out`).toBe(false) - expect(result.exitCode, `${shell.name} exit code`).toBe(0) + const result = await run(runner, env) + expect(result.timedOut, `${runner.name} timed out`).toBe(false) + expect(result.code, `${runner.name} exit code; stderr: ${result.stderr}`).toBe(0) // Why: assert delivery before stdout so a launcher that swallows the payload fails // on the symptom users report, not on some downstream difference in what it printed. - const posts = listener.posts.slice(before) - expect(posts, `${shell.name} posted exactly one hook`).toHaveLength(1) - // Why: byte-exact, not "non-empty" — a pseudoconsole host delivers nothing, and a + // Byte-exact, not "non-empty" — a pseudoconsole host delivers nothing, and a // code-page-translating launcher delivers a corrupted body that still looks posted. - expect(posts[0].payload, `${shell.name} payload`).toBe(PAYLOAD) - expect(posts[0].paneKey, `${shell.name} paneKey`).toBe(PANE_KEY) - expect(posts[0].token, `${shell.name} token`).toBe(HOOK_TOKEN) - - expect(result.stderr, `${shell.name} stderr`).toBe('') + expect(listener.posts.slice(before), `${runner.name} posts`).toEqual([ + { payload: PAYLOAD, paneKey: PANE_KEY, token: HOOK_TOKEN } + ]) + expect(result.stderr, `${runner.name} stderr`).toBe('') // Why: compat consumers gate the tool call on parseable stdout (#14818). - expect(result.stdout.trim(), `${shell.name} stdout`).toBe(provider.stdout) + expect(result.stdout.trim(), `${runner.name} stdout`).toBe(provider.stdout) - const outsideOrca = await runHookCommand( - shell.executable, - shell.args, - hookEnvironment({ USERPROFILE: home, HOME: home }) - ) - expect(outsideOrca.timedOut, `${shell.name} outside Orca timed out`).toBe(false) - expect(outsideOrca.exitCode, `${shell.name} outside Orca exit code`).toBe(0) + // Outside an Orca pane the hook exits without posting or waiting on stdin (#11549). + const outsideOrca = await run(runner, hookEnvironment({ USERPROFILE: home, HOME: home })) + expect(outsideOrca.timedOut, `${runner.name} outside Orca timed out`).toBe(false) + expect(outsideOrca.code, `${runner.name} outside Orca exit code`).toBe(0) expect(listener.posts).toHaveLength(before + 1) } - // Why: two shell launches plus a real install can overrun the default under load. + // Why: several shell launches plus a real install can overrun the default under load. }, 60_000 ) diff --git a/src/main/agent-hooks/windows-powershell-hook-launcher.ts b/src/main/agent-hooks/windows-powershell-hook-launcher.ts index c2a81d679cf..b2e419834eb 100644 --- a/src/main/agent-hooks/windows-powershell-hook-launcher.ts +++ b/src/main/agent-hooks/windows-powershell-hook-launcher.ts @@ -40,9 +40,8 @@ export function getWindowsPowerShellExecutablePath(): string { * AV host for a flicker. The answer is to shorten the interpreter chain — the * shipped doctrine of #15520 and #15595 — or a launcher that owns no console. * - * #18875 took that answer for the Claude lifecycle hook, which now registers the - * managed `.cmd` path directly (`windows-direct-cmd-hook-command.ts`) and reaches - * this launcher only when the profile path is not cmd-safe. Every other caller still comes through here on every event. + * #18875 took that answer for the Claude lifecycle hook, and `wrapWindowsBatchHookCommand` + * extends it to other `.cmd` hooks: they reach this launcher only when the profile path is not cmd-safe. */ export const WINDOWS_POWERSHELL_HOOK_SWITCHES = '-NoProfile' diff --git a/src/main/command-code/hook-service.test.ts b/src/main/command-code/hook-service.test.ts index 9765ef5b2f4..41402ae03af 100644 --- a/src/main/command-code/hook-service.test.ts +++ b/src/main/command-code/hook-service.test.ts @@ -20,8 +20,7 @@ vi.mock('os', async () => { import { CommandCodeHookService } from './hook-service' -const WINDOWS_POWERSHELL_LAUNCHER = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -EncodedCommand \S+$/ +import { wrapWindowsBatchHookCommand } from '../agent-hooks/windows-direct-cmd-hook-command' describe('CommandCodeHookService', () => { let homeDir: string @@ -52,7 +51,11 @@ describe('CommandCodeHookService', () => { expect(config.hooks.PostToolUse[0].matcher).toBe('.*') expect(config.hooks.Stop[0].matcher).toBeUndefined() expect(config.hooks.PreToolUse[0].hooks[0].command).toMatch( - process.platform === 'win32' ? WINDOWS_POWERSHELL_LAUNCHER : /command-code-hook/ + process.platform === 'win32' + ? wrapWindowsBatchHookCommand( + join(homeDir, '.orca', 'agent-hooks', 'command-code-hook.cmd') + ) + : /command-code-hook/ ) if (process.platform !== 'win32') { expect(config.hooks.PreToolUse[0].hooks[0].command).toContain(join(homeDir, '.orca')) @@ -62,30 +65,24 @@ describe('CommandCodeHookService', () => { } }) - // Why: #6078 — a Windows user profile path with a space used to be written - // verbatim as the hook command, so the agent split it at the space. The - // managed command must use an encoded launcher so the path never appears raw - // on the cmd.exe command line. - it.skipIf(process.platform !== 'win32')( - 'wraps the managed hook command to survive spaces in the profile path (#6078)', - () => { - const spaceHome = join(tmpdir(), 'orca command-code home with spaces') - mkdirSync(spaceHome, { recursive: true }) - homedirMock.mockReturnValue(spaceHome) - try { - expect(new CommandCodeHookService().install().state).toBe('installed') - - const config = JSON.parse( - readFileSync(join(spaceHome, '.commandcode', 'settings.json'), 'utf8') - ) as { hooks: Record } - - const command = config.hooks.PreToolUse[0].hooks[0].command - expect(command).toMatch(WINDOWS_POWERSHELL_LAUNCHER) - } finally { - rmSync(spaceHome, { recursive: true, force: true }) + // Why: runs on every OS; windows-hook-payload-delivery.test.ts runs each form under Command Code's shell. + it('registers the Windows batch hook command for every event', () => { + const platform = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + try { + expect(new CommandCodeHookService().install().state).toBe('installed') + const command = wrapWindowsBatchHookCommand( + join(homeDir, '.orca', 'agent-hooks', 'command-code-hook.cmd') + ) + const config = JSON.parse( + readFileSync(join(homeDir, '.commandcode', 'settings.json'), 'utf8') + ) + for (const eventName of ['PreToolUse', 'PostToolUse', 'Stop']) { + expect(config.hooks[eventName][0].hooks[0].command).toBe(command) } + } finally { + platform.mockRestore() } - ) + }) it('installs a hook script that can recover the endpoint when Command Code strips token env', () => { new CommandCodeHookService().install() diff --git a/src/main/command-code/hook-service.ts b/src/main/command-code/hook-service.ts index 1e1ab9e56af..0ac48d2d9d6 100644 --- a/src/main/command-code/hook-service.ts +++ b/src/main/command-code/hook-service.ts @@ -9,11 +9,11 @@ import { readHooksJson, removeManagedCommands, wrapPosixHookCommand, - wrapWindowsHookCommand, writeHooksJson, writeManagedScript, type HookDefinition } from '../agent-hooks/installer-utils' +import { wrapWindowsBatchHookCommand } from '../agent-hooks/windows-direct-cmd-hook-command' import { refreshManagedScriptIfPresent } from '../agent-hooks/managed-hook-script-refresh' import { readHooksJsonRemote, @@ -48,7 +48,7 @@ function getManagedScriptPath(): string { function getManagedCommand(scriptPath: string): string { return process.platform === 'win32' - ? wrapWindowsHookCommand(scriptPath) + ? wrapWindowsBatchHookCommand(scriptPath) : wrapPosixHookCommand(scriptPath) } diff --git a/src/main/droid/hook-service.test.ts b/src/main/droid/hook-service.test.ts index 7486a12b71b..5d399a8be80 100644 --- a/src/main/droid/hook-service.test.ts +++ b/src/main/droid/hook-service.test.ts @@ -24,8 +24,7 @@ vi.mock('os', async () => { import { DroidHookService } from './hook-service' -const WINDOWS_POWERSHELL_LAUNCHER = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -EncodedCommand \S+$/ +import { wrapWindowsBatchHookCommand } from '../agent-hooks/windows-direct-cmd-hook-command' describe('DroidHookService', () => { let homeDir: string @@ -74,7 +73,9 @@ describe('DroidHookService', () => { expect(config.hooks.PermissionRequest[0].matcher).toBe('*') expect(config.hooks.UserPromptSubmit[0].matcher).toBeUndefined() expect(config.hooks.PreToolUse[0].hooks[0].command).toMatch( - process.platform === 'win32' ? WINDOWS_POWERSHELL_LAUNCHER : /droid-hook/ + process.platform === 'win32' + ? wrapWindowsBatchHookCommand(join(homeDir, '.orca', 'agent-hooks', 'droid-hook.cmd')) + : /droid-hook/ ) if (process.platform !== 'win32') { expect(config.hooks.PreToolUse[0].hooks[0].command).toContain(join(homeDir, '.orca')) @@ -82,32 +83,22 @@ describe('DroidHookService', () => { expect(config.hooks.PreToolUse[0].hooks[0].command).not.toContain(userDataDir) }) - // Why: #6078 — a Windows user profile path with a space used to be written - // verbatim as the hook command, so the agent split it at the space. The - // managed command must use an encoded launcher so the path never appears raw - // on the cmd.exe command line. - it.skipIf(process.platform !== 'win32')( - 'wraps the managed hook command to survive spaces in the profile path (#6078)', - () => { - const spaceHome = join(tmpdir(), 'orca droid home with spaces') - mkdirSync(spaceHome, { recursive: true }) - homedirMock.mockReturnValue(spaceHome) - try { - expect(new DroidHookService().install().state).toBe('installed') - - const config = JSON.parse( - readFileSync(join(spaceHome, '.factory', 'settings.json'), 'utf8') - ) as { hooks: Record } - - for (const eventName of ['SessionStart', 'UserPromptSubmit', 'Stop']) { - const command = config.hooks[eventName]?.[0]?.hooks?.[0]?.command - expect(command).toMatch(WINDOWS_POWERSHELL_LAUNCHER) - } - } finally { - rmSync(spaceHome, { recursive: true, force: true }) + // Why: runs on every OS; windows-hook-payload-delivery.test.ts runs each form under Droid's shell. + it('registers the Windows batch hook command for every event', () => { + const platform = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + try { + expect(new DroidHookService().install().state).toBe('installed') + const command = wrapWindowsBatchHookCommand( + join(homeDir, '.orca', 'agent-hooks', 'droid-hook.cmd') + ) + const config = JSON.parse(readFileSync(join(homeDir, '.factory', 'settings.json'), 'utf8')) + for (const eventName of ['SessionStart', 'UserPromptSubmit', 'PreToolUse', 'Stop']) { + expect(config.hooks[eventName][0].hooks[0].command).toBe(command) } + } finally { + platform.mockRestore() } - ) + }) it('reports partial when Factory has hooks disabled globally', () => { const configPath = join(homeDir, '.factory', 'settings.json') diff --git a/src/main/droid/hook-service.ts b/src/main/droid/hook-service.ts index 3bdb715427f..a8b864046ca 100644 --- a/src/main/droid/hook-service.ts +++ b/src/main/droid/hook-service.ts @@ -10,12 +10,12 @@ import { readHooksJson, removeManagedCommands, wrapPosixHookCommand, - wrapWindowsHookCommand, writeHooksJson, writeManagedScript, type HookDefinition, type HooksConfig } from '../agent-hooks/installer-utils' +import { wrapWindowsBatchHookCommand } from '../agent-hooks/windows-direct-cmd-hook-command' import { refreshManagedScriptIfPresent } from '../agent-hooks/managed-hook-script-refresh' import { readHooksJsonRemote, @@ -70,11 +70,9 @@ function getManagedScriptPath(): string { } function getManagedCommand(scriptPath: string): string { - // Why: Factory invokes the .cmd via cmd.exe, but the raw path still splits at - // whitespace when the user profile contains a space (e.g. `C:\Users\Jane Doe`). - // The shared Windows wrapper keeps the path out of cmd.exe's raw command line. #6078. + // Why: Factory runs hooks via cmd.exe, which splits a bare path at a profile space (#6078). return process.platform === 'win32' - ? wrapWindowsHookCommand(scriptPath) + ? wrapWindowsBatchHookCommand(scriptPath) : wrapPosixHookCommand(scriptPath) } diff --git a/src/main/dsh/hook-service.test.ts b/src/main/dsh/hook-service.test.ts index 331ecd97888..91644106107 100644 --- a/src/main/dsh/hook-service.test.ts +++ b/src/main/dsh/hook-service.test.ts @@ -9,7 +9,8 @@ import { } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { wrapWindowsBatchHookCommand } from '../agent-hooks/windows-direct-cmd-hook-command' import { DshHookService } from './hook-service' import { DSH_HOOK_EVENTS } from './hook-settings' @@ -103,6 +104,22 @@ describe('DshHookService', () => { expect(restoreAt).toBeLessThan(guardAt) }) + // Why: runs on every OS; windows-hook-payload-delivery.test.ts runs each form under DSH's shell. + it('registers the Windows batch hook command for every event', () => { + const platform = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + try { + expect(new DshHookService().install().state).toBe('installed') + const command = wrapWindowsBatchHookCommand(scriptPath()) + for (const event of DSH_HOOK_EVENTS) { + expect(readManagedHooks().hooks[event]).toEqual([ + { hooks: [expect.objectContaining({ command })] } + ]) + } + } finally { + platform.mockRestore() + } + }) + it('is idempotent', () => { const service = new DshHookService() service.install() diff --git a/src/main/dsh/hook-settings.ts b/src/main/dsh/hook-settings.ts index 8ccbb8a9283..5fcd486af6b 100644 --- a/src/main/dsh/hook-settings.ts +++ b/src/main/dsh/hook-settings.ts @@ -5,9 +5,9 @@ import { createManagedCommandMatcher, getSharedManagedScriptPath, wrapPosixHookCommand, - wrapWindowsHookCommand, type HookDefinition } from '../agent-hooks/installer-utils' +import { wrapWindowsBatchHookCommand } from '../agent-hooks/windows-direct-cmd-hook-command' import { readManagedHookEventsFromJson } from '../agent-hooks/managed-hooks-json-events' const DSH_SCRIPT_BASE = 'dsh-hook' @@ -73,11 +73,9 @@ export function getDshRemoteManagedHooksPath(remoteHome: string): string { } export function getDshManagedCommand(scriptPath: string): string { - // Why: DSH runs hooks through `ctx.shell`, which the base profile binds to bash - // everywhere except Windows, where it binds to PowerShell — the same split these two - // wrappers already encode. + // Why: DSH runs hooks through `ctx.shell`: bash off Windows, PowerShell on it, which runs a bare .cmd too. return process.platform === 'win32' - ? wrapWindowsHookCommand(scriptPath) + ? wrapWindowsBatchHookCommand(scriptPath) : wrapPosixHookCommand(scriptPath) } diff --git a/src/main/gemini/hook-service.test.ts b/src/main/gemini/hook-service.test.ts index 551c40bb0b1..099403d35e2 100644 --- a/src/main/gemini/hook-service.test.ts +++ b/src/main/gemini/hook-service.test.ts @@ -24,9 +24,7 @@ vi.mock('os', async (importOriginal) => { }) import { GeminiHookService } from './hook-service' - -const WINDOWS_POWERSHELL_LAUNCHER = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -EncodedCommand \S+$/ +import { wrapWindowsBatchHookCommand } from '../agent-hooks/windows-direct-cmd-hook-command' describe('GeminiHookService', () => { let homeDir: string @@ -105,7 +103,7 @@ describe('GeminiHookService', () => { expect(config.hooks.BeforeAgent[0].hooks[0].command).toBe('echo user-before-agent') const managedCommandPattern = process.platform === 'win32' - ? WINDOWS_POWERSHELL_LAUNCHER + ? wrapWindowsBatchHookCommand(managedHookPath) : new RegExp(managedHookPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) expect(config.hooks.BeforeAgent[1].hooks[0].command).toMatch(managedCommandPattern) expect(config.hooks.AfterAgent[0].hooks[0].command).toMatch(managedCommandPattern) @@ -113,33 +111,22 @@ describe('GeminiHookService', () => { expect(config.hooks.BeforeTool[0].hooks[0].command).toMatch(managedCommandPattern) }) - // Why: #6078 — a Windows user profile path with a space used to be written - // verbatim as the hook command, so the agent split it at the space. The - // managed command must use an encoded launcher so the path never appears raw - // on the cmd.exe command line. - it.skipIf(process.platform !== 'win32')( - 'wraps the managed hook command to survive spaces in the profile path (#6078)', - () => { - const spaceHome = join(tmpdir(), 'orca gemini home with spaces') - mkdirSync(spaceHome, { recursive: true }) - homedirMock.mockReturnValue(spaceHome) - try { - expect(new GeminiHookService().install().state).toBe('installed') - - const config = JSON.parse( - readFileSync(join(spaceHome, '.gemini', 'settings.json'), 'utf8') - ) as { hooks: Record } - - for (const eventName of ['BeforeAgent', 'AfterAgent', 'AfterTool']) { - const command = config.hooks[eventName]?.[0]?.hooks?.[0]?.command - expect(command).toMatch(WINDOWS_POWERSHELL_LAUNCHER) - } - } finally { - rmSync(spaceHome, { recursive: true, force: true }) - homedirMock.mockReturnValue(homeDir) + // Why: runs on every OS; windows-hook-payload-delivery.test.ts runs each form under Gemini's shell. + it('registers the Windows batch hook command for every event', () => { + const platform = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + try { + expect(new GeminiHookService().install().state).toBe('installed') + const command = wrapWindowsBatchHookCommand( + join(homeDir, '.orca', 'agent-hooks', 'gemini-hook.cmd') + ) + const config = JSON.parse(readFileSync(join(homeDir, '.gemini', 'settings.json'), 'utf8')) + for (const eventName of ['BeforeAgent', 'AfterAgent', 'AfterTool', 'BeforeTool']) { + expect(config.hooks[eventName].at(-1).hooks[0].command).toBe(command) } + } finally { + platform.mockRestore() } - ) + }) it('preserves user-authored PreToolUse hooks while sweeping stale managed Gemini hooks', () => { const managedHookFileName = process.platform === 'win32' ? 'gemini-hook.cmd' : 'gemini-hook.sh' @@ -184,7 +171,7 @@ describe('GeminiHookService', () => { expect(preToolCommands).toEqual(['echo user-authored']) expect(config.hooks.BeforeTool[0].hooks[0].command).toMatch( process.platform === 'win32' - ? WINDOWS_POWERSHELL_LAUNCHER + ? wrapWindowsBatchHookCommand(join(homeDir, '.orca', 'agent-hooks', managedHookFileName)) : new RegExp(managedHookFileName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) ) }) diff --git a/src/main/gemini/hook-service.ts b/src/main/gemini/hook-service.ts index 6fd529e96e4..df640661a66 100644 --- a/src/main/gemini/hook-service.ts +++ b/src/main/gemini/hook-service.ts @@ -11,11 +11,11 @@ import { readHooksJson, removeManagedCommands, wrapPosixHookCommand, - wrapWindowsHookCommand, writeHooksJson, writeManagedScript, type HookDefinition } from '../agent-hooks/installer-utils' +import { wrapWindowsBatchHookCommand } from '../agent-hooks/windows-direct-cmd-hook-command' import { refreshManagedScriptIfPresent } from '../agent-hooks/managed-hook-script-refresh' import { readHooksJsonRemote, @@ -47,7 +47,7 @@ function getManagedScriptPath(): string { function getManagedCommand(scriptPath: string): string { return process.platform === 'win32' - ? wrapWindowsHookCommand(scriptPath) + ? wrapWindowsBatchHookCommand(scriptPath) : wrapPosixHookCommand(scriptPath) } diff --git a/src/main/kiro/hook-service.test.ts b/src/main/kiro/hook-service.test.ts index abfe38dbacb..5af123a0e09 100644 --- a/src/main/kiro/hook-service.test.ts +++ b/src/main/kiro/hook-service.test.ts @@ -1,9 +1,10 @@ import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { createManagedHookLocalFilesystem } from '../agent-hooks/managed-hook-local-filesystem.test-fixture' import { isPlainObject } from '../agent-hooks/installer-utils' +import { wrapWindowsBatchHookCommand } from '../agent-hooks/windows-direct-cmd-hook-command' import { KiroHookService } from './hook-service' import { getKiroManagedCommandMatcher, KIRO_HOOK_EVENTS } from './hook-settings' @@ -234,6 +235,25 @@ describe('KiroHookService', () => { expect(readAgent('kept').hooks).toBeDefined() }) + // Why: runs on every OS; windows-hook-payload-delivery.test.ts runs each form under Kiro's shells. + it('registers the Windows batch hook command for every event', () => { + const platform = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + try { + writeAgent('mine', { name: 'mine' }) + expect(new KiroHookService().install().state).toBe('installed') + const hooks = readAgent('mine').hooks + if (!isPlainObject(hooks)) { + throw new Error('Expected hooks object') + } + const command = wrapWindowsBatchHookCommand(scriptPath()) + for (const event of KIRO_HOOK_EVENTS) { + expect(hooks[event]).toEqual([expect.objectContaining({ command })]) + } + } finally { + platform.mockRestore() + } + }) + it('ignores non-JSON files such as the shipped example config', () => { mkdirSync(agentsDir(), { recursive: true }) writeFileSync(join(agentsDir(), 'agent_config.json.example'), '{}') diff --git a/src/main/kiro/hook-settings.ts b/src/main/kiro/hook-settings.ts index 97245f12e16..da2e8906bd9 100644 --- a/src/main/kiro/hook-settings.ts +++ b/src/main/kiro/hook-settings.ts @@ -8,9 +8,9 @@ import { MANAGED_HOOK_TIMEOUT_MILLISECONDS, removeManagedCommands, wrapPosixHookCommand, - wrapWindowsHookCommand, type HookDefinition } from '../agent-hooks/installer-utils' +import { wrapWindowsBatchHookCommand } from '../agent-hooks/windows-direct-cmd-hook-command' const KIRO_SCRIPT_BASE = 'kiro-hook' @@ -68,7 +68,7 @@ export function getKiroManagedScriptPath(): string { export function getKiroManagedCommand(scriptPath: string): string { // Why: Kiro runs hook commands through PowerShell on Windows and sh elsewhere. return process.platform === 'win32' - ? wrapWindowsHookCommand(scriptPath) + ? wrapWindowsBatchHookCommand(scriptPath) : wrapPosixHookCommand(scriptPath) } diff --git a/src/main/muse/hook-settings.test.ts b/src/main/muse/hook-settings.test.ts index 157a5ba28d9..40066423462 100644 --- a/src/main/muse/hook-settings.test.ts +++ b/src/main/muse/hook-settings.test.ts @@ -1,5 +1,8 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -import { wrapWindowsHookCommand } from '../agent-hooks/windows-hook-command' +import { + isSafeUnicodeWindowsBatchHookPath, + wrapWindowsHookCommand +} from '../agent-hooks/windows-hook-command' import { wrapPosixHookCommand } from '../agent-hooks/posix-hook-command' import { getMuseManagedCommand, getMuseRemoteManagedCommand } from './hook-settings' @@ -22,7 +25,9 @@ describe('Muse hook commands', () => { '\\\\server\\share\\.orca\\agent-hooks\\muse-hook.cmd' ])('retains the established quoting fallback for %s', (path) => { vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') - expect(getMuseManagedCommand(path)).toBe(wrapWindowsHookCommand(path)) + // Guarded Unicode .cmd paths take the cheaper policy switch (#26382). + const policy = { useProcessPolicyEnvironment: isSafeUnicodeWindowsBatchHookPath(path) } + expect(getMuseManagedCommand(path)).toBe(wrapWindowsHookCommand(path, {}, policy)) }) it.each(['darwin', 'linux'] as const)('keeps local %s commands POSIX', (platform) => { diff --git a/src/main/muse/hook-settings.ts b/src/main/muse/hook-settings.ts index 98d208bd935..392b5f048ff 100644 --- a/src/main/muse/hook-settings.ts +++ b/src/main/muse/hook-settings.ts @@ -6,11 +6,10 @@ import { createManagedCommandMatcher, getSharedManagedScriptPath, wrapPosixHookCommand, - wrapWindowsHookCommand, type HookDefinition } from '../agent-hooks/installer-utils' import { readManagedHookEventsFromJson } from '../agent-hooks/managed-hooks-json-events' -import { wrapWindowsDirectCmdHookCommand } from '../agent-hooks/windows-direct-cmd-hook-command' +import { wrapWindowsBatchHookCommand } from '../agent-hooks/windows-direct-cmd-hook-command' import { refreshManagedScriptIfPresent } from '../agent-hooks/managed-hook-script-refresh' import { parseMuseSettingsText } from './hook-config-json' @@ -68,7 +67,7 @@ export function getMuseRemoteManagedHooksPath(remoteHome: string): string { export function getMuseManagedCommand(scriptPath: string): string { return process.platform === 'win32' - ? (wrapWindowsDirectCmdHookCommand(scriptPath) ?? wrapWindowsHookCommand(scriptPath)) + ? wrapWindowsBatchHookCommand(scriptPath) : wrapPosixHookCommand(scriptPath) }