diff --git a/plugins/opengui/src/confirmation.ts b/plugins/opengui/src/confirmation.ts index daa20b4..ebd1b66 100644 --- a/plugins/opengui/src/confirmation.ts +++ b/plugins/opengui/src/confirmation.ts @@ -1,18 +1,22 @@ -import { execFile } from 'node:child_process' +import { execFile, type ExecFileException } from 'node:child_process' import type { ExternalSideEffect } from './codex/service.ts' export type ConfirmAction = (effect: ExternalSideEffect, args: Record, signal: AbortSignal) => Promise /** Fixed AppleScript; untrusted action descriptions are passed as argv, never code. */ -const SCRIPT = [ +export const CONFIRMATION_SCRIPT = [ 'on run argv', 'set messageText to item 1 of argv', 'try', - 'display dialog messageText with title "OpenGUI — Confirm Android action" buttons {"Cancel", "Allow once"} default button "Cancel" cancel button "Cancel" giving up after 60', - 'if gave up of result then return "cancel"', - 'return button returned of result', - 'on error', - 'return "cancel"', + 'activate', + 'set dialogReply to display dialog messageText with title "OpenGUI — Confirm Android action" buttons {"Cancel", "Allow once"} default button "Cancel" cancel button "Cancel" giving up after 60', + // AppleScript control statements can clear implicit `result`; retain the reply explicitly. + 'if gave up of dialogReply then return "timeout"', + 'return button returned of dialogReply', + 'on error errorMessage number errorNumber', + 'if errorNumber is -128 then return "cancel"', + 'if errorNumber is -1712 then return "timeout"', + 'return "unavailable"', 'end try', 'end run', ].join('\n') @@ -23,14 +27,28 @@ export const confirmAction: ConfirmAction = async (effect, args, signal) => { } export async function confirmLocalSetup(description: string, signal: AbortSignal): Promise { - if (process.platform !== 'darwin') return false signal.throwIfAborted() - return new Promise((resolve, reject) => { - execFile('/usr/bin/osascript', ['-e', SCRIPT, description], { - shell: false, signal, timeout: 65_000, maxBuffer: 8192, - }, (error, stdout) => { - if (signal.aborted) reject(signal.reason) - else resolve(error === null && stdout.trim() === 'Allow once') + if (process.platform !== 'darwin') throw new Error('opengui: confirmation UI unavailable') + let output: string + try { + output = await new Promise((resolve, reject) => { + execFile('/usr/bin/osascript', ['-e', CONFIRMATION_SCRIPT, description], { + shell: false, signal, timeout: 65_000, maxBuffer: 8192, + }, (error, stdout) => error ? reject(error) : resolve(stdout)) }) - }) + } catch (error) { + signal.throwIfAborted() + const failure = error as ExecFileException + if (failure?.killed && failure.signal === 'SIGTERM') throw new Error('opengui: confirmation timed out') + // Process errors may contain the full command and private action arguments. + throw new Error('opengui: confirmation UI unavailable') + } + signal.throwIfAborted() + switch (output.trim()) { + case 'Allow once': return true + case 'cancel': + case 'Cancel': return false + case 'timeout': throw new Error('opengui: confirmation timed out') + default: throw new Error('opengui: confirmation UI unavailable') + } } diff --git a/plugins/opengui/tests/confirmation-applescript.spec.ts b/plugins/opengui/tests/confirmation-applescript.spec.ts new file mode 100644 index 0000000..953fe65 --- /dev/null +++ b/plugins/opengui/tests/confirmation-applescript.spec.ts @@ -0,0 +1,27 @@ +import { execFileSync } from 'node:child_process' +import { describe, expect, it } from 'vitest' +import { CONFIRMATION_SCRIPT } from '../src/confirmation.ts' + +// Run actual AppleScript control flow with a synthetic dialog result. No UI or device is opened. +function runSyntheticDialog(body: string): string { + const dialogLine = CONFIRMATION_SCRIPT.split('\n').find(line => line.includes('display dialog messageText'))! + const replacement = dialogLine.startsWith('set dialogReply to ') ? 'set dialogReply to my syntheticDialog()' : 'my syntheticDialog()' + const script = `on syntheticDialog()\n${body}\nend syntheticDialog\n` + + CONFIRMATION_SCRIPT.replace('\nactivate\n', '\n').replace(dialogLine, replacement) + return execFileSync('/usr/bin/osascript', ['-e', script, 'synthetic "quoted" argument'], { + encoding: 'utf8', timeout: 5000, maxBuffer: 8192, + }).trim() +} + +describe.runIf(process.platform === 'darwin')('actual AppleScript response handling', () => { + it('preserves Allow once after checking the timeout flag', () => { + expect(runSyntheticDialog('return {button returned:"Allow once", gave up:false}')).toBe('Allow once') + }) + it('handles timeout without reading a missing button field', () => { + expect(runSyntheticDialog('return {gave up:true}')).toBe('timeout') + }) + it('keeps explicit cancellation separate from a UI error', () => { + expect(runSyntheticDialog('error number -128')).toBe('cancel') + expect(runSyntheticDialog('error "synthetic private detail" number -2753')).toBe('unavailable') + }) +}) diff --git a/plugins/opengui/tests/confirmation.spec.ts b/plugins/opengui/tests/confirmation.spec.ts new file mode 100644 index 0000000..29cea01 --- /dev/null +++ b/plugins/opengui/tests/confirmation.spec.ts @@ -0,0 +1,109 @@ +import type { ExecFileException, ExecFileOptions } from 'node:child_process' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { confirmAction, confirmLocalSetup } from '../src/confirmation.ts' + +const { run } = vi.hoisted(() => ({ + run: vi.fn<(file: string, args: string[], options: ExecFileOptions, + callback: (error: ExecFileException | null, stdout: string, stderr: string) => void) => void>(), +})) +vi.mock('node:child_process', () => ({ execFile: run })) + +const platform = Object.getOwnPropertyDescriptor(process, 'platform')! +const signal = () => new AbortController().signal +function complete(output: string, error: ExecFileException | null = null) { + run.mockImplementation((_file, _args, _options, callback) => callback(error, output, 'private stderr')) +} + +beforeEach(() => { + run.mockReset() + Object.defineProperty(process, 'platform', { ...platform, value: 'darwin' }) +}) +afterEach(() => Object.defineProperty(process, 'platform', platform)) + +describe('native confirmation', () => { + it('foregrounds the dialog and passes untrusted descriptions only as argv', async () => { + complete('Allow once\n') + const description = 'Private " text\n$(touch injected) `whoami` ; display dialog "injected"' + const requestSignal = signal() + await expect(confirmLocalSetup(description, requestSignal)).resolves.toBe(true) + expect(run).toHaveBeenCalledOnce() + const [file, args, options] = run.mock.calls[0]! + expect(file).toBe('/usr/bin/osascript') + expect(args).toHaveLength(3) + expect(args[0]).toBe('-e') + expect(args[2]).toBe(description) + const script = args[1]! + expect(script).not.toContain(description) + expect(script).toContain('activate\nset dialogReply to display dialog') + expect(script).toContain('default button "Cancel" cancel button "Cancel"') + expect(script).toContain('if errorNumber is -128 then return "cancel"') + expect(script).toContain('if gave up of dialogReply then return "timeout"') + expect(script).toContain('if errorNumber is -1712 then return "timeout"') + expect(options).toMatchObject({ shell: false, signal: requestSignal, timeout: 65_000, maxBuffer: 8192 }) + }) + + it('preserves the action context in the confirmation request', async () => { + complete('Allow once') + await expect(confirmAction('send', { text: 'Hello' }, signal())).resolves.toBe(true) + const args = run.mock.calls[0]![1] + expect(args[1]).toContain('Confirm Android action') + expect(args[2]).toContain('Allow one send action on the selected Android phone?') + expect(args[2]).toContain('"text": "Hello"') + }) + + it.each(['cancel', 'Cancel\n'])('returns false for explicit cancellation %j', async output => { + complete(output) + await expect(confirmLocalSetup('request', signal())).resolves.toBe(false) + }) + + it('reports a dialog timeout separately from user cancellation', async () => { + complete('timeout\n') + await expect(confirmLocalSetup('request', signal())).rejects.toThrow('opengui: confirmation timed out') + expect(run).toHaveBeenCalledOnce() + }) + + it('reports the child process timeout without exposing command arguments', async () => { + complete('', Object.assign(new Error('private command arguments'), { killed: true, signal: 'SIGTERM' as const })) + await expect(confirmLocalSetup('private request', signal())).rejects.toThrow(/^opengui: confirmation timed out$/) + }) + + it.each(['unavailable', '', 'Allow', 'true', 'Allow once\nprivate output'])('rejects unexpected or unavailable UI output %j', async output => { + complete(output) + await expect(confirmLocalSetup('private request', signal())).rejects.toThrow(/^opengui: confirmation UI unavailable$/) + }) + + it('redacts subprocess failures even when stdout claims approval', async () => { + complete('Allow once', Object.assign(new Error('private command arguments'), { code: 1 })) + await expect(confirmLocalSetup('private request', signal())).rejects.toThrow(/^opengui: confirmation UI unavailable$/) + expect(run).toHaveBeenCalledOnce() + }) + + it('redacts synchronous subprocess launch failures', async () => { + run.mockImplementation(() => { throw new Error('private invalid arguments') }) + await expect(confirmLocalSetup('private request', signal())).rejects.toThrow(/^opengui: confirmation UI unavailable$/) + }) + + it('preserves an existing abort reason without opening a dialog', async () => { + const controller = new AbortController() + const reason = new Error('caller cancelled') + controller.abort(reason) + await expect(confirmLocalSetup('request', controller.signal)).rejects.toBe(reason) + expect(run).not.toHaveBeenCalled() + }) + + it.each([null, new Error('process aborted')])('preserves an in-flight abort over a process result %j', async error => { + const controller = new AbortController() + const reason = new Error('caller cancelled') + run.mockImplementation((_file, _args, _options, callback) => { + controller.abort(reason) + callback(error, 'Allow once', '') + }) + await expect(confirmLocalSetup('request', controller.signal)).rejects.toBe(reason) + }) + + it('reports unsupported UI hosts without presenting them as cancellation', async () => { + Object.defineProperty(process, 'platform', { ...platform, value: 'linux' }) + await expect(confirmLocalSetup('request', signal())).rejects.toThrow(/^opengui: confirmation UI unavailable$/) + expect(run).not.toHaveBeenCalled() + }) +})