From 1fde7f553c6fc6a56edd538ba85583ac7699c960 Mon Sep 17 00:00:00 2001 From: Jorge Silva Date: Tue, 23 Jun 2026 00:04:01 +0100 Subject: [PATCH] fix(agent-hooks): wrap Windows hook commands in cmd.exe to survive spaces in profile path (#6078) (#6083) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(codex): wrap Windows hook command in cmd.exe to survive spaces in profile path (#6078) Windows splits raw hook commands on whitespace, so a user profile path like `C:\Users\Jane Doe` made Codex hooks exit with code 1. Add a wrapWindowsHookCommand helper that invokes the .cmd through `cmd.exe /d /c call "..."` and use it in getManagedCommand. * fix(agent-hooks): wrap Windows hook command in cmd.exe for all agents with raw .cmd path (#6078) Apply the wrapWindowsHookCommand helper to cursor, command-code, gemini, grok, and droid, which shared the same raw-scriptPath-on-Windows pattern as codex. A user profile path with a space (e.g. `C:\Users\Jane Doe`) used to split at the space and fail with exit code 1. Agents that already handle spaces correctly are left untouched: - claude/openclaude (Git Bash + forward slashes) - copilot (PowerShell with quoted path) - kimi (Git Bash + forward slashes) - antigravity (event-specific wrapper .cmd files) - devin (already wraps via `cmd /d /s /c ""...""`) Each fixed agent gets a Windows-only test asserting the cmd.exe wrapping survives spaces in the profile path. * fix(claude): wrap Windows hook command in cmd.exe to survive spaces in profile path (#6078) Claude Code runs hooks through Git Bash on Windows. The previous forward-slash trick only works when the path has no spaces — Git Bash splits `C:/Users/Jane Doe/...` at the space and tries to execute `C:/Users/Jane` as a command. Use wrapWindowsHookCommand so the .cmd is invoked through `cmd.exe /d /c call "..."`, which Git Bash treats as one argument. Applies to both Claude and OpenClaude (shared getManagedCommand). * Harden Windows agent hook launcher --------- Co-authored-by: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> --- src/main/agent-hooks/installer-utils.test.ts | 57 ++++++++++++++++++ src/main/agent-hooks/installer-utils.ts | 31 +++++++++- src/main/claude/hook-service.test.ts | 51 +++++++++++++++- src/main/claude/hook-settings.ts | 9 ++- src/main/codex/hook-service.test.ts | 62 +++++++++++++++++--- src/main/codex/hook-service.ts | 5 +- src/main/command-code/hook-service.test.ts | 41 +++++++++++-- src/main/command-code/hook-service.ts | 5 +- src/main/cursor/hook-service.test.ts | 45 +++++++++++++- src/main/cursor/hook-service.ts | 5 +- src/main/droid/hook-service.test.ts | 39 +++++++++++- src/main/droid/hook-service.ts | 10 +++- src/main/gemini/hook-service.test.ts | 48 +++++++++++++-- src/main/gemini/hook-service.ts | 5 +- src/main/grok/hook-service.test.ts | 47 ++++++++++++++- src/main/grok/hook-service.ts | 5 +- 16 files changed, 423 insertions(+), 42 deletions(-) diff --git a/src/main/agent-hooks/installer-utils.test.ts b/src/main/agent-hooks/installer-utils.test.ts index f7ca2d3ea40..43c08542c60 100644 --- a/src/main/agent-hooks/installer-utils.test.ts +++ b/src/main/agent-hooks/installer-utils.test.ts @@ -20,6 +20,7 @@ import { hookDefinitionHasManagedCommand, removeManagedCommands, wrapPosixHookCommand, + wrapWindowsHookCommand, writeManagedScript, writeHooksJson, type HooksConfig @@ -167,6 +168,11 @@ describe('createManagedCommandMatcher', () => { ).toBe(true) }) + it('matches encoded Windows launcher commands by decoding their script path', () => { + const command = wrapWindowsHookCommand('C:\\Users\\alice\\.orca\\agent-hooks\\claude-hook.cmd') + expect(match(command)).toBe(true) + }) + it('matches the legacy per-userData script path AND the new shared ~/.orca path', () => { // Why: install() must sweep old per-userData commands when migrating to // the shared ~/.orca script path, or stale launchers keep failing. @@ -332,6 +338,57 @@ describe('wrapPosixHookCommand', () => { ) }) +describe('wrapWindowsHookCommand', () => { + function decodeWindowsHookCommand(command: string): string { + const encodedCommand = command.match(/ -EncodedCommand (\S+)$/)?.[1] + expect(encodedCommand).toBeTruthy() + return Buffer.from(encodedCommand!, 'base64').toString('utf16le') + } + + it('invokes the .cmd through an encoded PowerShell command', () => { + const command = wrapWindowsHookCommand('C:\\Users\\alice\\.orca\\agent-hooks\\codex-hook.cmd') + expect(command).toMatch(/^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/) + expect(decodeWindowsHookCommand(command)).toBe( + "& 'C:\\Users\\alice\\.orca\\agent-hooks\\codex-hook.cmd'; exit $LASTEXITCODE" + ) + }) + + // Why: a user profile path like `C:\Users\Jane Doe` is the regression from + // #6078 — the raw path used to be split at the space. The wrapper must keep + // the whole path inside the encoded command so shells do not split it. + it('preserves spaces in the script path (user profile with space case)', () => { + const cmd = wrapWindowsHookCommand('C:\\Users\\Jorge Silva\\.orca\\agent-hooks\\codex-hook.cmd') + expect(decodeWindowsHookCommand(cmd)).toBe( + "& 'C:\\Users\\Jorge Silva\\.orca\\agent-hooks\\codex-hook.cmd'; exit $LASTEXITCODE" + ) + }) + + it('keeps cmd.exe percent expansion and caret escapes out of the command line', () => { + const cmd = wrapWindowsHookCommand('C:\\Users\\%ORCA_TEST%\\a^b\\codex-hook.cmd') + expect(cmd).not.toContain('%ORCA_TEST%') + expect(cmd).not.toContain('^') + expect(decodeWindowsHookCommand(cmd)).toBe( + "& 'C:\\Users\\%ORCA_TEST%\\a^b\\codex-hook.cmd'; exit $LASTEXITCODE" + ) + }) + + it.skipIf(process.platform !== 'win32')( + 'executes a script path containing a cmd.exe caret literally', + () => { + const scriptDir = join(tmpDir, 'home with ^ caret', '.orca', 'agent-hooks') + mkdirSync(scriptDir, { recursive: true }) + const scriptPath = join(scriptDir, 'codex-hook.cmd') + writeFileSync(scriptPath, '@echo off\r\nexit /b 7\r\n', 'utf-8') + + const result = spawnSync('cmd.exe', ['/d', '/c', wrapWindowsHookCommand(scriptPath)], { + env: { ...process.env, ORCA_WRAP_TEST: 'expanded' } + }) + + expect(result.status).toBe(7) + } + ) +}) + describe('buildWindowsAgentHookPostCommand', () => { it('forces UTF-8 for redirected hook stdin and POST bodies', () => { const command = buildWindowsAgentHookPostCommand('codex') diff --git a/src/main/agent-hooks/installer-utils.ts b/src/main/agent-hooks/installer-utils.ts index 07f8e834d9d..a533fb0ed1a 100644 --- a/src/main/agent-hooks/installer-utils.ts +++ b/src/main/agent-hooks/installer-utils.ts @@ -75,11 +75,25 @@ export function createManagedCommandMatcher( if (!command) { return false } - const normalizedCommand = command.replaceAll('\\', '/') + const decodedCommand = decodePowerShellEncodedCommand(command) + const searchText = decodedCommand ? `${command}\n${decodedCommand}` : command + const normalizedCommand = searchText.replaceAll('\\', '/') return needles.some((needle) => normalizedCommand.includes(needle)) } } +function decodePowerShellEncodedCommand(command: string): string | null { + const match = command.match(/\s-EncodedCommand\s+(\S+)/i) + if (!match) { + return null + } + try { + return Buffer.from(match[1], 'base64').toString('utf16le') + } catch { + return null + } +} + // Why: prod, dev, and parallel Orca instances must write the same managed // settings entry instead of racing between per-userData script paths. export function getSharedManagedScriptPath(scriptFileName: string): string { @@ -106,6 +120,21 @@ export function wrapPosixHookCommand(scriptPath: string, env: Record { ) expect(legacyCommands).toContain('/usr/local/bin/user-hook') expect( - legacyCommands.some((command: string) => command.includes(CLAUDE_SCRIPT_FILE_NAME)) + legacyCommands.some((command: string) => + process.platform === 'win32' + ? command.startsWith('powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand ') + : command.includes(CLAUDE_SCRIPT_FILE_NAME) + ) ).toBe(true) expect( legacyCommands.some((command: string) => command.includes('/Users/old/.orca/agent-hooks/claude-hook.sh') ) ).toBe(false) - expect(legacy.hooks.StopFailure[0].hooks[0].command).toContain(CLAUDE_SCRIPT_FILE_NAME) + expect(legacy.hooks.StopFailure[0].hooks[0].command).toMatch( + process.platform === 'win32' + ? /^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + : new RegExp(CLAUDE_SCRIPT_FILE_NAME) + ) expect( readFileSync(join(tmpHome, '.orca', 'agent-hooks', CLAUDE_SCRIPT_FILE_NAME), 'utf-8') ).toContain('DEVIN_PROJECT_DIR') @@ -177,6 +185,36 @@ describe('ClaudeHookService.install', () => { rmSync(tmpHome, { recursive: true, force: true }) } }) + + // Why: #6078 — Claude Code runs hooks through Git Bash, and an unquoted path + // with a space (e.g. `C:/Users/Jane Doe`) splits at the space. The managed + // command must use an encoded launcher so Git Bash/cmd.exe never splits or + // expands the raw path before invoking the managed .cmd. + it.skipIf(process.platform !== 'win32')( + 'wraps the managed hook command to survive spaces in the profile path (#6078)', + () => { + const tmpHome = mkdtempSync(join(tmpdir(), 'orca claude home with spaces ')) + vi.stubEnv('HOME', tmpHome) + vi.stubEnv('USERPROFILE', tmpHome) + try { + expect(new ClaudeHookService().install().state).toBe('installed') + + const settings = JSON.parse( + readFileSync(join(tmpHome, '.claude', 'settings.json'), 'utf-8') + ) as { hooks: Record } + + for (const eventName of ['UserPromptSubmit', 'Stop', 'StopFailure']) { + const command = settings.hooks[eventName]?.[0]?.hooks?.[0]?.command + expect(command).toMatch( + /^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + ) + } + } finally { + vi.unstubAllEnvs() + rmSync(tmpHome, { recursive: true, force: true }) + } + } + ) }) describe('ClaudeHookService.installRemote', () => { @@ -287,7 +325,14 @@ describe('OpenClaudeHookService-compatible install', () => { const parsed = JSON.parse(readFileSync(openClaudeSettings, 'utf-8')) for (const event of ['UserPromptSubmit', 'Stop', 'StopFailure']) { const command = parsed.hooks[event][0].hooks[0].command as string - expect(command).toContain(OPENCLAUDE_SCRIPT_FILE_NAME) + if (process.platform === 'win32') { + expect(command).toMatch( + /^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + ) + } else { + expect(command).toContain(OPENCLAUDE_SCRIPT_FILE_NAME) + expect(command).toMatch(/^if \[ -x /) + } } expect( readFileSync(join(tmpHome, '.orca', 'agent-hooks', OPENCLAUDE_SCRIPT_FILE_NAME), 'utf-8') diff --git a/src/main/claude/hook-settings.ts b/src/main/claude/hook-settings.ts index f6a7a2a0f73..fc94d6e2c6a 100644 --- a/src/main/claude/hook-settings.ts +++ b/src/main/claude/hook-settings.ts @@ -5,6 +5,7 @@ import { getSharedManagedScriptPath, removeManagedCommands, wrapPosixHookCommand, + wrapWindowsHookCommand, type HookDefinition, type HooksConfig } from '../agent-hooks/installer-utils' @@ -74,9 +75,11 @@ export function getRemoteConfigPath(remoteHome: string, settings = CLAUDE_HOOK_S export function getManagedCommand(scriptPath: string): string { if (process.platform === 'win32') { - // Why: Claude Code runs hooks through Git Bash on Windows; forward slashes - // survive that shell layer while native Windows APIs still accept them. - return scriptPath.replaceAll('\\', '/') + // Why: Claude Code runs hooks through Git Bash on Windows. Forward slashes + // alone don't survive a path with spaces — bash splits at the space and + // tries to execute `C:/Users/Jorge` as a command. Wrapping in + // `cmd.exe /d /c call "..."` keeps the path as one argument. #6078. + return wrapWindowsHookCommand(scriptPath) } return wrapPosixHookCommand(scriptPath) } diff --git a/src/main/codex/hook-service.test.ts b/src/main/codex/hook-service.test.ts index dc0dc060a62..94f365a213a 100644 --- a/src/main/codex/hook-service.test.ts +++ b/src/main/codex/hook-service.test.ts @@ -13,7 +13,7 @@ import { import { tmpdir } from 'os' import type * as Os from 'os' import { join } from 'path' -import { wrapPosixHookCommand } from '../agent-hooks/installer-utils' +import { createManagedCommandMatcher, wrapPosixHookCommand } from '../agent-hooks/installer-utils' import { upsertHookTrustEntriesInContent } from './config-toml-trust' const { getPathMock, homedirMock } = vi.hoisted(() => ({ @@ -39,6 +39,11 @@ import { CodexHookService } from './hook-service' let tmpHome: string let userDataDir: string + +function isCodexManagedCommand(command: string | undefined): boolean { + const scriptFileName = process.platform === 'win32' ? 'codex-hook.cmd' : 'codex-hook.sh' + return createManagedCommandMatcher(scriptFileName)(command) +} let previousUserDataPath: string | undefined beforeEach(() => { @@ -118,8 +123,9 @@ describe('CodexHookService', () => { 'UserPromptSubmit' ].sort() ) - expect(hooksConfig.hooks.PermissionRequest?.[0]?.hooks?.[0]?.command).toContain('agent-hooks') - expect(hooksConfig.hooks.PermissionRequest?.[0]?.hooks?.[0]?.command).toContain('codex-hook') + expect( + isCodexManagedCommand(hooksConfig.hooks.PermissionRequest?.[0]?.hooks?.[0]?.command) + ).toBe(true) const trustConfig = readFileSync(join(managedCodexHome, 'config.toml'), 'utf-8') expect(trustConfig).toContain('model = "gpt-5.2-codex"') @@ -127,6 +133,40 @@ describe('CodexHookService', () => { expect(trustConfig).toContain(':permission_request:0:0') }) + // Why: #6078 — a Windows user profile path like `C:\Users\Jane Doe` used to + // be written verbatim as the hook command, so Codex split it at the space and + // the hook exited with code 1. The managed command uses 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 in an encoded launcher when the profile path contains a space (#6078)', + () => { + const spaceHome = join(tmpdir(), 'orca home with spaces') + mkdirSync(spaceHome, { recursive: true }) + homedirMock.mockReturnValue(spaceHome) + try { + const systemCodexHome = join(spaceHome, '.codex') + mkdirSync(systemCodexHome, { recursive: true }) + + const status = new CodexHookService().install() + expect(status.state).toBe('installed') + + const managedCodexHome = join(userDataDir, 'codex-runtime-home', 'home') + const hooksConfig = JSON.parse( + readFileSync(join(managedCodexHome, 'hooks.json'), 'utf-8') + ) as { hooks: Record } + + for (const eventName of ['SessionStart', 'UserPromptSubmit', 'Stop']) { + const command = hooksConfig.hooks[eventName]?.[0]?.hooks?.[0]?.command + expect(command).toMatch( + /^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + ) + } + } finally { + rmSync(spaceHome, { recursive: true, force: true }) + } + } + ) + it('keeps hooks isolated by Orca userData instead of mutating system ~/.codex', () => { const systemCodexHome = join(tmpHome, '.codex') const systemHooksPath = join(systemCodexHome, 'hooks.json') @@ -177,12 +217,12 @@ describe('CodexHookService', () => { ).toBe(true) expect( devHooks.hooks.Stop?.some((definition) => - definition.hooks?.[0]?.command?.includes('codex-hook') + isCodexManagedCommand(definition.hooks?.[0]?.command) ) ).toBe(true) expect( prodHooks.hooks.Stop?.some((definition) => - definition.hooks?.[0]?.command?.includes('codex-hook') + isCodexManagedCommand(definition.hooks?.[0]?.command) ) ).toBe(true) expect(readFileSync(systemHooksPath, 'utf-8')).toBe(existingSystemHooks) @@ -296,7 +336,9 @@ describe('CodexHookService', () => { hooks: Record } - expect(runtimeHooks.hooks.PostToolUse?.[0]?.hooks?.[0]?.command).toContain('codex-hook') + expect(isCodexManagedCommand(runtimeHooks.hooks.PostToolUse?.[0]?.hooks?.[0]?.command)).toBe( + true + ) expect(runtimeHooks.hooks.PostToolUse?.[1]?.hooks?.[0]?.command).toBe( 'slow-user-post-tool-hook' ) @@ -430,7 +472,7 @@ describe('CodexHookService', () => { ) ?? [] expect(stopCommands).toContain(userCommand) - expect(stopCommands.some((command) => command.includes('codex-hook'))).toBe(true) + expect(stopCommands.some((command) => isCodexManagedCommand(command))).toBe(true) expect(runtimeHooks.hooks.PreCompact).toBeUndefined() for (const command of pluginCommands) { expect(runtimeHooksText).not.toContain(command) @@ -953,8 +995,10 @@ describe('CodexHookService', () => { (definition) => definition.hooks?.map((hook) => hook.command ?? '') ?? [] ) ?? [] expect(stopCommands).toContain(userCommand) - expect(stopCommands.some((command) => command.includes('codex-hook'))).toBe(true) - expect(runtimeHooks.hooks.PermissionRequest?.[0]?.hooks?.[0]?.command).toContain('codex-hook') + expect(stopCommands.some((command) => isCodexManagedCommand(command))).toBe(true) + expect( + isCodexManagedCommand(runtimeHooks.hooks.PermissionRequest?.[0]?.hooks?.[0]?.command) + ).toBe(true) const runtimeToml = readFileSync(join(managedCodexHome, 'config.toml'), 'utf-8') expect(runtimeToml).toContain('[features]\nhooks = true') diff --git a/src/main/codex/hook-service.ts b/src/main/codex/hook-service.ts index f53a180a15f..99638a787bc 100644 --- a/src/main/codex/hook-service.ts +++ b/src/main/codex/hook-service.ts @@ -11,6 +11,7 @@ import { readHooksJson, removeManagedCommands, wrapPosixHookCommand, + wrapWindowsHookCommand, writeHooksJson, writeManagedScript, type HookCommandConfig, @@ -111,7 +112,9 @@ function getManagedScriptPath(): string { } function getManagedCommand(scriptPath: string): string { - return process.platform === 'win32' ? scriptPath : wrapPosixHookCommand(scriptPath) + return process.platform === 'win32' + ? wrapWindowsHookCommand(scriptPath) + : wrapPosixHookCommand(scriptPath) } function getSystemConfigPath(): string { diff --git a/src/main/command-code/hook-service.test.ts b/src/main/command-code/hook-service.test.ts index c6b8bf1b8b9..7dd2abcb9a7 100644 --- a/src/main/command-code/hook-service.test.ts +++ b/src/main/command-code/hook-service.test.ts @@ -48,15 +48,46 @@ describe('CommandCodeHookService', () => { expect(config.hooks.PreToolUse[0].matcher).toBe('.*') expect(config.hooks.PostToolUse[0].matcher).toBe('.*') expect(config.hooks.Stop[0].matcher).toBeUndefined() - expect(config.hooks.PreToolUse[0].hooks[0].command).toContain('command-code-hook') - expect(config.hooks.PreToolUse[0].hooks[0].command).toContain(join(homeDir, '.orca')) - if (process.platform === 'win32') { - expect(config.hooks.PreToolUse[0].hooks[0].command).toContain('command-code-hook.cmd') - } else { + expect(config.hooks.PreToolUse[0].hooks[0].command).toMatch( + process.platform === 'win32' + ? /^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + : /command-code-hook/ + ) + if (process.platform !== 'win32') { + expect(config.hooks.PreToolUse[0].hooks[0].command).toContain(join(homeDir, '.orca')) + } + if (process.platform !== 'win32') { expect(config.hooks.PreToolUse[0].hooks[0].command).toMatch(/^if \[ -x /) } }) + // 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( + /^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + ) + } finally { + rmSync(spaceHome, { recursive: true, force: true }) + } + } + ) + 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 435344bd97c..a5a34a30512 100644 --- a/src/main/command-code/hook-service.ts +++ b/src/main/command-code/hook-service.ts @@ -10,6 +10,7 @@ import { readHooksJson, removeManagedCommands, wrapPosixHookCommand, + wrapWindowsHookCommand, writeHooksJson, writeManagedScript, type HookDefinition @@ -45,7 +46,9 @@ function getManagedScriptPath(): string { } function getManagedCommand(scriptPath: string): string { - return process.platform === 'win32' ? scriptPath : wrapPosixHookCommand(scriptPath) + return process.platform === 'win32' + ? wrapWindowsHookCommand(scriptPath) + : wrapPosixHookCommand(scriptPath) } function getManagedScript(target: 'local' | 'posix' = 'local'): string { diff --git a/src/main/cursor/hook-service.test.ts b/src/main/cursor/hook-service.test.ts index 6a1cee03498..4f310d595b8 100644 --- a/src/main/cursor/hook-service.test.ts +++ b/src/main/cursor/hook-service.test.ts @@ -58,8 +58,14 @@ describe('CursorHookService', () => { expect(Object.keys(config.hooks).sort()).toEqual([...CURSOR_EVENTS].sort()) for (const eventName of CURSOR_EVENTS) { const definition = config.hooks[eventName]?.[0] - expect(definition?.command).toContain('cursor-hook') - expect(definition?.command).toContain(join(homeDir, '.orca')) + expect(definition?.command).toMatch( + process.platform === 'win32' + ? /^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + : /cursor-hook/ + ) + if (process.platform !== 'win32') { + expect(definition?.command).toContain(join(homeDir, '.orca')) + } expect(definition?.hooks).toBeUndefined() } @@ -75,6 +81,35 @@ describe('CursorHookService', () => { } }) + // 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 cursor home with spaces') + mkdirSync(spaceHome, { recursive: true }) + homedirMock.mockReturnValue(spaceHome) + try { + expect(new CursorHookService().install().state).toBe('installed') + + const config = JSON.parse( + readFileSync(join(spaceHome, '.cursor', 'hooks.json'), 'utf8') + ) as { hooks: Record } + + for (const eventName of ['beforeSubmitPrompt', 'stop']) { + const command = config.hooks[eventName]?.[0]?.command + expect(command).toMatch( + /^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + ) + } + } finally { + rmSync(spaceHome, { recursive: true, force: true }) + } + } + ) + it('preserves user-authored Cursor hook entries and removes stale managed entries', () => { const configPath = join(homeDir, '.cursor', 'hooks.json') mkdirSync(dirname(configPath), { recursive: true }) @@ -107,7 +142,11 @@ describe('CursorHookService', () => { const promptCommands = config.hooks.beforeSubmitPrompt.map((definition) => definition.command) expect(promptCommands).toContain('/usr/local/bin/user-hook') expect( - promptCommands.filter((command) => command?.includes(CURSOR_SCRIPT_FILE_NAME)) + promptCommands.filter((command) => + process.platform === 'win32' + ? command?.startsWith('powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand ') + : command?.includes(CURSOR_SCRIPT_FILE_NAME) + ) ).toHaveLength(1) expect(config.hooks.retiredEvent.map((definition) => definition.command)).toEqual([ '/usr/local/bin/retired-user-hook' diff --git a/src/main/cursor/hook-service.ts b/src/main/cursor/hook-service.ts index 3b74d2538d3..fb57ce1b8fd 100644 --- a/src/main/cursor/hook-service.ts +++ b/src/main/cursor/hook-service.ts @@ -9,6 +9,7 @@ import { readHooksJson, removeManagedCommands, wrapPosixHookCommand, + wrapWindowsHookCommand, writeHooksJson, writeManagedScript, type HookDefinition @@ -59,7 +60,9 @@ function getManagedScriptPath(): string { } function getManagedCommand(scriptPath: string): string { - return process.platform === 'win32' ? scriptPath : wrapPosixHookCommand(scriptPath) + return process.platform === 'win32' + ? wrapWindowsHookCommand(scriptPath) + : wrapPosixHookCommand(scriptPath) } function getManagedScript(target: 'local' | 'posix' = 'local'): string { diff --git a/src/main/droid/hook-service.test.ts b/src/main/droid/hook-service.test.ts index 5608cc10dd8..65ac1538302 100644 --- a/src/main/droid/hook-service.test.ts +++ b/src/main/droid/hook-service.test.ts @@ -70,11 +70,46 @@ describe('DroidHookService', () => { expect(config.hooks.PreToolUse[0].matcher).toBe('*') expect(config.hooks.PermissionRequest[0].matcher).toBe('*') expect(config.hooks.UserPromptSubmit[0].matcher).toBeUndefined() - expect(config.hooks.PreToolUse[0].hooks[0].command).toContain('droid-hook') - expect(config.hooks.PreToolUse[0].hooks[0].command).toContain(join(homeDir, '.orca')) + expect(config.hooks.PreToolUse[0].hooks[0].command).toMatch( + process.platform === 'win32' + ? /^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + : /droid-hook/ + ) + if (process.platform !== 'win32') { + expect(config.hooks.PreToolUse[0].hooks[0].command).toContain(join(homeDir, '.orca')) + } 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( + /^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + ) + } + } finally { + rmSync(spaceHome, { recursive: true, force: true }) + } + } + ) + it('reports partial when Factory has hooks disabled globally', () => { const configPath = join(homeDir, '.factory', 'settings.json') mkdirSync(dirname(configPath), { recursive: true }) diff --git a/src/main/droid/hook-service.ts b/src/main/droid/hook-service.ts index 9b444d0d11a..f8e07cec3ab 100644 --- a/src/main/droid/hook-service.ts +++ b/src/main/droid/hook-service.ts @@ -8,6 +8,7 @@ import { readHooksJson, removeManagedCommands, wrapPosixHookCommand, + wrapWindowsHookCommand, writeHooksJson, writeManagedScript, type HookDefinition @@ -54,9 +55,12 @@ function getManagedScriptPath(): string { } function getManagedCommand(scriptPath: string): string { - // Why: Factory invokes the .cmd directly via cmd.exe (no bash), so native - // backslashes are correct on Windows. Matches the codex/cursor pattern. - return process.platform === 'win32' ? scriptPath : wrapPosixHookCommand(scriptPath) + // 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. + return process.platform === 'win32' + ? wrapWindowsHookCommand(scriptPath) + : wrapPosixHookCommand(scriptPath) } function getManagedScript(): string { diff --git a/src/main/gemini/hook-service.test.ts b/src/main/gemini/hook-service.test.ts index 7490a1b8434..587762de95a 100644 --- a/src/main/gemini/hook-service.test.ts +++ b/src/main/gemini/hook-service.test.ts @@ -100,12 +100,46 @@ describe('GeminiHookService', () => { expect(config.hooks.PreToolUse).toBeUndefined() expect(config.hooks.BeforeAgent).toHaveLength(2) expect(config.hooks.BeforeAgent[0].hooks[0].command).toBe('echo user-before-agent') - expect(config.hooks.BeforeAgent[1].hooks[0].command).toContain(managedHookPath) - expect(config.hooks.AfterAgent[0].hooks[0].command).toContain(managedHookPath) - expect(config.hooks.AfterTool[0].hooks[0].command).toContain(managedHookPath) - expect(config.hooks.BeforeTool[0].hooks[0].command).toContain(managedHookPath) + const managedCommandPattern = + process.platform === 'win32' + ? /^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + : 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) + expect(config.hooks.AfterTool[0].hooks[0].command).toMatch(managedCommandPattern) + 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( + /^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + ) + } + } finally { + rmSync(spaceHome, { recursive: true, force: true }) + homedirMock.mockReturnValue(homeDir) + } + } + ) + it('preserves user-authored PreToolUse hooks while sweeping stale managed Gemini hooks', () => { const managedHookFileName = process.platform === 'win32' ? 'gemini-hook.cmd' : 'gemini-hook.sh' const staleManagedHookPath = @@ -147,6 +181,10 @@ describe('GeminiHookService', () => { expect(status.state).toBe('installed') expect(preToolCommands).toEqual(['echo user-authored']) - expect(config.hooks.BeforeTool[0].hooks[0].command).toContain(managedHookFileName) + expect(config.hooks.BeforeTool[0].hooks[0].command).toMatch( + process.platform === 'win32' + ? /^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + : new RegExp(managedHookFileName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) + ) }) }) diff --git a/src/main/gemini/hook-service.ts b/src/main/gemini/hook-service.ts index ac687f6c6be..889e7a467f8 100644 --- a/src/main/gemini/hook-service.ts +++ b/src/main/gemini/hook-service.ts @@ -9,6 +9,7 @@ import { readHooksJson, removeManagedCommands, wrapPosixHookCommand, + wrapWindowsHookCommand, writeHooksJson, writeManagedScript, type HookDefinition @@ -43,7 +44,9 @@ function getManagedScriptPath(): string { } function getManagedCommand(scriptPath: string): string { - return process.platform === 'win32' ? scriptPath : wrapPosixHookCommand(scriptPath) + return process.platform === 'win32' + ? wrapWindowsHookCommand(scriptPath) + : wrapPosixHookCommand(scriptPath) } function getManagedScript(target: 'local' | 'posix' = 'local'): string { diff --git a/src/main/grok/hook-service.test.ts b/src/main/grok/hook-service.test.ts index fd5467b66b3..52d46fbcf19 100644 --- a/src/main/grok/hook-service.test.ts +++ b/src/main/grok/hook-service.test.ts @@ -59,8 +59,14 @@ describe('GrokHookService', () => { expect(config.hooks.PreToolUse[0].matcher).toBe('*') expect(config.hooks.PostToolUseFailure[0].matcher).toBe('*') expect(config.hooks.Notification[0].matcher).toBeUndefined() - expect(config.hooks.PreToolUse[0].hooks[0].command).toContain('grok-hook') - expect(config.hooks.PreToolUse[0].hooks[0].command).toContain(join(homeDir, '.orca')) + expect(config.hooks.PreToolUse[0].hooks[0].command).toMatch( + process.platform === 'win32' + ? /^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + : /grok-hook/ + ) + if (process.platform !== 'win32') { + expect(config.hooks.PreToolUse[0].hooks[0].command).toContain(join(homeDir, '.orca')) + } const script = readFileSync( join(homeDir, '.orca', 'agent-hooks', GROK_SCRIPT_FILE_NAME), @@ -74,6 +80,35 @@ describe('GrokHookService', () => { } }) + // 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 grok home with spaces') + mkdirSync(spaceHome, { recursive: true }) + homedirMock.mockReturnValue(spaceHome) + try { + expect(new GrokHookService().install().state).toBe('installed') + + const config = JSON.parse( + readFileSync(join(spaceHome, '.grok', 'hooks', 'orca-status.json'), 'utf8') + ) as { hooks: Record } + + for (const eventName of ['SessionStart', 'UserPromptSubmit', 'Stop']) { + const command = config.hooks[eventName]?.[0]?.hooks?.[0]?.command + expect(command).toMatch( + /^powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + ) + } + } finally { + rmSync(spaceHome, { recursive: true, force: true }) + } + } + ) + it('preserves user-authored hook entries in the Orca Grok config file', () => { const configPath = join(homeDir, '.grok', 'hooks', 'orca-status.json') mkdirSync(dirname(configPath), { recursive: true }) @@ -99,6 +134,12 @@ describe('GrokHookService', () => { definition.hooks.map((hook) => hook.command) ) expect(commands).toContain('/usr/local/bin/user-hook') - expect(commands.some((command) => command.includes(GROK_SCRIPT_FILE_NAME))).toBe(true) + expect( + commands.some((command) => + process.platform === 'win32' + ? command.startsWith('powershell -NoProfile -ExecutionPolicy Bypass -EncodedCommand ') + : command.includes(GROK_SCRIPT_FILE_NAME) + ) + ).toBe(true) }) }) diff --git a/src/main/grok/hook-service.ts b/src/main/grok/hook-service.ts index eefc6dae5fa..e8a81952a59 100644 --- a/src/main/grok/hook-service.ts +++ b/src/main/grok/hook-service.ts @@ -9,6 +9,7 @@ import { readHooksJson, removeManagedCommands, wrapPosixHookCommand, + wrapWindowsHookCommand, writeHooksJson, writeManagedScript, type HookDefinition @@ -55,7 +56,9 @@ function getManagedScriptPath(): string { } function getManagedCommand(scriptPath: string): string { - return process.platform === 'win32' ? scriptPath : wrapPosixHookCommand(scriptPath) + return process.platform === 'win32' + ? wrapWindowsHookCommand(scriptPath) + : wrapPosixHookCommand(scriptPath) } function getManagedScript(target: 'local' | 'posix' = 'local'): string {