mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
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:
@@ -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()
|
||||
})
|
||||
})
|
||||
@@ -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()
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user