fix(windows): make managed grok-hook.cmd safe when GROK_HOME is unset (#11782)

* fix(windows): make managed grok-hook.cmd safe when GROK_HOME is unset Fixes #9358 and #9941. cmd.exe expands %VAR:~n,m% at parse time. When GROK_HOME is unset (default outside Orca terminals), the generated length/trailing-backslash guards became a syntax error and every Grok hook event failed with exit 255. - Skip substring work when GROK_HOME is undefined (if defined + goto) - Replace if "%x:~-1%"=="\" (itself a quote-parser bug) with findstr - Extract Windows script builder; add template + spawn tests * fix(windows): harden grok-hook GROK_HOME guards and tests Address review on #11782: - Inject grokHome via buildWindowsAgentHookPostCommand extra form lines (no fragile string replace of the shared payload line) - Spawn tests delete GROK_HOME and keep PORT/TOKEN/PANE_KEY set so the GROK_HOME path actually runs before curl * fix(windows): cover Grok hook home boundaries --------- Co-authored-by: OrcaWin <alpha-eng@stably.ai>
2026-08-06 04:31:26 +08:00 · 2026-08-06 04:31:26 +08:00 · 74ac7049ec
parent d4dfc35ac4
commit 74ac7049ec
5 changed files with 243 additions and 32 deletions
--- 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",
--- 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')
 }
--- 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<WindowsGrokHookRun> {
+  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<void>((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<Omit<WindowsGrokHookRun, 'request'>>((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<void>((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
--- 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 [
--- a/src/main/grok/windows-grok-hook-script.ts
+++ 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')
+}