Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions src/main/agent-hooks/spool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
import { AgentHookServer, _internals } from './server'
import { buildBody } from './server.test-fixtures'
import { _internals as codexInternals } from '../codex/hook-service'
import { getManagedScript as getClaudeManagedScript } from '../claude/hook-script'
import { makePaneKey } from '../../shared/stable-pane-id'
import { buildPosixHookSpoolLines } from './hook-stdin-contract'

Expand Down Expand Up @@ -247,6 +248,37 @@ describe('agent hook spool', () => {
)
})

it('keeps a Claude hook attributed when CLAUDE_CODE_SUBPROCESS_ENV_SCRUB drops the pane key', () => {
const dir = mkdtempSync(join(tmpdir(), 'orca-spool-scrubbed-'))
const endpointDir = join(dir, 'agent-hooks')
mkdirSync(endpointDir, { recursive: true })
const endpoint = join(endpointDir, 'endpoint.env')
writeFileSync(endpoint, 'ORCA_AGENT_HOOK_PORT=9\nORCA_AGENT_HOOK_TOKEN=stale\n')
const script = join(dir, 'claude-hook.sh')
writeFileSync(script, getClaudeManagedScript('posix'))
const env: NodeJS.ProcessEnv = {
...process.env,
ORCA_AGENT_HOOK_ENDPOINT: endpoint,
ORCA_AGENT_PANE: 'tab-scrubbed:0',
ORCA_AGENT_LAUNCH: 'generation-token'
}
// Claude's scrub removes every *KEY/*TOKEN name; only the scrub-safe aliases reach the hook.
delete env.ORCA_PANE_KEY
delete env.ORCA_AGENT_LAUNCH_TOKEN
delete env.CLAUDE_JOB_DIR
execFileSync('/bin/sh', [script], {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major · Stability & Availability · Quick win

🔎 Supported by static analysis

Script executed:

sed -n '1,310p' src/main/agent-hooks/spool.test.ts
sed -n '1,100p' config/scripts/ci-unit-files.mjs
sed -n '45,95p' config/vitest.config.ts

Repository: stablyai/orca

Length of output: 16640


Script executed:

set -eu
printf '%s\n' '--- package scripts containing test/unit/vitest ---'
python3 - <<'PY'
import json
from pathlib import Path
p = Path('package.json')
data = json.loads(p.read_text())
for k, v in data.get('scripts', {}).items():
    if any(term in k.lower() or term in str(v).lower() for term in ('test', 'vitest', 'unit')):
        print(f'{k}: {v}')
PY
printf '%s\n' '--- workflow references to Windows and unit/Vitest commands ---'
rg -n -i -F --glob '.github/workflows/*' -e 'windows' -e 'vitest' -e 'pnpm test' -e 'ci-unit-files' -e 'unit' .github/workflows || test "$?" -eq 1
printf '%s\n' '--- all direct references to this test or its enclosing test command ---'
rg -n -F --glob '*.yml' --glob '*.yaml' --glob '*.json' --glob '*.mjs' --glob '*.ts' -e 'spool.test.ts' -e 'run test' -e 'test:unit' -e 'vitest run' . || test "$?" -eq 1

Repository: stablyai/orca

Length of output: 41116


Skip this POSIX-only test on native Windows.

The Windows preflight matrix runs the full Node Vitest configuration. This test has no enclosing or platform-specific exclusion. Without /bin/sh, execFileSync('/bin/sh', [script], ...) can fail before the assertions run.

🐛 Suggested fix
-  it('keeps a Claude hook attributed when CLAUDE_CODE_SUBPROCESS_ENV_SCRUB drops the pane key', () => {
+  it.skipIf(process.platform === 'win32')('keeps a Claude hook attributed when CLAUDE_CODE_SUBPROCESS_ENV_SCRUB drops the pane key', () => {

input: '{"hook_event_name":"Stop","session_id":"claude-session"}\n',
env,
timeout: 5000
})
const spoolFiles = readdirSync(join(endpointDir, 'spool'))
expect(spoolFiles).toHaveLength(1)
const record: unknown = JSON.parse(
readFileSync(join(endpointDir, 'spool', spoolFiles[0]!), 'utf8').trim()
)
expect(record).toMatchObject({ paneKey: 'tab-scrubbed:0', launchToken: 'generation-token' })
})

it('does not mark a non-terminal downtime replay as runtime-observed', async () => {
const userDataPath = mkdtempSync(join(tmpdir(), 'orca-spool-observed-'))
const paneKey = makePaneKey('tab-observed', '00000000-0000-4000-8000-000000000002')
Expand Down
8 changes: 8 additions & 0 deletions src/main/claude/hook-script.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import { buildHookProcessCapture } from '../agent-hooks/hook-process-capture'
import type { AgentHookSource } from '../../shared/agent-hook-relay'
import { buildWindowsAgentHookCurlPostCommand } from '../agent-hooks/installer-utils'
import { buildPosixAgentHookPostCommand } from '../agent-hooks/hook-post-command'
import {
buildPosixScrubSafeEnvRestoreLines,
buildWindowsScrubSafeEnvRestoreLines
} from '../../shared/agent-hook-scrub-safe-env'
import {
buildPosixCursorReplayGuardLines,
buildWindowsCursorReplayGuardLines
Expand Down Expand Up @@ -37,6 +41,8 @@ export function getManagedScript(
'setlocal',
// Permission hooks fail closed on empty stdout, including skip paths.
'echo {}',
// CLAUDE_CODE_SUBPROCESS_ENV_SCRUB drops ORCA_PANE_KEY; restore it before any guard reads it.
...buildWindowsScrubSafeEnvRestoreLines(),
// Refresh endpoint coordinates for PTYs surviving an Orca restart.
'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul',
// Guard abandoned stdin before any skip path can drain it.
Expand All @@ -62,6 +68,8 @@ export function getManagedScript(
'#!/bin/sh',
// Permission hooks fail closed on empty stdout, including skip paths.
'printf "{}\\n"',
// CLAUDE_CODE_SUBPROCESS_ENV_SCRUB drops ORCA_PANE_KEY; restore it before any guard or spool reads it.
...buildPosixScrubSafeEnvRestoreLines(),
...buildPosixHookPayloadCapture(),
...(options.skipWhenGrokImportsClaude ? buildPosixGrokReplayGuardLines() : []),
...(options.skipWhenCursorImportsClaude ? buildPosixCursorReplayGuardLines() : []),
Expand Down
11 changes: 6 additions & 5 deletions src/main/dsh/hook-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ import {
buildWindowsHookStdinDrainEpilogue
} from '../agent-hooks/hook-stdin-contract'
import { buildPosixAgentHookPostCommand } from '../agent-hooks/hook-post-command'
import {
buildPosixScrubSafeEnvRestoreLines,
buildWindowsScrubSafeEnvRestoreLines
} from '../../shared/agent-hook-scrub-safe-env'
import {
applyManagedDshPatch,
isDshPatchFileUnappendable,
Expand All @@ -48,8 +52,7 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string {
'@echo off',
'setlocal',
// Restore DSH-scrubbed KEY/TOKEN variables before any guard or spool code reads them.
'if not defined ORCA_PANE_KEY if defined ORCA_AGENT_PANE set "ORCA_PANE_KEY=%ORCA_AGENT_PANE%"',
'if not defined ORCA_AGENT_LAUNCH_TOKEN if defined ORCA_AGENT_LAUNCH set "ORCA_AGENT_LAUNCH_TOKEN=%ORCA_AGENT_LAUNCH%"',
...buildWindowsScrubSafeEnvRestoreLines(),
'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul',
...buildWindowsHookEnvironmentGuardLines(),
buildWindowsAgentHookCurlPostCommand('dsh'),
Expand All @@ -62,9 +65,7 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string {
return [
'#!/bin/sh',
// DSH scrubs KEY/TOKEN variables; recover canonical names from the aliases set at spawn.
': "${ORCA_PANE_KEY:=${ORCA_AGENT_PANE:-}}"',
': "${ORCA_AGENT_LAUNCH_TOKEN:=${ORCA_AGENT_LAUNCH:-}}"',
'export ORCA_PANE_KEY ORCA_AGENT_LAUNCH_TOKEN',
...buildPosixScrubSafeEnvRestoreLines(),
...buildPosixHookPayloadCapture(),
...buildPosixHookSpoolLines('dsh'),
// Refresh endpoint credentials for PTYs that outlive an Orca restart.
Expand Down
22 changes: 19 additions & 3 deletions src/shared/agent-hook-scrub-safe-env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
*
* DeepSeek Harness runs command hooks through its own shell executor, which drops every
* variable whose name contains `KEY`, `TOKEN`, `SECRET` or `PASSWORD` before the hook
* process starts. That is a reasonable credential-scrub default, and it is not something
* Orca can turn off — but it takes `ORCA_PANE_KEY` and `ORCA_AGENT_LAUNCH_TOKEN` with it,
* which are the two values a status hook needs to say *which pane* it is reporting for.
* process starts. Claude Code does the same for hooks when `CLAUDE_CODE_SUBPROCESS_ENV_SCRUB`
* is on (verified on 2.1.310). That is a reasonable credential-scrub default, and it is not
* something Orca can turn off — but it takes `ORCA_PANE_KEY` and `ORCA_AGENT_LAUNCH_TOKEN`
* with it, which are the two values a status hook needs to say *which pane* it is reporting for.
* Without them the hook has nothing to attribute to and exits without posting, so the pane
* reads as an agent Orca never hears from.
*
Expand Down Expand Up @@ -47,3 +48,18 @@ export function applyScrubSafeAgentEnvAliases(env: Record<string, string>): void
env[alias] = value
}
}

/** POSIX hook prologue: refill the canonical names from their aliases; never overwrites a live value. */
export function buildPosixScrubSafeEnvRestoreLines(): string[] {
return [
...ALIASES.map(([source, alias]) => `: "\${${source}:=\${${alias}:-}}"`),
`export ${ALIASES.map(([source]) => source).join(' ')}`
]
}

/** Windows `.cmd` hook prologue with the same contract. */
export function buildWindowsScrubSafeEnvRestoreLines(): string[] {
return ALIASES.map(
([source, alias]) => `if not defined ${source} if defined ${alias} set "${source}=%${alias}%"`
)
}
Loading