diff --git a/src/main/ipc/runtime-environment-connectivity-handlers.ts b/src/main/ipc/runtime-environment-connectivity-handlers.ts index 827b76c08e5..b1f26a2d385 100644 --- a/src/main/ipc/runtime-environment-connectivity-handlers.ts +++ b/src/main/ipc/runtime-environment-connectivity-handlers.ts @@ -30,7 +30,6 @@ import { import { clearRuntimeEnvironmentManualDisconnect, isRuntimeEnvironmentManuallyDisconnected, - markRuntimeEnvironmentManuallyDisconnected, RUNTIME_MANUALLY_DISCONNECTED_MESSAGE } from './runtime-environment-manual-disconnect' import { @@ -40,6 +39,11 @@ import { import { publicRuntimeEnvironmentWithHostKey } from './runtime-environment-host-key' import { withWslOrcadAccountEnv } from '../wsl/wsl-orcad-account-env' import { clearSshCredentialDeclinedForEnvironment } from '../ssh/ssh-credential-decline' +import { + disconnectManagedHostOfEnvironmentForUser, + disconnectRuntimeEnvironmentForUser, + liftSshUserDisconnectForEnvironment +} from './ssh-host-user-disconnect' import { observeWslOrcadHostResponse, wslOrcadHostSupports @@ -141,13 +145,16 @@ export function registerRuntimeEnvironmentConnectivityHandlers({ ) ipcMain.handle( 'runtimeEnvironments:disconnect', - (_event, args: { selector: string }): { disconnected: PublicKnownRuntimeEnvironment } => { + async ( + _event, + args: { selector: string } + ): Promise<{ disconnected: PublicKnownRuntimeEnvironment }> => { const environment = resolveEnvironment(getUserDataPath(), args.selector) - markRuntimeEnvironmentManuallyDisconnected(environment.id) - invalidateTransport(environment.id) + const runtimeLayer = { userDataPath: getUserDataPath(), invalidateTransport } + disconnectRuntimeEnvironmentForUser(environment.id, runtimeLayer) closeLegacySelectorTransport(args.selector, environment.id) - // Retain disconnected evidence for renderers that missed the teardown event. - getRuntimeEnvironmentStatusOwner(getUserDataPath(), environment.id) + // Why: otherwise Settings keeps showing the managed host's SSH connection as live (#30085). + await disconnectManagedHostOfEnvironmentForUser(environment, runtimeLayer) return { disconnected: redactRuntimeEnvironment(environment) } } ) @@ -161,6 +168,7 @@ export function registerRuntimeEnvironmentConnectivityHandlers({ assertWslHostReachable(environment) clearRuntimeEnvironmentManualDisconnect(environment.id) clearSshCredentialDeclinedForEnvironment(environment) + await liftSshUserDisconnectForEnvironment(environment) await connectThroughOwningService(environment) return getRuntimeEnvironmentStatus(getUserDataPath(), environment.id, args.timeoutMs, { reconnect: true diff --git a/src/main/ipc/runtime-environment-managed-host-disconnect.test.ts b/src/main/ipc/runtime-environment-managed-host-disconnect.test.ts new file mode 100644 index 00000000000..3862f99c9ee --- /dev/null +++ b/src/main/ipc/runtime-environment-managed-host-disconnect.test.ts @@ -0,0 +1,121 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const hoisted = vi.hoisted(() => { + const environment: Record = {} + return { + handlers: new Map unknown>(), + environment, + teardown: vi.fn(), + invalidate: vi.fn(), + connectHost: vi.fn() + } +}) + +vi.mock('electron', () => ({ + ipcMain: { + handle: (channel: string, handler: (event: unknown, args: unknown) => unknown) => + hoisted.handlers.set(channel, handler) + } +})) +vi.mock('../../shared/runtime-environment-store', async (importOriginal) => ({ + ...(await importOriginal()), + resolveEnvironment: () => hoisted.environment, + listEnvironments: () => [hoisted.environment] +})) +vi.mock('./runtime-environment-request-connections', async (importOriginal) => ({ + ...(await importOriginal()), + getRuntimeEnvironmentStatusOwner: () => ({ activate: vi.fn() }), + closeRemoteRuntimeRequestConnection: vi.fn() +})) +vi.mock('./runtime-environment-host-service-ops', () => ({ + connectThroughOwningService: vi.fn(async () => {}), + removeThroughOwningService: vi.fn() +})) +vi.mock('./runtime-environment-transport-routing', () => ({ + callRuntimeEnvironment: vi.fn(), + getRuntimeEnvironmentStatus: vi.fn(async () => ({ ok: true })) +})) +vi.mock('./ssh-session-teardown', () => ({ disconnectRegisteredSshTarget: hoisted.teardown })) +vi.mock('../ssh/ssh-target-registry', async (importOriginal) => ({ + ...(await importOriginal()), + connectRegisteredSshTarget: hoisted.connectHost +})) + +import { registerRuntimeEnvironmentConnectivityHandlers } from './runtime-environment-connectivity-handlers' +import { + clearRuntimeEnvironmentManualDisconnect, + isRuntimeEnvironmentManuallyDisconnected +} from './runtime-environment-manual-disconnect' +import { isSshUserDisconnected, resetSshUserDisconnectsForTests } from '../ssh/ssh-user-disconnect' + +const deployment = { + kind: 'ssh', + requestId: 'request-1', + sshTargetId: 'ssh-1', + sshTargetGeneration: 1, + localPort: 41000, + remotePort: 6768 +} + +describe("the status bar's Disconnect of a server", () => { + beforeEach(() => { + hoisted.handlers.clear() + hoisted.teardown.mockReset().mockResolvedValue(undefined) + hoisted.invalidate.mockReset() + hoisted.connectHost.mockReset().mockResolvedValue(undefined) + resetSshUserDisconnectsForTests() + registerRuntimeEnvironmentConnectivityHandlers({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: these handlers read only store.getSettings(), which this stub provides. + store: { getSettings: () => ({}) } as never, + getUserDataPath: () => '/tmp/orca-user-data', + invalidateTransport: hoisted.invalidate + }) + }) + + afterEach(() => { + clearRuntimeEnvironmentManualDisconnect('env-1') + }) + + function invoke(channel: string): Promise { + return Promise.resolve(hoisted.handlers.get(channel)!(null, { selector: 'env-1' })) + } + + it('also disconnects the SSH host a managed server runs on, so Settings agrees', async () => { + hoisted.environment = { id: 'env-1', endpoints: [], orcadDeployment: deployment } + + await invoke('runtimeEnvironments:disconnect') + + expect(isRuntimeEnvironmentManuallyDisconnected('env-1')).toBe(true) + expect(hoisted.teardown).toHaveBeenCalledWith('ssh-1') + expect(isSshUserDisconnected('ssh-1')).toBe(true) + expect(hoisted.invalidate).toHaveBeenCalledTimes(1) + }) + + it('leaves SSH alone for a server that is not managed over SSH', async () => { + hoisted.environment = { id: 'env-1', endpoints: [] } + + await invoke('runtimeEnvironments:disconnect') + + expect(hoisted.teardown).not.toHaveBeenCalled() + expect(isSshUserDisconnected('ssh-1')).toBe(false) + }) + + it("is lifted by the status bar's Connect, through the host's own Connect", async () => { + hoisted.environment = { id: 'env-1', endpoints: [], orcadDeployment: deployment } + await invoke('runtimeEnvironments:disconnect') + + await invoke('runtimeEnvironments:connect') + + expect(isRuntimeEnvironmentManuallyDisconnected('env-1')).toBe(false) + // That connect republishes the managed verdict and restores the host's port forwards. + expect(hoisted.connectHost).toHaveBeenCalledWith('ssh-1') + }) + + it('reconnects a server whose host the user never disconnected without dialing the host', async () => { + hoisted.environment = { id: 'env-1', endpoints: [], orcadDeployment: deployment } + + await invoke('runtimeEnvironments:connect') + + expect(hoisted.connectHost).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/ipc/ssh-app-shutdown.test.ts b/src/main/ipc/ssh-app-shutdown.test.ts index 336a447d4aa..f9e53778f24 100644 --- a/src/main/ipc/ssh-app-shutdown.test.ts +++ b/src/main/ipc/ssh-app-shutdown.test.ts @@ -109,7 +109,7 @@ describe('SSH IPC handlers', () => { for (let tick = 0; tick < 5; tick++) { await Promise.resolve() } - expect(mockConnectionManager.connect).toHaveBeenCalledWith(target) + expect(mockConnectionManager.connect).toHaveBeenCalledWith(target, { probe: true }) quitTeardownStartGate.tryStart({ preventDefault() {} }) const shutdown = beginSshShutdown() diff --git a/src/main/ipc/ssh-connect-flow.ts b/src/main/ipc/ssh-connect-flow.ts index dc40c72c633..fec364318d6 100644 --- a/src/main/ipc/ssh-connect-flow.ts +++ b/src/main/ipc/ssh-connect-flow.ts @@ -26,8 +26,13 @@ import { abandonDecisionTransport } from './ssh-session-teardown' import { awaitTargetLifecycle } from './ssh-target-lifecycle-queue' import { startLegacyRelayBridge } from '../ssh/legacy-relay-bridge/legacy-relay-bridge-registry' import { clearSshCredentialDeclined } from '../ssh/ssh-credential-decline' +import { createSshUserDisconnectedError, isSshUserDisconnected } from '../ssh/ssh-user-disconnect' +import { + liftSshHostUserDisconnect, + resumeRuntimeEnvironmentStatus +} from './ssh-host-user-disconnect' -/** `automatic`: no user asked for this connect, so a cancelled sign-in stays cancelled. */ +/** `automatic`: no user asked for this connect, so a cancelled sign-in or a Disconnect stays. */ export async function connectTarget( targetId: string, options: { automatic?: boolean } = {} @@ -37,9 +42,24 @@ export async function connectTarget( appendFileSync(e2eProbePath, `${JSON.stringify(targetId)}\n`) throw new Error('e2e_forbidden_local_ssh_connect') } - if (!options.automatic) { - clearSshCredentialDeclined(targetId) + if (options.automatic) { + // Why before any publish: the host must keep reading "disconnected", not a failed connect. + if (isSshUserDisconnected(targetId)) { + const label = getSshTargetRegistryStore()?.getTarget(targetId)?.label ?? targetId + throw createSshUserDisconnectedError(label) + } + return admitConnect(targetId) } + clearSshCredentialDeclined(targetId) + const liftedEnvironments = liftSshHostUserDisconnect(targetId) + try { + return await admitConnect(targetId) + } finally { + resumeRuntimeEnvironmentStatus(liftedEnvironments) + } +} + +async function admitConnect(targetId: string): Promise { // Why: fence callers that entered before a same-turn disconnect/reset but resume after its cleanup. const admissionAuthority = getSshProviderAuthority(targetId) await awaitTargetLifecycle(targetId) diff --git a/src/main/ipc/ssh-connection-handlers.ts b/src/main/ipc/ssh-connection-handlers.ts index 152e63b317d..b61b2395252 100644 --- a/src/main/ipc/ssh-connection-handlers.ts +++ b/src/main/ipc/ssh-connection-handlers.ts @@ -8,7 +8,7 @@ import { import { connectTarget } from './ssh-connect-flow' import { connectionManager } from './ssh-ipc-context' import { getPublicSshState } from './ssh-renderer-broadcast' -import { disconnectRegisteredSshTarget } from './ssh-session-teardown' +import { disconnectSshHostForUser } from './ssh-host-user-disconnect' import { errorMessage } from '../../shared/error-message' import { clearSshCredentialDeclined, isSshCredentialDeclined } from '../ssh/ssh-credential-decline' @@ -23,7 +23,7 @@ export function registerSshConnectionHandlers(): void { }) ipcMain.handle('ssh:disconnect', async (_event, args: { targetId: string }) => { - await disconnectRegisteredSshTarget(args.targetId) + await disconnectSshHostForUser(args.targetId) }) ipcMain.handle('ssh:getState', (_event, args: { targetId: string }) => { @@ -65,7 +65,8 @@ export function registerSshConnectionHandlers(): void { const probe = runSshTestConnectionProbe(args.targetId, async () => { // Why: a probe transport opened after the shutdown drain would outlive orderly teardown. assertSshConnectsNotFenced() - const conn = await connectionManager!.connect(target) + // Why: a test of a disconnected host may dial it, but must leave it disconnected. + const conn = await connectionManager!.connect(target, { probe: true }) const state = conn.getState() await connectionManager!.disconnect(args.targetId) return state diff --git a/src/main/ipc/ssh-host-user-disconnect.test.ts b/src/main/ipc/ssh-host-user-disconnect.test.ts new file mode 100644 index 00000000000..eaf567bc95a --- /dev/null +++ b/src/main/ipc/ssh-host-user-disconnect.test.ts @@ -0,0 +1,151 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = await vi.hoisted(async () => { + const { createSshIpcMocks } = await import('./ssh-ipc-module-mocks') + return createSshIpcMocks() +}) +const runtime = vi.hoisted(() => { + const environments: unknown[] = [] + return { environments, activate: vi.fn(), getStatusOwner: vi.fn(), invalidate: vi.fn() } +}) + +vi.mock('../ssh/ssh-config-host-picker', () => mocks.sshConfigHostPicker) +vi.mock('electron', () => mocks.electron) +vi.mock('../ssh/ssh-connection-store', () => mocks.sshConnectionStore) +vi.mock('./ssh-host-server-connect', () => mocks.hostServerConnect) +vi.mock('../ssh/ssh-connection-manager', () => mocks.sshConnectionManager) +vi.mock('../ssh/ssh-channel-multiplexer', () => mocks.sshChannelMultiplexer) +vi.mock('../ssh/ssh-port-forward', () => mocks.sshPortForward) +vi.mock('../ssh/ssh-port-scanner', () => mocks.sshPortScanner) +vi.mock('../../shared/runtime-environment-store', async (importOriginal) => ({ + ...(await importOriginal()), + listEnvironments: () => runtime.environments +})) +vi.mock('./runtime-environment-request-connections', async (importOriginal) => ({ + ...(await importOriginal()), + getRuntimeEnvironmentStatusOwner: runtime.getStatusOwner +})) +vi.mock('./runtime-environments', () => ({ + invalidateRuntimeEnvironmentTransport: runtime.invalidate +})) + +import type { SshConnectionState, SshTarget } from '../../shared/ssh-types' +import { + clearRuntimeEnvironmentManualDisconnect, + isRuntimeEnvironmentManuallyDisconnected +} from './runtime-environment-manual-disconnect' +import { isSshUserDisconnected } from '../ssh/ssh-user-disconnect' +import { createSshIpcHarness } from './ssh-ipc-test-harness' + +const { mockSshStore, mockConnectionManager } = mocks + +const target: SshTarget = { + id: 'ssh-1', + label: 'Server', + host: 'example.com', + port: 22, + username: 'deploy' +} +const managedServer = { + id: 'env-managed', + orcadDeployment: { + kind: 'ssh', + requestId: 'request-1', + sshTargetId: 'ssh-1', + sshTargetGeneration: 1, + localPort: 41000, + remotePort: 6768 + } +} +const otherServer = { id: 'env-other' } + +describe('a user Disconnect of an SSH host', () => { + const harness = createSshIpcHarness(mocks) + const { handlers } = harness + + beforeEach(async () => { + await harness.reset() + runtime.environments = [managedServer, otherServer] + runtime.getStatusOwner.mockReset().mockReturnValue({ activate: runtime.activate }) + runtime.activate.mockReset() + runtime.invalidate.mockReset() + mockSshStore.getTarget.mockReturnValue(target) + mockConnectionManager.disconnect.mockResolvedValue(undefined) + }) + + afterEach(() => { + clearRuntimeEnvironmentManualDisconnect(managedServer.id) + clearRuntimeEnvironmentManualDisconnect(otherServer.id) + }) + + async function disconnect(): Promise { + await handlers.get('ssh:disconnect')!(null, { targetId: target.id }) + } + + it('is not undone by a background connect such as a pane or automation restore', async () => { + await disconnect() + + await expect( + handlers.get('ssh:connect')!(null, { targetId: target.id, automatic: true }) + ).rejects.toThrow('Server was disconnected. Connect to use it again.') + expect(mockConnectionManager.connect).not.toHaveBeenCalled() + expect(isSshUserDisconnected(target.id)).toBe(true) + }) + + it("also stops the managed server's status checks, which would redial the host", async () => { + await disconnect() + + expect(isRuntimeEnvironmentManuallyDisconnected(managedServer.id)).toBe(true) + expect(runtime.invalidate).toHaveBeenCalledWith(managedServer.id) + expect(isRuntimeEnvironmentManuallyDisconnected(otherServer.id)).toBe(false) + }) + + /** Settings' Connect; the server decision is mocked, so only the lift matters here. */ + async function connectExplicitly(): Promise { + await Promise.resolve(handlers.get('ssh:connect')!(null, { targetId: target.id })).catch( + () => {} + ) + } + + it('is not re-applied by its own cleanup when a Connect follows at once', async () => { + const disconnecting = disconnect() + const connecting = connectExplicitly() + await Promise.all([disconnecting, connecting]) + + expect(isSshUserDisconnected(target.id)).toBe(false) + expect(isRuntimeEnvironmentManuallyDisconnected(managedServer.id)).toBe(false) + expect(runtime.invalidate).not.toHaveBeenCalled() + }) + + it('is lifted, with its server, by an explicit Connect, whose status checks resume after', async () => { + await disconnect() + mockConnectionManager.connect.mockResolvedValue({}) + const { publishManagedServerConnect } = await import('./ssh-host-server-connect') + const connected: SshConnectionState = { + targetId: target.id, + status: 'connected', + error: null, + reconnectAttempt: 0 + } + vi.mocked(publishManagedServerConnect).mockReturnValue(connected) + + await connectExplicitly() + + expect(isSshUserDisconnected(target.id)).toBe(false) + expect(isRuntimeEnvironmentManuallyDisconnected(managedServer.id)).toBe(false) + expect(runtime.activate).toHaveBeenCalledTimes(1) + }) + + it('still lets the user test the connection, which leaves it disconnected', async () => { + await disconnect() + mockConnectionManager.connect.mockResolvedValue({ + getState: () => ({ targetId: target.id, status: 'connected', error: null }) + }) + + await handlers.get('ssh:testConnection')!(null, { targetId: target.id }) + + expect(mockConnectionManager.connect).toHaveBeenCalledWith(target, { probe: true }) + expect(isSshUserDisconnected(target.id)).toBe(true) + expect(isRuntimeEnvironmentManuallyDisconnected(managedServer.id)).toBe(true) + }) +}) diff --git a/src/main/ipc/ssh-host-user-disconnect.ts b/src/main/ipc/ssh-host-user-disconnect.ts new file mode 100644 index 00000000000..10bc3d9f107 --- /dev/null +++ b/src/main/ipc/ssh-host-user-disconnect.ts @@ -0,0 +1,129 @@ +import { getAppEnvironment } from '../../shared/app-environment' +import { listEnvironments } from '../../shared/runtime-environment-store' +import { + getOrcadSshDeployment, + getRuntimeSshAccess, + type KnownRuntimeEnvironment +} from '../../shared/runtime-environments' +import { + clearSshUserDisconnected, + clearSshUserDisconnectedForEnvironment, + isSshUserDisconnected, + recordSshUserDisconnected +} from '../ssh/ssh-user-disconnect' +import { connectRegisteredSshTarget } from '../ssh/ssh-target-registry' +import { + clearRuntimeEnvironmentManualDisconnect, + isRuntimeEnvironmentManuallyDisconnected, + markRuntimeEnvironmentManuallyDisconnected +} from './runtime-environment-manual-disconnect' +import { getRuntimeEnvironmentStatusOwner } from './runtime-environment-request-connections' +import { disconnectRegisteredSshTarget } from './ssh-session-teardown' + +/** + * A server reached through an SSH host cannot work without it, and its status checks redial it. + * So a user's Disconnect of either one (Settings or the status bar) takes down both, and only an + * explicit Connect of either one brings both back; every surface then reads the same verdict. + */ + +type InvalidateTransport = (environmentId: string) => Promise | void +type RuntimeLayer = { userDataPath: string; invalidateTransport: InvalidateTransport } + +const appUserDataPath = (): string => getAppEnvironment().getPath('userData') + +function environmentsReachedThrough( + userDataPath: string, + targetId: string +): KnownRuntimeEnvironment[] { + return listEnvironments(userDataPath).filter( + (environment) => getRuntimeSshAccess(environment)?.sshTargetId === targetId + ) +} + +export function disconnectRuntimeEnvironmentForUser( + environmentId: string, + { userDataPath, invalidateTransport }: RuntimeLayer +): void { + markRuntimeEnvironmentManuallyDisconnected(environmentId) + void invalidateTransport(environmentId) + // Retain disconnected evidence for renderers that missed the teardown event. + getRuntimeEnvironmentStatusOwner(userDataPath, environmentId) +} + +export async function disconnectSshHostForUser( + targetId: string, + runtimeLayer?: RuntimeLayer +): Promise { + // Why synchronously, before any await: a status check already in flight must not redial, and the + // teardown must reserve the target's lifecycle so a later Connect queues behind it. + recordSshUserDisconnected(targetId) + const userDataPath = runtimeLayer?.userDataPath ?? appUserDataPath() + const environments = environmentsReachedThrough(userDataPath, targetId).filter( + (environment) => !isRuntimeEnvironmentManuallyDisconnected(environment.id) + ) + environments.forEach((environment) => markRuntimeEnvironmentManuallyDisconnected(environment.id)) + const teardown = disconnectRegisteredSshTarget(targetId) + if (environments.length > 0) { + // Why lazy: runtime-environments wires every runtime handler and imports the SSH layer. + const invalidateTransport = + runtimeLayer?.invalidateTransport ?? + (await import('./runtime-environments')).invalidateRuntimeEnvironmentTransport + for (const environment of environments) { + // A Connect during the import already lifted this server; tearing it down would undo that. + if (isRuntimeEnvironmentManuallyDisconnected(environment.id)) { + disconnectRuntimeEnvironmentForUser(environment.id, { userDataPath, invalidateTransport }) + } + } + } + await teardown +} + +/** The status bar's Disconnect of a managed server is a Disconnect of the machine it runs on. */ +export async function disconnectManagedHostOfEnvironmentForUser( + environment: Pick, + runtimeLayer: RuntimeLayer +): Promise { + const targetId = getOrcadSshDeployment(environment)?.sshTargetId + if (targetId) { + await disconnectSshHostForUser(targetId, runtimeLayer) + } +} + +/** The status bar's Connect of a server lifts the Disconnect of the SSH host it is reached through. */ +export async function liftSshUserDisconnectForEnvironment( + environment: Pick +): Promise { + const managedHostId = getOrcadSshDeployment(environment)?.sshTargetId + if (managedHostId && isSshUserDisconnected(managedHostId)) { + // Why: the host's own Connect republishes its managed verdict and restores its port forwards, + // which the status bar's Disconnect tore down with the host. + await connectRegisteredSshTarget(managedHostId) + return + } + clearSshUserDisconnectedForEnvironment(environment) +} + +/** + * An explicit Connect of an SSH host lifts its Disconnect and that of the servers reached through it. + * Returns the servers lifted, whose status checks resume once the connect settles. + */ +export function liftSshHostUserDisconnect(targetId: string): string[] { + clearSshUserDisconnected(targetId) + const lifted: string[] = [] + for (const environment of environmentsReachedThrough(appUserDataPath(), targetId)) { + if (isRuntimeEnvironmentManuallyDisconnected(environment.id)) { + clearRuntimeEnvironmentManualDisconnect(environment.id) + lifted.push(environment.id) + } + } + return lifted +} + +/** Why after the connect: an earlier status check would race the connect for the same host. */ +export function resumeRuntimeEnvironmentStatus(environmentIds: readonly string[]): void { + for (const environmentId of environmentIds) { + if (!isRuntimeEnvironmentManuallyDisconnected(environmentId)) { + getRuntimeEnvironmentStatusOwner(appUserDataPath(), environmentId).activate() + } + } +} diff --git a/src/main/ipc/ssh.ts b/src/main/ipc/ssh.ts index d4d9660a0e0..ff80652a03d 100644 --- a/src/main/ipc/ssh.ts +++ b/src/main/ipc/ssh.ts @@ -30,6 +30,7 @@ import { } from './ssh-connect-attempt-registry' import { createSshConnectionCallbacks } from './ssh-connection-state-callbacks' import { resetSshCredentialDeclinesForTests } from '../ssh/ssh-credential-decline' +import { resetSshUserDisconnectsForTests } from '../ssh/ssh-user-disconnect' import { registerSshConnectionHandlers } from './ssh-connection-handlers' import { registerPowerMonitorReconnect, @@ -170,6 +171,7 @@ export async function resetSshHandlerStateForTests(): Promise { testConnectionProbes.clear() credentialRequestedForTarget.clear() resetSshCredentialDeclinesForTests() + resetSshUserDisconnectsForTests() quitTeardownStartGate.resetForTests() resetSshShutdownDrain() diff --git a/src/main/ssh/ssh-connection-manager.test.ts b/src/main/ssh/ssh-connection-manager.test.ts index eacb991fbea..4418fb2a674 100644 --- a/src/main/ssh/ssh-connection-manager.test.ts +++ b/src/main/ssh/ssh-connection-manager.test.ts @@ -55,6 +55,7 @@ import { resetSshCredentialDeclinesForTests } from './ssh-credential-decline' import { isAuthError } from './ssh-connection-utils' +import { recordSshUserDisconnected, resetSshUserDisconnectsForTests } from './ssh-user-disconnect' const target = { id: 'target-1', @@ -271,3 +272,43 @@ describe('SshConnectionManager after a cancelled sign-in', () => { expect(mockState.instances).toHaveLength(1) }) }) + +describe('SshConnectionManager after a user Disconnect', () => { + beforeEach(() => { + mockState.connectResults.length = 0 + mockState.instances.length = 0 + resetSshUserDisconnectsForTests() + }) + + it('refuses the background dials that revived the host, without opening a connection', async () => { + // A managed server's tunnel, its status checks, a census and host-resume recovery dial here. + const manager = new SshConnectionManager({ onStateChange: vi.fn() }) + await manager.connect(target) + await manager.disconnect(target.id) + recordSshUserDisconnected(target.id) + mockState.instances.length = 0 + + const error = await manager.connect(target).catch((err: unknown) => err) + + expect(error instanceof Error && error.message).toBe( + 'Target 1 was disconnected. Connect to use it again.' + ) + expect(error instanceof Error && isAuthError(error)).toBe(false) + expect(mockState.instances).toHaveLength(0) + }) + + it("does not lend a connection test's transport to a background dial", async () => { + const manager = new SshConnectionManager({ onStateChange: vi.fn() }) + recordSshUserDisconnected(target.id) + await manager.connect(target, { probe: true }) + + await expect(manager.connect(target)).rejects.toThrow('was disconnected') + }) + + it("dials a user's connection test", async () => { + const manager = new SshConnectionManager({ onStateChange: vi.fn() }) + recordSshUserDisconnected(target.id) + + await expect(manager.connect(target, { probe: true })).resolves.toBeDefined() + }) +}) diff --git a/src/main/ssh/ssh-connection-manager.ts b/src/main/ssh/ssh-connection-manager.ts index 8eadf5cdd4d..3a5e2a4e50a 100644 --- a/src/main/ssh/ssh-connection-manager.ts +++ b/src/main/ssh/ssh-connection-manager.ts @@ -6,6 +6,7 @@ import { } from './ssh-connection' import { recordSshConnectionOpened, recordSshConnectionReused } from './ssh-connection-attribution' import { createSshCredentialDeclinedError, isSshCredentialDeclined } from './ssh-credential-decline' +import { createSshUserDisconnectedError, isSshUserDisconnected } from './ssh-user-disconnect' /** A remembered sign-in only applies to the endpoint it was given for; an edited host must ask. */ function signInEndpoint(target: SshTarget): string { @@ -37,7 +38,13 @@ export class SshConnectionManager { } } - async connect(target: SshTarget): Promise { + /** `probe`: a user's connection test, which a Disconnect does not refuse. */ + async connect(target: SshTarget, options?: { probe?: boolean }): Promise { + // Why: background dials would revive a host the user disconnected (#30086); before reuse, so + // none borrows the transport a connection test opened. + if (!options?.probe && isSshUserDisconnected(target.id)) { + throw createSshUserDisconnectedError(target.label) + } const existing = this.connections.get(target.id) if (existing?.getState().status === 'connected') { recordSshConnectionReused(existing) diff --git a/src/main/ssh/ssh-user-disconnect.ts b/src/main/ssh/ssh-user-disconnect.ts new file mode 100644 index 00000000000..37a0cc20e4d --- /dev/null +++ b/src/main/ssh/ssh-user-disconnect.ts @@ -0,0 +1,46 @@ +import { + getRuntimeSshAccess, + type KnownRuntimeEnvironment +} from '../../shared/runtime-environments' + +/** + * Hosts the user pressed Disconnect on. Until the user explicitly connects again, no background + * dial (a managed server's tunnel or status check, a census, a pane or automation restore) may + * reconnect them. Session-only, like a paired server's manual disconnect, so both halves of a + * managed host are forgotten together at restart. + */ +const userDisconnectedTargets = new Set() + +export function recordSshUserDisconnected(targetId: string): void { + userDisconnectedTargets.add(targetId) +} + +export function isSshUserDisconnected(targetId: string): boolean { + return userDisconnectedTargets.has(targetId) +} + +/** Only an explicit user connect may lift it. */ +export function clearSshUserDisconnected(targetId: string): void { + userDisconnectedTargets.delete(targetId) +} + +/** An explicit connect to a server reached over SSH is a connect of that SSH host too. */ +export function clearSshUserDisconnectedForEnvironment( + environment: Pick +): void { + const targetId = getRuntimeSshAccess(environment)?.sshTargetId + if (targetId) { + clearSshUserDisconnected(targetId) + } +} + +export function resetSshUserDisconnectsForTests(): void { + userDisconnectedTargets.clear() +} + +/** Not an auth or transient failure: callers neither ask for a sign-in nor retry it. */ +export function createSshUserDisconnectedError(label: string): Error { + return Object.assign(new Error(`${label} was disconnected. Connect to use it again.`), { + code: 'ssh_user_disconnected' + }) +}