From e51aa13180dd2119c0e95876de5c9317ff7de6a4 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 4 Sep 2026 16:29:08 -0400 Subject: [PATCH] fix(runtime): authorize opcode-18 query replies against the authenticated device token Both binary query-reply paths called isAcceptableTerminalQueryReplyFrame without connectionClientId, so the guard fell back to the client-DECLARED client.id and one paired phone could author a reply as another. Thread the RpcContext clientId through TerminalSubscriptionArgs and TerminalMultiplexConnectionBase as a distinct connectionClientId, pass it at both binary sites, and make the guard field required so a new call site cannot silently degrade. Also covers browser.tabCreate file: confinement over RPC for a paired mobile device. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- ...browser-tab-create-paired-file-url.test.ts | 33 +++++ .../terminal-legacy-binary-control-frames.ts | 9 +- .../terminal-legacy-subscription-types.ts | 3 + .../terminal/terminal-multiplex-connection.ts | 2 + .../terminal-multiplex-input-frame.ts | 3 +- .../terminal/terminal-multiplex-method.ts | 3 +- .../terminal/terminal-query-reply-guard.ts | 7 +- .../terminal/terminal-subscribe-method.ts | 10 +- .../rpc/terminal-multiplex-test-harness.ts | 4 +- .../terminal-query-reply-negotiation.test.ts | 115 +++++++++++++++++- 10 files changed, 181 insertions(+), 8 deletions(-) diff --git a/src/main/runtime/browser-tab-create-paired-file-url.test.ts b/src/main/runtime/browser-tab-create-paired-file-url.test.ts index f2e966696ad..82c6f436e0b 100644 --- a/src/main/runtime/browser-tab-create-paired-file-url.test.ts +++ b/src/main/runtime/browser-tab-create-paired-file-url.test.ts @@ -8,6 +8,8 @@ import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' import { OrcaRuntimeService } from './orca-runtime' import { setRuntimeBrowserCommandsFactory } from './runtime-browser-commands-factory' +import { RpcDispatcher } from './rpc/dispatcher' +import { BROWSER_CORE_METHODS } from './rpc/methods/browser-core' const { browserSessionRegistryMock } = vi.hoisted(() => ({ browserSessionRegistryMock: { @@ -144,6 +146,37 @@ describe('browser.tabCreate file: URLs from a paired client', () => { expect(createTab).not.toHaveBeenCalled() }) + // Why: the shell-side confinement is page code; a paired client that is not this shell must still be fenced. + it('refuses file:///etc/passwd dispatched over RPC by a paired mobile device', async () => { + const { runtime, createTab } = createRuntime({ + id: WT, + path: WORKTREE_PATH + }) + const dispatcher = new RpcDispatcher({ runtime, methods: BROWSER_CORE_METHODS }) + const replies: string[] = [] + await dispatcher.dispatchStreaming( + { + id: 'req-1', + authToken: 'tok', + method: 'browser.tabCreate', + params: { + worktree: `id:${WT}`, + page: 'page-new', + url: 'file:///etc/passwd', + activate: true, + navigation: 'caller' + } + }, + (reply) => replies.push(reply), + { pairedDeviceId: 'device-1', clientKind: 'mobile' } + ) + expect(JSON.parse(replies[0]!)).toMatchObject({ + ok: false, + error: { message: expect.stringMatching(/outside the requested workspace/) } + }) + expect(createTab).not.toHaveBeenCalled() + }) + it('still opens an HTML artifact inside the workspace, the native file-tap feature', async () => { const { runtime, createTab } = createRuntime({ id: WT, diff --git a/src/main/runtime/rpc/methods/terminal/terminal-legacy-binary-control-frames.ts b/src/main/runtime/rpc/methods/terminal/terminal-legacy-binary-control-frames.ts index 5c4ffef70cc..eb6ada1c890 100644 --- a/src/main/runtime/rpc/methods/terminal/terminal-legacy-binary-control-frames.ts +++ b/src/main/runtime/rpc/methods/terminal/terminal-legacy-binary-control-frames.ts @@ -30,6 +30,7 @@ export function registerLegacyBinaryControlFrames( registerBinaryStreamHandler, ptyId, clientId, + connectionClientId, isMobile, supportsDesktopViewportClaims, supportsQueryReply, @@ -61,7 +62,13 @@ export function registerLegacyBinaryControlFrames( // Why: opcode 18 skips the mobile input floor, so it needs every guard terminal.send applies; drop otherwise. if ( isQueryReply && - !isAcceptableTerminalQueryReplyFrame({ runtime, ptyId, text, client: params.client }) + !isAcceptableTerminalQueryReplyFrame({ + runtime, + ptyId, + text, + client: params.client, + connectionClientId + }) ) { return } diff --git a/src/main/runtime/rpc/methods/terminal/terminal-legacy-subscription-types.ts b/src/main/runtime/rpc/methods/terminal/terminal-legacy-subscription-types.ts index 652d7c2e506..d335c755106 100644 --- a/src/main/runtime/rpc/methods/terminal/terminal-legacy-subscription-types.ts +++ b/src/main/runtime/rpc/methods/terminal/terminal-legacy-subscription-types.ts @@ -19,7 +19,10 @@ export type TerminalSubscriptionArgs = { signal: RpcContext['signal'] emit: TerminalSubscriptionEmit ptyId: string + /** Client-declared id from `params.client`; never an authenticated identity. */ clientId: string | undefined + /** Authenticated device token carried by the transport; undefined for in-process/desktop callers. */ + connectionClientId: string | undefined isMobile: boolean supportsDesktopViewportClaims: boolean supportsQueryReply: boolean diff --git a/src/main/runtime/rpc/methods/terminal/terminal-multiplex-connection.ts b/src/main/runtime/rpc/methods/terminal/terminal-multiplex-connection.ts index f3b48a755f0..d9871c4a5dd 100644 --- a/src/main/runtime/rpc/methods/terminal/terminal-multiplex-connection.ts +++ b/src/main/runtime/rpc/methods/terminal/terminal-multiplex-connection.ts @@ -20,6 +20,8 @@ export type MultiplexEmit = (result: unknown) => void export type TerminalMultiplexConnectionBase = { runtime: RpcContext['runtime'] connectionId: string + /** Authenticated device token for this socket; undefined for in-process/desktop callers. */ + connectionClientId: string | undefined sendBinary: NonNullable registerBinaryStreamHandler: NonNullable signal: RpcContext['signal'] diff --git a/src/main/runtime/rpc/methods/terminal/terminal-multiplex-input-frame.ts b/src/main/runtime/rpc/methods/terminal/terminal-multiplex-input-frame.ts index 21ecf9372cb..45dcf6a9827 100644 --- a/src/main/runtime/rpc/methods/terminal/terminal-multiplex-input-frame.ts +++ b/src/main/runtime/rpc/methods/terminal/terminal-multiplex-input-frame.ts @@ -45,7 +45,8 @@ export function handleMultiplexInputFrame( runtime, ptyId: stream.ptyId, text, - client: stream.client + client: stream.client, + connectionClientId: state.connectionClientId }) ) { return diff --git a/src/main/runtime/rpc/methods/terminal/terminal-multiplex-method.ts b/src/main/runtime/rpc/methods/terminal/terminal-multiplex-method.ts index 11fd0c4d039..b59b062a55f 100644 --- a/src/main/runtime/rpc/methods/terminal/terminal-multiplex-method.ts +++ b/src/main/runtime/rpc/methods/terminal/terminal-multiplex-method.ts @@ -17,7 +17,7 @@ export const TERMINAL_MULTIPLEX_METHODS: RpcAnyMethod[] = [ params: TerminalMultiplex, handler: async ( _params, - { runtime, connectionId, sendBinary, registerBinaryStreamHandler, signal }, + { runtime, connectionId, clientId, sendBinary, registerBinaryStreamHandler, signal }, emit ) => { if (!sendBinary || !registerBinaryStreamHandler || !connectionId) { @@ -31,6 +31,7 @@ export const TERMINAL_MULTIPLEX_METHODS: RpcAnyMethod[] = [ const state: TerminalMultiplexConnectionBase = { runtime, connectionId, + connectionClientId: clientId, sendBinary, registerBinaryStreamHandler, signal, diff --git a/src/main/runtime/rpc/methods/terminal/terminal-query-reply-guard.ts b/src/main/runtime/rpc/methods/terminal/terminal-query-reply-guard.ts index 9fc6160b3ad..2c97fd0885c 100644 --- a/src/main/runtime/rpc/methods/terminal/terminal-query-reply-guard.ts +++ b/src/main/runtime/rpc/methods/terminal/terminal-query-reply-guard.ts @@ -5,8 +5,11 @@ import type { TerminalViewportClient } from './terminal-stream-types' type TerminalQueryReplyIdentity = { text: string | undefined client: TerminalViewportClient | undefined - /** Authenticated device token when the transport carries one; the declared client id must match it. */ - connectionClientId?: string | undefined + /** + * Authenticated device token when the transport carries one; the declared client id must match it. + * Required (not optional) so a new call site cannot silently fall back to the declared id. + */ + connectionClientId: string | undefined } /** diff --git a/src/main/runtime/rpc/methods/terminal/terminal-subscribe-method.ts b/src/main/runtime/rpc/methods/terminal/terminal-subscribe-method.ts index d3e6dba971e..1695f9ffa90 100644 --- a/src/main/runtime/rpc/methods/terminal/terminal-subscribe-method.ts +++ b/src/main/runtime/rpc/methods/terminal/terminal-subscribe-method.ts @@ -15,7 +15,14 @@ export const TERMINAL_SUBSCRIBE_METHODS: RpcAnyMethod[] = [ params: TerminalSubscribe, handler: async ( params, - { runtime, connectionId, sendBinary, registerBinaryStreamHandler, signal }, + { + runtime, + connectionId, + clientId: connectionClientId, + sendBinary, + registerBinaryStreamHandler, + signal + }, emit ) => { let leaf = runtime.resolveLeafForHandle(params.terminal) @@ -70,6 +77,7 @@ export const TERMINAL_SUBSCRIBE_METHODS: RpcAnyMethod[] = [ emit, ptyId, clientId, + connectionClientId, isMobile, supportsDesktopViewportClaims: params.capabilities?.desktopViewportClaims === 1, supportsQueryReply: params.capabilities?.queryReply === 1, diff --git a/src/main/runtime/rpc/terminal-multiplex-test-harness.ts b/src/main/runtime/rpc/terminal-multiplex-test-harness.ts index 13368bd680d..09d1ddf8551 100644 --- a/src/main/runtime/rpc/terminal-multiplex-test-harness.ts +++ b/src/main/runtime/rpc/terminal-multiplex-test-harness.ts @@ -51,7 +51,8 @@ export function makeRequest(method: string, params?: unknown): RpcRequest { export function startDesktopMultiplexSubscribe( overrides: Partial = {}, trace?: string[], - sendBinaryOverride?: (bytes: Uint8Array) => boolean | void + sendBinaryOverride?: (bytes: Uint8Array) => boolean | void, + connectionClientId?: string ) { const messages: string[] = [] const binaryFrames: Uint8Array[] = [] @@ -93,6 +94,7 @@ export function startDesktopMultiplexSubscribe( }, { connectionId: 'conn-desktop-first-paint', + clientId: connectionClientId, sendBinary: (bytes) => { const sent = sendBinaryOverride?.(bytes) if (sent === false) { diff --git a/src/main/runtime/rpc/terminal-query-reply-negotiation.test.ts b/src/main/runtime/rpc/terminal-query-reply-negotiation.test.ts index ff8a94baa3f..42a711bca80 100644 --- a/src/main/runtime/rpc/terminal-query-reply-negotiation.test.ts +++ b/src/main/runtime/rpc/terminal-query-reply-negotiation.test.ts @@ -239,6 +239,118 @@ describe('terminal query-reply opcode guards', () => { }) }) +describe('terminal query-reply opcode 18 author identity', () => { + // Why: the declared `client.id` is free-form page input; only the transport's device token is authenticated. + const IDENTITY_CASES = [ + { + name: 'a declared id impersonating another paired device', + declaredClientId: 'mobile-authority', + connectionClientId: 'mobile-impostor', + delivered: false + }, + { + name: 'a declared id matching the connection device token', + declaredClientId: 'mobile-authority', + connectionClientId: 'mobile-authority', + delivered: true + }, + { + name: 'a connection carrying no device token', + declaredClientId: 'mobile-authority', + connectionClientId: undefined, + delivered: true + } + ] + + it.each(IDENTITY_CASES)('gates multiplex opcode 18 on $name', async (testCase) => { + const sendTerminal = vi.fn().mockResolvedValue({ accepted: true }) + const harness = startDesktopMultiplexSubscribe( + { + sendTerminal, + handleMobileSubscribe: vi.fn().mockResolvedValue(undefined), + handleMobileUnsubscribe: vi.fn(), + isMobileTerminalQueryReplyAuthority: vi.fn().mockReturnValue(true) + }, + undefined, + undefined, + testCase.connectionClientId + ) + await vi.waitFor(() => expect(harness.handlers.has(0)).toBe(true)) + harness.handlers.get(0)?.( + subscribeFrame({ + streamId: 21, + clientType: 'mobile', + negotiated: true, + clientId: testCase.declaredClientId + }) + ) + await vi.waitFor(() => expect(harness.handlers.has(21)).toBe(true)) + + harness.handlers.get(21)?.(queryReplyFrame(21)) + // Ordinary input on the same stream proves the stream is live and the drop is guard-specific. + harness.handlers.get(21)?.(inputFrame(21)) + await vi.waitFor(() => expect(sendTerminal).toHaveBeenCalledTimes(testCase.delivered ? 2 : 1)) + expect(sendTerminal.mock.calls.some((call) => call[1]?.text === QUERY_REPLY)).toBe( + testCase.delivered + ) + + harness.registry.cleanupSubscription('terminal-multiplex:conn-desktop-first-paint') + await harness.dispatchPromise + }) + + it.each(IDENTITY_CASES)('gates direct opcode 18 on $name', async (testCase) => { + const registry = createSubscriptionRegistryDouble() + const messages: string[] = [] + const handlers = new Map< + number, + (frame: NonNullable>) => void + >() + const sendTerminal = vi.fn().mockResolvedValue({ accepted: true }) + const runtime = stubRuntime({ + ...directSubscribeRuntime(registry), + isMobileTerminalQueryReplyAuthority: vi.fn().mockReturnValue(true), + sendTerminal + }) + const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + const dispatchPromise = dispatcher.dispatchStreaming( + makeRequest('terminal.subscribe', { + terminal: 'terminal-1', + client: { id: testCase.declaredClientId, type: 'mobile' }, + capabilities: { terminalBinaryStream: 1, queryReply: 1 } + }), + (message) => messages.push(message), + { + connectionId: `conn-identity-${testCase.name}`, + clientId: testCase.connectionClientId, + sendBinary: vi.fn(), + registerBinaryStreamHandler: (streamId, handler) => { + handlers.set(streamId, handler) + return () => handlers.delete(streamId) + } + } + ) + + await vi.waitFor(() => + expect(messages.some((message) => JSON.parse(message).result?.type === 'subscribed')).toBe( + true + ) + ) + const streamId = messages + .map((message) => JSON.parse(message).result) + .find((event) => event?.type === 'subscribed').streamId + + handlers.get(streamId)?.(queryReplyFrame(streamId)) + handlers.get(streamId)?.(inputFrame(streamId)) + await vi.waitFor(() => expect(sendTerminal).toHaveBeenCalledTimes(testCase.delivered ? 2 : 1)) + expect(sendTerminal.mock.calls.some((call) => call[1]?.text === QUERY_REPLY)).toBe( + testCase.delivered + ) + + runtime.cleanupSubscription(`terminal-1:${testCase.declaredClientId}`) + await dispatchPromise + }) +}) + function directSubscribeRuntime(registry: ReturnType) { return { resolveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), @@ -263,6 +375,7 @@ function subscribeFrame(options: { streamId: number clientType: 'mobile' | 'desktop' negotiated: boolean + clientId?: string }) { return decodeTerminalStreamFrame( encodeTerminalStreamFrame({ @@ -272,7 +385,7 @@ function subscribeFrame(options: { payload: encodeTerminalStreamJson({ streamId: options.streamId, terminal: 'terminal-1', - client: { id: 'client-1', type: options.clientType }, + client: { id: options.clientId ?? 'client-1', type: options.clientType }, capabilities: { ackOutput: 1, ...(options.negotiated ? { queryReply: 1 } : {})