From 5bd8a2152b01549b9679f9a0bc3115578ce85778 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 25 Jun 2026 22:17:51 -0700 Subject: [PATCH] Keep spaced Windows Codex hook paths fast --- src/main/codex/hook-service.test.ts | 40 +++++++++++++++++++++++++---- src/main/codex/hook-service.ts | 12 ++++----- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/main/codex/hook-service.test.ts b/src/main/codex/hook-service.test.ts index c0f6c1aac00..b93301d84cd 100644 --- a/src/main/codex/hook-service.test.ts +++ b/src/main/codex/hook-service.test.ts @@ -183,10 +183,10 @@ describe('CodexHookService', () => { // 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. + // the hook exited with code 1. Codex's cmd.exe runner can launch a bare .cmd + // path with spaces directly, avoiding a PowerShell startup for this case. it.skipIf(process.platform !== 'win32')( - 'wraps the managed hook command in an encoded launcher when the profile path contains a space (#6078)', + 'launches the managed .cmd directly when the profile path contains a space (#6078)', () => { const spaceHome = join(tmpdir(), 'orca home with spaces') mkdirSync(spaceHome, { recursive: true }) @@ -203,6 +203,36 @@ describe('CodexHookService', () => { readFileSync(join(managedCodexHome, 'hooks.json'), 'utf-8') ) as { hooks: Record } + for (const eventName of localManagedCodexEvents()) { + const command = hooksConfig.hooks[eventName]?.[0]?.hooks?.[0]?.command + expect(command).toBe(join(spaceHome, '.orca', 'agent-hooks', 'codex-hook.cmd')) + } + } finally { + rmSync(spaceHome, { recursive: true, force: true }) + } + } + ) + + // Why: cmd.exe expands `%` and treats `^` as an escape even inside otherwise + // plausible paths. Keep those rare cases on the encoded launcher from #6078. + it.skipIf(process.platform !== 'win32')( + 'keeps the encoded launcher when the profile path contains cmd metacharacters', + () => { + const metacharHome = join(tmpdir(), 'orca %ORCA_TEST% ^ home') + mkdirSync(metacharHome, { recursive: true }) + homedirMock.mockReturnValue(metacharHome) + try { + const systemCodexHome = join(metacharHome, '.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 localManagedCodexEvents()) { const command = hooksConfig.hooks[eventName]?.[0]?.hooks?.[0]?.command expect(command).toMatch( @@ -210,7 +240,7 @@ describe('CodexHookService', () => { ) } } finally { - rmSync(spaceHome, { recursive: true, force: true }) + rmSync(metacharHome, { recursive: true, force: true }) } } ) @@ -232,7 +262,7 @@ describe('CodexHookService', () => { // Why: the temp home is normally cmd-safe; guard so a runner whose tmpdir // holds an exotic character still asserts the correct (fallback) branch. const command = hooksConfig.hooks.Stop?.[0]?.hooks?.[0]?.command ?? '' - const cmdSafe = /^[A-Za-z0-9_.:\\~-]+$/.test(join(tmpHome, '.orca', 'agent-hooks')) + const cmdSafe = /^[A-Za-z0-9_.:\\~ -]+$/.test(join(tmpHome, '.orca', 'agent-hooks')) if (cmdSafe) { expect(command).not.toMatch(/powershell/i) expect(command).toMatch(/\\agent-hooks\\codex-hook\.cmd$/) diff --git a/src/main/codex/hook-service.ts b/src/main/codex/hook-service.ts index b090bc5278c..fad0a467b2a 100644 --- a/src/main/codex/hook-service.ts +++ b/src/main/codex/hook-service.ts @@ -121,13 +121,11 @@ function getManagedScriptPath(): string { } // Why: a Windows script path is cmd-safe when it holds only characters cmd.exe -// passes through untouched (drive letter, backslash, dot, dash, underscore). -// Codex runs hooks as `cmd.exe /C ` and forwards our string verbatim -// when it has no spaces/quotes, so a bare path then launches with zero shell -// startup. Spaces or cmd metacharacters (`% ^ & ! ( )` etc.) force the encoded -// PowerShell launcher (#6078), which hides the path in base64. `~` is allowed -// because 8.3 short paths (e.g. `RUNNER~1`) are cmd-safe and common. -const WINDOWS_CMD_SAFE_PATH = /^[A-Za-z0-9_.:\\~-]+$/ +// passes through untouched (drive letter, backslash, dot, dash, underscore, and +// spaces). Codex runs hooks as `cmd.exe /C `; a bare `.cmd` path with +// spaces still executes directly, while `%`, `^`, `&`, quotes, and other shell +// metacharacters must fall back to the encoded PowerShell launcher. +const WINDOWS_CMD_SAFE_PATH = /^[A-Za-z0-9_.:\\~ -]+$/ function getManagedCommand(scriptPath: string): string { if (process.platform !== 'win32') {