fix(ssh): quarantine ambiguous direct reattach exits

This commit is contained in:
Brennan Benson
2026-08-16 22:43:25 -07:00
parent 972cc2b45c
commit 2ed57ae61b
3 changed files with 65 additions and 22 deletions
@@ -1,5 +1,6 @@
import { expect, it, vi } from 'vitest'
import { SshPtyProvider } from './ssh-pty-provider'
import { SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR } from './ssh-pty-errors'
function sourceActivation(ptyIncarnation: string) {
return {
@@ -130,7 +131,7 @@ it('rejects an SSH reattach whose matching exit shares the attach reply batch',
})
})
it('does not classify a legacy exit as current while reattach resolves its incarnation', async () => {
it('keeps a legacy exit unverifiable while reattach resolves its incarnation', async () => {
const mux = {
request: vi.fn(),
notify: vi.fn(),
@@ -166,13 +167,9 @@ it('does not classify a legacy exit as current while reattach resolves its incar
expect(exitListener).not.toHaveBeenCalled()
resolveAttach?.()
await expect(spawn).resolves.toMatchObject({
id: 'ssh:conn-1@@pty-existing',
incarnationId: 'incarnation-current',
isReattach: true
})
await expect(spawn).rejects.toThrow(SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR)
expect(exitListener).not.toHaveBeenCalled()
await expect(provider.probePtyLiveness('ssh:conn-1@@pty-existing')).resolves.toBe(true)
await expect(provider.probePtyLiveness('ssh:conn-1@@pty-existing')).resolves.toBeNull()
notify?.('pty.exit', {
id: 'pty-existing',
@@ -188,6 +185,34 @@ it('does not classify a legacy exit as current while reattach resolves its incar
})
})
it('keeps an incarnation-less attach and exit race unverifiable', async () => {
const mux = {
request: vi.fn(),
notify: vi.fn(),
onNotification: vi.fn(),
dispose: vi.fn(),
isDisposed: vi.fn().mockReturnValue(false)
}
const provider = new SshPtyProvider('conn-1', mux as never)
const exitListener = vi.fn()
provider.onExit(exitListener)
mux.request.mockImplementation(async (method: string, _params, options) => {
if (method !== 'pty.attach') {
return undefined
}
options?.beforeResolve?.({})
const notify = mux.onNotification.mock.calls[0]?.[0]
notify?.('pty.exit', { id: 'pty-existing', code: 0 })
return {}
})
await expect(
provider.spawn({ cols: 80, rows: 24, sessionId: 'ssh:conn-1@@pty-existing' })
).rejects.toThrow(SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR)
expect(exitListener).not.toHaveBeenCalled()
await expect(provider.probePtyLiveness('ssh:conn-1@@pty-existing')).resolves.toBeNull()
})
it('returns a provisional source activation lease to reconnect authority', async () => {
const mux = {
request: vi.fn(),
+15 -15
View File
@@ -244,14 +244,16 @@ export async function reattachSshPtySessionWithExitFence(
try {
result = await reattachSshPtySession(args)
const relayPtyId = toRelaySshPtyId(args.connectionId, result.id)
if (
args.exitRaceTracker.didMatchingExitArrive(operation, {
id: relayPtyId,
incarnationId: result.incarnationId
})
) {
const exitOutcome = args.exitRaceTracker.classifyReattachExit(operation, {
id: relayPtyId,
incarnationId: result.incarnationId
})
if (exitOutcome === 'exited') {
throw new Error('agent_session_exited_during_start')
}
if (exitOutcome === 'unverifiable') {
throw new Error(`${SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR}: ${relayPtyId}`)
}
return result
} catch (error) {
result?.sourceActivationLease?.rollback()
@@ -294,17 +296,15 @@ export async function reattachSshPtySessionForSpawn(
await result?.sourceActivationLease?.rollback()
if (isSshPtyExitedEvidenceError(error)) {
args.acceptExitedPty(result?.id ?? toAppSshPtyId(args.connectionId, args.sessionId))
} else if (
!isSshPtyIdentityMismatchError(error) &&
!isSshPtyRestoreRequiredError(error) &&
!isSshPtyLivenessUnverifiableError(error)
) {
} else if (!isSshPtyIdentityMismatchError(error) && !isSshPtyRestoreRequiredError(error)) {
const id = result?.id ?? toAppSshPtyId(args.connectionId, args.sessionId)
args.acceptUnverifiablePty(id)
throw new Error(
`${SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR}: ${toRelaySshPtyId(args.connectionId, id)}`,
{ cause: error }
)
if (!isSshPtyLivenessUnverifiableError(error)) {
throw new Error(
`${SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR}: ${toRelaySshPtyId(args.connectionId, id)}`,
{ cause: error }
)
}
}
throw error
}
@@ -9,6 +9,8 @@ type PendingSshPtySpawn = {
}[]
}
type SshPtyPendingExitOutcome = 'exited' | 'unverifiable' | null
export class SshPtySpawnExitRaceTracker {
private pending = new Set<PendingSshPtySpawn>()
@@ -52,6 +54,22 @@ export class SshPtySpawnExitRaceTracker {
return matchingExit !== undefined
}
classifyReattachExit(
operation: PendingSshPtySpawn,
result: { id: string; incarnationId?: PtyIncarnationId }
): SshPtyPendingExitOutcome {
const sameIdExits = operation.exits.filter((exit) => exit.relayPtyId === result.id)
if (!result.incarnationId) {
return sameIdExits.length > 0 ? 'unverifiable' : null
}
const matchingExit = sameIdExits.find((exit) => exit.incarnationId === result.incarnationId)
if (matchingExit) {
matchingExit.publish?.()
return 'exited'
}
return sameIdExits.some((exit) => !exit.incarnationId) ? 'unverifiable' : null
}
finish(operation: PendingSshPtySpawn): void {
this.pending.delete(operation)
}