diff --git a/src/main/agent-hooks/hook-stdin-contract.ts b/src/main/agent-hooks/hook-stdin-contract.ts index 902ec5432..6c01f9756 100644 --- a/src/main/agent-hooks/hook-stdin-contract.ts +++ b/src/main/agent-hooks/hook-stdin-contract.ts @@ -1,6 +1,11 @@ export type PosixHookEmptyPayloadPolicy = 'exit' | 'empty-object' -export const POSIX_HOOK_STDIN_DRAIN_COMMAND = 'cat >/dev/null 2>&1 || :' +// Why: a stripped PATH must not stop a hook from consuming stdin, or the agent +// sees exit 127 and a broken pipe mid-write (#8110). `command -p` resolves from +// the shell's built-in default PATH, so it also survives hosts without /bin/cat +// (NixOS) and ignores a worktree-local `cat` that could capture the payload. +export const POSIX_HOOK_STDIN_READER = '{ command -p cat 2>/dev/null || cat; }' +export const POSIX_HOOK_STDIN_DRAIN_COMMAND = `${POSIX_HOOK_STDIN_READER} >/dev/null 2>&1 || :` // Why: every POSIX hook must own stdin before any no-op exit; sharing this // prelude prevents agent templates from inventing different drain semantics. @@ -9,7 +14,12 @@ export function buildPosixHookPayloadCapture( ): string[] { const emptyPayloadLines = emptyPayloadPolicy === 'empty-object' ? [" payload='{}'"] : [' exit 0'] - return ['payload=$(cat)', 'if [ -z "$payload" ]; then', ...emptyPayloadLines, 'fi'] + return [ + `payload=$(${POSIX_HOOK_STDIN_READER})`, + 'if [ -z "$payload" ]; then', + ...emptyPayloadLines, + 'fi' + ] } export const WINDOWS_HOOK_STDIN_DRAIN_LABEL = 'orca_agent_hook_drain_stdin' diff --git a/src/main/agent-hooks/installer-utils.test.ts b/src/main/agent-hooks/installer-utils.test.ts index 466792009..487a58195 100644 --- a/src/main/agent-hooks/installer-utils.test.ts +++ b/src/main/agent-hooks/installer-utils.test.ts @@ -31,6 +31,7 @@ import { writeHooksJson, type HooksConfig } from './installer-utils' +import { POSIX_HOOK_STDIN_DRAIN_COMMAND } from './hook-stdin-contract' let tmpDir: string let configPath: string @@ -357,7 +358,7 @@ describe('wrapPosixHookCommand', () => { it('produces a guarded command that no-ops when the script is missing', () => { const cmd = wrapPosixHookCommand('/does/not/exist.sh') expect(cmd).toBe( - "if [ -f '/does/not/exist.sh' ] && [ -r '/does/not/exist.sh' ] && [ -x '/does/not/exist.sh' ]; then /bin/sh '/does/not/exist.sh'; else cat >/dev/null 2>&1 || :; fi" + `if [ -f '/does/not/exist.sh' ] && [ -r '/does/not/exist.sh' ] && [ -x '/does/not/exist.sh' ]; then /bin/sh '/does/not/exist.sh'; else ${POSIX_HOOK_STDIN_DRAIN_COMMAND}; fi` ) }) @@ -375,7 +376,7 @@ describe('wrapPosixHookCommand', () => { // /bin/sh as a single argument. const cmd = wrapPosixHookCommand("/path/with'quote/x.sh") expect(cmd).toBe( - "if [ -f '/path/with'\\''quote/x.sh' ] && [ -r '/path/with'\\''quote/x.sh' ] && [ -x '/path/with'\\''quote/x.sh' ]; then /bin/sh '/path/with'\\''quote/x.sh'; else cat >/dev/null 2>&1 || :; fi" + `if [ -f '/path/with'\\''quote/x.sh' ] && [ -r '/path/with'\\''quote/x.sh' ] && [ -x '/path/with'\\''quote/x.sh' ]; then /bin/sh '/path/with'\\''quote/x.sh'; else ${POSIX_HOOK_STDIN_DRAIN_COMMAND}; fi` ) }) @@ -384,7 +385,7 @@ describe('wrapPosixHookCommand', () => { ORCA_COPILOT_HOOK_EVENT: 'UserPromptSubmit' }) expect(cmd).toBe( - "if [ -f '/does/not/exist.sh' ] && [ -r '/does/not/exist.sh' ] && [ -x '/does/not/exist.sh' ]; then ORCA_COPILOT_HOOK_EVENT='UserPromptSubmit' /bin/sh '/does/not/exist.sh'; else cat >/dev/null 2>&1 || :; fi" + `if [ -f '/does/not/exist.sh' ] && [ -r '/does/not/exist.sh' ] && [ -x '/does/not/exist.sh' ]; then ORCA_COPILOT_HOOK_EVENT='UserPromptSubmit' /bin/sh '/does/not/exist.sh'; else ${POSIX_HOOK_STDIN_DRAIN_COMMAND}; fi` ) }) @@ -558,7 +559,7 @@ describe('wrapWindowsGitBashHookCommand', () => { expect( wrapWindowsGitBashHookCommand('C:\\Users\\alice\\.orca\\agent-hooks\\claude-hook.cmd') ).toBe( - "if [ -f 'C:/Users/alice/.orca/agent-hooks/claude-hook.cmd' ]; then 'C:/Users/alice/.orca/agent-hooks/claude-hook.cmd'; else cat >/dev/null 2>&1 || :; fi" + `if [ -f 'C:/Users/alice/.orca/agent-hooks/claude-hook.cmd' ]; then 'C:/Users/alice/.orca/agent-hooks/claude-hook.cmd'; else ${POSIX_HOOK_STDIN_DRAIN_COMMAND}; fi` ) }) diff --git a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts index 0645de342..aa303ce31 100644 --- a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts +++ b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts @@ -2,7 +2,7 @@ // matrix catches an unread early exit without duplicating template assertions. import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { spawn } from 'node:child_process' -import { existsSync, mkdtempSync, readFileSync, readdirSync, rmSync } from 'node:fs' +import { existsSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import type { SFTPWrapper } from 'ssh2' @@ -82,6 +82,7 @@ import { wrapWindowsGitBashHookCommand, wrapWindowsHookCommand } from './installer-utils' +import { POSIX_HOOK_STDIN_READER } from './hook-stdin-contract' import { createAgentHookMemorySftp } from './agent-hook-memory-sftp.test-fixture' const REMOTE_HOME = '/home/dev' @@ -155,6 +156,7 @@ const LOCAL_INSTALLERS = [ type HookRun = { exitCode: number | null stdinErrors: NodeJS.ErrnoException[] + stdout: string } function runHookProcess( @@ -163,8 +165,9 @@ function runHookProcess( env: NodeJS.ProcessEnv ): Promise { return new Promise((resolve, reject) => { - const child = spawn(executable, args, { env, stdio: ['pipe', 'ignore', 'ignore'] }) + const child = spawn(executable, args, { env, stdio: ['pipe', 'pipe', 'ignore'] }) const stdinErrors: NodeJS.ErrnoException[] = [] + let stdout = '' const timeout = setTimeout(() => { child.kill('SIGKILL') reject(new Error('hook did not finish after stdin closed')) @@ -173,10 +176,13 @@ function runHookProcess( clearTimeout(timeout) reject(error) }) + child.stdout.on('data', (chunk: Buffer) => { + stdout += chunk.toString() + }) child.stdin.on('error', (error: NodeJS.ErrnoException) => stdinErrors.push(error)) child.on('close', (exitCode) => { clearTimeout(timeout) - resolve({ exitCode, stdinErrors }) + resolve({ exitCode, stdinErrors, stdout }) }) child.stdin.end(LARGE_PAYLOAD) }) @@ -274,7 +280,9 @@ describe('Windows managed hook stdin structure', () => { copilot.indexOf('if (-not $env:ORCA_AGENT_HOOK_PORT') ) const kimi = readFileSync(join(hooksDir, 'kimi-hook.sh'), 'utf8') - expect(kimi.indexOf('payload=$(cat)')).toBeLessThan(kimi.indexOf('exit 0')) + expect(kimi.indexOf(`payload=$(${POSIX_HOOK_STDIN_READER})`)).toBeLessThan( + kimi.indexOf('exit 0') + ) } finally { homedirMock.mockImplementation(() => process.env.HOME ?? tmpdir()) if (previousGrokHome === undefined) { @@ -370,7 +378,7 @@ describe.skipIf(process.platform === 'win32')('managed hook stdin lifecycle', () it('captures stdin before every possible whole-script success exit', async () => { const scripts = await generatePosixScripts() for (const [agent, script] of scripts) { - const captureIndex = script.indexOf('payload=$(cat)') + const captureIndex = script.indexOf(`payload=$(${POSIX_HOOK_STDIN_READER})`) const firstExitIndex = script.indexOf('exit 0') expect(captureIndex, `${agent} payload capture`).toBeGreaterThanOrEqual(0) expect(firstExitIndex, `${agent} first success exit`).toBeGreaterThan(captureIndex) @@ -393,6 +401,50 @@ describe.skipIf(process.platform === 'win32')('managed hook stdin lifecycle', () } }) + it('does not need PATH to capture or drain POSIX hook stdin', async () => { + const scripts = await generatePosixScripts() + for (const [agent, script] of scripts) { + const result = await runPosixHook(script, { PATH: '' }) + expect(result.exitCode, `${agent} exit code`).toBe(0) + expect(result.stdinErrors, `${agent} stdin errors`).toHaveLength(0) + } + + const missing = await runPosixHook(wrapPosixHookCommand('/missing/orca-hook.sh'), { PATH: '' }) + expect(missing.exitCode, 'missing script launcher exit code').toBe(0) + expect(missing.stdinErrors, 'missing script launcher stdin errors').toHaveLength(0) + }) + + // Why: an unread stdin still exits 0, so exit codes alone cannot prove the + // reader consumed the payload. Assert the captured byte count directly. + it.each([ + ['empty PATH', ''], + // Why: /bin/cat is absent on NixOS-style hosts, so an absolute path alone is + // not enough; the reader must fall back to the shell's default PATH. + ['PATH without coreutils', '/nonexistent'], + // Why: a worktree-local `cat` must never receive the hook payload. + ['PATH whose first cat is a decoy', ''] + ])('captures the whole payload with %s', async (label, pathValue) => { + const decoyDir = mkdtempSync(join(tmpdir(), 'orca-hook-stdin-decoy-')) + try { + let effectivePath = pathValue + if (label === 'PATH whose first cat is a decoy') { + const decoy = join(decoyDir, 'cat') + writeFileSync(decoy, '#!/bin/sh\nexit 0\n', { mode: 0o755 }) + effectivePath = decoyDir + } + const result = await runHookProcess( + '/bin/sh', + ['-c', `payload=$(${POSIX_HOOK_STDIN_READER}); printf '%s' "${'${#payload}'}"`], + { ...hookEnvironment(), PATH: effectivePath } + ) + expect(result.exitCode, `${label} exit code`).toBe(0) + expect(result.stdinErrors, `${label} stdin errors`).toHaveLength(0) + expect(result.stdout, `${label} captured bytes`).toBe(String(LARGE_PAYLOAD.length)) + } finally { + rmSync(decoyDir, { recursive: true, force: true }) + } + }) + it('drains before Claude skips hooks imported by Devin', async () => { const script = (await generatePosixScripts()).get('claude claude-hook.sh') expect(script).toBeDefined() diff --git a/src/main/antigravity/hook-service.test.ts b/src/main/antigravity/hook-service.test.ts index fffe15052..5861d5e0c 100644 --- a/src/main/antigravity/hook-service.test.ts +++ b/src/main/antigravity/hook-service.test.ts @@ -16,6 +16,7 @@ vi.mock('os', async () => { }) import { AntigravityHookService } from './hook-service' +import { POSIX_HOOK_STDIN_READER } from '../agent-hooks/hook-stdin-contract' import { createManagedCommandMatcher } from '../agent-hooks/installer-utils' const ANTIGRAVITY_SCRIPT_FILE_NAME = @@ -94,7 +95,7 @@ describe('AntigravityHookService', () => { expect(script).not.toContain('[string]::IsNullOrWhiteSpace($inputData)) { exit 0 }') } else { expect(script).toContain('hook_event_name=${ORCA_ANTIGRAVITY_EVENT}') - expect(script).toContain('payload=$(cat)') + expect(script).toContain(`payload=$(${POSIX_HOOK_STDIN_READER})`) expect(script).toContain("payload='{}'") expect(script).not.toContain('if [ -z "$payload" ]; then\n exit 0\nfi') // Why: payload is piped to curl via stdin (`payload@-`) so it never lands diff --git a/src/main/codex/hook-service-wsl-runtime.test.ts b/src/main/codex/hook-service-wsl-runtime.test.ts index f56ea1449..dbc51f39b 100644 --- a/src/main/codex/hook-service-wsl-runtime.test.ts +++ b/src/main/codex/hook-service-wsl-runtime.test.ts @@ -5,6 +5,7 @@ import { tmpdir } from 'node:os' import { dirname, join, win32 as pathWin32 } from 'node:path' import { MANAGED_HOOK_TIMEOUT_SECONDS } from '../agent-hooks/installer-utils' +import { POSIX_HOOK_STDIN_DRAIN_COMMAND } from '../agent-hooks/hook-stdin-contract' import { computeTrustKey, computeTrustedHash, @@ -77,7 +78,7 @@ function getManagedTrustEntry( } function expectedManagedCommand(scriptPath: string): string { - return `if [ -f '${scriptPath}' ] && [ -r '${scriptPath}' ]; then /bin/sh '${scriptPath}'; else cat >/dev/null 2>&1 || :; fi` + return `if [ -f '${scriptPath}' ] && [ -r '${scriptPath}' ]; then /bin/sh '${scriptPath}'; else ${POSIX_HOOK_STDIN_DRAIN_COMMAND}; fi` } describe('Codex WSL runtime hook install', () => { diff --git a/src/main/cursor/hook-service.test.ts b/src/main/cursor/hook-service.test.ts index d04609044..f861d4f2a 100644 --- a/src/main/cursor/hook-service.test.ts +++ b/src/main/cursor/hook-service.test.ts @@ -16,6 +16,7 @@ vi.mock('os', async () => { }) import { CursorHookService } from './hook-service' +import { POSIX_HOOK_STDIN_READER } from '../agent-hooks/hook-stdin-contract' const CURSOR_EVENTS = [ 'beforeSubmitPrompt', @@ -79,7 +80,7 @@ describe('CursorHookService', () => { } else { // Why: payload is piped to curl via stdin (`payload@-`) so it never lands // on the curl command line (EDR oversized-command-line false positive). - expect(script).toContain('payload=$(cat)') + expect(script).toContain(`payload=$(${POSIX_HOOK_STDIN_READER})`) expect(script).toContain('printf \'%s\' "$payload" | curl') expect(script).toContain('--data-urlencode "payload@-"') expect(script).not.toContain('--data-urlencode "payload=${payload}"') diff --git a/src/main/grok/hook-service.test.ts b/src/main/grok/hook-service.test.ts index c6f89509e..028107263 100644 --- a/src/main/grok/hook-service.test.ts +++ b/src/main/grok/hook-service.test.ts @@ -16,6 +16,7 @@ vi.mock('os', async () => { }) import { getGrokToolEventMatcherForTests, GrokHookService } from './hook-service' +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 = @@ -97,7 +98,7 @@ describe('GrokHookService', () => { } else { // Why: payload is piped to curl via stdin (`payload@-`) so it never lands // on the curl command line (EDR oversized-command-line false positive). - expect(script).toContain('payload=$(cat)') + expect(script).toContain(`payload=$(${POSIX_HOOK_STDIN_READER})`) expect(script).toContain('printf \'%s\' "$payload" | curl') expect(script).toContain('--data-urlencode "payload@-"') expect(script).toContain('${#GROK_HOME}" -le 4096')