diff --git a/src/main/providers/ssh-pty-liveness-probe.test.ts b/src/main/providers/ssh-pty-liveness-probe.test.ts new file mode 100644 index 00000000000..3e5cfbf66f6 --- /dev/null +++ b/src/main/providers/ssh-pty-liveness-probe.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, it, vi } from 'vitest' +import { probeSshPtyLiveness } from './ssh-pty-liveness-probe' +import { + parseRelayPtyMintEpoch, + toRelayPtyIdWithMintEpoch +} from '../../shared/relay-pty-mint-epoch' + +const EPOCH = '0f8f3a1e-1111-4111-8111-111111111111' +const PTY_ID = toRelayPtyIdWithMintEpoch(EPOCH, 7) + +function relay(options: { + listed?: { id: string }[] | null + epoch?: string | null + listThrows?: Error + capabilitiesThrow?: Error +}) { + return vi.fn(async (method: string) => { + if (method === 'pty.listProcesses') { + if (options.listThrows) { + throw options.listThrows + } + return options.listed === undefined ? [] : options.listed + } + if (method === 'pty.getCapabilities') { + if (options.capabilitiesThrow) { + throw options.capabilitiesThrow + } + return options.epoch === null ? {} : { ptyIdMintEpoch: options.epoch ?? EPOCH } + } + throw new Error(`unexpected relay method ${method}`) + }) +} + +describe('SSH PTY liveness probe', () => { + it('answers live for an id the relay still lists', async () => { + const request = relay({ listed: [{ id: PTY_ID }] }) + + await expect(probeSshPtyLiveness({ request, relayPtyId: PTY_ID })).resolves.toBe(true) + // A listed id needs no epoch comparison, so the second round trip is not spent. + expect(request).toHaveBeenCalledTimes(1) + }) + + it('certifies the exit when the relay that minted the id no longer lists it', async () => { + const request = relay({ listed: [{ id: toRelayPtyIdWithMintEpoch(EPOCH, 8) }] }) + + await expect(probeSshPtyLiveness({ request, relayPtyId: PTY_ID })).resolves.toBe(false) + }) + + it('certifies the exit even when the relay now lists nothing at all', async () => { + // The population the recovery sweep meets: the worker was the host's only terminal and its + // shell died while Orca was closed, so no listed id can name the current generation. + const request = relay({ listed: [] }) + + await expect(probeSshPtyLiveness({ request, relayPtyId: PTY_ID })).resolves.toBe(false) + }) + + it('stays unverifiable when a restarted relay disowns an id it never minted', async () => { + const request = relay({ listed: [], epoch: 'ffffffff-2222-4222-8222-222222222222' }) + + await expect(probeSshPtyLiveness({ request, relayPtyId: PTY_ID })).resolves.toBeNull() + }) + + it('stays unverifiable for a relay that names no mint epoch', async () => { + const request = relay({ listed: [], epoch: null }) + + await expect(probeSshPtyLiveness({ request, relayPtyId: PTY_ID })).resolves.toBeNull() + }) + + it('stays unverifiable for a legacy id that carries no epoch', async () => { + const request = relay({ listed: [] }) + + await expect(probeSshPtyLiveness({ request, relayPtyId: 'pty-4' })).resolves.toBeNull() + expect(request).toHaveBeenCalledTimes(1) + }) + + it.each([ + ['the listing', { listThrows: new Error('Multiplexer disposed') }], + ['the capability read', { capabilitiesThrow: new Error('SSH connection lost') }] + ])('stays unverifiable when %s fails', async (_label, failure) => { + const request = relay({ listed: [], ...failure }) + + await expect(probeSshPtyLiveness({ request, relayPtyId: PTY_ID })).resolves.toBeNull() + }) + + it('stays unverifiable when the relay answers with no listing at all', async () => { + const request = relay({ listed: null }) + + await expect(probeSshPtyLiveness({ request, relayPtyId: PTY_ID })).resolves.toBeNull() + }) +}) + +describe('relay PTY mint epoch', () => { + it('round-trips an epoch through an id', () => { + expect(parseRelayPtyMintEpoch(toRelayPtyIdWithMintEpoch(EPOCH, 3))).toBe(EPOCH) + }) + + it('round-trips an epoch that needs escaping', () => { + expect(parseRelayPtyMintEpoch(toRelayPtyIdWithMintEpoch('a:b/c d', 1))).toBe('a:b/c d') + }) + + it.each(['pty-4', 'pty2:', 'pty2::9', 'shell-1'])('names no epoch in %s', (id) => { + expect(parseRelayPtyMintEpoch(id)).toBeNull() + }) +}) diff --git a/src/main/providers/ssh-pty-liveness-probe.ts b/src/main/providers/ssh-pty-liveness-probe.ts new file mode 100644 index 00000000000..b0cae3fe354 --- /dev/null +++ b/src/main/providers/ssh-pty-liveness-probe.ts @@ -0,0 +1,60 @@ +import { parseRelayPtyMintEpoch } from '../../shared/relay-pty-mint-epoch' + +type RelayRequest = ( + method: string, + params?: Record, + options?: { timeoutMs?: number } +) => Promise + +const PROBE_TIMEOUT_MS = 10_000 + +/** + * Whether the relay still owns this PTY, as an answer a caller may retire durable state on. + * + * `pty.listProcesses` is the relay's own live set: it reaps a torn-down record and re-probes a pid + * before reporting, so a listed id is live. An unlisted id is only an exit when this relay is also + * the one that minted it — after a restart the relay disowns every id the previous one allocated + * without having checked anything, which is why absence alone was never evidence + * (docs/reference/ssh-execution-boundary.md). + * + * Deliberately not `pty.attach`, the only refusal that carries the proven-exited marker: a + * successful attach opens a delivery, retires the previous one, and restores retired pane surfaces, + * so probing with it would disturb a live consumer in exactly the case where the answer is "live". + * + * Never throws. A transport failure, a disposed multiplexer, a timeout, a legacy `pty-N` id, and a + * relay that names no mint epoch all answer null, because none of them observed the process. + */ +export async function probeSshPtyLiveness(args: { + request: RelayRequest + relayPtyId: string +}): Promise { + try { + const listed = (await args.request( + 'pty.listProcesses', + { includeForegroundProcessEvidence: false }, + { timeoutMs: PROBE_TIMEOUT_MS } + )) as { id?: unknown }[] | null + if (!Array.isArray(listed)) { + return null + } + if (listed.some((session) => session.id === args.relayPtyId)) { + return true + } + const mintEpoch = parseRelayPtyMintEpoch(args.relayPtyId) + if (!mintEpoch) { + return null + } + // Read after the listing on purpose: both answers come from one relay process, and a restart + // between them breaks the multiplexer rather than pairing a new epoch with an old listing. + const capabilities = (await args.request('pty.getCapabilities', undefined, { + timeoutMs: PROBE_TIMEOUT_MS + })) as { ptyIdMintEpoch?: unknown } | null + const currentEpoch = capabilities?.ptyIdMintEpoch + if (typeof currentEpoch !== 'string' || currentEpoch !== mintEpoch) { + return null + } + return false + } catch { + return null + } +} diff --git a/src/main/providers/ssh-pty-provider-liveness-readback.test.ts b/src/main/providers/ssh-pty-provider-liveness-readback.test.ts new file mode 100644 index 00000000000..94eae4a043e --- /dev/null +++ b/src/main/providers/ssh-pty-provider-liveness-readback.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it, vi } from 'vitest' +import { SshPtyProvider } from './ssh-pty-provider' +import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer' +import { toAppSshPtyId } from '../../shared/ssh-pty-id' +import { toRelayPtyIdWithMintEpoch } from '../../shared/relay-pty-mint-epoch' + +const CONNECTION_ID = 'conn-1' +const RELAY_EPOCH = '0f8f3a1e-1111-4111-8111-111111111111' +const RELAY_PTY_ID = toRelayPtyIdWithMintEpoch(RELAY_EPOCH, 42) +const APP_PTY_ID = toAppSshPtyId(CONNECTION_ID, RELAY_PTY_ID) + +/** + * The provider is the owner the liveness rule routes to, so this pins the contract seam itself: + * without `probePtyLiveness` on this class, `probePtyLivenessFromRuntimeController` answers null + * for every SSH id and no SSH worker can ever be certified exited. + */ +function makeProvider(listed: { id: string }[]) { + const request = vi.fn(async (method: string) => { + if (method === 'pty.listProcesses') { + return listed + } + if (method === 'pty.getCapabilities') { + return { ptyIdMintEpoch: RELAY_EPOCH } + } + throw new Error(`unexpected relay method ${method}`) + }) + const mux = { + request, + onNotification: () => () => {}, + onRequest: () => () => {} + } as unknown as SshChannelMultiplexer + return { provider: new SshPtyProvider(CONNECTION_ID, mux), request } +} + +describe('SshPtyProvider liveness readback', () => { + it('exposes the readback the liveness rule asks the owning provider for', () => { + const { provider } = makeProvider([]) + + expect(typeof provider.probePtyLiveness).toBe('function') + }) + + it('answers live from the relay for an id its own cache has never seen', async () => { + const { provider } = makeProvider([{ id: RELAY_PTY_ID }]) + + expect(provider.hasPty(APP_PTY_ID)).toBe(false) + await expect(provider.probePtyLiveness(APP_PTY_ID)).resolves.toBe(true) + }) + + it('certifies an exit the relay observed', async () => { + const { provider } = makeProvider([]) + + await expect(provider.probePtyLiveness(APP_PTY_ID)).resolves.toBe(false) + }) + + it('refuses to answer for an id belonging to another SSH target', async () => { + const { provider, request } = makeProvider([]) + + await expect( + provider.probePtyLiveness(toAppSshPtyId('other-conn', RELAY_PTY_ID)) + ).resolves.toBeNull() + expect(request).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/providers/ssh-pty-provider.ts b/src/main/providers/ssh-pty-provider.ts index 76b5d9f85e4..1e3097ef009 100644 --- a/src/main/providers/ssh-pty-provider.ts +++ b/src/main/providers/ssh-pty-provider.ts @@ -25,6 +25,7 @@ import { SshAgentSessionCapabilities } from './ssh-agent-session-capabilities' import type { PtyProcessInspection } from './pty-process-inspection' import { spawnWithTerminalRuntimeRepair, type TerminalRepairHook } from './ssh-pty-spawn-repair' import { createSshPtyProviderRpcOperations } from './ssh-pty-provider-rpc-operations' +import { probeSshPtyLiveness } from './ssh-pty-liveness-probe' // Why: sequential relay teardown calls share one absolute budget; convert to the mux-relative timeout only at dispatch. function relayTimeoutOptions(deadlineMs: number | undefined): { timeoutMs: number } | undefined { @@ -288,6 +289,21 @@ export class SshPtyProvider implements IPtyProvider { hasPty = (id: string): boolean => this.livePtyIds.has(id) + /** `hasPty` is this process's cache of what it has seen; only the relay may answer absent. */ + probePtyLiveness = async (id: string): Promise => { + let relayPtyId: string + try { + relayPtyId = this.toRelayPtyId(id) + } catch { + // The id names another SSH target, so this provider is not its owner and cannot answer. + return null + } + return await probeSshPtyLiveness({ + request: (method, params, options) => this.mux.request(method, params, options), + relayPtyId + }) + } + onData = (callback: SshPtyDataCallback): (() => void) => this.outputState.onData(callback) onRejectedData = (callback: SshPtyDataCallback): (() => void) => this.outputState.onRejectedData(callback) diff --git a/src/main/runtime/runtime-legacy-worker-terminal-exit-certification.test.ts b/src/main/runtime/runtime-legacy-worker-terminal-exit-certification.test.ts index 7be1e8a173d..4ae66e45eb4 100644 --- a/src/main/runtime/runtime-legacy-worker-terminal-exit-certification.test.ts +++ b/src/main/runtime/runtime-legacy-worker-terminal-exit-certification.test.ts @@ -6,8 +6,15 @@ import type { LegacyWorkerRecoveryPorts, LegacyWorkerRecoveryResolution } from './runtime-legacy-worker-terminal-recovery-types' +import { probeSshPtyLiveness } from '../providers/ssh-pty-liveness-probe' +import { toAppSshPtyId, toRelaySshPtyId } from '../../shared/ssh-pty-id' +import { toRelayPtyIdWithMintEpoch } from '../../shared/relay-pty-mint-epoch' -const PTY_ID = 'app-ssh:conn-1:pty-42' +const CONNECTION_ID = 'conn-1' +const RELAY_EPOCH = '0f8f3a1e-1111-4111-8111-111111111111' +// A real SSH id, so the population this suite claims to cover is the one it drives. +const PTY_ID = toAppSshPtyId(CONNECTION_ID, toRelayPtyIdWithMintEpoch(RELAY_EPOCH, 42)) +const OTHER_PTY_ID = toAppSshPtyId(CONNECTION_ID, toRelayPtyIdWithMintEpoch(RELAY_EPOCH, 43)) const candidate = { dispatchId: 'dispatch_1', @@ -36,7 +43,7 @@ function inventory(options: { } as unknown as LegacyWorkerRecoveryInventory } -const listingWithoutThePty = inventory({ ptyIds: ['app-ssh:conn-1:pty-other'] }) +const listingWithoutThePty = inventory({ ptyIds: [OTHER_PTY_ID] }) const listingWithThePty = inventory({ ptyIds: [PTY_ID], identity: { handle: 'term_worker', incarnationId: 'inc-1' } @@ -44,7 +51,7 @@ const listingWithThePty = inventory({ function reconcile(options: { inventory: LegacyWorkerRecoveryInventory - isPtyProvenAbsent: () => Promise + isPtyProvenAbsent: (ptyId: string) => Promise refreshInventory?: LegacyWorkerRecoveryPorts['refreshInventory'] }) { const pendingResolutions: LegacyWorkerRecoveryResolution[] = [] @@ -145,6 +152,40 @@ describe('legacy worker recovery: certifying that a worker PTY exited', () => { expect(ports.isPtyProvenAbsent).not.toHaveBeenCalled() }) + // The rule `isLeafPtyProvenAbsent` applies: the owning provider's readback is proven absence + // only when it answers false, and the SSH provider's readback is the relay itself. + function provenAbsentViaRelay(request: Parameters[0]['request']) { + return async (appPtyId: string): Promise => + (await probeSshPtyLiveness({ + request, + relayPtyId: toRelaySshPtyId(CONNECTION_ID, appPtyId) + })) === false + } + + it('certifies an SSH exit when the relay that minted the id no longer lists it', async () => { + const { pendingResolutions, deferredDispatchIds } = await reconcile({ + inventory: listingWithoutThePty, + isPtyProvenAbsent: provenAbsentViaRelay(async (method) => + method === 'pty.listProcesses' ? [] : { ptyIdMintEpoch: RELAY_EPOCH } + ) + }) + + expect(pendingResolutions).toEqual([{ candidate, resolution: 'exited' }]) + expect(deferredDispatchIds.size).toBe(0) + }) + + it('defers an SSH candidate a restarted relay merely disowns', async () => { + const { pendingResolutions, deferredDispatchIds } = await reconcile({ + inventory: listingWithoutThePty, + isPtyProvenAbsent: provenAbsentViaRelay(async (method) => + method === 'pty.listProcesses' ? [] : { ptyIdMintEpoch: 'a-later-relay-generation' } + ) + }) + + expect(pendingResolutions).toEqual([]) + expect([...deferredDispatchIds]).toEqual(['dispatch_1']) + }) + it('control: still adopts a PTY the listing names with the recorded identity', async () => { const { pendingResolutions, deferredDispatchIds, ports } = await reconcile({ inventory: listingWithThePty, diff --git a/src/relay/pty-handler-mint-epoch-capability.test.ts b/src/relay/pty-handler-mint-epoch-capability.test.ts new file mode 100644 index 00000000000..93d1d521bcc --- /dev/null +++ b/src/relay/pty-handler-mint-epoch-capability.test.ts @@ -0,0 +1,82 @@ +// The relay half of the SSH liveness readback. A client may only read "this id is absent from my +// listing" as an exit when the relay that minted the id is the one answering, so the generation +// stamped into every id has to be the generation `pty.getCapabilities` names. If these two ever +// disagree, every SSH absence silently becomes unverifiable forever and no SSH worker can be +// retired (docs/reference/ssh-execution-boundary.md). +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest' + +const { mockPtySpawn, mockPtyInstance, mockCreateShellPromptReadinessProbe } = vi.hoisted(() => ({ + mockPtySpawn: vi.fn(), + mockCreateShellPromptReadinessProbe: vi.fn(), + mockPtyInstance: { + pid: process.pid, + process: 'zsh', + onData: vi.fn(), + onExit: vi.fn(), + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn(), + clear: vi.fn(), + pause: vi.fn(), + resume: vi.fn() + } +})) + +vi.mock('node-pty', () => ({ spawn: mockPtySpawn })) + +vi.mock('../main/pty/posix-pty-process-groups', () => ({ + forceKillPosixPtyProcessGroups: vi.fn((_pid: number, fallback: () => void) => fallback()) +})) + +vi.mock('../main/shell-prompt-readiness-probe', () => ({ + createShellPromptReadinessProbe: mockCreateShellPromptReadinessProbe +})) + +import type { PtyHandler } from './pty-handler' +import { + beginPtyHandlerTest, + createPtyRequestHelpers, + endPtyHandlerTest +} from './pty-handler-test-harness' +import type { MockDispatcher } from './pty-handler-test-harness' +import { parseRelayPtyMintEpoch } from '../shared/relay-pty-mint-epoch' + +describe('PtyHandler mint epoch', () => { + let dispatcher: MockDispatcher + let handler: PtyHandler + let originalPlatform: PropertyDescriptor | undefined + + const { spawnPty } = createPtyRequestHelpers(() => dispatcher) + + beforeEach(() => { + ;({ dispatcher, handler, originalPlatform } = beginPtyHandlerTest({ + mockPtySpawn, + mockPtyInstance, + mockCreateShellPromptReadinessProbe + })) + }) + + afterEach(() => { + endPtyHandlerTest(handler, originalPlatform) + }) + + async function capabilities(): Promise<{ ptyIdMintEpoch?: unknown }> { + return (await dispatcher.callRequest('pty.getCapabilities', {})) as { ptyIdMintEpoch?: unknown } + } + + it('names the generation that minted its PTY ids', async () => { + const { id } = await spawnPty() + + const mintEpoch = parseRelayPtyMintEpoch(id) + expect(mintEpoch).toBeTruthy() + expect((await capabilities()).ptyIdMintEpoch).toBe(mintEpoch) + }) + + it('keeps that generation stable across ids and reads', async () => { + const first = await spawnPty() + const second = await spawnPty() + + expect(parseRelayPtyMintEpoch(second.id)).toBe(parseRelayPtyMintEpoch(first.id)) + expect((await capabilities()).ptyIdMintEpoch).toBe((await capabilities()).ptyIdMintEpoch) + }) +}) diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index cdf436bca2a..6089916cd2c 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -36,6 +36,7 @@ import { } from './pty-spawn-cwd' import { PhysicalExitTracker } from '../shared/physical-exit-tracker' import { PTY_ATTACH_PROVEN_EXITED_MARKER } from '../shared/pty-attach-absence-evidence' +import { toRelayPtyIdWithMintEpoch } from '../shared/relay-pty-mint-epoch' import { SHELL_READY_MARKER_PREFIX } from '../main/shell-ready-marker-scanner' import { createShellStartupOutputScanState, @@ -1092,7 +1093,11 @@ export class PtyHandler { agentSessionCreateOperationVersion: AGENT_SESSION_CREATE_OPERATION_PROTOCOL_VERSION, // Additive capability: clients may request the no-process-table inventory // projection and consume fenced inspect evidence on this host. - foregroundProcessEvidenceVersion: 1 + foregroundProcessEvidenceVersion: 1, + // Additive: names the generation that minted this relay's ids, so a client can read an id's + // absence from `pty.listProcesses` as an exit this host observed rather than as a restart. + // A relay that omits it leaves every absence unverifiable, which is the shipped behaviour. + ptyIdMintEpoch: this.ptyIdMintEpoch })) this.dispatcher.onRequest('pty.listProcesses', (params) => this.listProcesses(params)) this.dispatcher.onRequest('pty.getDefaultShell', async () => resolveDefaultShell()) @@ -1863,7 +1868,7 @@ export class PtyHandler { const shell = resolvedShellOverride || requestedEnvShell || resolveDefaultShell() let id: string do { - id = `pty2:${encodeURIComponent(this.ptyIdMintEpoch)}:${this.nextId++}` + id = toRelayPtyIdWithMintEpoch(this.ptyIdMintEpoch, this.nextId++) } while (this.ptys.has(id) || this.pendingReviveIds.has(id)) // Why: augmenter values override renderer env so remote paths and hook coords win over local userData. diff --git a/src/shared/relay-pty-mint-epoch.ts b/src/shared/relay-pty-mint-epoch.ts new file mode 100644 index 00000000000..a6fbd854365 --- /dev/null +++ b/src/shared/relay-pty-mint-epoch.ts @@ -0,0 +1,30 @@ +/** + * A relay PTY id carries the mint epoch of the relay process that allocated it, so a client can + * tell the two halves of "this relay does not list that id" apart: the relay minted it and no + * longer has it, which is an exit the host observed, versus the relay never had it, which is every + * id minted before a restart and is evidence of nothing (docs/reference/ssh-execution-boundary.md). + * + * The id shape lives here so the relay that mints and the client that reads it cannot drift. + */ +const MINT_EPOCH_PTY_ID_PREFIX = 'pty2:' + +export function toRelayPtyIdWithMintEpoch(mintEpoch: string, sequence: number): string { + return `${MINT_EPOCH_PTY_ID_PREFIX}${encodeURIComponent(mintEpoch)}:${sequence}` +} + +/** Null for a legacy `pty-N` id, which names no epoch and so can never certify an exit. */ +export function parseRelayPtyMintEpoch(relayPtyId: string): string | null { + if (!relayPtyId.startsWith(MINT_EPOCH_PTY_ID_PREFIX)) { + return null + } + const remainder = relayPtyId.slice(MINT_EPOCH_PTY_ID_PREFIX.length) + const sequenceSeparator = remainder.lastIndexOf(':') + if (sequenceSeparator <= 0) { + return null + } + try { + return decodeURIComponent(remainder.slice(0, sequenceSeparator)) || null + } catch { + return null + } +}