fix(ssh): let a reconnect start its own loopback route while a stale start is in flight

Key the route-starting guard by connect generation so a start superseded by reconnect cannot block the new generation's route. Adds SshConnection lifecycle tests: route adoption, reconnect during a pending start, forward exit, and disconnect.
This commit is contained in:
Bingxin Xu
2026-10-21 20:35:30 -04:00
parent b1cdd46d5c
commit 3835c2d8bc
2 changed files with 149 additions and 5 deletions
@@ -0,0 +1,138 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { resetSshConnectionMocks } from './ssh-connection-test-harness'
import { createCallbacks, createTarget } from './ssh-connection-test-fixtures'
import { SshConnection } from './ssh-connection'
import type { SystemSshLoopbackRoute } from '../system-transport/system-ssh-loopback-route'
const { startRouteMock } = vi.hoisted(() => ({ startRouteMock: vi.fn() }))
vi.mock('ssh2', async () =>
(await import('../__tests__/ssh-connection-test-client')).createSsh2Module()
)
vi.mock('../system-transport/system-ssh-binary', async () =>
(await import('./ssh-connection-test-harness')).createSystemSshBinaryModule()
)
vi.mock('../system-transport/system-ssh-args', async (importOriginal) =>
(await import('./ssh-connection-test-harness')).withSystemSshArgsMocks(await importOriginal())
)
vi.mock('../system-transport/system-ssh-command', async (importOriginal) =>
(await import('./ssh-connection-test-harness')).withSystemSshCommandMocks(await importOriginal())
)
vi.mock('../ssh-control-socket', async () =>
(await import('./ssh-connection-test-harness')).createControlSocketModule()
)
vi.mock('../config/ssh-config-parser', async () =>
(await import('./ssh-connection-test-harness')).createSshConfigParserModule()
)
vi.mock('../config/ssh-g-config-resolution', async (importOriginal) =>
(await import('./ssh-connection-test-harness')).withSshGConfigResolutionMocks(
await importOriginal()
)
)
// Why the gate is forced on: it is platform-gated to win32 and covered in its own suite; this suite is the lifecycle.
vi.mock('../system-transport/system-ssh-loopback-route', () => ({
shouldUseSystemSshLoopbackRoute: () => true,
startSystemSshLoopbackRoute: startRouteMock
}))
function createRoute(localPort: number): SystemSshLoopbackRoute {
return {
args: { localPort, hostKeyAlias: 'example.com' },
onExit: vi.fn(),
dispose: vi.fn()
}
}
function deferRouteStart(): (route: SystemSshLoopbackRoute) => void {
let resolveRoute: (route: SystemSshLoopbackRoute) => void = () => undefined
startRouteMock.mockImplementationOnce(
() =>
new Promise<SystemSshLoopbackRoute>((resolve) => {
resolveRoute = resolve
})
)
return (route) => resolveRoute(route)
}
describe('SshConnection loopback route', () => {
beforeEach(() => {
resetSshConnectionMocks()
startRouteMock.mockReset()
})
it('routes later commands through the route once it is proven', async () => {
const finishStart = deferRouteStart()
const conn = new SshConnection(
createTarget({ proxyCommand: 'ssh -W %h:%p bastion.example.com' }),
createCallbacks()
)
await conn.connect()
expect(conn.getSystemSshBuildArgsOptions().loopbackRoute).toBeUndefined()
finishStart(createRoute(50001))
await vi.waitFor(() =>
expect(conn.getSystemSshBuildArgsOptions().loopbackRoute).toEqual({
localPort: 50001,
hostKeyAlias: 'example.com'
})
)
})
it('still builds a route for a reconnect that superseded a start in flight', async () => {
const finishStaleStart = deferRouteStart()
const finishCurrentStart = deferRouteStart()
const conn = new SshConnection(
createTarget({ proxyCommand: 'ssh -W %h:%p bastion.example.com' }),
createCallbacks()
)
await conn.connect()
await conn.reconnect()
expect(conn.getState().status).toBe('connected')
expect(startRouteMock).toHaveBeenCalledTimes(2)
const stale = createRoute(50001)
finishStaleStart(stale)
await vi.waitFor(() => expect(stale.dispose).toHaveBeenCalled())
expect(conn.getSystemSshBuildArgsOptions().loopbackRoute).toBeUndefined()
finishCurrentStart(createRoute(50002))
await vi.waitFor(() =>
expect(conn.getSystemSshBuildArgsOptions().loopbackRoute?.localPort).toBe(50002)
)
})
it('drops the route when its forward exits, so commands fall back to the proxy', async () => {
const finishStart = deferRouteStart()
const conn = new SshConnection(
createTarget({ proxyCommand: 'ssh -W %h:%p bastion.example.com' }),
createCallbacks()
)
await conn.connect()
const route = createRoute(50001)
finishStart(route)
await vi.waitFor(() => expect(route.onExit).toHaveBeenCalled())
const onExit = vi.mocked(route.onExit).mock.calls[0]?.[0]
onExit?.()
expect(conn.getSystemSshBuildArgsOptions().loopbackRoute).toBeUndefined()
})
it('disposes the route on disconnect', async () => {
const finishStart = deferRouteStart()
const conn = new SshConnection(
createTarget({ proxyCommand: 'ssh -W %h:%p bastion.example.com' }),
createCallbacks()
)
await conn.connect()
const route = createRoute(50001)
finishStart(route)
await vi.waitFor(() => expect(conn.getSystemSshBuildArgsOptions().loopbackRoute).toBeDefined())
await conn.disconnect()
expect(route.dispose).toHaveBeenCalled()
})
})
+11 -5
View File
@@ -199,7 +199,7 @@ export class SshConnection {
private systemSshControlMasterDisabledForSession = false
private systemSshGssapiOnlyForSession = false
private systemSshLoopbackRoute: SystemSshLoopbackRoute | null = null
private systemSshLoopbackRouteStarting = false
private systemSshLoopbackRouteStartingGeneration: number | null = null
private useSystemSshTransport = false
private credentialAbortController = new AbortController()
private readonly pendingSsh2Clients = new Set<SshClient>()
@@ -1107,15 +1107,21 @@ export class SshConnection {
const options = this.getSystemSshBuildArgsOptions()
if (
this.systemSshLoopbackRoute ||
this.systemSshLoopbackRouteStarting ||
this.systemSshLoopbackRouteStartingGeneration === connectGeneration ||
!shouldUseSystemSshLoopbackRoute(this.target, this.systemSshResolvedConfig, options)
) {
return
}
this.systemSshLoopbackRouteStarting = true
// Why per generation: a start a reconnect superseded can still be in flight, and must not block the new one.
this.systemSshLoopbackRouteStartingGeneration = connectGeneration
const settle = (): void => {
if (this.systemSshLoopbackRouteStartingGeneration === connectGeneration) {
this.systemSshLoopbackRouteStartingGeneration = null
}
}
startSystemSshLoopbackRoute(this.target, this.systemSshResolvedConfig, options).then(
(route) => {
this.systemSshLoopbackRouteStarting = false
settle()
if (!this.isCurrentConnectAttempt(connectGeneration) || !this.useSystemSshTransport) {
route.dispose()
return
@@ -1129,7 +1135,7 @@ export class SshConnection {
},
() => {
// Why: per-command proxy logins still work; the route only makes them cheaper.
this.systemSshLoopbackRouteStarting = false
settle()
}
)
}