diff --git a/src/main/providers/pty-process-info.ts b/src/main/providers/pty-process-info.ts index 848ff07c78a..942cab84f73 100644 --- a/src/main/providers/pty-process-info.ts +++ b/src/main/providers/pty-process-info.ts @@ -18,4 +18,13 @@ export type PtyProcessInfo = { /** Optional host-side process evidence attached to an inventory seed. */ foregroundProcessEvidence?: ForegroundProcessEvidence agentSessionOwners?: AgentSessionOwnerBinding[] + /** Age measured on the OWNING host's clock. Absent means the host did not measure it, which is + * not the same as "new" or "old" — a reader that needs an age must defer instead of assuming. */ + hostAgeMs?: number + /** True when the host spawned this PTY for an Orca pane, false for a bare host shell. Absent from + * a host that never published it; absence is neither value. */ + paneBound?: boolean + /** The client identity the OWNING host recorded as having asked it to create this PTY. Absent + * whenever the host could not attest one, and absence must never be read as "unowned". */ + ownerClientInstanceId?: string } diff --git a/src/main/ssh/ssh-orphan-relay-pty-sweep.test.ts b/src/main/ssh/ssh-orphan-relay-pty-sweep.test.ts new file mode 100644 index 00000000000..841a8ff21f3 --- /dev/null +++ b/src/main/ssh/ssh-orphan-relay-pty-sweep.test.ts @@ -0,0 +1,162 @@ +// #9819, the client half: what the sweep actually asks the store and the host, and what it does +// with the answers. The rule itself is covered in ssh-relay-pty-ownership-proof.test.ts. +import { describe, expect, it, vi } from 'vitest' +import type { Store } from '../persistence' +import type { IPtyProvider } from '../providers/types' +import type { PtyProcessInfo } from '../providers/pty-process-info' +import type { SshRemotePtyLease } from '../../shared/ssh-types' +import { sweepOrphanedRelayPtys } from './ssh-orphan-relay-pty-sweep' +import { RELAY_PTY_SWEEP_MIN_AGE_MS } from '../../shared/ssh-relay-pty-ownership-proof' + +const TARGET = 'target-1' +const OURS = 'client-instance-ours' + +function hostEntry(overrides: Partial = {}): PtyProcessInfo { + return { + id: `ssh:${TARGET}@@pty-1`, + incarnationId: 'inc-1', + cwd: '/home/user', + title: 'zsh', + ownerClientInstanceId: OURS, + hostAgeMs: RELAY_PTY_SWEEP_MIN_AGE_MS * 2, + paneBound: true, + ...overrides + } +} + +function createHarness( + processes: PtyProcessInfo[], + leases: SshRemotePtyLease[] = [] +): { provider: IPtyProvider; store: Store; shutdown: ReturnType } { + const shutdown = vi.fn().mockResolvedValue(undefined) + const provider = { + listProcesses: vi.fn().mockResolvedValue(processes), + shutdown + } as unknown as IPtyProvider + const store = { + getSshRemotePtyLeases: vi.fn().mockReturnValue(leases) + } as unknown as Store + return { provider, store, shutdown } +} + +function run( + harness: ReturnType, + overrides: Partial[0]> = {} +): Promise { + return sweepOrphanedRelayPtys({ + targetId: TARGET, + store: harness.store, + provider: harness.provider, + clientInstanceId: OURS, + isSessionOwner: true, + routedPtyIds: [], + shouldContinue: () => true, + ...overrides + }) +} + +function lease(ptyId: string, state: SshRemotePtyLease['state']): SshRemotePtyLease { + return { ptyId, state } as SshRemotePtyLease +} + +describe('sweepOrphanedRelayPtys', () => { + it('stops an attested orphan, fenced on the incarnation the same listing published', async () => { + const harness = createHarness([hostEntry()]) + + await run(harness) + + expect(harness.shutdown).toHaveBeenCalledWith(`ssh:${TARGET}@@pty-1`, { + immediate: true, + expectedIncarnationId: 'inc-1' + }) + }) + + it('leaves a PTY the caller just reattached alone', async () => { + const harness = createHarness([hostEntry()]) + + await run(harness, { routedPtyIds: ['pty-1'] }) + + expect(harness.shutdown).not.toHaveBeenCalled() + }) + + it.each([['attached'], ['detached']] as const)( + 'leaves a PTY holding a live %s lease alone', + async (state) => { + const harness = createHarness([hostEntry()], [lease('pty-1', state)]) + + await run(harness) + + expect(harness.shutdown).not.toHaveBeenCalled() + } + ) + + it('leaves a PTY with an undelivered stop to the replay pass', async () => { + // The kill-intent journal owns those: it re-fences and retries them, and a second stop issued + // from here would race that decision with weaker evidence. + const tombstoned = { + ...lease('pty-1', 'terminated'), + pendingKill: { requestedAt: 1, incarnationId: 'inc-1', attempts: 0 } + } as SshRemotePtyLease + const harness = createHarness([hostEntry()], [tombstoned]) + + await run(harness) + + expect(harness.shutdown).not.toHaveBeenCalled() + }) + + it('does sweep a PTY whose lease this client already tombstoned without an order', async () => { + const harness = createHarness([hostEntry()], [lease('pty-1', 'terminated')]) + + await run(harness) + + expect(harness.shutdown).toHaveBeenCalledTimes(1) + }) + + it('asks the host nothing when this connection is not the session owner', async () => { + const harness = createHarness([hostEntry()]) + + await run(harness, { isSessionOwner: false }) + + expect(harness.provider.listProcesses).not.toHaveBeenCalled() + expect(harness.shutdown).not.toHaveBeenCalled() + }) + + it('stops nothing against a host that publishes no attestation', async () => { + const legacy = hostEntry() + delete legacy.ownerClientInstanceId + delete legacy.hostAgeMs + delete legacy.paneBound + const harness = createHarness([legacy]) + + await run(harness) + + expect(harness.shutdown).not.toHaveBeenCalled() + }) + + it('swallows a failed listing rather than failing the connect it runs on', async () => { + const harness = createHarness([]) + vi.mocked(harness.provider.listProcesses).mockRejectedValue(new Error('relay went away')) + + await expect(run(harness)).resolves.toBeUndefined() + }) + + it('swallows a failed stop and leaves the order to the next connect', async () => { + const harness = createHarness([hostEntry()]) + harness.shutdown.mockRejectedValue(new Error('connection lost')) + + await expect(run(harness)).resolves.toBeUndefined() + }) + + it('abandons the pass when the attempt is superseded mid-flight', async () => { + const harness = createHarness([hostEntry()]) + let alive = true + vi.mocked(harness.provider.listProcesses).mockImplementation(async () => { + alive = false + return [hostEntry()] + }) + + await run(harness, { shouldContinue: () => alive }) + + expect(harness.shutdown).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/ssh/ssh-orphan-relay-pty-sweep.ts b/src/main/ssh/ssh-orphan-relay-pty-sweep.ts new file mode 100644 index 00000000000..964864ca2cf --- /dev/null +++ b/src/main/ssh/ssh-orphan-relay-pty-sweep.ts @@ -0,0 +1,117 @@ +import type { Store } from '../persistence' +import type { IPtyProvider } from '../providers/types' +import { toAppSshPtyId, toRelaySshPtyId } from '../providers/ssh-pty-id' +import { + planRelayPtySweep, + RELAY_PTY_SWEEP_MIN_AGE_MS, + type RelayPtyOwnershipEvidence +} from '../../shared/ssh-relay-pty-ownership-proof' + +export type SshOrphanRelayPtySweepArgs = { + targetId: string + store: Store + provider: IPtyProvider + /** This client's persisted consumer identity for the target. */ + clientInstanceId: string + /** True only when the relay granted this connection the negotiated `session-owner` role. */ + isSessionOwner: boolean + /** Relay PTY ids this connect just reattached, plus any the caller otherwise knows are live. */ + routedPtyIds: Iterable + shouldContinue: () => boolean + now?: () => number + minimumHostAgeMs?: number +} + +/** Every relay PTY id this client still has a route to. Read across all lease states except the + * tombstones, plus the undelivered stops, which belong to the replay pass and not to this one. */ +function routedIds(args: SshOrphanRelayPtySweepArgs): Set { + const routed = new Set(args.routedPtyIds) + for (const lease of args.store.getSshRemotePtyLeases(args.targetId)) { + if (lease.state !== 'terminated' && lease.state !== 'expired') { + routed.add(lease.ptyId) + } + if (lease.pendingKill) { + routed.add(lease.ptyId) + } + } + return routed +} + +function toEvidence( + targetId: string, + process: Awaited>[number] +): RelayPtyOwnershipEvidence { + return { + ptyId: toRelaySshPtyId(targetId, process.id), + ...(process.incarnationId ? { incarnationId: process.incarnationId } : {}), + ...(process.ownerClientInstanceId + ? { ownerClientInstanceId: process.ownerClientInstanceId } + : {}), + ...(typeof process.hostAgeMs === 'number' ? { hostAgeMs: process.hostAgeMs } : {}), + ...(typeof process.paneBound === 'boolean' ? { paneBound: process.paneBound } : {}), + ...(process.agentSessionOwners ? { agentSessionOwners: process.agentSessionOwners } : {}) + } +} + +/** Stops the relay PTYs this client can prove it created and has since lost every route to. + * + * Runs after reattach, so a PTY this connect reclaimed is already routed and can never be a + * candidate. Best-effort and never throws: it is opportunistic cleanup on the connect path, and a + * failed connection is a much worse outcome than a slot left leaked for another session. + * + * Costs one `pty.listProcesses` per connect. That is the price of reconciling at all — there is no + * cheaper question than asking the authoritative host what it is holding. */ +export async function sweepOrphanedRelayPtys(args: SshOrphanRelayPtySweepArgs): Promise { + if (!args.isSessionOwner || !args.clientInstanceId || !args.shouldContinue()) { + return + } + try { + const processes = await args.provider.listProcesses() + if (!args.shouldContinue()) { + return + } + const plan = planRelayPtySweep( + processes.map((process) => toEvidence(args.targetId, process)), + { + clientInstanceId: args.clientInstanceId, + isSessionOwner: args.isSessionOwner, + routedPtyIds: routedIds(args), + minimumHostAgeMs: args.minimumHostAgeMs ?? RELAY_PTY_SWEEP_MIN_AGE_MS + } + ) + if (plan.sweep.length === 0) { + return + } + await Promise.all( + plan.sweep.map(async (target) => { + if (!args.shouldContinue()) { + return + } + try { + // Fenced on the incarnation the same listing published, so a relay that renumbered its + // ids between the read and this call refuses the stop instead of hitting a stranger. + await args.provider.shutdown(toAppSshPtyId(args.targetId, target.ptyId), { + immediate: true, + expectedIncarnationId: target.incarnationId + }) + console.log( + `[ssh-orphan-sweep] stopped orphaned relay PTY ${args.targetId}/${target.ptyId}` + ) + } catch (err) { + // Unverifiable, not failed: the next connect re-reads the inventory and decides again. + console.warn( + `[ssh-orphan-sweep] stop for ${args.targetId}/${target.ptyId} is unverifiable: ${ + err instanceof Error ? err.message : String(err) + }` + ) + } + }) + ) + } catch (err) { + console.warn( + `[ssh-orphan-sweep] pass on ${args.targetId} stopped early: ${ + err instanceof Error ? err.message : String(err) + }` + ) + } +} diff --git a/src/main/ssh/ssh-relay-session-orphan-sweep.test.ts b/src/main/ssh/ssh-relay-session-orphan-sweep.test.ts new file mode 100644 index 00000000000..960cbd58a4f --- /dev/null +++ b/src/main/ssh/ssh-relay-session-orphan-sweep.test.ts @@ -0,0 +1,203 @@ +// #9819 end to end on the client: the sweep runs only after reattach, only under a negotiated +// session-owner grant, and only against PTYs this relay itself attributes to this client. +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { SshRelaySession } from './ssh-relay-session' +import { createMockDeps, mockDeploySuccess } from './ssh-relay-session-test-fixtures' + +const { muxRequestMock, openConsumerSessionMock } = vi.hoisted(() => ({ + muxRequestMock: vi.fn(), + openConsumerSessionMock: vi.fn(async (_mux: unknown, options: { clientInstanceId: string }) => ({ + state: { + mode: 'negotiated' as const, + clientInstanceId: options.clientInstanceId, + clientGeneration: 1, + ownerGeneration: 1, + ownerLease: 'test-owner-lease' + }, + resumed: false + })) +})) + +vi.mock('./ssh-relay-deploy', () => ({ deployAndLaunchRelay: vi.fn() })) +vi.mock('./ssh-pty-consumer-session', () => ({ + openSshPtyConsumerSession: openConsumerSessionMock +})) +vi.mock('../ipc/ssh-pty-output-intake-registry', () => ({ + acceptSshPtyOutputData: vi.fn().mockResolvedValue(undefined), + acceptSshPtyOutputExit: vi.fn().mockResolvedValue(undefined), + allocateSshPtyProviderGeneration: vi.fn(() => 17), + beginSshPtyOutputGenerationMigration: vi.fn(() => ({ + byPty: new Map(), + completion: Promise.resolve() + })), + closeSshPtyOutputGeneration: vi.fn(), + getSshPtyAcceptedSourceCheckpoints: vi.fn(() => []), + applySshPtySourceCancellationProof: vi.fn(() => true), + applySshPtySourceRecoveryCancellationProof: vi.fn(() => true), + installSshPtySourceAckPublisher: vi.fn(() => () => {}), + installSshPtySourceCancellationPublisher: vi.fn(() => () => {}) +})) +vi.mock('./ssh-relay-deploy-helpers', () => ({ execCommand: vi.fn().mockResolvedValue('') })) +vi.mock('./ssh-remote-orca-cli', () => ({ + runRemoteOrcaCli: vi.fn().mockResolvedValue({ exitCode: 0, stdout: '', stderr: '' }) +})) +vi.mock('./ssh-channel-multiplexer', () => ({ + SshChannelMultiplexer: class MockSshChannelMultiplexer { + notify = vi.fn() + notifyWithSettlement = vi.fn() + request = muxRequestMock + onNotification = vi.fn().mockReturnValue(() => {}) + onNotificationByMethod = vi.fn().mockReturnValue(() => {}) + onRequest = vi.fn().mockReturnValue(() => {}) + onDispose = vi.fn().mockReturnValue(() => {}) + dispose = vi.fn() + isDisposed = vi.fn().mockReturnValue(false) + } +})) +vi.mock('../agent-hooks/remote-managed-hook-installers', () => ({ + installRemoteManagedAgentHooks: vi.fn() +})) +vi.mock('../providers/ssh-pty-provider', () => ({ + SshPtyProvider: class MockSshPtyProvider { + onData = vi.fn().mockReturnValue(() => {}) + onReplay = vi.fn().mockReturnValue(() => {}) + onExit = vi.fn().mockReturnValue(() => {}) + attach = vi.fn().mockResolvedValue(undefined) + attachForReconnect = vi.fn().mockResolvedValue({}) + dispose = vi.fn() + } +})) +vi.mock('../providers/ssh-filesystem-provider', () => ({ + SshFilesystemProvider: class MockSshFilesystemProvider { + dispose = vi.fn() + } +})) +vi.mock('../providers/ssh-git-provider', () => ({ + SshGitProvider: class MockSshGitProvider {} +})) +vi.mock('../ipc/pty', () => ({ + registerSshPtyProvider: vi.fn(), + unregisterSshPtyProvider: vi.fn(), + getSshPtyProvider: vi.fn(), + getPtyIdsForConnection: vi.fn().mockReturnValue([]), + clearPtyOwnershipForConnection: vi.fn(), + clearProviderPtyState: vi.fn(), + deletePtyOwnership: vi.fn(), + setPtyOwnership: vi.fn(), + restorePtyIncarnation: vi.fn(), + isCurrentPtyExit: vi.fn(() => true), + answerStartupTerminalColorQueriesForPty: vi.fn((_id: string, data: string) => data) +})) +vi.mock('../providers/ssh-filesystem-dispatch', () => ({ + registerSshFilesystemProvider: vi.fn(), + unregisterSshFilesystemProvider: vi.fn(), + getSshFilesystemProvider: vi.fn().mockReturnValue({ dispose: vi.fn() }) +})) +vi.mock('../providers/ssh-git-dispatch', () => ({ + registerSshGitProvider: vi.fn(), + unregisterSshGitProvider: vi.fn() +})) + +const { getSshPtyProvider, getPtyIdsForConnection } = await import('../ipc/pty') + +const TARGET = 'target-1' +const OUR_CLIENT = 'client-instance-1' + +function hostEntry(overrides: Record = {}): Record { + return { + id: `ssh:${TARGET}@@pty-orphan`, + incarnationId: 'inc-orphan', + cwd: '/home/user', + title: 'zsh', + // The relay stamps this from the live consumer grant, so it names THIS client. + ownerClientInstanceId: OUR_CLIENT, + hostAgeMs: 120_000, + paneBound: true, + ...overrides + } +} + +describe('SshRelaySession orphaned relay PTY sweep', () => { + beforeEach(() => { + vi.clearAllMocks() + muxRequestMock.mockReset() + muxRequestMock.mockResolvedValue([]) + mockDeploySuccess() + vi.mocked(getPtyIdsForConnection).mockReturnValue([]) + }) + + async function establish( + processes: Record[], + leases: { ptyId: string; state: string }[] = [] + ): Promise<{ shutdown: ReturnType }> { + const deps = createMockDeps() + // Why the recovery row: it pins this session's clientInstanceId, and the comparison is + // meaningless unless the id it uses is the persisted one. + vi.mocked(deps.mockStore.getSshPtyConsumerRecovery).mockReturnValue({ + targetId: TARGET, + clientInstanceId: OUR_CLIENT, + serverBuildId: 'build-1', + clientGeneration: 1, + ownerGeneration: 1, + ownerLease: 'test-owner-lease' + } as ReturnType) + vi.mocked(deps.mockStore.getSshRemotePtyLeases).mockReturnValue( + leases.map((lease) => ({ targetId: TARGET, ...lease })) as ReturnType< + typeof deps.mockStore.getSshRemotePtyLeases + > + ) + const shutdown = vi.fn().mockResolvedValue(undefined) + vi.mocked(getSshPtyProvider).mockReturnValue({ + attachForReconnect: vi.fn().mockResolvedValue({}), + listProcesses: vi.fn().mockResolvedValue(processes), + shutdown, + dispose: vi.fn() + } as unknown as ReturnType) + + const session = new SshRelaySession( + TARGET, + deps.getMainWindow, + deps.mockStore, + deps.mockPortForward + ) + await session.establish(deps.mockConn) + return { shutdown } + } + + it('stops an attested orphan the client has no lease for', async () => { + const { shutdown } = await establish([hostEntry()]) + + expect(shutdown).toHaveBeenCalledWith(`ssh:${TARGET}@@pty-orphan`, { + immediate: true, + expectedIncarnationId: 'inc-orphan' + }) + }) + + it('never stops a PTY that still holds a live lease', async () => { + const { shutdown } = await establish( + [hostEntry({ id: `ssh:${TARGET}@@pty-live`, incarnationId: 'inc-live' })], + [{ ptyId: 'pty-live', state: 'detached' }] + ) + + expect(shutdown).not.toHaveBeenCalled() + }) + + it('never stops a PTY this relay attributes to a different client', async () => { + const { shutdown } = await establish([ + hostEntry({ ownerClientInstanceId: 'someone-elses-laptop' }) + ]) + + expect(shutdown).not.toHaveBeenCalled() + }) + + it('never stops anything a relay predating the attestation lists', async () => { + const legacy = hostEntry() + delete legacy.ownerClientInstanceId + delete legacy.hostAgeMs + delete legacy.paneBound + + const { shutdown } = await establish([legacy]) + + expect(shutdown).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index 68254fbb108..04fe6020749 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -10,6 +10,7 @@ import { isRelayEndpointHeldError } from './ssh-relay-endpoint-incumbent' import { forgetRelayNodePtyRepairs, recoverRelayNodePtyForSpawn } from './ssh-relay-node-pty-repair' import type { TerminalUnavailableCause } from '../../shared/terminal-unavailable-cause' import { replayPendingSshPtyKills } from './ssh-pending-pty-kill-replay' +import { sweepOrphanedRelayPtys } from './ssh-orphan-relay-pty-sweep' import { SshChannelMultiplexer } from './ssh-channel-multiplexer' import { SshPtyProvider } from '../providers/ssh-pty-provider' import type { SshPtyAttachResult } from '../providers/ssh-pty-session-reattach' @@ -2398,6 +2399,17 @@ export class SshRelaySession { Array.from(attachedLeaseIds) ) } + // Why last: reclaiming comes first, so every PTY this connect could route to is routed before + // anything asks which ones are unreachable (#9819). + await sweepOrphanedRelayPtys({ + targetId: this.targetId, + store: this.store, + provider: ptyProvider, + clientInstanceId: this.ptyConsumerClientInstanceId, + isSessionOwner: this.activePtyConsumerOwner() !== null, + routedPtyIds: ptyIds, + shouldContinue + }) } private async reattachKnownPty(args: { diff --git a/src/relay/pty-handler-ownership-attestation.test.ts b/src/relay/pty-handler-ownership-attestation.test.ts new file mode 100644 index 00000000000..7ab726fd533 --- /dev/null +++ b/src/relay/pty-handler-ownership-attestation.test.ts @@ -0,0 +1,107 @@ +// The host half of #9819: a client may only reap a relay PTY it can prove it created, so the relay +// has to say who created each one. The attestation is read from the live consumer grant, never from +// a spawn parameter — otherwise it would just echo the caller's claim back at it. +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, + 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, + endPtyHandlerTest, + type MockDispatcher +} from './pty-handler-test-harness' + +const PANE_KEY = 'tab-agent:22222222-2222-4222-8222-222222222222' + +type Summary = { + id: string + paneBound?: boolean + hostAgeMs?: number + ownerClientInstanceId?: string +} + +describe('PtyHandler publishes host-attested PTY ownership', () => { + let dispatcher: MockDispatcher + let handler: PtyHandler + let originalPlatform: PropertyDescriptor | undefined + + async function spawnFrom( + clientId: number, + params: Record = {} + ): Promise<{ id: string }> { + mockPtySpawn.mockReturnValue({ ...mockPtyInstance, onData: vi.fn(), onExit: vi.fn() }) + return (await dispatcher.callRequest('pty.spawn', params, { + clientId, + isStale: () => false + } as never)) as { id: string } + } + + async function listProcesses(): Promise { + return (await dispatcher.callRequest('pty.listProcesses', {})) as Summary[] + } + + beforeEach(() => { + ;({ dispatcher, handler, originalPlatform } = beginPtyHandlerTest({ + mockPtySpawn, + mockPtyInstance, + mockCreateShellPromptReadinessProbe + })) + handler.setConsumerIdentityResolver((clientId) => (clientId === 7 ? 'client-A' : null)) + }) + + afterEach(async () => { + await endPtyHandlerTest(handler, originalPlatform) + }) + + it('attributes a pane spawn to the identity the consumer grant names', async () => { + const { id } = await spawnFrom(7, { env: { ORCA_PANE_KEY: PANE_KEY } }) + vi.advanceTimersByTime(45_000) + + const entry = (await listProcesses()).find((process) => process.id === id) + + expect(entry?.ownerClientInstanceId).toBe('client-A') + expect(entry?.paneBound).toBe(true) + expect(entry?.hostAgeMs).toBeGreaterThanOrEqual(45_000) + }) + + it('omits the attestation entirely when the connection holds no active grant', async () => { + const { id } = await spawnFrom(9, { env: { ORCA_PANE_KEY: PANE_KEY } }) + + const entry = (await listProcesses()).find((process) => process.id === id) + + // Absent, not empty-string or null: a reader must be able to tell "unattested" from any value. + expect(entry).not.toHaveProperty('ownerClientInstanceId') + }) + + it('reports a bare shell as not pane-bound', async () => { + const { id } = await spawnFrom(7, {}) + + const entry = (await listProcesses()).find((process) => process.id === id) + + expect(entry?.paneBound).toBe(false) + expect(entry?.ownerClientInstanceId).toBe('client-A') + }) +}) diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index f8bd2351cf2..d8fe370b50c 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -190,6 +190,13 @@ type ManagedPty = { startupIngressIntent?: ReturnType ownerBackend: PtyOwnerBackend agentSessionOwners?: AgentSessionOwnerBinding[] + /** Host clock, host-relative only: published as an age so no client has to trust our wall clock. */ + createdAt: number + /** The authenticated consumer identity that asked this host to create this PTY, read from the + * live grant rather than from a spawn parameter. Absent whenever the host could not attest one + * (no consumer session, or a revive replaying state some other client serialized), and absence + * must never be read as "nobody owns it". */ + ownerClientInstanceId?: string } type RelayAgentSessionCreateResult = { @@ -369,6 +376,14 @@ type PtyProcessSummary = { terminalHandle?: string foregroundProcessEvidence?: ForegroundProcessEvidence agentSessionOwners?: AgentSessionOwnerBinding[] + /** Age on the HOST's clock. Published instead of a creation timestamp so a client with a skewed + * clock cannot compute a negative or enormous age and act on it. */ + hostAgeMs?: number + /** True when this PTY was spawned for an Orca pane (`ORCA_PANE_KEY`). False means a bare relay + * shell. Absent from a host that predates the field — which is neither. */ + paneBound?: boolean + /** See {@link ManagedPty.ownerClientInstanceId}. Omitted when this host cannot attest one. */ + ownerClientInstanceId?: string } type SerializedPtyEntry = { @@ -456,6 +471,7 @@ export class PtyHandler { private consumerPausedOutputPtys = new Set() private removeLegacyCapacityListener: (() => void) | null = null private sourcePublication: RelayPtySourcePublication | null = null + private consumerIdentityResolver: ((clientId: number) => string | null) | null = null private lastInputAtByPty = new Map() private interactiveOutputCharsByPty = new Map() private pendingSpawnCount = 0 @@ -510,6 +526,12 @@ export class PtyHandler { this.sourcePublication = publication } + /** Supplies the authenticated client identity behind a transport connection, so a spawn can be + * attributed to the consumer session that requested it. */ + setConsumerIdentityResolver(resolve: ((clientId: number) => string | null) | null): void { + this.consumerIdentityResolver = resolve + } + handleSourceCreditAvailable(id: string): void { this.sourcePublication?.onCreditAvailable(id) } @@ -1871,11 +1893,15 @@ export class PtyHandler { params.startupIngressVersion === PTY_STARTUP_INGRESS_VERSION ? parsePtyStartupIngressIntent(params.startupIngress) : undefined + const ownerClientInstanceId = + context === undefined ? null : (this.consumerIdentityResolver?.(context.clientId) ?? null) const managed: ManagedPty = { id, incarnationId: randomUUID(), pty: term, initialCwd: cwd, + createdAt: Date.now(), + ...(ownerClientInstanceId ? { ownerClientInstanceId } : {}), buffered: new RecentPtyOutputBuffer({ preserveChunkBoundaries: false, limit: REPLAY_BUFFER_MAX @@ -2451,6 +2477,11 @@ export class PtyHandler { incarnationId: managed.incarnationId, cwd: managed.initialCwd, title, + hostAgeMs: Math.max(0, Date.now() - managed.createdAt), + paneBound: Boolean(managed.paneKey ?? managed.attachIdentity?.paneKey), + ...(managed.ownerClientInstanceId + ? { ownerClientInstanceId: managed.ownerClientInstanceId } + : {}), ...(managed.worktreeId ? { worktreeId: managed.worktreeId } : {}), ...(managed.terminalHandle ? { terminalHandle: managed.terminalHandle } : {}), ...(foregroundProcessEvidence ? { foregroundProcessEvidence } : {}), @@ -2624,6 +2655,10 @@ export class PtyHandler { incarnationId: randomUUID(), pty: term, initialCwd: entry.cwd, + createdAt: Date.now(), + // Deliberately no ownerClientInstanceId: revive replays state a client serialized, which is + // not this host observing who asked for the shell. Unattested means never swept. + buffered: new RecentPtyOutputBuffer({ preserveChunkBoundaries: false, limit: REPLAY_BUFFER_MAX diff --git a/src/relay/relay-runtime-services.ts b/src/relay/relay-runtime-services.ts index 36276ed9b70..8fa5dc9d1d0 100644 --- a/src/relay/relay-runtime-services.ts +++ b/src/relay/relay-runtime-services.ts @@ -44,6 +44,11 @@ export class RelayRuntimeServices { (id, paused) => this.ptyHandler.setConsumerDeliveryPaused(id, paused), (id) => this.ptyHandler.handleSourceCreditAvailable(id) ) + // Why wired after construction: the handler is built first, but PTY ownership has to be + // attested from the consumer grant the adapter holds. + this.ptyHandler.setConsumerIdentityResolver((clientId) => + this.ptyConsumerSessionAdapter.clientInstanceIdFor(clientId) + ) this.ptySourcePublication = new RelayPtySourcePublication( dispatcher, this.ptyConsumerSessionAdapter, diff --git a/src/relay/ssh-pty-consumer-session-adapter.ts b/src/relay/ssh-pty-consumer-session-adapter.ts index 83e61f17fa3..906edd47ff8 100644 --- a/src/relay/ssh-pty-consumer-session-adapter.ts +++ b/src/relay/ssh-pty-consumer-session-adapter.ts @@ -112,6 +112,12 @@ export class SshPtyConsumerSessionAdapter { }) } + /** The authenticated client identity behind a transport connection, or null when it holds no + * active grant. Used to stamp host-attested ownership on a PTY at spawn. */ + clientInstanceIdFor(clientId: number): string | null { + return this.session.activeClientInstanceId(String(clientId)) + } + openDelivery( clientId: number, id: string, diff --git a/src/shared/pty-consumer-session.ts b/src/shared/pty-consumer-session.ts index 6b040be47cf..7c5b29aacfd 100644 --- a/src/shared/pty-consumer-session.ts +++ b/src/shared/pty-consumer-session.ts @@ -153,6 +153,15 @@ export class PtyConsumerSession { return client?.state === 'active' ? client.grant : null } + /** The authenticated client identity behind an active connection, or null. + * + * Why the host reads it here instead of taking a spawn parameter: this is what makes a later + * "this PTY belongs to you" attestation evidence rather than an echo of what a caller claimed. */ + activeClientInstanceId(connectionId: string): string | null { + const client = this.clients.get(connectionId) + return client?.state === 'active' ? client.clientInstanceId : null + } + private admissionFor( client: ClientRecord, displacedOwner?: Readonly diff --git a/src/shared/ssh-relay-pty-ownership-proof.test.ts b/src/shared/ssh-relay-pty-ownership-proof.test.ts new file mode 100644 index 00000000000..4dc8cb1ce14 --- /dev/null +++ b/src/shared/ssh-relay-pty-ownership-proof.test.ts @@ -0,0 +1,159 @@ +// #9819. Every case here is the same question asked from a different angle: can this client PROVE +// the host is holding a process nobody can reach? A "no" has to mean "leave it running". +import { describe, expect, it } from 'vitest' +import { + planRelayPtySweep, + RELAY_PTY_SWEEP_MAX_PER_PASS, + RELAY_PTY_SWEEP_MIN_AGE_MS, + type RelayPtyOwnershipEvidence, + type RelayPtySweepContext +} from './ssh-relay-pty-ownership-proof' + +const OURS = 'client-instance-ours' + +function orphan(overrides: Partial = {}): RelayPtyOwnershipEvidence { + return { + ptyId: 'pty-1', + incarnationId: 'inc-1', + ownerClientInstanceId: OURS, + hostAgeMs: RELAY_PTY_SWEEP_MIN_AGE_MS * 2, + paneBound: true, + ...overrides + } +} + +function context(overrides: Partial = {}): RelayPtySweepContext { + return { + clientInstanceId: OURS, + isSessionOwner: true, + routedPtyIds: new Set(), + minimumHostAgeMs: RELAY_PTY_SWEEP_MIN_AGE_MS, + ...overrides + } +} + +function reasonFor(plan: ReturnType, ptyId: string): string | undefined { + return plan.skipped.find((entry) => entry.ptyId === ptyId)?.reason +} + +describe('planRelayPtySweep', () => { + it('sweeps a pane PTY this host attests we created and we have lost every route to', () => { + const plan = planRelayPtySweep([orphan()], context()) + + expect(plan.sweep).toEqual([{ ptyId: 'pty-1', incarnationId: 'inc-1' }]) + }) + + it('never sweeps a PTY this client still routes to', () => { + const plan = planRelayPtySweep([orphan()], context({ routedPtyIds: new Set(['pty-1']) })) + + expect(plan.sweep).toEqual([]) + expect(reasonFor(plan, 'pty-1')).toBe('this client still has a route to it') + }) + + it('never sweeps a PTY the host attributes to another client instance', () => { + // The case that makes local absence useless as evidence: a second machine on the same build + // connects to the same relay, and its live agents are missing from our store exactly like an + // orphan is. + const plan = planRelayPtySweep( + [orphan({ ownerClientInstanceId: 'client-instance-theirs' })], + context() + ) + + expect(plan.sweep).toEqual([]) + expect(reasonFor(plan, 'pty-1')).toBe('host attests another client created it') + }) + + it('never sweeps a PTY younger than the floor', () => { + const plan = planRelayPtySweep( + [orphan({ hostAgeMs: RELAY_PTY_SWEEP_MIN_AGE_MS - 1 })], + context() + ) + + expect(plan.sweep).toEqual([]) + expect(reasonFor(plan, 'pty-1')).toBe('younger than the sweep floor') + }) + + it('never sweeps a bare host shell', () => { + const plan = planRelayPtySweep([orphan({ paneBound: false })], context()) + + expect(plan.sweep).toEqual([]) + expect(reasonFor(plan, 'pty-1')).toBe('not a pane-bound PTY') + }) + + it('never sweeps a PTY whose agent session the host still advertises as adoptable', () => { + const plan = planRelayPtySweep( + [orphan({ agentSessionOwners: [{ ptyId: 'pty-1' }] })], + context() + ) + + expect(plan.sweep).toEqual([]) + expect(reasonFor(plan, 'pty-1')).toBe('host still advertises an adoptable agent session') + }) + + it('never sweeps without the negotiated session-owner grant', () => { + const plan = planRelayPtySweep([orphan()], context({ isSessionOwner: false })) + + expect(plan.sweep).toEqual([]) + expect(reasonFor(plan, 'pty-1')).toBe('this client does not hold the relay session-owner grant') + }) + + it('refuses a pass larger than the per-pass ceiling instead of truncating it', () => { + const entries = Array.from({ length: RELAY_PTY_SWEEP_MAX_PER_PASS + 1 }, (_, index) => + orphan({ ptyId: `pty-${index}`, incarnationId: `inc-${index}` }) + ) + + const plan = planRelayPtySweep(entries, context()) + + expect(plan.sweep).toEqual([]) + expect(plan.skipped).toHaveLength(entries.length) + }) + + describe('against a host that predates the attestation', () => { + // Mixed versions: every new field is optional, and an older host publishes none of them. The + // sweep has to read each absence as "unknown", never as a permissive default. + it('skips an entry with no owner attestation', () => { + const { ownerClientInstanceId: _absent, ...legacy } = orphan() + + const plan = planRelayPtySweep([legacy], context()) + + expect(plan.sweep).toEqual([]) + expect(reasonFor(plan, 'pty-1')).toBe('host attested no owning client') + }) + + it('skips an entry with no published age', () => { + const { hostAgeMs: _absent, ...legacy } = orphan() + + const plan = planRelayPtySweep([legacy], context()) + + expect(plan.sweep).toEqual([]) + expect(reasonFor(plan, 'pty-1')).toBe('host published no age') + }) + + it('skips an entry with no paneBound field', () => { + const { paneBound: _absent, ...legacy } = orphan() + + const plan = planRelayPtySweep([legacy], context()) + + expect(plan.sweep).toEqual([]) + expect(reasonFor(plan, 'pty-1')).toBe('not a pane-bound PTY') + }) + + it('skips an entry with no incarnation, so no stop is ever unfenced', () => { + const { incarnationId: _absent, ...legacy } = orphan() + + const plan = planRelayPtySweep([legacy], context()) + + expect(plan.sweep).toEqual([]) + expect(reasonFor(plan, 'pty-1')).toBe('host published no PTY incarnation') + }) + + it('sweeps nothing at all when the whole listing predates the fields', () => { + const legacy = [ + { ptyId: 'pty-1', incarnationId: 'inc-1' }, + { ptyId: 'pty-2', incarnationId: 'inc-2' } + ] + + expect(planRelayPtySweep(legacy, context()).sweep).toEqual([]) + }) + }) +}) diff --git a/src/shared/ssh-relay-pty-ownership-proof.ts b/src/shared/ssh-relay-pty-ownership-proof.ts new file mode 100644 index 00000000000..1f9505db03e --- /dev/null +++ b/src/shared/ssh-relay-pty-ownership-proof.ts @@ -0,0 +1,144 @@ +/** Which relay PTYs a client may prove it orphaned, and therefore may stop (#9819). + * + * A relay PTY is a child of the detached relay daemon. Stopping one destroys a running process — + * often a running agent — on the user's remote machine, and the relay's 50-slot cap is a far + * cheaper failure than that. So every rule below is written to answer "can this client PROVE + * nobody owns this?" and to answer "no" whenever it cannot. + * + * The rule #9819 proposed — "pane-bound and the app no longer owns or leases it" — is not that + * proof. Absence from a client-side set is `unverifiable` by construction + * (`docs/reference/ssh-execution-boundary.md`): a second machine running the same Orca build + * connects to the SAME relay and displaces the session owner, and its PTYs are missing from THIS + * client's store for exactly the same reason a genuine orphan is. Sweeping on local absence alone + * would let one laptop reap another laptop's live agents. + * + * What replaces it: the host itself records which authenticated consumer identity asked it to + * create each PTY, and publishes that back. A PTY is sweepable only when the OWNING HOST names + * this client as its creator and this client's own durable state has no route to it. Both halves + * are required; either alone is a guess. + */ + +/** One `pty.listProcesses` entry, as far as this decision is concerned. Every field a host may + * omit is optional here, because a host predating it publishes nothing rather than a default. */ +export type RelayPtyOwnershipEvidence = { + /** Relay-scoped PTY id. */ + ptyId: string + incarnationId?: string + ownerClientInstanceId?: string + hostAgeMs?: number + paneBound?: boolean + /** Non-empty when the host still advertises an adoptable agent session on this PTY. */ + agentSessionOwners?: readonly unknown[] +} + +export type RelayPtySweepContext = { + /** This client's persisted consumer identity for the target. */ + clientInstanceId: string + /** Whether the relay granted THIS connection the `session-owner` role. A subscriber, or a client + * that fell back to the unnegotiated legacy path, never sweeps. */ + isSessionOwner: boolean + /** Every relay PTY id this client still has any route to: a live provider PTY, a lease it has + * not tombstoned, an id it just reattached, or a stop it has recorded and not yet delivered. */ + routedPtyIds: ReadonlySet + /** Host-measured age a PTY must exceed. Guards a spawn that is in flight from another window of + * this same client and has not written its lease yet. */ + minimumHostAgeMs: number +} + +export type RelayPtySweepTarget = { ptyId: string; incarnationId: string } + +export type RelayPtySweepSkip = { ptyId: string; reason: string } + +export type RelayPtySweepPlan = { + sweep: RelayPtySweepTarget[] + skipped: RelayPtySweepSkip[] +} + +/** Deliberately longer than any single connect round trip. A PTY younger than this is never worth + * the risk: the leak it represents costs one slot for 30 more seconds, and reaping a shell that a + * concurrent spawn is still recording costs the user a terminal. */ +export const RELAY_PTY_SWEEP_MIN_AGE_MS = 30_000 + +/** Bounds one pass. A relay is capped at 50 PTYs, so a pass that wants to stop more than this is + * not reclaiming a leak — it is a disagreement about ownership, and stopping is the wrong move. */ +export const RELAY_PTY_SWEEP_MAX_PER_PASS = 8 + +function skipReason( + entry: RelayPtyOwnershipEvidence, + context: RelayPtySweepContext +): string | null { + if (typeof entry.incarnationId !== 'string' || entry.incarnationId.length === 0) { + // Without the host's own incarnation there is no fence, and an unfenced stop aimed at a relay + // id can hit whatever holds that id by the time it lands. + return 'host published no PTY incarnation' + } + if (typeof entry.ownerClientInstanceId !== 'string' || entry.ownerClientInstanceId.length === 0) { + return 'host attested no owning client' + } + if (entry.ownerClientInstanceId !== context.clientInstanceId) { + return 'host attests another client created it' + } + if (entry.paneBound !== true) { + // Covers both a bare host shell (a remote CLI terminal nobody's pane owns) and a host that + // never published the field. Neither is a pane this client lost. + return 'not a pane-bound PTY' + } + if (entry.agentSessionOwners !== undefined && entry.agentSessionOwners.length > 0) { + // The host still advertises this session as adoptable, so a later spawn can reclaim the running + // agent. Reaping it converts a recoverable session into a destroyed one. + return 'host still advertises an adoptable agent session' + } + if (typeof entry.hostAgeMs !== 'number' || !Number.isFinite(entry.hostAgeMs)) { + return 'host published no age' + } + if (entry.hostAgeMs < context.minimumHostAgeMs) { + return 'younger than the sweep floor' + } + if (context.routedPtyIds.has(entry.ptyId)) { + return 'this client still has a route to it' + } + return null +} + +/** Plans one sweep pass. Pure: every input is evidence the caller already gathered, so the rule can + * be tested without a relay, and the irreversible call sits with the caller. */ +export function planRelayPtySweep( + entries: readonly RelayPtyOwnershipEvidence[], + context: RelayPtySweepContext +): RelayPtySweepPlan { + if (!context.isSessionOwner || !context.clientInstanceId) { + return { + sweep: [], + skipped: entries.map((entry) => ({ + ptyId: entry.ptyId, + reason: 'this client does not hold the relay session-owner grant' + })) + } + } + const sweep: RelayPtySweepTarget[] = [] + const skipped: RelayPtySweepSkip[] = [] + for (const entry of entries) { + const reason = skipReason(entry, context) + if (reason !== null) { + skipped.push({ ptyId: entry.ptyId, reason }) + } else { + sweep.push({ ptyId: entry.ptyId, incarnationId: entry.incarnationId as string }) + } + } + if (sweep.length > RELAY_PTY_SWEEP_MAX_PER_PASS) { + // Why refuse rather than truncate: at this size the disagreement is about ownership, not about + // a handful of leaked slots, and a truncated pass would work through the same list one connect + // at a time and destroy it all anyway. + return { + sweep: [], + skipped: [ + ...skipped, + ...sweep.map((target) => ({ + ptyId: target.ptyId, + reason: `refusing a ${sweep.length}-PTY sweep; over the ${RELAY_PTY_SWEEP_MAX_PER_PASS} per-pass ceiling` + })) + ] + } + } + return { sweep, skipped } +}