diff --git a/src/main/agent-hooks/installer-utils.ts b/src/main/agent-hooks/installer-utils.ts index ac4abee9469..19c190b12d4 100644 --- a/src/main/agent-hooks/installer-utils.ts +++ b/src/main/agent-hooks/installer-utils.ts @@ -106,7 +106,7 @@ export function getSharedManagedScriptPath(scriptFileName: string): string { return join(homedir(), '.orca', 'agent-hooks', scriptFileName) } -export { wrapPosixHookCommand } from './posix-hook-command' +export { wrapPosixHookCommand, wrapPosixHookCommandForExec } from './posix-hook-command' export function quotePowerShellString(value: string): string { return `'${value.replaceAll("'", "''")}'` diff --git a/src/main/agent-hooks/posix-hook-command.ts b/src/main/agent-hooks/posix-hook-command.ts index 23d8f1acba8..35e7ee475bd 100644 --- a/src/main/agent-hooks/posix-hook-command.ts +++ b/src/main/agent-hooks/posix-hook-command.ts @@ -31,3 +31,12 @@ export function wrapPosixHookCommand( ].join(' && ') return `if ${guards}; then ${invocation}; else ${fallback}; fi` } + +// ACP launches argv[0] directly; give the shell guard an executable interpreter. +export function wrapPosixHookCommandForExec( + scriptPath: string, + env: Record = {}, + options: { fallbackStdout?: string; requiredEnvVar?: string } = {} +): string { + return `/bin/sh -c ${quotePosixShellString(wrapPosixHookCommand(scriptPath, env, options))}` +} diff --git a/src/main/agent-hooks/posix-hook-exec-command.test-fixture.ts b/src/main/agent-hooks/posix-hook-exec-command.test-fixture.ts new file mode 100644 index 00000000000..9498e62f8fe --- /dev/null +++ b/src/main/agent-hooks/posix-hook-exec-command.test-fixture.ts @@ -0,0 +1,53 @@ +// Mirrors the ACP host's POSIX `shlex.split` so tests can exercise the exec path (#16087). +// Throws on malformed input like shlex does, so a broken quoting change fails loudly here. +export function shlexSplit(command: string): string[] { + const tokens: string[] = [] + let current = '' + let started = false + let quote: '"' | "'" | null = null + for (let index = 0; index < command.length; index += 1) { + const char = command[index] + if (char === '\\' && quote !== "'") { + const next = command[index + 1] + if (next === undefined) { + throw new Error(`shlexSplit: trailing backslash with nothing to escape: ${command}`) + } + current += quote === '"' && next !== '"' && next !== '\\' ? char + next : next + started = true + index += 1 + continue + } + if (quote === null && (char === '"' || char === "'")) { + quote = char + started = true + continue + } + if (quote === char) { + quote = null + continue + } + if (quote === null && /\s/.test(char)) { + if (started) { + tokens.push(current) + current = '' + started = false + } + continue + } + current += char + started = true + } + if (quote !== null) { + throw new Error(`shlexSplit: unbalanced ${quote} quote: ${command}`) + } + if (started) { + tokens.push(current) + } + return tokens +} + +// Unwrap the `/bin/sh -c` payload so assertions match the snippet, not its escaping. +export function posixHookInnerCommand(command: string): string { + const argv = shlexSplit(command) + return argv[0] === '/bin/sh' && argv[1] === '-c' && argv[2] !== undefined ? argv[2] : command +} diff --git a/src/main/agent-hooks/posix-hook-exec-command.test.ts b/src/main/agent-hooks/posix-hook-exec-command.test.ts new file mode 100644 index 00000000000..12ffb2f4b97 --- /dev/null +++ b/src/main/agent-hooks/posix-hook-exec-command.test.ts @@ -0,0 +1,66 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { chmodSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { runProcess } from '../../shared/child-process/run-process' +import { wrapPosixHookCommandForExec } from './posix-hook-command' +import { shlexSplit } from './posix-hook-exec-command.test-fixture' + +it('rejects incomplete command quoting', () => { + expect(() => shlexSplit("/bin/sh -c 'printf ok")).toThrow(/unbalanced/) + expect(() => shlexSplit('/bin/sh -c foo\\')).toThrow(/trailing backslash/) +}) + +it('preserves quoted backslashes and escaped single quotes', () => { + expect(shlexSplit(String.raw`cmd "a\q" 'a'\''b'`)).toEqual(['cmd', 'a\\q', "a'b"]) +}) + +describe.skipIf(process.platform === 'win32')('POSIX exec hook command', () => { + let root: string + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'orca-exec-hook-')) + }) + afterEach(() => rmSync(root, { recursive: true, force: true })) + + it.each(['present', 'missing', 'not-executable', 'missing-env'])( + 'preserves the gate response with a %s script', + async (state) => { + const script = join(root, "hook 'quoted' $literal `name` 日本語.sh") + writeFileSync(script, '#!/bin/sh\nprintf \'%s\\n\' "$ORCA_TEST_EVENT"\ncat >/dev/null\n', { + mode: 0o700 + }) + if (state === 'missing') { + rmSync(script) + } + if (state === 'not-executable') { + chmodSync(script, 0o600) + } + const response = '{"decision":"ask"}' + const command = wrapPosixHookCommandForExec( + script, + { ORCA_TEST_EVENT: 'managed hook ran' }, + { fallbackStdout: response, requiredEnvVar: 'ORCA_TEST_REQUIRED' } + ) + const [program, ...args] = shlexSplit(command) + expect(program).toBe('/bin/sh') + if (!program) { + throw new Error('missing hook program') + } + for (const route of ['exec', 'shell']) { + const result = await runProcess({ + program: route === 'exec' ? program : '/bin/sh', + args: route === 'exec' ? args : ['-c', command], + input: '{"toolCall":{"name":"read_file"}}', + env: { ...process.env, ORCA_TEST_REQUIRED: state === 'missing-env' ? '' : '1' }, + timeoutMs: 5000 + }) + expect(result).toMatchObject({ + code: 0, + stdout: `${state === 'present' ? 'managed hook ran' : response}\n`, + stderr: '', + timedOut: false + }) + } + } + ) +}) diff --git a/src/main/agent-hooks/remote-hook-service-installers.test.ts b/src/main/agent-hooks/remote-hook-service-installers.test.ts index 2e4cbb06496..fb2a7ee4498 100644 --- a/src/main/agent-hooks/remote-hook-service-installers.test.ts +++ b/src/main/agent-hooks/remote-hook-service-installers.test.ts @@ -1,6 +1,8 @@ +import { ANTIGRAVITY_EVENTS } from '../antigravity/hook-events' import { describe, expect, it, vi } from 'vitest' import { parse as parseJsonc } from 'jsonc-parser' import type { SFTPWrapper } from 'ssh2' +import { posixHookInnerCommand } from './posix-hook-exec-command.test-fixture' vi.mock('electron', () => ({ app: { @@ -348,25 +350,23 @@ describe('remote hook service installers', () => { { matcher?: string; command?: string; hooks?: { command: string }[] }[] > } - for (const eventName of ['PreInvocation', 'PostInvocation', 'Stop']) { - const command = antigravityConfig['orca-status'][eventName]?.[0]?.command + const agyHooks = antigravityConfig['orca-status'] + for (const { eventName, schema } of ANTIGRAVITY_EVENTS) { + const definition = agyHooks[eventName]?.[0] + const raw = definition?.command ?? definition?.hooks?.[0]?.command ?? '' + expect(raw).toMatch(/^\/bin\/sh -c /) + const command = posixHookInnerCommand(raw) + if (schema === 'tool') { + expect(definition?.matcher).toBe('*') + } expect(command).toContain('/home/dev/.orca/agent-hooks/antigravity-hook.sh') expect(command).toContain(`ORCA_ANTIGRAVITY_EVENT='${eventName}'`) } - for (const eventName of ['PreToolUse', 'PostToolUse']) { - const definition = antigravityConfig['orca-status'][eventName]?.[0] - const command = definition?.hooks?.[0]?.command - expect(definition?.matcher).toBe('*') - expect(command).toContain('/home/dev/.orca/agent-hooks/antigravity-hook.sh') - expect(command).toContain(`ORCA_ANTIGRAVITY_EVENT='${eventName}'`) - } - // Why: #2426 was an SSH report — a remote host missing the script must still answer the gate, not deny every tool. - expect(antigravityConfig['orca-status'].PreToolUse[0].hooks?.[0]?.command).toContain( - `printf '%s\\n' '{"decision":"ask"}'` - ) - expect(antigravityConfig['orca-status'].PostToolUse[0].hooks?.[0]?.command).not.toContain( - '{"decision"' - ) + // A remote host missing the script must still answer the gate (#2426). + const preTool = posixHookInnerCommand(agyHooks.PreToolUse[0].hooks?.[0]?.command ?? '') + const postTool = posixHookInnerCommand(agyHooks.PostToolUse[0].hooks?.[0]?.command ?? '') + expect(preTool).toContain(`printf '%s\\n' '{"decision":"ask"}'`) + expect(postTool).not.toContain('{"decision"') const ampPlugin = amp.fs.files.get('/home/dev/.config/amp/plugins/orca-agent-status.ts') expect(ampPlugin).toContain('/hook/amp') diff --git a/src/main/antigravity/hook-service.test.ts b/src/main/antigravity/hook-service.test.ts index a0deb253188..31436e09006 100644 --- a/src/main/antigravity/hook-service.test.ts +++ b/src/main/antigravity/hook-service.test.ts @@ -18,6 +18,7 @@ vi.mock('os', async () => { import { AntigravityHookService } from './hook-service' import { createManagedCommandMatcher } from '../agent-hooks/installer-utils' +import { posixHookInnerCommand } from '../agent-hooks/posix-hook-exec-command.test-fixture' const ANTIGRAVITY_SCRIPT_FILE_NAME = process.platform === 'win32' ? 'antigravity-hook.cmd' : 'antigravity-hook.sh' @@ -128,10 +129,13 @@ describe('AntigravityHookService', () => { if (process.platform === 'win32') { expect(config['orca-status'].PreInvocation[0].command).not.toContain('ORCA_ANTIGRAVITY_EVENT') } else { - expect(config['orca-status'].PreInvocation[0].command).toContain( + expect(config['orca-status'].PreToolUse[0].hooks?.[0]?.command).toMatch(/^\/bin\/sh -c /) + expect(posixHookInnerCommand(config['orca-status'].PreInvocation[0].command ?? '')).toContain( "ORCA_ANTIGRAVITY_EVENT='PreInvocation'" ) - expect(config['orca-status'].Stop[0].command).toContain("ORCA_ANTIGRAVITY_EVENT='Stop'") + expect(posixHookInnerCommand(config['orca-status'].Stop[0].command ?? '')).toContain( + "ORCA_ANTIGRAVITY_EVENT='Stop'" + ) } const script = readFileSync( diff --git a/src/main/antigravity/hook-service.ts b/src/main/antigravity/hook-service.ts index e49a522257a..3cdd8129eeb 100644 --- a/src/main/antigravity/hook-service.ts +++ b/src/main/antigravity/hook-service.ts @@ -6,7 +6,7 @@ import type { AgentHookInstallState, AgentHookInstallStatus } from '../../shared import { getSharedManagedScriptPath, readHooksJson, - wrapPosixHookCommand, + wrapPosixHookCommandForExec, wrapWindowsCmdHookCommand, writeHooksJson, writeManagedScript, @@ -60,7 +60,9 @@ function getWindowsWrapperScriptPath(event: AntigravityEvent): string { } function getPosixManagedCommand(scriptPath: string, event: AntigravityEvent): string { - return wrapPosixHookCommand( + // Why: Antigravity's ACP host execs this string as argv[0] rather than running it through a shell, + // so it must stay one spawnable token (#16087) — see wrapPosixHookCommandForExec. + return wrapPosixHookCommandForExec( scriptPath, { ORCA_ANTIGRAVITY_EVENT: event.eventName }, // Why: a missing managed script must not brick tools; the guard answers PreToolUse itself instead of staying silent.