mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 00:03:52 +00:00
fix(agent-hooks): drain stdin before hook script early exits so agents never hit EPIPE (#8430)
* Fix hook scripts to drain stdin before any early-exit path Generated agent hook scripts and missing-script launchers could exit successfully before consuming the payload written to their stdin, leaving the writer with a broken pipe (EPIPE/ERROR_BROKEN_PIPE) once the reader closed early. Capture stdin (or drain it via a shared epilogue/fast-path guard) before any whole-script success exit across all POSIX, batch, PowerShell, and Git Bash launcher variants, and add a cross-agent lifecycle test suite plus a live Electron verification script to guard the contract going forward. * Harden hook scripts against unreadable managed scripts and add a Claude/ - Extend the POSIX launcher guard to also require `[ -r ]`, not just `-f`/`-x`, so an executable-but-unreadable managed script still drains stdin instead of erroring or silently misbehaving. - Add a verifier case (`verifyClaudeDevinSkip`) that spins up a local HTTP server and confirms the Claude hook never forwards a request that Devin already imported, catching accidental double-forwarding. - Update installer-utils tests and stdin-lifecycle docs to match the new readable-file guard and the added verification case. * Fix hook-launcher verification to derive script paths from the installed Extract the quoted path from the launcher's `if [ -f '...'` clause instead of reconstructing it via join(home, ...), so missing/failing-script test cases can't silently fall through to the real script if the install layout changes. --------- Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com>
This commit is contained in:
co-authored by
Jinjing
parent
8303d955e8
commit
c3ab805d12
@@ -66,6 +66,10 @@ function getManagedTrustEntry(
|
||||
}
|
||||
}
|
||||
|
||||
function expectedManagedCommand(scriptPath: string): string {
|
||||
return `if [ -f '${scriptPath}' ] && [ -r '${scriptPath}' ]; then /bin/sh '${scriptPath}'; else cat >/dev/null 2>&1 || :; fi`
|
||||
}
|
||||
|
||||
describe('Codex WSL runtime hook install', () => {
|
||||
it('plans WSL hook files with Linux command and trust paths', () => {
|
||||
const runtimeHome =
|
||||
@@ -133,7 +137,7 @@ describe('Codex WSL runtime hook install', () => {
|
||||
trustConfigPath: '/old/home/hooks.json'
|
||||
}
|
||||
expect(_internals.installManagedHooksIntoWslRuntime(oldPlan).state).toBe('installed')
|
||||
const oldCommand = `if [ -r '${oldPlan.commandScriptPath}' ]; then /bin/sh '${oldPlan.commandScriptPath}'; fi`
|
||||
const oldCommand = expectedManagedCommand(oldPlan.commandScriptPath)
|
||||
const oldKey = computeTrustKey(getManagedTrustEntry(oldPlan, oldCommand))
|
||||
|
||||
const newPlan = {
|
||||
@@ -142,7 +146,7 @@ describe('Codex WSL runtime hook install', () => {
|
||||
trustConfigPath: '/new/home/hooks.json'
|
||||
}
|
||||
expect(_internals.installManagedHooksIntoWslRuntime(newPlan).state).toBe('installed')
|
||||
const newCommand = `if [ -r '${newPlan.commandScriptPath}' ]; then /bin/sh '${newPlan.commandScriptPath}'; fi`
|
||||
const newCommand = expectedManagedCommand(newPlan.commandScriptPath)
|
||||
const newKey = computeTrustKey(getManagedTrustEntry(newPlan, newCommand))
|
||||
const trustEntries = readHookTrustEntries(plan.tomlPath)
|
||||
|
||||
@@ -150,6 +154,30 @@ describe('Codex WSL runtime hook install', () => {
|
||||
expect(trustEntries.has(newKey)).toBe(true)
|
||||
})
|
||||
|
||||
it.skipIf(process.platform === 'win32')(
|
||||
'drains stdin when the WSL runtime script is missing',
|
||||
() => {
|
||||
const basePlan = createTestPlan()
|
||||
const plan = {
|
||||
...basePlan,
|
||||
commandScriptPath: join(dirname(basePlan.configPath), 'missing-codex-hook.sh')
|
||||
}
|
||||
writeFileSync(plan.configPath, '{"hooks":{}}\n', 'utf-8')
|
||||
writeFileSync(plan.tomlPath, '', 'utf-8')
|
||||
|
||||
expect(_internals.installManagedHooksIntoWslRuntime(plan).state).toBe('installed')
|
||||
const installed = JSON.parse(readFileSync(plan.configPath, 'utf-8')) as HooksConfig
|
||||
const command = installed.hooks.UserPromptSubmit[0]?.hooks?.[0]?.command
|
||||
expect(command).toBe(expectedManagedCommand(plan.commandScriptPath))
|
||||
|
||||
const result = spawnSync('/bin/sh', ['-c', command!], {
|
||||
input: Buffer.alloc(1_000_000, 'x')
|
||||
})
|
||||
expect(result.error).toBeUndefined()
|
||||
expect(result.status).toBe(0)
|
||||
}
|
||||
)
|
||||
|
||||
it('sweeps all managed WSL trust for disable or confirmed absence', () => {
|
||||
// Why: disable and confirmed absence intentionally pass []. Transient
|
||||
// unavailability must NOT use this path — last known-good trust remains.
|
||||
@@ -351,9 +379,7 @@ describe('Codex WSL runtime hook install', () => {
|
||||
const installed = JSON.parse(readFileSync(plan.configPath, 'utf-8')) as HooksConfig
|
||||
expect(Object.keys(installed.hooks).sort()).toEqual([...managedEvents].sort())
|
||||
const managedCommand = installed.hooks.UserPromptSubmit[0]?.hooks?.[0]?.command
|
||||
expect(managedCommand).toBe(
|
||||
`if [ -r '${plan.commandScriptPath}' ]; then /bin/sh '${plan.commandScriptPath}'; fi`
|
||||
)
|
||||
expect(managedCommand).toBe(expectedManagedCommand(plan.commandScriptPath))
|
||||
expect(installed.hooks.UserPromptSubmit[1]?.hooks?.[0]?.command).toBe(userCommand)
|
||||
expect(readFileSync(plan.scriptPath, 'utf-8')).toContain('command -v curl.exe')
|
||||
|
||||
|
||||
@@ -25,6 +25,12 @@ import {
|
||||
writeManagedScriptRemote,
|
||||
writeTextFileRemoteAtomic
|
||||
} from '../agent-hooks/installer-utils-remote'
|
||||
import {
|
||||
buildPosixHookPayloadCapture,
|
||||
buildWindowsHookEnvironmentGuardLines,
|
||||
buildWindowsHookStdinDrainEpilogue,
|
||||
POSIX_HOOK_STDIN_DRAIN_COMMAND
|
||||
} from '../agent-hooks/hook-stdin-contract'
|
||||
import {
|
||||
computeTrustKey,
|
||||
computeTrustedHash,
|
||||
@@ -136,8 +142,8 @@ export type { CodexWslRuntimeHookInstallPlan }
|
||||
function wrapReadablePosixHookCommand(scriptPath: string): string {
|
||||
const quoted = `'${scriptPath.replaceAll("'", "'\\''")}'`
|
||||
// Why: WSL runtime hooks are written from Windows through UNC, where the
|
||||
// executable bit is not reliable. /bin/sh only needs the script to be readable.
|
||||
return `if [ -r ${quoted} ]; then /bin/sh ${quoted}; fi`
|
||||
// executable bit is not reliable; a missing script must still own stdin.
|
||||
return `if [ -f ${quoted} ] && [ -r ${quoted} ]; then /bin/sh ${quoted}; else ${POSIX_HOOK_STDIN_DRAIN_COMMAND}; fi`
|
||||
}
|
||||
|
||||
function getSystemConfigPath(): string {
|
||||
@@ -781,17 +787,17 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string {
|
||||
// surviving PTY reach the current server even though its env points at
|
||||
// the prior Orca's coordinates.
|
||||
'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul',
|
||||
'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0',
|
||||
'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0',
|
||||
'if "%ORCA_PANE_KEY%"=="" exit /b 0',
|
||||
...buildWindowsHookEnvironmentGuardLines(),
|
||||
buildWindowsAgentHookCurlPostCommand('codex'),
|
||||
'exit /b 0',
|
||||
...buildWindowsHookStdinDrainEpilogue(),
|
||||
''
|
||||
].join('\r\n')
|
||||
}
|
||||
|
||||
return [
|
||||
'#!/bin/sh',
|
||||
...buildPosixHookPayloadCapture(),
|
||||
// Why: see claude/hook-service.ts for rationale. Sourcing refreshes
|
||||
// PORT/TOKEN/ENV/VERSION from the current Orca so a surviving PTY keeps
|
||||
// reporting after a restart.
|
||||
@@ -823,10 +829,6 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string {
|
||||
'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then',
|
||||
' exit 0',
|
||||
'fi',
|
||||
'payload=$(cat)',
|
||||
'if [ -z "$payload" ]; then',
|
||||
' exit 0',
|
||||
'fi',
|
||||
'post_codex_hook() {',
|
||||
' curl_bin="$1"',
|
||||
' connect_timeout="${2:-0.5}"',
|
||||
|
||||
Reference in New Issue
Block a user