From 3aec590e1ff319f790f200e2c4af46b334c77b55 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 10 Jul 2026 04:48:40 -0400 Subject: [PATCH] fix(remote): guard terminal geometry RPCs with the live-leaf resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit terminal.send was already hardened (#7827) to resolve handles through the guarded runtime.resolveLiveLeafForHandle, which throws terminal_handle_stale when a pane's PTY was replaced under a handle (restart/re-spawn bumps ptyId/generation). The geometry family still used the UNGUARDED resolveLeafForHandle and then mutated PTY state: - terminal.resizeForClient - terminal.setDisplayMode - terminal.restoreFit - terminal.updateViewport The unguarded resolver returns the pane's CURRENT pty, so a remote client holding a stale handle resized/refit/viewported the WRONG (new) pty — mysterious geometry corruption on the fresh session. Switch all four to resolveLiveLeafForHandle. The return shape is identical ({ ptyId }), so no follow-on logic changes. The read-only terminal.getDisplayMode, the already-guarded multiplex path, and the legacy subscribe-time resolution are left untouched. Clients already handle terminal_handle_stale gracefully (no red banners): updateViewport/restoreFit/setDisplayMode callers swallow the error, and the remote-runtime transport classifies terminal_handle_stale as "terminal gone" → retires the mirror and re-derives the handle from the next session-tabs snapshot. Applies to both SSH remote-runtime and remote Orca Server, which share these control-plane RPCs. --- src/main/runtime/rpc/methods/terminal.ts | 20 ++- .../terminal-geometry-stale-handle.test.ts | 154 ++++++++++++++++++ src/main/runtime/rpc/terminal-send.test.ts | 2 +- 3 files changed, 171 insertions(+), 5 deletions(-) create mode 100644 src/main/runtime/rpc/terminal-geometry-stale-handle.test.ts diff --git a/src/main/runtime/rpc/methods/terminal.ts b/src/main/runtime/rpc/methods/terminal.ts index a750438e03e..e14d2c7cb21 100644 --- a/src/main/runtime/rpc/methods/terminal.ts +++ b/src/main/runtime/rpc/methods/terminal.ts @@ -1083,7 +1083,10 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ name: 'terminal.resizeForClient', params: TerminalResizeForClient, handler: async (params, { runtime }) => { - const leaf = runtime.resolveLeafForHandle(params.terminal) + // Why: guarded resolution — a stale handle (pane's PTY replaced under it) + // must fail with terminal_handle_stale instead of resizing the wrong PTY + // (#7718). Clients recover by re-deriving the handle. + const leaf = runtime.resolveLiveLeafForHandle(params.terminal) if (!leaf?.ptyId) { throw new Error('no_connected_pty') } @@ -1137,7 +1140,10 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ name: 'terminal.setDisplayMode', params: TerminalSetDisplayMode, handler: async (params, { runtime }) => { - const leaf = runtime.resolveLeafForHandle(params.terminal) + // Why: guarded resolution — a stale handle must fail with + // terminal_handle_stale instead of mutating the wrong PTY's display + // mode/viewport (#7718). Clients recover by re-deriving the handle. + const leaf = runtime.resolveLiveLeafForHandle(params.terminal) if (!leaf?.ptyId) { throw new Error('no_connected_pty') } @@ -1159,7 +1165,10 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ name: 'terminal.restoreFit', params: TerminalHandle, handler: async (params, { runtime }) => { - const leaf = runtime.resolveLeafForHandle(params.terminal) + // Why: guarded resolution — a stale handle must fail with + // terminal_handle_stale instead of reclaiming the wrong PTY back to + // desktop dims (#7718). Clients recover by re-deriving the handle. + const leaf = runtime.resolveLiveLeafForHandle(params.terminal) if (!leaf?.ptyId) { throw new Error('no_connected_pty') } @@ -1180,7 +1189,10 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ name: 'terminal.updateViewport', params: TerminalUpdateViewport, handler: async (params, { runtime }) => { - const leaf = runtime.resolveLeafForHandle(params.terminal) + // Why: guarded resolution — a stale handle must fail with + // terminal_handle_stale instead of writing viewport state to the wrong + // PTY (#7718). Clients recover by re-deriving the handle. + const leaf = runtime.resolveLiveLeafForHandle(params.terminal) if (!leaf?.ptyId) { throw new Error('no_connected_pty') } diff --git a/src/main/runtime/rpc/terminal-geometry-stale-handle.test.ts b/src/main/runtime/rpc/terminal-geometry-stale-handle.test.ts new file mode 100644 index 00000000000..41242ad98ef --- /dev/null +++ b/src/main/runtime/rpc/terminal-geometry-stale-handle.test.ts @@ -0,0 +1,154 @@ +import { describe, expect, it, vi } from 'vitest' +import { RpcDispatcher } from './dispatcher' +import type { RpcRequest } from './core' +import type { OrcaRuntimeService } from '../orca-runtime' +import { TERMINAL_METHODS } from './methods/terminal' + +// Why: the terminal geometry family (resize/setDisplayMode/restoreFit/ +// updateViewport) mutates PTY state. A remote client can hold a handle minted +// for a PTY that was later replaced under the pane (restart/re-spawn bumps +// ptyId/generation). The UNGUARDED resolveLeafForHandle returns the pane's +// CURRENT pty, so a stale client would mutate the wrong (new) PTY — visible as +// geometry corruption on the fresh session (#7718). These methods must use the +// guarded resolveLiveLeafForHandle, which throws terminal_handle_stale instead. + +// Models a stale handle exactly as the runtime does: the unguarded resolver +// silently adopts the replacement PTY ('pty-b'); the guarded resolver throws. +const NEW_PTY_UNDER_PANE = 'pty-b' + +function stubStaleHandleRuntime(overrides: Partial = {}): OrcaRuntimeService { + return { + getRuntimeId: () => 'test-runtime', + // Unguarded path: returns the pane's current (replaced) PTY — the misroute. + resolveLeafForHandle: vi.fn().mockReturnValue({ ptyId: NEW_PTY_UNDER_PANE }), + // Guarded path: surfaces the staleness. + resolveLiveLeafForHandle: vi.fn(() => { + throw new Error('terminal_handle_stale') + }), + ...overrides + } as unknown as OrcaRuntimeService +} + +function makeRequest(method: string, params?: unknown): RpcRequest { + return { id: 'req-1', authToken: 'tok', method, params } +} + +async function expectStale(method: string, params: unknown, mutators: string[]): Promise { + const spies: Record> = {} + for (const name of mutators) { + spies[name] = vi.fn() + } + const runtime = stubStaleHandleRuntime(spies as Partial) + const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + + const response = await dispatcher.dispatch(makeRequest(method, params)) + + expect(response.ok).toBe(false) + if (response.ok) { + throw new Error(`expected ${method} to reject a stale handle`) + } + expect(response.error.message).toContain('terminal_handle_stale') + // The wrong (replacement) PTY must never be mutated. + for (const name of mutators) { + expect(spies[name]).not.toHaveBeenCalled() + } +} + +describe('terminal geometry family rejects stale handles instead of mutating the wrong PTY', () => { + it('terminal.resizeForClient fails with terminal_handle_stale', async () => { + await expectStale( + 'terminal.resizeForClient', + { terminal: 'stale-terminal', mode: 'restore', clientId: 'client-1' }, + ['resizeForClient'] + ) + }) + + it('terminal.setDisplayMode fails with terminal_handle_stale', async () => { + await expectStale( + 'terminal.setDisplayMode', + { + terminal: 'stale-terminal', + mode: 'auto', + client: { id: 'client-1', type: 'mobile' }, + viewport: { cols: 80, rows: 24 } + }, + [ + 'setMobileDisplayMode', + 'applyMobileDisplayMode', + 'updateMobileSubscriberViewport', + 'markMobileActor' + ] + ) + }) + + it('terminal.restoreFit fails with terminal_handle_stale', async () => { + await expectStale('terminal.restoreFit', { terminal: 'stale-terminal' }, [ + 'reclaimTerminalForDesktop' + ]) + }) + + it('terminal.updateViewport fails with terminal_handle_stale', async () => { + await expectStale( + 'terminal.updateViewport', + { + terminal: 'stale-terminal', + client: { id: 'client-1', type: 'mobile' }, + viewport: { cols: 80, rows: 24 } + }, + ['updateMobileViewport', 'updateDesktopViewport'] + ) + }) +}) + +describe('terminal geometry family still mutates the live PTY for a fresh handle', () => { + it('terminal.restoreFit reclaims the resolved PTY when the handle is live', async () => { + const reclaimTerminalForDesktop = vi.fn().mockResolvedValue(true) + const runtime = { + getRuntimeId: () => 'test-runtime', + resolveLiveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-a' }), + reclaimTerminalForDesktop + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + + const response = await dispatcher.dispatch( + makeRequest('terminal.restoreFit', { terminal: 'live-terminal' }) + ) + + expect(response.ok).toBe(true) + if (!response.ok) { + throw new Error(response.error.message) + } + expect(response.result).toEqual({ restored: true }) + expect(reclaimTerminalForDesktop).toHaveBeenCalledWith('pty-a') + }) + + it('terminal.resizeForClient resizes the resolved PTY when the handle is live', async () => { + const resizeForClient = vi.fn().mockResolvedValue({ cols: 80, rows: 24 }) + const runtime = { + getRuntimeId: () => 'test-runtime', + resolveLiveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-a' }), + resizeForClient + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + + const response = await dispatcher.dispatch( + makeRequest('terminal.resizeForClient', { + terminal: 'live-terminal', + mode: 'restore', + clientId: 'client-1' + }) + ) + + expect(response.ok).toBe(true) + if (!response.ok) { + throw new Error(response.error.message) + } + expect(resizeForClient).toHaveBeenCalledWith( + 'pty-a', + 'restore', + 'client-1', + undefined, + undefined + ) + }) +}) diff --git a/src/main/runtime/rpc/terminal-send.test.ts b/src/main/runtime/rpc/terminal-send.test.ts index b778f814977..c426513c9f5 100644 --- a/src/main/runtime/rpc/terminal-send.test.ts +++ b/src/main/runtime/rpc/terminal-send.test.ts @@ -417,7 +417,7 @@ describe('terminal send RPC', () => { it('routes terminal restore fit through the runtime driver state machine', async () => { const runtime = stubRuntime({ - resolveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), + resolveLiveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), reclaimTerminalForDesktop: vi.fn().mockResolvedValue(true) }) const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS })