diff --git a/src/main/ssh/connection/ssh-connection-loopback-route.test.ts b/src/main/ssh/connection/ssh-connection-loopback-route.test.ts new file mode 100644 index 00000000000..ab20392d642 --- /dev/null +++ b/src/main/ssh/connection/ssh-connection-loopback-route.test.ts @@ -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((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() + }) +}) diff --git a/src/main/ssh/connection/ssh-connection.ts b/src/main/ssh/connection/ssh-connection.ts index 6f4e7920d87..11d39f5a0b8 100644 --- a/src/main/ssh/connection/ssh-connection.ts +++ b/src/main/ssh/connection/ssh-connection.ts @@ -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() @@ -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() } ) }