diff --git a/src/main/agent-hooks/managed-hook-script.test.ts b/src/main/agent-hooks/managed-hook-script.test.ts index 4f4b5ed6bdb..20a46fd7c88 100644 --- a/src/main/agent-hooks/managed-hook-script.test.ts +++ b/src/main/agent-hooks/managed-hook-script.test.ts @@ -20,10 +20,8 @@ describe('buildPosixManagedHookScript — static invariants', () => { expect(script).toContain('done < "$ORCA_AGENT_HOOK_ENDPOINT"') }) - it('keeps the token off the command line (streamed as a header via stdin)', () => { - expect(script).not.toContain('X-Orca-Agent-Hook-Token: ${ORCA_AGENT_HOOK_TOKEN}') - expect(script).toContain('printf \'X-Orca-Agent-Hook-Token: %s\\n\' "$ORCA_AGENT_HOOK_TOKEN" |') - expect(script).toContain('-H @-') + it('sends the token as a header (matching the other hook transports)', () => { + expect(script).toContain('-H "X-Orca-Agent-Hook-Token: ${ORCA_AGENT_HOOK_TOKEN}"') }) it('keeps the payload off the command line (read from a temp file)', () => { diff --git a/src/main/agent-hooks/managed-hook-script.ts b/src/main/agent-hooks/managed-hook-script.ts index 90e33ff5646..bf4e09c8e67 100644 --- a/src/main/agent-hooks/managed-hook-script.ts +++ b/src/main/agent-hooks/managed-hook-script.ts @@ -10,10 +10,11 @@ import type { AgentHookSource } from '../../shared/agent-hook-relay' // it (`. "$file"` / `call "%file%"`), so any shell/batch written into that // path ran with the agent's privileges. We only ever copy the four known // `ORCA_AGENT_HOOK_*` values out of it. -// 2. The hook token and the raw payload never appear on the process command -// line. `ps`/`/proc//cmdline` are readable by other same-UID (and, on -// many systems, cross-UID) processes, so the token went to a header read -// from stdin and the payload to a private temp file read via `@file`. +// 2. The raw payload never appears on the process command line. `ps`/ +// `/proc//cmdline` are readable by other same-UID processes, so the +// payload (the user's prompt and tool output) goes to a private temp file +// read via `@file`. The token stays an argv header — see the note on +// `buildPosixSecureCurlPostLines` for why that is deliberate. // `--noproxy 127.0.0.1` keeps the loopback POST from being redirected through a // configured `http_proxy`/`ALL_PROXY`, which would send the token and payload // off-box. @@ -77,13 +78,17 @@ export function buildPosixHookGuardLines(): string[] { // Why: capture the payload into a private temp file (mktemp is 0600) and post // it with `--data-urlencode payload@` so the raw prompt never lands on -// argv. This also keeps tens-of-KB tool output off the curl command line, which -// avoids EDR command-line-length false positives (the concern #4475 fixed via a -// stdin pipe). We use a temp file rather than that stdin pipe because stdin is -// needed for the token: it is streamed into curl as a header via `-H @-`, so the -// secret stays off argv AND off disk. Non-secret identifiers -// (paneKey/tabId/worktreeId/…) stay as argv fields — they are not sensitive and -// keeping them there avoids escaping filesystem paths into the temp file. +// argv. This keeps tens-of-KB tool output off the curl command line, which +// avoids EDR command-line-length false positives (the concern #4475 fixed). +// The token stays an argv header: it is a low-value, per-session, loopback-only +// credential already present in the 0600 endpoint file, so its `ps` exposure +// matches the Windows path's documented same-user exposure. Moving it off argv +// on the curl versions we still support would mean routing it through curl's +// config-file loader (the only pre-7.55 stdin-header mechanism), which adds a +// config-directive injection surface out of proportion to what a fake-status +// loopback token is worth. Non-secret identifiers (paneKey/tabId/worktreeId/…) +// stay as argv fields — they are not sensitive and keeping them there avoids +// escaping filesystem paths into the temp file. export function buildPosixSecureCurlPostLines(options: PosixManagedHookScriptOptions): string[] { const { source, extraDataFields = [], emptyPayload = 'exit' } = options const emptyLines = @@ -103,12 +108,11 @@ export function buildPosixSecureCurlPostLines(options: PosixManagedHookScriptOpt 'trap \'rm -f "$__orca_payload_file"\' EXIT', 'cat > "$__orca_payload_file"', ...emptyLines, - 'printf \'X-Orca-Agent-Hook-Token: %s\\n\' "$ORCA_AGENT_HOOK_TOKEN" | ' + - `curl -sS -X POST "http://127.0.0.1:\${ORCA_AGENT_HOOK_PORT}/hook/${source}" \\`, + `curl -sS -X POST "http://127.0.0.1:\${ORCA_AGENT_HOOK_PORT}/hook/${source}" \\`, ' --noproxy 127.0.0.1 \\', ' --connect-timeout 0.5 --max-time 1.5 \\', ' -H "Content-Type: application/x-www-form-urlencoded" \\', - ' -H @- \\', + ' -H "X-Orca-Agent-Hook-Token: ${ORCA_AGENT_HOOK_TOKEN}" \\', ' --data-urlencode "paneKey=${ORCA_PANE_KEY}" \\', ' --data-urlencode "tabId=${ORCA_TAB_ID}" \\', ' --data-urlencode "launchToken=${ORCA_AGENT_LAUNCH_TOKEN}" \\', diff --git a/src/main/claude/hook-service.test.ts b/src/main/claude/hook-service.test.ts index 1167dfdacab..3ecfbd80e11 100644 --- a/src/main/claude/hook-service.test.ts +++ b/src/main/claude/hook-service.test.ts @@ -269,12 +269,12 @@ describe('ClaudeHookService.installRemote', () => { const script = fs.files.get('/home/dev/.orca/agent-hooks/claude-hook.sh') expect(script).toContain('#!/bin/sh') expect(script).toContain('DEVIN_PROJECT_DIR') - // Why: payload posts from a private temp file and the token streams in as a - // header via stdin, so neither the prompt nor the token lands on the curl - // command line (also avoids EDR oversized-command-line false positives). - // The endpoint file is parsed, never sourced. + // Why: the payload (the prompt/tool output) posts from a private temp file + // so it never lands on the curl command line (also avoids EDR + // oversized-command-line false positives). The endpoint file is parsed, + // never sourced. The token is a low-value loopback header on argv. expect(script).toContain('--data-urlencode "payload@${__orca_payload_file}"') - expect(script).toContain('-H @-') + expect(script).toContain('-H "X-Orca-Agent-Hook-Token: ${ORCA_AGENT_HOOK_TOKEN}"') expect(script).not.toContain('--data-urlencode "payload=${payload}"') expect(script).not.toContain('. "$ORCA_AGENT_HOOK_ENDPOINT"') expect(fs.modes.get('/home/dev/.orca/agent-hooks/claude-hook.sh')).toBe(0o755) diff --git a/src/main/copilot/hook-service.test.ts b/src/main/copilot/hook-service.test.ts index 68ae69629c4..8958d776306 100644 --- a/src/main/copilot/hook-service.test.ts +++ b/src/main/copilot/hook-service.test.ts @@ -117,6 +117,28 @@ describe('CopilotHookService', () => { } }) + // Why: the shared-builder tests use synthetic options, so copilot's + // agent-specific stdout prelude and hookEventName field are only pinned here. + it.skipIf(process.platform === 'win32')( + 'managed script prints {} first and posts the hookEventName field', + () => { + new CopilotHookService().install() + const script = readFileSync(join(tmpDir, '.orca', 'agent-hooks', 'copilot-hook.sh'), 'utf-8') + // Copilot parses hook stdout; {} must be emitted before any guard can + // exit the script. + expect(script.indexOf("printf '{}\\n'")).toBeGreaterThan(-1) + expect(script.indexOf("printf '{}\\n'")).toBeLessThan( + script.indexOf('done < "$ORCA_AGENT_HOOK_ENDPOINT"') + ) + expect(script).toContain('--data-urlencode "hookEventName=${ORCA_COPILOT_HOOK_EVENT}"') + expect(script).toContain('/hook/copilot') + expect(script).not.toContain('. "$ORCA_AGENT_HOOK_ENDPOINT"') + expect(script).toContain('-H "X-Orca-Agent-Hook-Token: ${ORCA_AGENT_HOOK_TOKEN}"') + expect(script).toContain('--data-urlencode "payload@${__orca_payload_file}"') + expect(script).toContain('--noproxy 127.0.0.1') + } + ) + it('preserves user-authored hooks and sweeps stale managed entries', () => { const configPath = join(copilotHome, 'hooks', 'orca.json') mkdirSync(join(copilotHome, 'hooks'), { recursive: true }) diff --git a/src/main/cursor/hook-service.test.ts b/src/main/cursor/hook-service.test.ts index bcf7a5618d9..3d507fe45e5 100644 --- a/src/main/cursor/hook-service.test.ts +++ b/src/main/cursor/hook-service.test.ts @@ -77,8 +77,8 @@ describe('CursorHookService', () => { if (process.platform === 'win32') { expect(script).toContain('%SystemRoot%\\System32\\curl.exe') } else { - // Payload goes to a private temp file (off argv); token is streamed as a - // header via `-H @-`; the endpoint file is parsed, never sourced. + // Payload goes to a private temp file (off argv); the endpoint file is + // parsed, never sourced. expect(script).toContain('--data-urlencode "payload@${__orca_payload_file}"') expect(script).not.toContain('. "$ORCA_AGENT_HOOK_ENDPOINT"') } diff --git a/src/main/devin/hook-service.test.ts b/src/main/devin/hook-service.test.ts index 850b1ee8ed4..a4072d6b11d 100644 --- a/src/main/devin/hook-service.test.ts +++ b/src/main/devin/hook-service.test.ts @@ -64,13 +64,21 @@ describe('DevinHookService', () => { } const script = readFileSync(getDevinManagedScriptPath(), 'utf8') expect(script).toContain('/hook/devin') - // Why: payload posts from a private temp file and the token streams in as a - // header via stdin, so neither lands on the curl command line (also avoids - // EDR oversized-command-line false positives). Endpoint file parsed, not sourced. - expect(script).toContain('--data-urlencode "payload@${__orca_payload_file}"') - expect(script).toContain('-H @-') - expect(script).not.toContain('--data-urlencode "payload=${payload}"') - expect(script).not.toContain('. "$ORCA_AGENT_HOOK_ENDPOINT"') + if (process.platform === 'win32') { + // Why: cmd has no mktemp, so the payload streams from stdin and the + // endpoint file is parsed by the for/f subroutine, never call-executed. + expect(script).toContain('--data-urlencode "payload@-"') + expect(script).toContain('call :__orcaLoadEndpoint') + expect(script).not.toContain('call "%ORCA_AGENT_HOOK_ENDPOINT%"') + } else { + // Why: the payload posts from a private temp file so it never lands on + // the curl command line (also avoids EDR oversized-command-line false + // positives). Endpoint file parsed, not sourced. Token is a loopback header. + expect(script).toContain('--data-urlencode "payload@${__orca_payload_file}"') + expect(script).toContain('-H "X-Orca-Agent-Hook-Token: ${ORCA_AGENT_HOOK_TOKEN}"') + expect(script).not.toContain('--data-urlencode "payload=${payload}"') + expect(script).not.toContain('. "$ORCA_AGENT_HOOK_ENDPOINT"') + } }) it('preserves unrelated keys in Devin config when installing hooks', () => { diff --git a/src/main/droid/hook-service.test.ts b/src/main/droid/hook-service.test.ts index ef31754a7e7..e2f28d32d01 100644 --- a/src/main/droid/hook-service.test.ts +++ b/src/main/droid/hook-service.test.ts @@ -109,6 +109,18 @@ describe('DroidHookService', () => { } ) + // Why: nothing else pins the droid script body at the service level; assert + // the secure post shape so a builder-call regression fails here too. + it.skipIf(process.platform === 'win32')('managed script posts securely to /hook/droid', () => { + expect(new DroidHookService().install().state).toBe('installed') + const script = readFileSync(join(homeDir, '.orca', 'agent-hooks', 'droid-hook.sh'), 'utf8') + expect(script).toContain('/hook/droid') + expect(script).not.toContain('. "$ORCA_AGENT_HOOK_ENDPOINT"') + expect(script).toContain('-H "X-Orca-Agent-Hook-Token: ${ORCA_AGENT_HOOK_TOKEN}"') + expect(script).toContain('--data-urlencode "payload@${__orca_payload_file}"') + expect(script).toContain('--noproxy 127.0.0.1') + }) + 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/gemini/hook-service.test.ts b/src/main/gemini/hook-service.test.ts index 1a80c1568bf..f901af6b094 100644 --- a/src/main/gemini/hook-service.test.ts +++ b/src/main/gemini/hook-service.test.ts @@ -188,4 +188,25 @@ describe('GeminiHookService', () => { : new RegExp(managedHookFileName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) ) }) + + // Why: the shared-builder tests use synthetic options, so gemini's + // agent-specific stdout prelude is only pinned here. + it.skipIf(process.platform === 'win32')( + 'managed script prints {} before the endpoint parse and posts securely', + () => { + expect(new GeminiHookService().install().state).toBe('installed') + const script = readFileSync(join(homeDir, '.orca', 'agent-hooks', 'gemini-hook.sh'), 'utf8') + // Gemini parses hook stdout as JSON; {} must be emitted before any guard + // can exit the script. + expect(script.indexOf('printf "{}\\n"')).toBeGreaterThan(-1) + expect(script.indexOf('printf "{}\\n"')).toBeLessThan( + script.indexOf('done < "$ORCA_AGENT_HOOK_ENDPOINT"') + ) + expect(script).toContain('/hook/gemini') + expect(script).not.toContain('. "$ORCA_AGENT_HOOK_ENDPOINT"') + expect(script).toContain('-H "X-Orca-Agent-Hook-Token: ${ORCA_AGENT_HOOK_TOKEN}"') + expect(script).toContain('--data-urlencode "payload@${__orca_payload_file}"') + expect(script).toContain('--noproxy 127.0.0.1') + } + ) }) diff --git a/src/main/kimi/hook-service.test.ts b/src/main/kimi/hook-service.test.ts index aeb188b2b8c..4e56e31eaec 100644 --- a/src/main/kimi/hook-service.test.ts +++ b/src/main/kimi/hook-service.test.ts @@ -8,16 +8,20 @@ import { KIMI_HOOK_EVENTS } from './kimi-hook-config-toml' // Why: getSharedManagedScriptPath() writes the managed script under // homedir()/.orca, and getKimiHome() honors KIMI_CODE_HOME. Point both at a // temp dir so the local install/remove cycle never touches the real ~/.orca or -// ~/.kimi-code. os.homedir() resolves $HOME on POSIX (verified at write time). +// ~/.kimi-code. os.homedir() resolves $HOME on POSIX and USERPROFILE on +// Windows, so both must be pinned. let home: string let originalHome: string | undefined +let originalUserProfile: string | undefined let originalKimiHome: string | undefined beforeEach(() => { home = mkdtempSync(join(tmpdir(), 'orca-kimi-hook-')) originalHome = process.env.HOME + originalUserProfile = process.env.USERPROFILE originalKimiHome = process.env.KIMI_CODE_HOME process.env.HOME = home + process.env.USERPROFILE = home process.env.KIMI_CODE_HOME = join(home, '.kimi-code') }) @@ -27,6 +31,11 @@ afterEach(() => { } else { process.env.HOME = originalHome } + if (originalUserProfile === undefined) { + delete process.env.USERPROFILE + } else { + process.env.USERPROFILE = originalUserProfile + } if (originalKimiHome === undefined) { delete process.env.KIMI_CODE_HOME } else { @@ -55,11 +64,11 @@ describe('KimiHookService', () => { // The managed script must exist and POST to the Kimi hook endpoint. const script = readFileSync(scriptPath(), 'utf-8') expect(script).toContain('/hook/kimi') - // Why: payload posts from a private temp file and the token streams in as a - // header via stdin, so neither lands on the curl command line (also avoids - // EDR oversized-command-line false positives). Endpoint file parsed, not sourced. + // Why: the payload posts from a private temp file so it never lands on the + // curl command line (also avoids EDR oversized-command-line false + // positives). Endpoint file parsed, not sourced. Token is a loopback header. expect(script).toContain('--data-urlencode "payload@${__orca_payload_file}"') - expect(script).toContain('-H @-') + expect(script).toContain('-H "X-Orca-Agent-Hook-Token: ${ORCA_AGENT_HOOK_TOKEN}"') expect(script).not.toContain('--data-urlencode "payload=${payload}"') expect(script).not.toContain('. "$ORCA_AGENT_HOOK_ENDPOINT"') // The command Kimi runs points at the managed script via sh.