diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index f42472813..3e0e59381 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -63,6 +63,7 @@ "../src/main/droid/hook-service.ts", "../src/main/gemini/hook-service.ts", "../src/main/grok/hook-service.ts", + "../src/main/grok/windows-grok-hook-script.ts", "../src/main/devin/hook-settings.ts", "../src/main/devin/hook-service.ts", "../src/main/devin/hook-config-json.ts", diff --git a/src/main/agent-hooks/installer-utils.ts b/src/main/agent-hooks/installer-utils.ts index e0688b443..a6d5d61fd 100644 --- a/src/main/agent-hooks/installer-utils.ts +++ b/src/main/agent-hooks/installer-utils.ts @@ -161,7 +161,14 @@ export function wrapWindowsGitBashHookCommand(scriptPath: string): string { : wrapWindowsHookCommand(scriptPath) } -export function buildWindowsAgentHookPostCommand(source: AgentHookSource): string { +/** + * Extra form lines inserted before the final `payload@-` line (each should end with ` ^`). + * Used by Grok to attach `grokHome` without fragile string replace on the shared template. + */ +export function buildWindowsAgentHookPostCommand( + source: AgentHookSource, + extraFormLines: readonly string[] = [] +): string { // Why: PowerShell startup makes inline per-turn Codex hooks visibly slow, so mirror the POSIX curl path. // Why: fully-qualify curl so a repo-local curl.exe can't hijack hook payloads. return [ @@ -175,6 +182,7 @@ export function buildWindowsAgentHookPostCommand(source: AgentHookSource): strin ' --data-urlencode "worktreeId=%ORCA_WORKTREE_ID%" ^', ' --data-urlencode "env=%ORCA_AGENT_HOOK_ENV%" ^', ' --data-urlencode "version=%ORCA_AGENT_HOOK_VERSION%" ^', + ...extraFormLines, ' --data-urlencode "payload@-" >nul 2>nul' ].join('\r\n') } diff --git a/src/main/grok/hook-service.test.ts b/src/main/grok/hook-service.test.ts index 028107263..069b3b954 100644 --- a/src/main/grok/hook-service.test.ts +++ b/src/main/grok/hook-service.test.ts @@ -1,5 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { spawn } from 'node:child_process' import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { createServer } from 'node:http' import { tmpdir } from 'node:os' import { dirname, join } from 'node:path' @@ -16,12 +18,86 @@ vi.mock('os', async () => { }) import { getGrokToolEventMatcherForTests, GrokHookService } from './hook-service' +import { buildWindowsGrokHookScript } from './windows-grok-hook-script' import { POSIX_HOOK_STDIN_READER } from '../agent-hooks/hook-stdin-contract' const GROK_SCRIPT_FILE_NAME = process.platform === 'win32' ? 'grok-hook.cmd' : 'grok-hook.sh' const WINDOWS_POWERSHELL_LAUNCHER = /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ +type WindowsGrokHookRun = { + status: number | null + stderr: string + stdout: string + request?: { path: string; body: string } +} + +function createWindowsGrokHookEnvironment(grokHome?: string): NodeJS.ProcessEnv { + const env = { ...process.env } as NodeJS.ProcessEnv + delete env.ORCA_AGENT_HOOK_ENDPOINT + if (grokHome === undefined) { + delete env.GROK_HOME + } else { + env.GROK_HOME = grokHome + } + env.ORCA_AGENT_HOOK_TOKEN = 'test-token' + env.ORCA_PANE_KEY = 'pane-test' + return env +} + +async function runWindowsGrokHook( + scriptPath: string, + env: NodeJS.ProcessEnv, + input: string +): Promise { + let request: WindowsGrokHookRun['request'] + const server = createServer((incoming, response) => { + let body = '' + incoming.setEncoding('utf8') + incoming.on('data', (chunk: string) => { + body += chunk + }) + incoming.on('end', () => { + request = { path: incoming.url ?? '', body } + response.writeHead(204).end() + }) + }) + await new Promise((resolve, reject) => { + server.once('error', reject) + server.listen(0, '127.0.0.1', () => { + server.off('error', reject) + resolve() + }) + }) + const address = server.address() + if (!address || typeof address === 'string') { + throw new Error('Could not resolve Windows Grok hook test listener port') + } + env.ORCA_AGENT_HOOK_PORT = String(address.port) + try { + const result = await new Promise>((resolve, reject) => { + const child = spawn(process.env.ComSpec ?? 'cmd.exe', ['/d', '/c', scriptPath], { + env, + stdio: ['pipe', 'pipe', 'pipe'] + }) + let stderr = '' + let stdout = '' + child.stderr.setEncoding('utf8').on('data', (chunk: string) => { + stderr += chunk + }) + child.stdout.setEncoding('utf8').on('data', (chunk: string) => { + stdout += chunk + }) + child.once('error', reject) + child.once('close', (status) => resolve({ status, stderr, stdout })) + child.stdin.end(input) + }) + return { ...result, request } + } finally { + await new Promise((resolve) => server.close(() => resolve())) + } +} + describe('GrokHookService', () => { let homeDir: string @@ -35,6 +111,108 @@ describe('GrokHookService', () => { rmSync(homeDir, { recursive: true, force: true }) }) + // Why: #9358 / #9941 — empty GROK_HOME + parse-time %VAR:~n,m% / `"\"` broke + // every SessionStart/UserPromptSubmit on Windows outside Orca terminals. + it('guards Windows GROK_HOME substring checks when empty (#9358)', () => { + const script = buildWindowsGrokHookScript() + expect(script).toContain('set "ORCA_GROK_HOME="') + expect(script).toContain('if not defined GROK_HOME goto :orca_grok_home_ready') + expect(script).toContain('%GROK_HOME:~4096,1%') + expect(script).toContain('set "ORCA_GROK_HOME=%GROK_HOME%"') + expect(script).toContain('%ORCA_GROK_HOME:~4096,1%') + expect(script).toContain(':orca_grok_home_ready') + expect(script).toContain('if not defined ORCA_GROK_HOME goto :orca_grok_home_ready') + expect(script).toContain('if "%ORCA_GROK_HOME:~-1%"=="\\"') + expect(script).toContain('if not "%GROK_HOME:~4096,1%"=="" goto :orca_grok_home_ready') + // Why: parenthesized `if defined (...)` still parse-expands the body early. + expect(script).not.toMatch(/if defined GROK_HOME \(/) + }) + + it.skipIf(process.platform !== 'win32')( + 'generated grok-hook.cmd exits 0 when GROK_HOME is unset (#9358)', + async () => { + const scriptPath = join(homeDir, 'grok-hook-unset.cmd') + writeFileSync(scriptPath, buildWindowsGrokHookScript(), 'utf8') + // Why: delete GROK_HOME rather than set '' so cmd sees "not defined". + const result = await runWindowsGrokHook( + scriptPath, + createWindowsGrokHookEnvironment(), + '{"hook_event_name":"SessionStart"}' + ) + expect(result.status, `stderr=${result.stderr}\nstdout=${result.stdout}`).toBe(0) + expect(`${result.stderr ?? ''}${result.stdout ?? ''}`).not.toMatch( + /syntax of the command is incorrect|命令语法不正确/i + ) + expect(result.request?.path).toBe('/hook/grok') + const form = new URLSearchParams(result.request?.body) + expect(form.get('grokHome')).toBe('') + expect(form.get('payload')).toBe('{"hook_event_name":"SessionStart"}') + } + ) + + it.skipIf(process.platform !== 'win32')( + 'generated grok-hook.cmd exits 0 with trailing-backslash GROK_HOME (#9358)', + async () => { + const scriptPath = join(homeDir, 'grok-hook-slash.cmd') + writeFileSync(scriptPath, buildWindowsGrokHookScript(), 'utf8') + const trailing = `${join(homeDir, 'grok-home-with-slash')}\\` + const result = await runWindowsGrokHook( + scriptPath, + createWindowsGrokHookEnvironment(trailing), + '{"hook_event_name":"UserPromptSubmit"}' + ) + expect(result.status, `stderr=${result.stderr}\nstdout=${result.stdout}`).toBe(0) + expect(`${result.stderr ?? ''}${result.stdout ?? ''}`).not.toMatch( + /syntax of the command is incorrect|命令语法不正确/i + ) + expect(result.request?.path).toBe('/hook/grok') + const form = new URLSearchParams(result.request?.body) + expect(form.get('grokHome')).toBe(`${trailing}.`) + expect(form.get('payload')).toBe('{"hook_event_name":"UserPromptSubmit"}') + } + ) + + it.skipIf(process.platform !== 'win32')( + 'generated grok-hook.cmd exits 0 with an oversized GROK_HOME (#9358)', + async () => { + const scriptPath = join(homeDir, 'grok-hook-oversized.cmd') + writeFileSync(scriptPath, buildWindowsGrokHookScript(), 'utf8') + const result = await runWindowsGrokHook( + scriptPath, + createWindowsGrokHookEnvironment(`C:\\${'a'.repeat(9000)}`), + '{"hook_event_name":"UserPromptSubmit"}' + ) + expect(result.status, `stderr=${result.stderr}\nstdout=${result.stdout}`).toBe(0) + expect(`${result.stderr ?? ''}${result.stdout ?? ''}`).not.toMatch( + /syntax of the command is incorrect|命令语法不正确/i + ) + expect(result.request?.path).toBe('/hook/grok') + const form = new URLSearchParams(result.request?.body) + expect(form.get('grokHome')).toBe('') + expect(form.get('payload')).toBe('{"hook_event_name":"UserPromptSubmit"}') + } + ) + + it.skipIf(process.platform !== 'win32')( + 'omits a max-length trailing-backslash GROK_HOME after safe normalization (#9358)', + async () => { + const scriptPath = join(homeDir, 'grok-hook-max-trailing.cmd') + writeFileSync(scriptPath, buildWindowsGrokHookScript(), 'utf8') + const trailingAtLimit = `C:\\${'a'.repeat(4092)}\\` + expect(trailingAtLimit).toHaveLength(4096) + const result = await runWindowsGrokHook( + scriptPath, + createWindowsGrokHookEnvironment(trailingAtLimit), + '{"hook_event_name":"UserPromptSubmit"}' + ) + expect(result.status, `stderr=${result.stderr}\nstdout=${result.stdout}`).toBe(0) + expect(result.request?.path).toBe('/hook/grok') + const form = new URLSearchParams(result.request?.body) + expect(form.get('grokHome')).toBe('') + expect(form.get('payload')).toBe('{"hook_event_name":"UserPromptSubmit"}') + } + ) + it('installs a dedicated global Grok hook config and managed script', () => { const status = new GrokHookService().install() @@ -89,11 +267,12 @@ describe('GrokHookService', () => { expect(script).toContain('/hook/grok') if (process.platform === 'win32') { expect(script).toContain('%SystemRoot%\\System32\\curl.exe') - expect(script).toContain('set "ORCA_GROK_HOME=%GROK_HOME%"') + expect(script).toContain('if not defined GROK_HOME goto :orca_grok_home_ready') expect(script).toContain('%GROK_HOME:~4096,1%') - expect(script).toContain( - 'if "%ORCA_GROK_HOME:~-1%"=="\\" set "ORCA_GROK_HOME=%ORCA_GROK_HOME%."' - ) + expect(script).toContain('set "ORCA_GROK_HOME=%GROK_HOME%"') + expect(script).toContain('%ORCA_GROK_HOME:~4096,1%') + expect(script).toContain('if not defined ORCA_GROK_HOME goto :orca_grok_home_ready') + expect(script).toContain('if "%ORCA_GROK_HOME:~-1%"=="\\"') expect(script).toContain('--data-urlencode "grokHome=%ORCA_GROK_HOME%"') } else { // Why: payload is piped to curl via stdin (`payload@-`) so it never lands diff --git a/src/main/grok/hook-service.ts b/src/main/grok/hook-service.ts index 564565d0d..51a6150ea 100644 --- a/src/main/grok/hook-service.ts +++ b/src/main/grok/hook-service.ts @@ -5,7 +5,6 @@ import { resolveGrokHomeDir } from '../../shared/grok-session-paths' import { buildManagedCommandHook, createManagedCommandMatcher, - buildWindowsAgentHookPostCommand, getSharedManagedScriptPath, readHooksJson, removeManagedCommands, @@ -20,19 +19,17 @@ import { writeHooksJsonRemote, writeManagedScriptRemote } from '../agent-hooks/installer-utils-remote' +import { buildPosixHookPayloadCapture } from '../agent-hooks/hook-stdin-contract' import { - buildPosixHookPayloadCapture, - buildWindowsHookEnvironmentGuardLines, - buildWindowsHookStdinDrainEpilogue -} from '../agent-hooks/hook-stdin-contract' + buildWindowsGrokHookScript, + GROK_HOME_ENVELOPE_MAX_LENGTH +} from './windows-grok-hook-script' // Why: Grok's tool-event matcher is a real regex (see Grok hooks docs). Bare // `*` is not a valid "match all" pattern and can fail to load/match, so tool // lifecycle hooks never fire. `.*` matches every tool name (same as Command // Code's managed hooks). const GROK_TOOL_EVENT_MATCHER = '.*' -const GROK_HOME_ENVELOPE_MAX_LENGTH = 4096 -const WINDOWS_HOOK_PAYLOAD_FORM_LINE = ' --data-urlencode "payload@-" >nul 2>nul' const GROK_EVENTS = [ { eventName: 'SessionStart', definition: { hooks: [{ type: 'command', command: '' }] } }, @@ -95,11 +92,6 @@ function hasControlCharacter(value: string): boolean { }) } -const WINDOWS_GROK_HOOK_POST_COMMAND = buildWindowsAgentHookPostCommand('grok').replace( - WINDOWS_HOOK_PAYLOAD_FORM_LINE, - ` --data-urlencode "grokHome=%ORCA_GROK_HOME%" ^\r\n${WINDOWS_HOOK_PAYLOAD_FORM_LINE}` -) - function getManagedScriptFileName(): string { return process.platform === 'win32' ? 'grok-hook.cmd' : 'grok-hook.sh' } @@ -116,21 +108,7 @@ function getManagedCommand(scriptPath: string): string { function getManagedScript(target: 'local' | 'posix' = 'local'): string { if (target === 'local' && process.platform === 'win32') { - return [ - '@echo off', - 'setlocal', - 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', - ...buildWindowsHookEnvironmentGuardLines(), - 'set "ORCA_GROK_HOME=%GROK_HOME%"', - `if not "%GROK_HOME:~${GROK_HOME_ENVELOPE_MAX_LENGTH},1%"=="" set "ORCA_GROK_HOME="`, - // Why: a trailing backslash escapes curl's closing argv quote on Windows, - // merging the payload option into grokHome and dropping the hook body. - 'if "%ORCA_GROK_HOME:~-1%"=="\\" set "ORCA_GROK_HOME=%ORCA_GROK_HOME%."', - WINDOWS_GROK_HOOK_POST_COMMAND, - 'exit /b 0', - ...buildWindowsHookStdinDrainEpilogue(), - '' - ].join('\r\n') + return buildWindowsGrokHookScript() } return [ diff --git a/src/main/grok/windows-grok-hook-script.ts b/src/main/grok/windows-grok-hook-script.ts new file mode 100644 index 000000000..fb396dfaf --- /dev/null +++ b/src/main/grok/windows-grok-hook-script.ts @@ -0,0 +1,45 @@ +import { buildWindowsAgentHookPostCommand } from '../agent-hooks/installer-utils' +import { + buildWindowsHookEnvironmentGuardLines, + buildWindowsHookStdinDrainEpilogue +} from '../agent-hooks/hook-stdin-contract' + +/** Matches the envelope length cap used by the POSIX grok-hook branch. */ +export const GROK_HOME_ENVELOPE_MAX_LENGTH = 4096 + +const WINDOWS_GROK_HOOK_POST_COMMAND = buildWindowsAgentHookPostCommand('grok', [ + // Why: attach grokHome before payload@- without string-replacing the shared template. + ' --data-urlencode "grokHome=%ORCA_GROK_HOME%" ^' +]) + +/** + * Windows `grok-hook.cmd` body. + * + * Why (#9358 / #9941): cmd expands `%VAR:~n,m%` at parse time. When `GROK_HOME` + * is unset (the default outside an Orca-managed terminal), length/trailing + * guards become a syntax error and every Grok hook event fails with exit 255. + * + * - Guard substring ops behind `if defined` + goto (not a parenthesized block). + * - Reject oversized values before copying them onto a cmd input line. + */ +export function buildWindowsGrokHookScript(): string { + return [ + '@echo off', + 'setlocal', + 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', + ...buildWindowsHookEnvironmentGuardLines(), + 'set "ORCA_GROK_HOME="', + 'if not defined GROK_HOME goto :orca_grok_home_ready', + `if not "%GROK_HOME:~${GROK_HOME_ENVELOPE_MAX_LENGTH},1%"=="" goto :orca_grok_home_ready`, + 'set "ORCA_GROK_HOME=%GROK_HOME%"', + 'if not defined ORCA_GROK_HOME goto :orca_grok_home_ready', + 'if "%ORCA_GROK_HOME:~-1%"=="\\" set "ORCA_GROK_HOME=%ORCA_GROK_HOME%."', + // Why: the trailing-backslash safety sentinel counts toward the relay envelope. + `if not "%ORCA_GROK_HOME:~${GROK_HOME_ENVELOPE_MAX_LENGTH},1%"=="" set "ORCA_GROK_HOME="`, + ':orca_grok_home_ready', + WINDOWS_GROK_HOOK_POST_COMMAND, + 'exit /b 0', + ...buildWindowsHookStdinDrainEpilogue(), + '' + ].join('\r\n') +}