fix(computer): bind macOS helper to supervised peer pid (#11475)

This commit is contained in:
Neil
2026-07-29 19:49:36 -07:00
committed by GitHub
parent 80c42d38c7
commit 1f2f809a11
13 changed files with 352 additions and 146 deletions
+10
View File
@@ -313,6 +313,16 @@ function verifyNativeArgumentGuardrails() {
'macOS typeText must try verified focused AX text replacement before synthetic keyboard fallback'
)
}
if (
!macos.includes('AgentLaunchArguments.parse(arguments)') ||
!macos.includes('expectedProcessId: expectedPeerProcessId') ||
macos.includes('/bin/ps') ||
macos.includes('computer-sidecar.js')
) {
failures.push(
'macOS agent authorization must bind the kernel peer pid to the main-selected launch pid'
)
}
if (!macos.includes('even when the click uses an AX action path')) {
failures.push(
'macOS clicks must activate the target window before accessibility or pointer input'
@@ -2583,12 +2583,14 @@ private final class AgentRuntime: NSObject, NSApplicationDelegate {
private let socketPath: String
private let token: String?
private let expectedPeerProcessId: pid_t
private var listener: SocketListener?
private var unclaimedSessionTimeout: DispatchWorkItem?
init(socketPath: String, token: String?) {
init(socketPath: String, token: String?, expectedPeerProcessId: pid_t) {
self.socketPath = socketPath
self.token = token
self.expectedPeerProcessId = expectedPeerProcessId
}
func applicationDidFinishLaunching(_ notification: Notification) {
@@ -2601,6 +2603,7 @@ private final class AgentRuntime: NSObject, NSApplicationDelegate {
let listener = try SocketListener(
socketPath: socketPath,
token: token,
expectedPeerProcessId: expectedPeerProcessId,
onSessionClaimed: {
DispatchQueue.main.async {
timeout.cancel()
@@ -3532,6 +3535,7 @@ private final class ButtonTarget: NSObject {
private final class SocketListener: @unchecked Sendable {
private let socketPath: String
private let token: String?
private let expectedPeerProcessId: pid_t
private let onSessionClaimed: () -> Void
private let onSessionClosed: () -> Void
private let provider = Provider()
@@ -3544,11 +3548,13 @@ private final class SocketListener: @unchecked Sendable {
init(
socketPath: String,
token: String?,
expectedPeerProcessId: pid_t,
onSessionClaimed: @escaping () -> Void,
onSessionClosed: @escaping () -> Void
) throws {
self.socketPath = socketPath
self.token = token
self.expectedPeerProcessId = expectedPeerProcessId
self.onSessionClaimed = onSessionClaimed
self.onSessionClosed = onSessionClosed
try bindSocket()
@@ -3641,7 +3647,10 @@ private final class SocketListener: @unchecked Sendable {
onSessionClosed()
}
}
let authorizedPeer = peerProcessId(fd).map(isAuthorizedAgentPeer) == true
let authorizedPeer = isAuthorizedAgentPeer(
peerProcessId: peerProcessId(fd),
expectedProcessId: expectedPeerProcessId
)
let decoder = JSONDecoder()
var registrationComplete = false
while let line = readLine(from: fd) {
@@ -3700,70 +3709,14 @@ private func peerProcessId(_ fd: Int32) -> pid_t? {
return result == 0 && pid > 0 ? pid : nil
}
private func isAuthorizedAgentPeer(_ pid: pid_t) -> Bool {
guard let command = processCommand(pid),
command.contains("/out/main/computer-sidecar.js")
|| command.contains("/Contents/Resources/app.asar.unpacked/out/main/computer-sidecar.js")
else {
return false
}
if isTrustedOrcaApplication(pid) {
return true
}
guard let parentPid = parentProcessId(pid) else { return false }
return isTrustedOrcaApplication(parentPid)
}
private func isTrustedOrcaApplication(_ pid: pid_t) -> Bool {
guard let app = NSRunningApplication(processIdentifier: pid),
let bundleId = app.bundleIdentifier
else {
return false
}
// Why: dev validation runs from per-worktree wrapper apps with stable
// Orca-owned bundle ids; the sidecar peer check must still authorize them.
return bundleId == "com.stablyai.orca" ||
bundleId.hasPrefix("com.stablyai.orca.dev.") ||
bundleId == "com.github.Electron"
}
private func parentProcessId(_ pid: pid_t) -> pid_t? {
guard let output = processField(pid: pid, field: "ppid=") else {
return nil
}
let trimmed = output.trimmingCharacters(in: .whitespacesAndNewlines)
guard let parentPid = pid_t(trimmed), parentPid > 1 else {
return nil
}
return parentPid
}
private func processCommand(_ pid: pid_t) -> String? {
return processField(pid: pid, field: "command=")
}
private func processField(pid: pid_t, field: String) -> String? {
let process = Process()
let pipe = Pipe()
process.executableURL = URL(fileURLWithPath: "/bin/ps")
process.arguments = ["-p", "\(pid)", "-o", field]
process.standardOutput = pipe
process.standardError = Pipe()
do {
try process.run()
process.waitUntilExit()
guard process.terminationStatus == 0 else { return nil }
let data = pipe.fileHandleForReading.readDataToEndOfFile()
return String(data: data, encoding: .utf8)
} catch {
return nil
}
}
@MainActor
private func runAgent(socketPath: String, token: String?) {
private func runAgent(socketPath: String, token: String?, expectedPeerProcessId: pid_t) {
let app = NSApplication.shared
let delegate = AgentRuntime(socketPath: socketPath, token: token)
let delegate = AgentRuntime(
socketPath: socketPath,
token: token,
expectedPeerProcessId: expectedPeerProcessId
)
app.delegate = delegate
// Why: SCK is reliable once this code runs as a signed app with a real TCC identity.
setenv("ORCA_COMPUTER_USE_SCK_SCREENSHOTS", "1", 1)
@@ -3896,23 +3849,23 @@ private func writeAll(_ data: Data, to fd: Int32) -> Bool {
let arguments = Array(CommandLine.arguments.dropFirst())
if arguments.first == "--agent" {
guard arguments.count >= 2 else {
fputs("usage: orca-computer-use-macos --agent <socket-path> --token-file <token-path>\n", stderr)
guard let launchArguments = AgentLaunchArguments.parse(arguments) else {
fputs("usage: orca-computer-use-macos --agent <socket-path> --token-file <token-path> --peer-pid <pid>\n", stderr)
exit(2)
}
let tokenFileIndex = arguments.firstIndex(of: "--token-file")
let token = tokenFileIndex.flatMap { index -> String? in
let valueIndex = index + 1
guard valueIndex < arguments.count else { return nil }
let tokenPath = arguments[valueIndex]
return try? String(contentsOfFile: tokenPath, encoding: .utf8)
.trimmingCharacters(in: .whitespacesAndNewlines)
}
let token = try? String(
contentsOfFile: launchArguments.tokenFilePath,
encoding: .utf8
).trimmingCharacters(in: .whitespacesAndNewlines)
guard let token, !token.isEmpty else {
fputs("orca-computer-use-macos --agent requires a non-empty --token-file\n", stderr)
exit(2)
}
runAgent(socketPath: arguments[1], token: token)
runAgent(
socketPath: launchArguments.socketPath,
token: token,
expectedPeerProcessId: launchArguments.expectedPeerProcessId
)
} else if arguments.first == "--permissions" {
runPermissionCheck()
} else if arguments.first == "--permission" {
@@ -0,0 +1,31 @@
public struct AgentLaunchArguments: Equatable, Sendable {
public let socketPath: String
public let tokenFilePath: String
public let expectedPeerProcessId: Int32
public static func parse(_ arguments: [String]) -> AgentLaunchArguments? {
guard arguments.count == 6,
arguments[0] == "--agent",
!arguments[1].isEmpty,
arguments[2] == "--token-file",
!arguments[3].isEmpty,
arguments[4] == "--peer-pid",
let expectedPeerProcessId = Int32(arguments[5]),
expectedPeerProcessId > 0
else {
return nil
}
return AgentLaunchArguments(
socketPath: arguments[1],
tokenFilePath: arguments[3],
expectedPeerProcessId: expectedPeerProcessId
)
}
}
public func isAuthorizedAgentPeer(
peerProcessId: Int32?,
expectedProcessId: Int32
) -> Bool {
expectedProcessId > 0 && peerProcessId == expectedProcessId
}
@@ -34,6 +34,20 @@ final class AgentEntrypointSourceSafetyTests: XCTestCase {
XCTAssertTrue(source.contains("event.flags = flags\n event.postToPid(pid)"))
}
func testAgentUsesExactKernelPeerPidWithoutProcessInspection() throws {
let source = try agentEntrypointSource()
XCTAssertTrue(source.contains(
"""
let authorizedPeer = isAuthorizedAgentPeer(
peerProcessId: peerProcessId(fd),
expectedProcessId: expectedPeerProcessId
"""
))
XCTAssertFalse(source.contains("/bin/ps"))
XCTAssertFalse(source.contains("computer-sidecar.js"))
}
private func agentEntrypointSource() throws -> String {
let testFile = URL(fileURLWithPath: #filePath)
let packageRoot = testFile
@@ -0,0 +1,52 @@
import XCTest
@testable import OrcaComputerUseMacOSCore
final class AgentPeerAuthorizationTests: XCTestCase {
func testParsesExactAgentArguments() {
XCTAssertEqual(
AgentLaunchArguments.parse([
"--agent",
"/private/tmp/provider.sock",
"--token-file",
"/private/tmp/provider.token",
"--peer-pid",
"4321",
]),
AgentLaunchArguments(
socketPath: "/private/tmp/provider.sock",
tokenFilePath: "/private/tmp/provider.token",
expectedPeerProcessId: 4321
)
)
}
func testRejectsMissingReorderedOrExtraAgentArguments() {
XCTAssertNil(AgentLaunchArguments.parse([
"--agent", "/tmp/provider.sock", "--token-file", "/tmp/provider.token",
]))
XCTAssertNil(AgentLaunchArguments.parse([
"--agent", "/tmp/provider.sock", "--peer-pid", "4321",
"--token-file", "/tmp/provider.token",
]))
XCTAssertNil(AgentLaunchArguments.parse([
"--agent", "/tmp/provider.sock", "--token-file", "/tmp/provider.token",
"--peer-pid", "4321", "--extra",
]))
}
func testRejectsInvalidPeerProcessIds() {
for value in ["", "0", "-1", "not-a-pid", "2147483648"] {
XCTAssertNil(AgentLaunchArguments.parse([
"--agent", "/tmp/provider.sock", "--token-file", "/tmp/provider.token",
"--peer-pid", value,
]))
}
}
func testAuthorizesOnlyTheExpectedPeerProcess() {
XCTAssertTrue(isAuthorizedAgentPeer(peerProcessId: 4321, expectedProcessId: 4321))
XCTAssertFalse(isAuthorizedAgentPeer(peerProcessId: 4322, expectedProcessId: 4321))
XCTAssertFalse(isAuthorizedAgentPeer(peerProcessId: nil, expectedProcessId: 4321))
XCTAssertFalse(isAuthorizedAgentPeer(peerProcessId: 4321, expectedProcessId: 0))
}
}
@@ -27,7 +27,7 @@ describe('ComputerProviderSupervisorHost', () => {
const macOS = macOSSupervisor()
const host = new ComputerProviderSupervisorHost(macOS as never)
const sent: unknown[] = []
host.attach((message) => sent.push(message))
host.attach((message) => sent.push(message), 4321)
expect(
host.handle({
@@ -40,7 +40,7 @@ describe('ComputerProviderSupervisorHost', () => {
).toBe(true)
await vi.waitFor(() => expect(sent).toHaveLength(1))
expect(macOS.start).toHaveBeenCalledTimes(1)
expect(macOS.start).toHaveBeenCalledWith(4321)
expect(sent).toEqual([
{
channel: COMPUTER_PROVIDER_SUPERVISOR_CHANNEL,
@@ -61,7 +61,7 @@ describe('ComputerProviderSupervisorHost', () => {
const desktop = desktopSupervisor()
const host = new ComputerProviderSupervisorHost(macOS as never, desktop as never)
const sent: unknown[] = []
host.attach((message) => sent.push(message))
host.attach((message) => sent.push(message), 4321)
expect(
host.handle({
@@ -112,7 +112,7 @@ describe('ComputerProviderSupervisorHost', () => {
const host = new ComputerProviderSupervisorHost(macOSSupervisor() as never, desktop as never)
const originalSend = vi.fn()
const replacementSend = vi.fn()
host.attach(originalSend)
host.attach(originalSend, 4321)
host.handle({
channel: COMPUTER_PROVIDER_SUPERVISOR_CHANNEL,
kind: 'request',
@@ -122,7 +122,7 @@ describe('ComputerProviderSupervisorHost', () => {
})
host.shutdown()
host.attach(replacementSend)
host.attach(replacementSend, 5432)
resolveExecution({ stdout: '{"ok":true}', stderr: '', error: null })
await new Promise<void>((resolve) => setImmediate(resolve))
@@ -130,6 +130,28 @@ describe('ComputerProviderSupervisorHost', () => {
expect(replacementSend).not.toHaveBeenCalled()
})
it('uses the replacement owner pid for a restarted macOS helper', async () => {
const macOS = macOSSupervisor()
const host = new ComputerProviderSupervisorHost(macOS as never)
const replacementSend = vi.fn()
host.attach(vi.fn(), 4321)
host.shutdown()
host.attach(replacementSend, 5432)
expect(
host.handle({
channel: COMPUTER_PROVIDER_SUPERVISOR_CHANNEL,
kind: 'request',
id: 5,
method: 'macos.start',
params: {}
})
).toBe(true)
await vi.waitFor(() => expect(replacementSend).toHaveBeenCalledTimes(1))
expect(macOS.start).toHaveBeenCalledWith(5432)
})
it('rejects arbitrary executable and argument fields at the protocol boundary', () => {
const macOS = macOSSupervisor()
const host = new ComputerProviderSupervisorHost(macOS as never)
@@ -140,7 +162,7 @@ describe('ComputerProviderSupervisorHost', () => {
kind: 'request',
id: 1,
method: 'macos.start',
params: { executablePath: '/tmp/untrusted', args: ['--anything'] }
params: { executablePath: '/tmp/untrusted', args: ['--anything'], peerPid: 9999 }
})
).toBe(false)
expect(macOS.start).not.toHaveBeenCalled()
@@ -151,7 +173,7 @@ describe('ComputerProviderSupervisorHost', () => {
const desktop = desktopSupervisor()
const host = new ComputerProviderSupervisorHost(macOS as never, desktop as never)
const send = vi.fn()
host.attach(send)
host.attach(send, 4321)
host.shutdown()
@@ -169,4 +191,15 @@ describe('ComputerProviderSupervisorHost', () => {
expect(macOS.start).not.toHaveBeenCalled()
expect(send).not.toHaveBeenCalled()
})
it.each([0, -1, 1.5, Number.NaN, 0x80000000])(
'rejects invalid owner pid %s before accepting requests',
(ownerProcessId) => {
const host = new ComputerProviderSupervisorHost(macOSSupervisor() as never)
expect(() => host.attach(vi.fn(), ownerProcessId)).toThrow(
'owner process did not report a valid pid'
)
}
)
})
@@ -15,6 +15,7 @@ type SupervisorResponsePayload =
export class ComputerProviderSupervisorHost {
private sender: SupervisorMessageSender | null = null
private ownerProcessId: number | null = null
private ownerGeneration = 0
private readonly macOS: MacOSNativeProviderSupervisor
private readonly desktop: DesktopScriptProviderSupervisor
@@ -24,17 +25,26 @@ export class ComputerProviderSupervisorHost {
this.desktop = desktop ?? new DesktopScriptProviderSupervisor()
}
attach(sender: SupervisorMessageSender): void {
attach(sender: SupervisorMessageSender, ownerProcessId: number): void {
if (!Number.isInteger(ownerProcessId) || ownerProcessId <= 0 || ownerProcessId > 0x7fffffff) {
throw new Error('computer provider owner process did not report a valid pid')
}
this.ownerGeneration++
this.sender = sender
this.ownerProcessId = ownerProcessId
}
handle(message: unknown): boolean {
if (!this.sender || !isComputerProviderSupervisorRequest(message)) {
if (
!this.sender ||
this.ownerProcessId === null ||
!isComputerProviderSupervisorRequest(message)
) {
return false
}
const ownerGeneration = this.ownerGeneration
void this.dispatch(message).then(
const ownerProcessId = this.ownerProcessId
void this.dispatch(message, ownerProcessId).then(
(result) => this.send(ownerGeneration, { id: message.id, ok: true, result }),
(error) =>
this.send(ownerGeneration, {
@@ -49,14 +59,18 @@ export class ComputerProviderSupervisorHost {
shutdown(): void {
this.ownerGeneration++
this.sender = null
this.ownerProcessId = null
this.macOS.shutdown()
this.desktop.shutdown()
}
private async dispatch(request: ComputerProviderSupervisorRequest): Promise<unknown> {
private async dispatch(
request: ComputerProviderSupervisorRequest,
ownerProcessId: number
): Promise<unknown> {
switch (request.method) {
case 'macos.start':
return this.macOS.start()
return this.macOS.start(ownerProcessId)
case 'macos.claim':
this.macOS.claim(request.params.sessionId)
return { claimed: true }
@@ -18,6 +18,20 @@ function desktopRequest(request: Record<string, unknown>): Record<string, unknow
}
describe('computer provider supervisor protocol', () => {
it('does not let the sidecar choose the macOS helper peer pid', () => {
const request = {
channel: COMPUTER_PROVIDER_SUPERVISOR_CHANNEL,
kind: 'request',
id: 1,
method: 'macos.start'
}
expect(isComputerProviderSupervisorRequest({ ...request, params: {} })).toBe(true)
expect(isComputerProviderSupervisorRequest({ ...request, params: { peerPid: 4321 } })).toBe(
false
)
})
it('accepts exact desktop tool requests with validated nested element state', () => {
expect(
isComputerProviderSupervisorRequest(
+26 -33
View File
@@ -2,6 +2,7 @@ import { fork, type ChildProcess } from 'node:child_process'
import { join } from 'node:path'
import { ComputerProviderSupervisorHost } from './computer-provider-supervisor-host'
import type { ComputerProviderSupervisorMessage } from './computer-provider-supervisor-protocol'
import { terminateComputerSidecarChild } from './computer-sidecar-termination'
import { RuntimeClientError } from './runtime-client-error'
export type ComputerSidecarMethod =
@@ -36,38 +37,10 @@ type PendingRequest = {
}
const REQUEST_TIMEOUT_MS = 60_000
export const COMPUTER_SIDECAR_FORCE_KILL_GRACE_MS = 5_000
// Why: stale children need an error listener without retaining their former owner.
function ignoreStaleChildError(): void {}
function terminateSidecarChild(child: ChildProcess): void {
let exited = false
let forceKillTimer: NodeJS.Timeout | null = null
const onExit = (): void => {
exited = true
if (forceKillTimer) {
clearTimeout(forceKillTimer)
forceKillTimer = null
}
}
child.once('exit', onExit)
try {
child.kill('SIGTERM')
} catch {}
if (exited) {
return
}
forceKillTimer = setTimeout(() => {
forceKillTimer = null
child.off('exit', onExit)
try {
child.kill('SIGKILL')
} catch {}
}, COMPUTER_SIDECAR_FORCE_KILL_GRACE_MS)
forceKillTimer.unref()
}
export class ComputerSidecarProcess {
private child: ChildProcess | null = null
private childListenerCleanup: (() => void) | null = null
@@ -116,12 +89,17 @@ export class ComputerSidecarProcess {
this.pending.delete(id)
}
if (child) {
terminateSidecarChild(child)
terminateComputerSidecarChild(child)
}
}
private send(method: ComputerSidecarMethod, params: unknown): Promise<unknown> {
const child = this.ensureStarted()
let child: ChildProcess
try {
child = this.ensureStarted()
} catch (error) {
return Promise.reject(error)
}
if (!child.send) {
const error = new RuntimeClientError(
'accessibility_error',
@@ -196,8 +174,23 @@ export class ComputerSidecarProcess {
child.on('error', ignoreStaleChildError)
}
this.child = child
this.providerSupervisor.attach((message) =>
this.sendComputerProviderSupervisorMessage(child, message)
const childProcessId = child.pid
if (
typeof childProcessId !== 'number' ||
!Number.isInteger(childProcessId) ||
childProcessId <= 0 ||
childProcessId > 0x7fffffff
) {
const error = new RuntimeClientError(
'accessibility_error',
'computer sidecar process did not report a valid pid'
)
this.failActiveChild(child, error)
throw error
}
this.providerSupervisor.attach(
(message) => this.sendComputerProviderSupervisorMessage(child, message),
childProcessId
)
return child
}
@@ -250,7 +243,7 @@ export class ComputerSidecarProcess {
this.child = null
this.queueGeneration++
this.providerSupervisor.shutdown()
terminateSidecarChild(child)
terminateComputerSidecarChild(child)
this.rejectPending(error)
}
@@ -0,0 +1,30 @@
import type { ChildProcess } from 'node:child_process'
export const COMPUTER_SIDECAR_FORCE_KILL_GRACE_MS = 5_000
export function terminateComputerSidecarChild(child: ChildProcess): void {
let exited = false
let forceKillTimer: NodeJS.Timeout | null = null
const onExit = (): void => {
exited = true
if (forceKillTimer) {
clearTimeout(forceKillTimer)
forceKillTimer = null
}
}
child.once('exit', onExit)
try {
child.kill('SIGTERM')
} catch {}
if (exited) {
return
}
forceKillTimer = setTimeout(() => {
forceKillTimer = null
child.off('exit', onExit)
try {
child.kill('SIGKILL')
} catch {}
}, COMPUTER_SIDECAR_FORCE_KILL_GRACE_MS)
forceKillTimer.unref()
}
@@ -48,7 +48,7 @@ describe('MacOSNativeProviderSupervisor', () => {
it('registers the helper pid and private session state before returning', () => {
const supervisor = new MacOSNativeProviderSupervisor((event) => events.push(event), deps)
const started = supervisor.start()
const started = supervisor.start(7654)
expect(started).toEqual({
sessionId: 'uuid-2',
@@ -61,7 +61,9 @@ describe('MacOSNativeProviderSupervisor', () => {
'--agent',
join('/private/tmp/orca-computer-use-session', 'provider.sock'),
'--token-file',
join('/private/tmp/orca-computer-use-session', 'provider.token')
join('/private/tmp/orca-computer-use-session', 'provider.token'),
'--peer-pid',
'7654'
],
{ detached: true, stdio: 'ignore' }
)
@@ -77,7 +79,7 @@ describe('MacOSNativeProviderSupervisor', () => {
it('claims a live session and removes only its token file', async () => {
const supervisor = new MacOSNativeProviderSupervisor((event) => events.push(event), deps)
const started = supervisor.start()
const started = supervisor.start(7654)
supervisor.claim(started.sessionId)
await vi.advanceTimersByTimeAsync(MACOS_HELPER_CLAIM_TIMEOUT_MS)
@@ -92,7 +94,7 @@ describe('MacOSNativeProviderSupervisor', () => {
it('reaps an unclaimed helper and removes private state on the parent deadline', async () => {
const supervisor = new MacOSNativeProviderSupervisor((event) => events.push(event), deps)
supervisor.start()
supervisor.start(7654)
await vi.advanceTimersByTimeAsync(MACOS_HELPER_CLAIM_TIMEOUT_MS)
@@ -118,7 +120,7 @@ describe('MacOSNativeProviderSupervisor', () => {
it('releases a claimed session with graceful-to-force escalation', async () => {
const supervisor = new MacOSNativeProviderSupervisor((event) => events.push(event), deps)
const started = supervisor.start()
const started = supervisor.start(7654)
supervisor.claim(started.sessionId)
supervisor.release(started.sessionId)
@@ -147,8 +149,8 @@ describe('MacOSNativeProviderSupervisor', () => {
.mockReturnValueOnce('/private/tmp/orca-computer-use-first')
.mockReturnValueOnce('/private/tmp/orca-computer-use-second')
const supervisor = new MacOSNativeProviderSupervisor((event) => events.push(event), deps)
supervisor.start()
supervisor.start()
supervisor.start(7654)
supervisor.start(7654)
supervisor.shutdown()
@@ -172,7 +174,7 @@ describe('MacOSNativeProviderSupervisor', () => {
it('reports unexpected helper exit and removes its private directory', () => {
const supervisor = new MacOSNativeProviderSupervisor((event) => events.push(event), deps)
supervisor.start()
supervisor.start(7654)
child.emit('exit', 13, null)
@@ -190,7 +192,7 @@ describe('MacOSNativeProviderSupervisor', () => {
it('reports a registered helper error and retains ownership until exit', () => {
const supervisor = new MacOSNativeProviderSupervisor((event) => events.push(event), deps)
const started = supervisor.start()
const started = supervisor.start(7654)
child.emit('error', new Error('helper handle failed'))
@@ -212,7 +214,7 @@ describe('MacOSNativeProviderSupervisor', () => {
child.pid = undefined
const supervisor = new MacOSNativeProviderSupervisor((event) => events.push(event), deps)
expect(() => supervisor.start()).toThrow('helper process did not report a pid')
expect(() => supervisor.start(7654)).toThrow('helper process did not report a pid')
expect(child.kill).toHaveBeenCalledWith('SIGKILL')
expect(deps.rmSync).toHaveBeenCalledWith('/private/tmp/orca-computer-use-session', {
@@ -229,7 +231,7 @@ describe('MacOSNativeProviderSupervisor', () => {
})
const supervisor = new MacOSNativeProviderSupervisor((event) => events.push(event), deps)
expect(() => supervisor.start()).toThrow('token write failed')
expect(() => supervisor.start(7654)).toThrow('token write failed')
expect(deps.spawn).not.toHaveBeenCalled()
expect(deps.rmSync).toHaveBeenCalledWith('/private/tmp/orca-computer-use-session', {
@@ -237,4 +239,18 @@ describe('MacOSNativeProviderSupervisor', () => {
force: true
})
})
it.each([0, -1, 1.5, Number.NaN, 0x80000000])(
'rejects invalid expected peer pid %s before creating private state',
(expectedPeerProcessId) => {
const supervisor = new MacOSNativeProviderSupervisor((event) => events.push(event), deps)
expect(() => supervisor.start(expectedPeerProcessId)).toThrow(
'owner process did not report a valid pid'
)
expect(deps.mkdtempSync).not.toHaveBeenCalled()
expect(deps.spawn).not.toHaveBeenCalled()
expect(events).toEqual([])
}
)
})
@@ -41,6 +41,10 @@ export type MacOSNativeProviderSupervisorDeps = {
function ignoreUnownedChildError(): void {}
function isValidPeerProcessId(pid: number): boolean {
return Number.isInteger(pid) && pid > 0 && pid <= 0x7fffffff
}
export class MacOSNativeProviderSupervisor {
private readonly sessions = new Map<string, SupervisedMacOSProviderSession>()
private readonly deps: MacOSNativeProviderSupervisorDeps
@@ -52,7 +56,13 @@ export class MacOSNativeProviderSupervisor {
this.deps = deps ?? createDefaultDeps()
}
start(): StartedSupervisedMacOSProvider {
start(expectedPeerProcessId: number): StartedSupervisedMacOSProvider {
if (!isValidPeerProcessId(expectedPeerProcessId)) {
throw new RuntimeClientError(
'accessibility_error',
'computer provider owner process did not report a valid pid'
)
}
const executablePath = this.deps.resolveExecutablePath()
if (!executablePath) {
throw new RuntimeClientError('accessibility_error', 'Orca Computer Use.app was not found')
@@ -69,7 +79,14 @@ export class MacOSNativeProviderSupervisor {
this.deps.writeFileSync(socketTokenPath, socketToken, { encoding: 'utf8', mode: 0o600 })
child = this.deps.spawn(
executablePath,
['--agent', socketPath, '--token-file', socketTokenPath],
[
'--agent',
socketPath,
'--token-file',
socketTokenPath,
'--peer-pid',
String(expectedPeerProcessId)
],
{ detached: true, stdio: 'ignore' }
)
child.on('error', ignoreUnownedChildError)
+42 -13
View File
@@ -10,17 +10,23 @@ import {
callComputerSidecarCapabilities,
resetComputerSidecarForTest
} from './sidecar-client'
import { COMPUTER_SIDECAR_FORCE_KILL_GRACE_MS } from './computer-sidecar-process'
import { COMPUTER_SIDECAR_FORCE_KILL_GRACE_MS } from './computer-sidecar-termination'
const { forkMock, supervisorHandleMock, supervisorSenderState, supervisorShutdownMock } =
vi.hoisted(() => ({
forkMock: vi.fn(),
supervisorHandleMock: vi.fn(() => false),
supervisorSenderState: {
current: null as null | ((message: Record<string, unknown>) => void)
},
supervisorShutdownMock: vi.fn()
}))
const {
forkMock,
supervisorAttachedPids,
supervisorHandleMock,
supervisorSenderState,
supervisorShutdownMock
} = vi.hoisted(() => ({
forkMock: vi.fn(),
supervisorAttachedPids: [] as number[],
supervisorHandleMock: vi.fn(() => false),
supervisorSenderState: {
current: null as null | ((message: Record<string, unknown>) => void)
},
supervisorShutdownMock: vi.fn()
}))
vi.mock('child_process', () => ({
fork: forkMock
@@ -28,8 +34,9 @@ vi.mock('child_process', () => ({
vi.mock('./computer-provider-supervisor-host', () => ({
ComputerProviderSupervisorHost: class {
attach(sender: (message: Record<string, unknown>) => void): void {
attach(sender: (message: Record<string, unknown>) => void, ownerProcessId: number): void {
supervisorSenderState.current = sender
supervisorAttachedPids.push(ownerProcessId)
}
handle = supervisorHandleMock
shutdown(): void {
@@ -46,6 +53,10 @@ type SentRequest = {
}
class FakeChildProcess extends EventEmitter {
constructor(readonly pid: number | undefined) {
super()
}
killed = false
killSignals: NodeJS.Signals[] = []
sent: SentRequest[] = []
@@ -83,9 +94,10 @@ describe('computer sidecar client', () => {
children.length = 0
deferNextSendCallback = false
supervisorSenderState.current = null
supervisorAttachedPids.length = 0
supervisorHandleMock.mockReturnValue(false)
forkMock.mockImplementation(() => {
const child = new FakeChildProcess()
const child = new FakeChildProcess(4321 + children.length)
child.deferSendCallback = deferNextSendCallback
deferNextSendCallback = false
children.push(child)
@@ -107,6 +119,7 @@ describe('computer sidecar client', () => {
'computer sidecar capabilities timed out'
)
const firstChild = children[0]!
expect(supervisorAttachedPids).toEqual([4321])
await vi.advanceTimersByTimeAsync(60_000)
await firstRejection
@@ -154,6 +167,7 @@ describe('computer sidecar client', () => {
const secondCall = callComputerSidecarCapabilities()
void secondCall.catch(() => undefined)
expect(children).toHaveLength(2)
expect(supervisorAttachedPids).toEqual([4321, 4322])
const secondChild = children[1]!
const secondRequest = secondChild.sent[0]!
@@ -375,7 +389,7 @@ describe('computer sidecar client', () => {
it('fails immediately when the forked sidecar has no IPC send channel', async () => {
forkMock.mockImplementationOnce(() => {
const child = new FakeChildProcess()
const child = new FakeChildProcess(4321)
child.send = undefined
children.push(child)
return child
@@ -390,6 +404,21 @@ describe('computer sidecar client', () => {
expect(children[0]!.listenerCount('error')).toBe(1)
})
it('rejects and reaps a forked sidecar without a valid pid', async () => {
forkMock.mockImplementationOnce(() => {
const child = new FakeChildProcess(undefined)
children.push(child)
return child
})
await expect(callComputerSidecarCapabilities()).rejects.toThrow(
'computer sidecar process did not report a valid pid'
)
expect(children[0]!.killed).toBe(true)
expect(supervisorAttachedPids).toEqual([])
})
it('force-kills a sidecar that does not exit after the grace period', async () => {
const call = callComputerSidecarCapabilities()
const rejection = expect(call).rejects.toThrow('computer sidecar shut down')