mirror of
https://github.com/stablyai/orca.git
synced 2026-10-26 00:03:55 +00:00
fix(remote): guard terminal geometry RPCs with the live-leaf resolver
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.
This commit is contained in:
@@ -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')
|
||||
}
|
||||
|
||||
@@ -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> = {}): 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<void> {
|
||||
const spies: Record<string, ReturnType<typeof vi.fn>> = {}
|
||||
for (const name of mutators) {
|
||||
spies[name] = vi.fn()
|
||||
}
|
||||
const runtime = stubStaleHandleRuntime(spies as Partial<OrcaRuntimeService>)
|
||||
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
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -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 })
|
||||
|
||||
Reference in New Issue
Block a user