From 63246eba3a55ae34bd37ff134c28d4572f912482 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 20 Oct 2026 03:09:31 -0700 Subject: [PATCH] fix(pairing): adding a server you already saved updates it instead of saving it twice (#31080) * fix(pairing): pairing a saved server again refreshes its entry instead of saving it twice * fix(pairing): only Settings refreshes a saved server in place; other add paths refuse the duplicate * fix(pairing): a same-key refresh keeps the server's workspaces and tabs; raw add refuses; match any endpoint key * fix(pairing): refuse a code for a hidden saved server; one choke point publishes verified status only for the pairing still saved * refactor(pairing): verified-pairing publish in its own module; an unreadable store skips the stamp instead of failing a saved pairing * fix(pairing): match a host key however its Base64 is spelled * fix(pairing): a refresh keeps the saved key spelling so the host fingerprint stays put --- src/cli/runtime/environments.test.ts | 24 ++- ...ntime-environment-connectivity-handlers.ts | 15 +- ...untime-environment-pairing-verification.ts | 13 +- .../runtime-environment-preference.test.ts | 3 +- .../runtime-environment-repair-handler.ts | 18 +- ...ent-session-tabs-delta.integration.test.ts | 32 ++- ...me-environment-verified-pairing-publish.ts | 42 ++++ .../runtime-environments-pairing.test.ts | 98 ++++++++- .../runtime-environments-repair.test.ts | 4 +- .../settings/RuntimeEnvironmentsPane.tsx | 1 - ...time-environment-mutation-actions.test.tsx | 111 ++++++++++ ...se-runtime-environment-mutation-actions.ts | 29 ++- .../sidebar/AddRemoteHostDialog.tsx | 11 +- src/renderer/src/i18n/locales/en.json | 3 +- src/renderer/src/i18n/locales/es.json | 3 +- src/renderer/src/i18n/locales/fr.json | 3 +- src/renderer/src/i18n/locales/ja.json | 3 +- src/renderer/src/i18n/locales/ko.json | 3 +- src/renderer/src/i18n/locales/zh.json | 3 +- ...ntime-environment-peer-replacement.test.ts | 19 +- .../runtime-environment-peer-replacement.ts | 5 + ...me-managed-repair-keeps-workspaces.test.ts | 25 +++ src/shared/remote-pairing-verification.ts | 2 + .../runtime-environment-host-key-match.ts | 36 ++++ ...time-environment-pairing-replace-guards.ts | 70 +++++++ .../runtime-environment-sidecar.test.ts | 5 +- ...ntime-environment-ssh-access-store.test.ts | 6 +- ...time-environment-store-same-server.test.ts | 197 ++++++++++++++++++ ...vironment-store-unreadable-records.test.ts | 16 +- src/shared/runtime-environment-store.test.ts | 18 +- src/shared/runtime-environment-store.ts | 127 +++++++---- src/shared/wsl-host-deployment-store.test.ts | 3 + 32 files changed, 828 insertions(+), 120 deletions(-) create mode 100644 src/main/ipc/runtime-environments/runtime-environment-verified-pairing-publish.ts create mode 100644 src/renderer/src/components/settings/use-runtime-environment-mutation-actions.test.tsx create mode 100644 src/shared/runtime-environment-host-key-match.ts create mode 100644 src/shared/runtime-environment-pairing-replace-guards.ts create mode 100644 src/shared/runtime-environment-store-same-server.test.ts diff --git a/src/cli/runtime/environments.test.ts b/src/cli/runtime/environments.test.ts index 67d9c10c502..aad4e9b42f3 100644 --- a/src/cli/runtime/environments.test.ts +++ b/src/cli/runtime/environments.test.ts @@ -11,12 +11,12 @@ import { resolveEnvironmentPairingOffer } from './environments' -function pairingCode(endpoint = 'ws://127.0.0.1:6768'): string { +function pairingCode(endpoint = 'ws://127.0.0.1:6768', keyFill = 1): string { return encodePairingOffer({ v: 2, endpoint, deviceToken: 'device-token', - publicKeyB64: Buffer.from(new Uint8Array(32).fill(1)).toString('base64') + publicKeyB64: Buffer.from(new Uint8Array(32).fill(keyFill)).toString('base64') }) } @@ -71,7 +71,7 @@ describe('CLI runtime environments', () => { expect(() => addEnvironmentFromPairingCode(userDataPath, { name: 'workstation', - pairingCode: pairingCode('ws://127.0.0.1:2222'), + pairingCode: pairingCode('ws://127.0.0.1:2222', 2), now: 200 }) ).toThrow('A server named "workstation" already exists.') @@ -81,4 +81,22 @@ describe('CLI runtime environments', () => { ) expect(listEnvironments(userDataPath)[0]?.id).toBe(first.id) }) + + it('refuses saving the same server twice and points at Settings, which can switch its connection', () => { + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-env-store-')) + addEnvironmentFromPairingCode(userDataPath, { + name: 'workstation', + pairingCode: pairingCode('ws://192.0.2.10:6768'), + now: 100 + }) + + expect(() => + addEnvironmentFromPairingCode(userDataPath, { + name: 'workstation over relay', + pairingCode: pairingCode('ws://198.51.100.7:6768'), + now: 200 + }) + ).toThrow('already paired as "workstation"') + expect(listEnvironments(userDataPath)).toHaveLength(1) + }) }) diff --git a/src/main/ipc/runtime-environments/runtime-environment-connectivity-handlers.ts b/src/main/ipc/runtime-environments/runtime-environment-connectivity-handlers.ts index 0dff59ec0b4..7d01e11eb1b 100644 --- a/src/main/ipc/runtime-environments/runtime-environment-connectivity-handlers.ts +++ b/src/main/ipc/runtime-environments/runtime-environment-connectivity-handlers.ts @@ -21,9 +21,9 @@ import type { Store } from '../../persistence' import { retireRemovedRuntimeEnvironment } from './runtime-environment-removal-cleanup' import { readSettingsWithRuntimeEnvironmentPreference } from './runtime-environment-preference' import { verifyAndAddRuntimeEnvironmentFromPairingCode } from './runtime-environment-pairing-verification' +import { publishVerifiedPairing } from './runtime-environment-verified-pairing-publish' import { closeRemoteRuntimeRequestConnection, - getRuntimeEnvironmentStatusOwner, getRuntimeEnvironmentStatusSnapshots, retryRemoteRuntimeSharedControlConnectionNow } from './runtime-environment-request-connections' @@ -92,6 +92,7 @@ export function registerRuntimeEnvironmentConnectivityHandlers({ _event, args: { name: string; pairingCode: string } ): { environment: PublicKnownRuntimeEnvironment } => { + // Why refuse, not refresh: nothing here proves the code works before it would replace one that does. const environment = addEnvironmentFromPairingCode(getUserDataPath(), args) clearRuntimeEnvironmentManualDisconnect(environment.id) return { environment: redactRuntimeEnvironment(environment) } @@ -102,13 +103,11 @@ export function registerRuntimeEnvironmentConnectivityHandlers({ async (_event, args: { name: string; pairingCode: string; allowLoopback?: boolean }) => { const result = await verifyAndAddRuntimeEnvironmentFromPairingCode(getUserDataPath(), args) if (result.ok) { - clearRuntimeEnvironmentManualDisconnect(result.environment.id) - getRuntimeEnvironmentStatusOwner(getUserDataPath(), result.environment.id).acceptVerified({ - id: 'status.get', - ok: true, - result: result.runtimeStatus, - _meta: { runtimeId: result.runtimeStatus.runtimeId } - }) + await publishVerifiedPairing( + getUserDataPath(), + result, + result.updatedExisting ? { invalidateTransport } : null + ) } return result } diff --git a/src/main/ipc/runtime-environments/runtime-environment-pairing-verification.ts b/src/main/ipc/runtime-environments/runtime-environment-pairing-verification.ts index 4b7c9b09800..0d01662fc3c 100644 --- a/src/main/ipc/runtime-environments/runtime-environment-pairing-verification.ts +++ b/src/main/ipc/runtime-environments/runtime-environment-pairing-verification.ts @@ -1,6 +1,6 @@ import { pairedRuntimeConnectionFromOffer } from '../../../shared/paired-runtime-connection' import { - addEnvironmentFromPairingCode, + addOrUpdateEnvironmentFromPairingCode, resolveEnvironment, RuntimeEnvironmentStoreError, updateEnvironmentFromPairingCode @@ -93,10 +93,12 @@ export async function verifyAndAddRuntimeEnvironmentFromPairingCode( } const { link, runtimeStatus } = verified const usesSshTunnel = link.endpointKind === 'loopback' && args.allowLoopback === true - let environment: ReturnType + let outcome: ReturnType try { - environment = addEnvironmentFromPairingCode(userDataPath, { + outcome = addOrUpdateEnvironmentFromPairingCode(userDataPath, { ...args, + // Why: recorded only if the link is for a saved server, which the key match proves it is. + runtimeId: runtimeStatus.runtimeId, ...(usesSshTunnel ? { connectionDependency: 'ssh-tunnel' as const } : {}) }) } catch (error) { @@ -111,8 +113,9 @@ export async function verifyAndAddRuntimeEnvironmentFromPairingCode( } return { ok: true, - environment: redactRuntimeEnvironment(environment), - runtimeStatus + environment: redactRuntimeEnvironment(outcome.environment), + runtimeStatus, + ...(outcome.updatedExisting ? { updatedExisting: true as const } : {}) } } diff --git a/src/main/ipc/runtime-environments/runtime-environment-preference.test.ts b/src/main/ipc/runtime-environments/runtime-environment-preference.test.ts index ae8b61f943f..cabc7af7ce1 100644 --- a/src/main/ipc/runtime-environments/runtime-environment-preference.test.ts +++ b/src/main/ipc/runtime-environments/runtime-environment-preference.test.ts @@ -43,7 +43,8 @@ function saveEnvironment(name = 'server') { v: 2, endpoint: 'ws://127.0.0.1:59999', deviceToken: 'test-token', - publicKeyB64: Buffer.alloc(32, 1).toString('base64') + // Why a key per name: one key is one server, which the store refuses to save twice. + publicKeyB64: Buffer.alloc(32, name).toString('base64') }) }) } diff --git a/src/main/ipc/runtime-environments/runtime-environment-repair-handler.ts b/src/main/ipc/runtime-environments/runtime-environment-repair-handler.ts index fc919b431f0..ec7cd099c01 100644 --- a/src/main/ipc/runtime-environments/runtime-environment-repair-handler.ts +++ b/src/main/ipc/runtime-environments/runtime-environment-repair-handler.ts @@ -1,8 +1,7 @@ import { ipcMain } from 'electron' import type { VerifyAndAddRuntimeEnvironmentResult } from '../../../shared/remote-pairing-verification' import { verifyAndRepairRuntimeEnvironmentFromPairingCode } from './runtime-environment-pairing-verification' -import { getRuntimeEnvironmentStatusOwner } from './runtime-environment-request-connections' -import { clearRuntimeEnvironmentManualDisconnect } from './runtime-environment-manual-disconnect' +import { publishVerifiedPairing } from './runtime-environment-verified-pairing-publish' export function registerRuntimeEnvironmentRepairHandler({ getUserDataPath, @@ -18,20 +17,9 @@ export function registerRuntimeEnvironmentRepairHandler({ args: { selector: string; pairingCode: string } ): Promise => { const result = await verifyAndRepairRuntimeEnvironmentFromPairingCode(getUserDataPath(), args) - if (!result.ok) { - return result + if (result.ok) { + await publishVerifiedPairing(getUserDataPath(), result, { invalidateTransport }) } - const environmentId = result.environment.id - // Why before invalidating: a manually disconnected server would not get a fresh owner. - clearRuntimeEnvironmentManualDisconnect(environmentId) - // Retires the owner blocked under the revoked pairing; its replacement keys on the new revision. - await invalidateTransport(environmentId) - getRuntimeEnvironmentStatusOwner(getUserDataPath(), environmentId).acceptVerified({ - id: 'status.get', - ok: true, - result: result.runtimeStatus, - _meta: { runtimeId: result.runtimeStatus.runtimeId } - }) return result } ) diff --git a/src/main/ipc/runtime-environments/runtime-environment-session-tabs-delta.integration.test.ts b/src/main/ipc/runtime-environments/runtime-environment-session-tabs-delta.integration.test.ts index 71687da6dbb..3fc60b44491 100644 --- a/src/main/ipc/runtime-environments/runtime-environment-session-tabs-delta.integration.test.ts +++ b/src/main/ipc/runtime-environments/runtime-environment-session-tabs-delta.integration.test.ts @@ -85,11 +85,7 @@ describe('session.tabs tab delta across mixed client versions', () => { runtime.registerPty(`pty-${index + 1}`, SESSION_WORKTREE_ID) } const hostDataPath = mkdtempSync(join(tmpdir(), 'orca-tab-delta-host-')) - const userDataPath = mkdtempSync(join(tmpdir(), 'orca-tab-delta-client-')) - cleanups.push(() => { - rmSync(hostDataPath, { recursive: true, force: true }) - rmSync(userDataPath, { recursive: true, force: true }) - }) + cleanups.push(() => rmSync(hostDataPath, { recursive: true, force: true })) const server = new OrcaRuntimeRpcServer({ runtime, userDataPath: hostDataPath, @@ -98,18 +94,31 @@ describe('session.tabs tab delta across mixed client versions', () => { }) await server.start() cleanups.push(() => server.stop()) - const pair = (name: string): string => { + // Why a profile per desktop: one profile saves a server once, so each desktop needs its own. + const pair = (name: string): { userDataPath: string; id: string } => { const offer = server.createPairingOffer({ address: '127.0.0.1', name, scope: 'runtime' }) if (!offer.available) { throw new Error('pairing_unavailable') } - return addEnvironmentFromPairingCode(userDataPath, { name, pairingCode: offer.pairingUrl }).id + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-tab-delta-client-')) + cleanups.push(() => rmSync(userDataPath, { recursive: true, force: true })) + const { id } = addEnvironmentFromPairingCode(userDataPath, { + name, + pairingCode: offer.pairingUrl + }) + return { userDataPath, id } + } + const pairedConnection = (name: string) => { + const desktop = pair(name) + return getPreferredPairedRuntimeConnection( + resolveEnvironment(desktop.userDataPath, desktop.id) + ) } // A desktop released before the delta: same handshake, minus the one capability. const legacy = recorder() const legacySubscription = await subscribeRemoteRuntimeRequest( - getPreferredPairedRuntimeConnection(resolveEnvironment(userDataPath, pair('old-desktop'))), + pairedConnection('old-desktop'), 'session.tabs.subscribe', { worktree: `id:${SESSION_WORKTREE_ID}` }, 10_000, @@ -125,7 +134,7 @@ describe('session.tabs tab delta across mixed client versions', () => { // What a new desktop's socket carries before main decodes it. const wire = recorder() const wireSubscription = await subscribeRemoteRuntimeRequest( - getPreferredPairedRuntimeConnection(resolveEnvironment(userDataPath, pair('new-wire'))), + pairedConnection('new-wire'), 'session.tabs.subscribe', { worktree: `id:${SESSION_WORKTREE_ID}` }, 10_000, @@ -136,9 +145,10 @@ describe('session.tabs tab delta across mixed client versions', () => { // This desktop: the real main-process path, which decodes before the window sees a frame. const current = recorder() + const desktop = pair('new-desktop') const currentSubscription = await subscribeRuntimeEnvironment( - userDataPath, - pair('new-desktop'), + desktop.userDataPath, + desktop.id, 'session.tabs.subscribe', { worktree: `id:${SESSION_WORKTREE_ID}` }, 10_000, diff --git a/src/main/ipc/runtime-environments/runtime-environment-verified-pairing-publish.ts b/src/main/ipc/runtime-environments/runtime-environment-verified-pairing-publish.ts new file mode 100644 index 00000000000..774c07394d8 --- /dev/null +++ b/src/main/ipc/runtime-environments/runtime-environment-verified-pairing-publish.ts @@ -0,0 +1,42 @@ +import type { VerifyAndAddRuntimeEnvironmentResult } from '../../../shared/remote-pairing-verification' +import { listEnvironments } from '../../../shared/runtime-environment-store' +import { getRuntimeEnvironmentStatusOwner } from './runtime-environment-request-connections' +import { clearRuntimeEnvironmentManualDisconnect } from './runtime-environment-manual-disconnect' + +type VerifiedPairing = Extract + +function savedPairingRevision(userDataPath: string, environmentId: string): number | undefined { + try { + return listEnvironments(userDataPath).find((entry) => entry.id === environmentId) + ?.pairingRevision + } catch { + // Why: the pairing is already saved; an unreadable store only means its status waits for a probe. + return undefined + } +} + +/** + * Publishes the status a pairing was verified with, retiring the replaced pairing's owner first when + * `replaced`. One choke point so every save-after-verify path fences against an overlapping save. + */ +export async function publishVerifiedPairing( + userDataPath: string, + result: VerifiedPairing, + replaced: { invalidateTransport: (environmentId: string) => Promise | void } | null +): Promise { + const environmentId = result.environment.id + // Why before invalidating: a manually disconnected server would not get a fresh owner. + clearRuntimeEnvironmentManualDisconnect(environmentId) + // Retires the owner blocked under the replaced pairing; its replacement keys on the new revision. + await replaced?.invalidateTransport(environmentId) + // Why: an overlapping save may have replaced this pairing during the await. + if (savedPairingRevision(userDataPath, environmentId) !== result.environment.pairingRevision) { + return + } + getRuntimeEnvironmentStatusOwner(userDataPath, environmentId).acceptVerified({ + id: 'status.get', + ok: true, + result: result.runtimeStatus, + _meta: { runtimeId: result.runtimeStatus.runtimeId } + }) +} diff --git a/src/main/ipc/runtime-environments/runtime-environments-pairing.test.ts b/src/main/ipc/runtime-environments/runtime-environments-pairing.test.ts index f9f3fcb73fd..f22850206db 100644 --- a/src/main/ipc/runtime-environments/runtime-environments-pairing.test.ts +++ b/src/main/ipc/runtime-environments/runtime-environments-pairing.test.ts @@ -1,5 +1,8 @@ import type { RuntimeHostStatusSnapshot } from '../../../shared/runtime-host-status' -import { resetRuntimeEnvironmentStatusOwners } from './runtime-environment-request-connections' +import { + getRuntimeEnvironmentStatusSnapshots, + resetRuntimeEnvironmentStatusOwners +} from './runtime-environment-request-connections' import { mkdtempSync, rmSync } from 'node:fs' import { EventEmitter } from 'node:events' import { tmpdir } from 'node:os' @@ -410,7 +413,7 @@ describe('registerRuntimeEnvironmentHandlers', () => { await expect( verifyAndAdd(null, { name: 'desk', - pairingCode: pairingCode('ws://100.76.32.125:6768') + pairingCode: pairingCode('ws://100.76.32.125:6768', { keyByte: 2 }) }) ).resolves.toMatchObject({ ok: false, @@ -420,6 +423,97 @@ describe('registerRuntimeEnvironmentHandlers', () => { expect(environmentStore.listEnvironments(userDataPath)).toHaveLength(1) }) + it('refreshes the saved server when a verified link is for it, and retires its old connection', async () => { + registerRuntimeEnvironmentHandlers(store as never) + const saved = environmentStore.addEnvironmentFromPairingCode(userDataPath, { + name: 'desk', + pairingCode: pairingCode('ws://100.76.32.125:6768') + }) + sendRemoteRuntimeRequestMock.mockResolvedValue({ + id: 'status', + ok: true, + result: runtimeStatus(), + _meta: { runtimeId: 'runtime-a' } + }) + const verifyAndAdd = handler< + { name: string; pairingCode: string }, + { ok: boolean; updatedExisting?: boolean; environment?: { id: string; name: string } } + >('runtimeEnvironments:verifyAndAddFromPairingCode') + + await expect( + verifyAndAdd(null, { + name: 'desk over relay', + pairingCode: pairingCode('ws://192.0.2.10:6768', { deviceToken: 'second-token' }) + }) + ).resolves.toMatchObject({ + ok: true, + updatedExisting: true, + environment: { id: saved.id, name: 'desk' } + }) + expect(environmentStore.listEnvironments(userDataPath)).toHaveLength(1) + expect(environmentStore.resolveEnvironmentPairingOffer(userDataPath, saved.id)).toMatchObject({ + endpoint: 'ws://192.0.2.10:6768', + deviceToken: 'second-token' + }) + expect(closeRemoteRuntimeRequestConnectionMock).toHaveBeenCalledWith(saved.id) + }) + + it('does not stamp a refresh that a newer one overtook with its stale verification', async () => { + registerRuntimeEnvironmentHandlers(store as never) + const saved = environmentStore.addEnvironmentFromPairingCode(userDataPath, { + name: 'desk', + pairingCode: pairingCode('ws://100.76.32.125:6768') + }) + sendRemoteRuntimeRequestMock.mockResolvedValue({ + id: 'status', + ok: true, + result: runtimeStatus(), + _meta: { runtimeId: 'runtime-a' } + }) + // An overlapping re-add lands while this one retires the old connection. + let overtaken = false + closeRemoteRuntimeRequestConnectionMock.mockImplementation(() => { + const current = environmentStore.resolveEnvironmentPairingOffer(userDataPath, saved.id) + if (overtaken || current.deviceToken !== 'second-token') { + return + } + overtaken = true + // The newer pairing answers as another process, so only a stale stamp reads runtime-a. + const newer = { ...runtimeStatus(), runtimeId: 'runtime-b' } + for (const mock of [ + sendRemoteRuntimeRequestMock, + sendRemoteRuntimeSharedControlRequestMock + ]) { + mock.mockResolvedValue({ + id: 'status', + ok: true, + result: newer, + _meta: { runtimeId: 'runtime-b' } + }) + } + environmentStore.addOrUpdateEnvironmentFromPairingCode(userDataPath, { + name: 'desk', + pairingCode: pairingCode('ws://192.0.2.11:6768', { deviceToken: 'third-token' }), + now: Date.now() + 60_000 + }) + }) + const verifyAndAdd = handler<{ name: string; pairingCode: string }, { ok: boolean }>( + 'runtimeEnvironments:verifyAndAddFromPairingCode' + ) + + await verifyAndAdd(null, { + name: 'desk again', + pairingCode: pairingCode('ws://192.0.2.10:6768', { deviceToken: 'second-token' }) + }) + + expect(overtaken).toBe(true) + const snapshot = getRuntimeEnvironmentStatusSnapshots().find( + (entry) => entry.environmentId === saved.id + ) + expect(snapshot).toBeDefined() + expect(['runtime-b', undefined]).toContain(snapshot?.status?.runtimeId) + }) + it('requires an explicit Advanced selection before removing the Active Server', async () => { registerRuntimeEnvironmentHandlers(store as never) const add = handler< diff --git a/src/main/ipc/runtime-environments/runtime-environments-repair.test.ts b/src/main/ipc/runtime-environments/runtime-environments-repair.test.ts index d03ff802925..0a6b82b6150 100644 --- a/src/main/ipc/runtime-environments/runtime-environments-repair.test.ts +++ b/src/main/ipc/runtime-environments/runtime-environments-repair.test.ts @@ -179,11 +179,11 @@ describe('runtimeEnvironments:repairFromPairingCode', () => { it('declines a server another Orca service pairs', async () => { environmentStore.addEnvironmentFromPairingCode(userDataPath, { name: 'vm', - pairingCode: pairingCode(ENDPOINT, { deviceToken: 'vm-token' }), + pairingCode: pairingCode(ENDPOINT, { deviceToken: 'vm-token', keyByte: 2 }), source: 'ephemeral-vm' }) await expect( - repair(null, { selector: 'vm', pairingCode: pairingCode(ENDPOINT) }) + repair(null, { selector: 'vm', pairingCode: pairingCode(ENDPOINT, { keyByte: 2 }) }) ).resolves.toMatchObject({ ok: false, kind: 'environment-save-failed' }) expect(sendRemoteRuntimeRequestMock).not.toHaveBeenCalled() }) diff --git a/src/renderer/src/components/settings/RuntimeEnvironmentsPane.tsx b/src/renderer/src/components/settings/RuntimeEnvironmentsPane.tsx index 5bb88e8a7fe..622b3ec9748 100644 --- a/src/renderer/src/components/settings/RuntimeEnvironmentsPane.tsx +++ b/src/renderer/src/components/settings/RuntimeEnvironmentsPane.tsx @@ -120,7 +120,6 @@ export function RuntimeEnvironmentsPane({ addEnvironment, removeEnvironment } = useRuntimeEnvironmentMutationActions({ - environments, settings, allowLocalRuntime, mountedRef, diff --git a/src/renderer/src/components/settings/use-runtime-environment-mutation-actions.test.tsx b/src/renderer/src/components/settings/use-runtime-environment-mutation-actions.test.tsx new file mode 100644 index 00000000000..28c4d8a7530 --- /dev/null +++ b/src/renderer/src/components/settings/use-runtime-environment-mutation-actions.test.tsx @@ -0,0 +1,111 @@ +// @vitest-environment happy-dom +import { act, cleanup, renderHook } from '@testing-library/react' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import type { GlobalSettings } from '../../../../shared/global-settings-types' +import type { PublicKnownRuntimeEnvironment } from '../../../../shared/runtime-environments' + +const { toastSuccess, toastError } = vi.hoisted(() => ({ + toastSuccess: vi.fn(), + toastError: vi.fn() +})) +vi.mock('sonner', () => ({ toast: { success: toastSuccess, error: toastError } })) + +import { useRuntimeEnvironmentMutationActions } from './use-runtime-environment-mutation-actions' + +const verifyAndAddFromPairingCode = vi.fn() +const remove = vi.fn() + +const saved: PublicKnownRuntimeEnvironment = { + id: 'env-studio', + name: 'Studio', + createdAt: 1, + updatedAt: 2, + lastUsedAt: null, + runtimeId: 'runtime-a', + endpoints: [{ id: 'ws-env-studio', kind: 'websocket', label: 'WebSocket', endpoint: 'ws://x' }], + preferredEndpointId: 'ws-env-studio' +} + +beforeEach(() => { + toastSuccess.mockReset() + toastError.mockReset() + verifyAndAddFromPairingCode.mockReset() + remove.mockReset() + Object.defineProperty(window, 'api', { + configurable: true, + value: { runtimeEnvironments: { verifyAndAddFromPairingCode, remove } } + }) +}) +afterEach(() => cleanup()) + +function renderActions(connectEnvironment: () => Promise, name = 'Studio over relay') { + const view = renderHook(() => + useRuntimeEnvironmentMutationActions({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: addEnvironment reads no settings. + settings: {} as GlobalSettings, + allowLocalRuntime: false, + mountedRef: { current: true }, + setAddServerFormOpen: vi.fn(), + loadEnvironments: vi.fn(async () => {}), + connectEnvironment + }) + ) + act(() => { + view.result.current.setName(name) + view.result.current.setPairingCode('orca://pair#code') + }) + return view +} + +it('keeps a refreshed saved server when the follow-up connect fails, and says which one it was', async () => { + verifyAndAddFromPairingCode.mockResolvedValue({ + ok: true, + environment: saved, + runtimeStatus: { runtimeId: 'runtime-a' }, + updatedExisting: true + }) + const view = renderActions(async () => false) + + await act(async () => { + await view.result.current.addEnvironment(false) + }) + + expect(remove).not.toHaveBeenCalled() + expect(toastSuccess).toHaveBeenCalledWith( + 'This server is already paired as Studio; updated its connection.' + ) +}) + +it('still discards a brand-new server whose first connect fails', async () => { + verifyAndAddFromPairingCode.mockResolvedValue({ + ok: true, + environment: { ...saved, id: 'env-new', name: 'Laptop' }, + runtimeStatus: { runtimeId: 'runtime-b' } + }) + const view = renderActions(async () => false) + + await act(async () => { + await view.result.current.addEnvironment(false) + }) + + expect(remove).toHaveBeenCalledWith({ selector: 'env-new' }) +}) + +it("lets main judge a saved server's own name, so its code can refresh it", async () => { + verifyAndAddFromPairingCode.mockResolvedValue({ + ok: true, + environment: saved, + runtimeStatus: { runtimeId: 'runtime-a' }, + updatedExisting: true + }) + const view = renderActions(async () => true, 'Studio') + + await act(async () => { + await view.result.current.addEnvironment(false) + }) + + expect(verifyAndAddFromPairingCode).toHaveBeenCalledWith( + expect.objectContaining({ name: 'Studio' }) + ) + expect(toastError).not.toHaveBeenCalled() +}) diff --git a/src/renderer/src/components/settings/use-runtime-environment-mutation-actions.ts b/src/renderer/src/components/settings/use-runtime-environment-mutation-actions.ts index 88e831be030..1862664f71d 100644 --- a/src/renderer/src/components/settings/use-runtime-environment-mutation-actions.ts +++ b/src/renderer/src/components/settings/use-runtime-environment-mutation-actions.ts @@ -3,13 +3,11 @@ import { toast } from 'sonner' import { translate } from '@/i18n/i18n' import type { GlobalSettings } from '../../../../shared/global-settings-types' import type { PublicKnownRuntimeEnvironment } from '../../../../shared/runtime-environments' -import { findCollidingEnvironmentName } from '../../../../shared/runtime-environment-name-collision' import type { RuntimeStatus } from '../../../../shared/runtime-types' import { isRuntimeEnvironmentRemovalBlocked } from './runtime-environment-host-details' import type { RuntimeHostAccessFailure } from './RuntimeHostAccessFailureAlert' type RuntimeEnvironmentMutationActionParams = { - environments: PublicKnownRuntimeEnvironment[] settings: GlobalSettings allowLocalRuntime: boolean mountedRef: MutableRefObject @@ -22,7 +20,6 @@ type RuntimeEnvironmentMutationActionParams = { } export function useRuntimeEnvironmentMutationActions({ - environments, settings, allowLocalRuntime, mountedRef, @@ -59,20 +56,10 @@ export function useRuntimeEnvironmentMutationActions({ ) return } - const duplicate = findCollidingEnvironmentName(environments, trimmedName) - if (duplicate) { - toast.error( - translate( - 'auto.components.settings.RuntimeEnvironmentsPane.5ef712f407', - 'A server named "{{value0}}" already exists.', - { value0: duplicate.name } - ) - ) - return - } setAddServerFailure(null) setIsSaving(true) try { + // Why main judges a taken name: a code for that same saved server refreshes it instead. const result = await window.api.runtimeEnvironments.verifyAndAddFromPairingCode({ name: trimmedName, pairingCode: trimmedPairingCode, @@ -92,14 +79,24 @@ export function useRuntimeEnvironmentMutationActions({ environmentId: result.environment.id, runtimeStatus: result.runtimeStatus }) + if (result.updatedExisting && mountedRef.current) { + toast.success( + translate( + 'auto.components.settings.RuntimeEnvironmentsPane.serverAlreadyPaired', + 'This server is already paired as {{value0}}; updated its connection.', + { value0: result.environment.name } + ) + ) + } if (!allowLocalRuntime) { const connected = await connectEnvironment(result.environment) - if (!connected) { + // Why not a refreshed entry: removing it would drop a server the user had already saved. + if (!connected && !result.updatedExisting) { await window.api.runtimeEnvironments.remove({ selector: result.environment.id }) await loadEnvironments() return } - } else { + } else if (!result.updatedExisting) { if (mountedRef.current) { toast.success( translate( diff --git a/src/renderer/src/components/sidebar/AddRemoteHostDialog.tsx b/src/renderer/src/components/sidebar/AddRemoteHostDialog.tsx index 7dcf73c2864..af01a60ff15 100644 --- a/src/renderer/src/components/sidebar/AddRemoteHostDialog.tsx +++ b/src/renderer/src/components/sidebar/AddRemoteHostDialog.tsx @@ -286,7 +286,16 @@ export function AddRemoteHostDialog({ setRuntimeEnvironments(environments) await readRuntimeHostStatusSnapshots() toast.success( - translate('auto.components.sidebar.AddRemoteHostDialog.serverSaved', 'Remote server added.') + result.updatedExisting + ? translate( + 'auto.components.sidebar.AddRemoteHostDialog.serverAlreadyPaired', + 'This server is already paired as {{value0}}; updated its connection.', + { value0: result.environment.name } + ) + : translate( + 'auto.components.sidebar.AddRemoteHostDialog.serverSaved', + 'Remote server added.' + ) ) reset() onOpenChange(null) diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index be3227eef7b..8831a0fc0e8 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -6643,6 +6643,7 @@ "sshSaveFailed": "Failed to add SSH host.", "serverFieldsRequired": "Server name and pairing code are required.", "serverSaved": "Remote server added.", + "serverAlreadyPaired": "This server is already paired as {{value0}}; updated its connection.", "serverSaveFailed": "Failed to add remote server.", "serverTitle": "Add remote server", "sshTitle": "Add SSH host", @@ -8677,7 +8678,7 @@ "b5b5114cb0": "Removed {{value0}}.", "6cb6eae14f": "Failed to save runtime environment.", "7b5986c8df": "Connected to {{value0}}. To create new projects there by default, choose it under Advanced > Default host for new projects.", - "5ef712f407": "A server named \"{{value0}}\" already exists.", + "serverAlreadyPaired": "This server is already paired as {{value0}}; updated its connection.", "0c55a47480": "Name and pairing code are required.", "e6410d72c3": "Failed to load runtime environments.", "6ef71985da": "No endpoint", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 58a73ea3cc1..ca9f196b4ca 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -6389,6 +6389,7 @@ "sshSaveFailed": "No se pudo agregar el host SSH.", "serverFieldsRequired": "Se requieren el nombre del servidor y el código de emparejamiento.", "serverSaved": "Se agregó servidor remoto.", + "serverAlreadyPaired": "Este servidor ya está emparejado como {{value0}}; se actualizó su conexión.", "serverSaveFailed": "No se pudo agregar el servidor remoto.", "serverTitle": "Agregar servidor remoto", "sshTitle": "Agregar host SSH", @@ -8342,7 +8343,7 @@ "b5b5114cb0": "Se eliminó {{value0}}.", "6cb6eae14f": "No se pudo guardar el runtime.", "7b5986c8df": "Conectado a {{value0}}. Para crear proyectos nuevos allí de forma predeterminada, elígelo en Avanzado > Host predeterminado para proyectos nuevos.", - "5ef712f407": "Ya existe un servidor llamado \"{{value0}}\".", + "serverAlreadyPaired": "Este servidor ya está emparejado como {{value0}}; se actualizó su conexión.", "0c55a47480": "Se requieren nombre y código de emparejamiento.", "e6410d72c3": "No se pudieron cargar los runtimes.", "6ef71985da": "Sin punto final", diff --git a/src/renderer/src/i18n/locales/fr.json b/src/renderer/src/i18n/locales/fr.json index b65ae069c11..940217e2b27 100644 --- a/src/renderer/src/i18n/locales/fr.json +++ b/src/renderer/src/i18n/locales/fr.json @@ -6476,6 +6476,7 @@ "sshSaveFailed": "Échec de l'ajout de l'hôte SSH.", "serverFieldsRequired": "Le nom du serveur et le code d'appairage sont requis.", "serverSaved": "Serveur distant ajouté.", + "serverAlreadyPaired": "Ce serveur est déjà associé sous le nom {{value0}} ; sa connexion a été mise à jour.", "serverSaveFailed": "Échec de l'ajout du serveur distant.", "serverTitle": "Ajouter un serveur distant", "sshTitle": "Ajouter un hôte SSH", @@ -8500,7 +8501,7 @@ "b5b5114cb0": "{{value0}} supprimé.", "6cb6eae14f": "Échec de l'enregistrement de l'environnement d'exécution.", "7b5986c8df": "Connecté à {{value0}}. Pour y créer les nouveaux projets par défaut, choisissez-le dans Avancé > Hôte par défaut des nouveaux projets.", - "5ef712f407": "Un serveur nommé « {{value0}} » existe déjà.", + "serverAlreadyPaired": "Ce serveur est déjà associé sous le nom {{value0}} ; sa connexion a été mise à jour.", "0c55a47480": "Le nom et le code d'appairage sont requis.", "e6410d72c3": "Échec du chargement des environnements d'exécution.", "6ef71985da": "Aucun endpoint", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index d093872f283..48276318db8 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -6394,6 +6394,7 @@ "sshSaveFailed": "SSH ホストの追加に失敗しました。", "serverFieldsRequired": "サーバー名とペアリングコードは必須です。", "serverSaved": "リモートサーバーが追加されました。", + "serverAlreadyPaired": "このサーバーは {{value0}} としてペアリング済みです。接続情報を更新しました。", "serverSaveFailed": "リモートサーバーの追加に失敗しました。", "serverTitle": "リモートサーバーを追加する", "sshTitle": "SSH ホストを追加", @@ -8369,7 +8370,7 @@ "b5b5114cb0": "{{value0}} を削除しました。", "6cb6eae14f": "実行環境の保存に失敗しました。", "7b5986c8df": "{{value0}} に接続しました。新規プロジェクトを既定でそこに作成するには、[詳細設定] > [新規プロジェクトの既定ホスト] で選択してください。", - "5ef712f407": "「{{value0}}」という名前のサーバーはすでに存在します。", + "serverAlreadyPaired": "このサーバーは {{value0}} としてペアリング済みです。接続情報を更新しました。", "0c55a47480": "名前とペアリングコードは必須です。", "e6410d72c3": "ランタイム環境の読み込みに失敗しました。", "6ef71985da": "エンドポイントなし", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index e2c8fcfa24f..2c3e6528d77 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -6394,6 +6394,7 @@ "sshSaveFailed": "SSH 호스트를 추가하지 못했습니다.", "serverFieldsRequired": "서버 이름과 페어링 코드가 필요합니다.", "serverSaved": "원격 서버가 추가되었습니다.", + "serverAlreadyPaired": "이 서버는 이미 {{value0}}(으)로 페어링되어 있습니다. 연결 정보를 업데이트했습니다.", "serverSaveFailed": "원격 서버를 추가하지 못했습니다.", "serverTitle": "원격 서버 추가", "sshTitle": "SSH 호스트 추가", @@ -8332,7 +8333,7 @@ "b5b5114cb0": "{{value0}}을(를) 삭제했습니다.", "6cb6eae14f": "런타임 환경을 저장하지 못했습니다.", "7b5986c8df": "{{value0}}에 연결되었습니다. 새 프로젝트를 기본적으로 그곳에 만들려면 고급 > 새 프로젝트의 기본 호스트에서 선택하세요.", - "5ef712f407": "\"{{value0}}\"이라는 서버가 이미 존재합니다.", + "serverAlreadyPaired": "이 서버는 이미 {{value0}}(으)로 페어링되어 있습니다. 연결 정보를 업데이트했습니다.", "0c55a47480": "이름과 페어링 코드가 필요합니다.", "e6410d72c3": "런타임 환경을 로드하지 못했습니다.", "6ef71985da": "엔드포인트 없음", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index bef843c2e52..75abb4e5a76 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -6394,6 +6394,7 @@ "sshSaveFailed": "添加 SSH 主机失败。", "serverFieldsRequired": "必须填写服务器名称和配对码。", "serverSaved": "已添加远程服务器。", + "serverAlreadyPaired": "此服务器已作为 {{value0}} 配对;已更新其连接。", "serverSaveFailed": "添加远程服务器失败。", "serverTitle": "添加远程服务器", "sshTitle": "添加 SSH 主机", @@ -8332,7 +8333,7 @@ "b5b5114cb0": "删除了 {{value0}}。", "6cb6eae14f": "无法保存运行时环境。", "7b5986c8df": "已连接到 {{value0}}。若要默认在其上创建新项目,请在“高级 > 新项目的默认主机”中选择它。", - "5ef712f407": "名为“{{value0}}”的服务器已存在。", + "serverAlreadyPaired": "此服务器已作为 {{value0}} 配对;已更新其连接。", "0c55a47480": "需要名称和配对代码。", "e6410d72c3": "无法加载运行时环境。", "6ef71985da": "无终点", diff --git a/src/renderer/src/store/slices/runtime-environment-peer-replacement.test.ts b/src/renderer/src/store/slices/runtime-environment-peer-replacement.test.ts index 3133b4c7cc8..92938274dc8 100644 --- a/src/renderer/src/store/slices/runtime-environment-peer-replacement.test.ts +++ b/src/renderer/src/store/slices/runtime-environment-peer-replacement.test.ts @@ -58,13 +58,28 @@ describe('which re-paired environments name a different machine', () => { expect(retired(managed(), managed({ generation: 2, pairingRevision: 2 }))).toEqual(['env-1']) }) - it('retires a re-paired environment that is not a managed server', () => { + it('retires a hand-paired server re-paired to a different key', () => { const paired = { id: 'env-1', createdAt: 1, pairingRevision: 1, hostKeyFingerprint: 'k' } expect( - classifyPeerReplacements([paired], [{ ...paired, pairingRevision: 2 }], ['env-1']).retired + classifyPeerReplacements( + [paired], + [{ ...paired, pairingRevision: 2, hostKeyFingerprint: 'other' }], + ['env-1'] + ).retired ).toEqual(['env-1']) }) + it('keeps a hand-paired server whose new pairing carries the same key, so its tabs survive', () => { + const paired = { id: 'env-1', createdAt: 1, pairingRevision: 1, hostKeyFingerprint: 'k' } + expect( + classifyPeerReplacements([paired], [{ ...paired, pairingRevision: 2 }], ['env-1']) + ).toEqual({ + retired: [], + sameHost: [{ id: 'env-1', fromRevision: 1, toRevision: 2 }], + pending: [] + }) + }) + it('leaves an environment alone while its pairing is unchanged', () => { expect(replacedRuntimeEnvironmentIds([managed()], [managed()])).toEqual([]) }) diff --git a/src/renderer/src/store/slices/runtime-environment-peer-replacement.ts b/src/renderer/src/store/slices/runtime-environment-peer-replacement.ts index 7f5d46d7e85..0b2c5e09462 100644 --- a/src/renderer/src/store/slices/runtime-environment-peer-replacement.ts +++ b/src/renderer/src/store/slices/runtime-environment-peer-replacement.ts @@ -33,6 +33,11 @@ function sameRegistration( before: CatalogEnvironment | undefined, after: CatalogEnvironment | undefined ): boolean { + // Why a hand-paired server counts only with both keys: main replaces its pairing in place only for + // the saved key, so the key check below is the whole proof; without one there is none. + if (before && after && !before.orcadDeployment && !after.orcadDeployment) { + return Boolean(before.hostKeyFingerprint && after.hostKeyFingerprint) + } // Why SSH only: a WSL deployment takes the stricter re-pair path until its host identity lands. const left = before && getOrcadSshDeployment(before) const right = after && getOrcadSshDeployment(after) diff --git a/src/renderer/src/store/slices/runtime-managed-repair-keeps-workspaces.test.ts b/src/renderer/src/store/slices/runtime-managed-repair-keeps-workspaces.test.ts index a56491a6b23..93742c402de 100644 --- a/src/renderer/src/store/slices/runtime-managed-repair-keeps-workspaces.test.ts +++ b/src/renderer/src/store/slices/runtime-managed-repair-keeps-workspaces.test.ts @@ -106,3 +106,28 @@ describe('an on-connect update re-pairs a managed server', () => { expect(keptWorkspaces(store)).toEqual({ repos: 0, tabs: 0 }) }) }) + +describe('Settings refreshes a hand-paired server with a new code for it', () => { + function handPaired(pairingRevision: number, hostKeyFingerprint: string) { + const { orcadDeployment: _managed, ...rest } = server(pairingRevision, hostKeyFingerprint) + return rest + } + + function userOnHandPairedServer() { + const store = userOnServer() + store.getState().setRuntimeEnvironments([handPaired(1, 'key-a')]) + return store + } + + it('keeps the workspaces and tabs when the new pairing carries the same key', () => { + const store = userOnHandPairedServer() + store.getState().setRuntimeEnvironments([handPaired(2, 'key-a')]) + expect(keptWorkspaces(store)).toEqual({ repos: 1, tabs: 1 }) + }) + + it('retires them when the new pairing is for another machine', () => { + const store = userOnHandPairedServer() + store.getState().setRuntimeEnvironments([handPaired(2, 'key-other')]) + expect(keptWorkspaces(store)).toEqual({ repos: 0, tabs: 0 }) + }) +}) diff --git a/src/shared/remote-pairing-verification.ts b/src/shared/remote-pairing-verification.ts index 578db956739..1ca160a6c88 100644 --- a/src/shared/remote-pairing-verification.ts +++ b/src/shared/remote-pairing-verification.ts @@ -24,6 +24,8 @@ export type VerifyAndAddRuntimeEnvironmentResult = ok: true environment: PublicKnownRuntimeEnvironment runtimeStatus: RuntimeStatus + /** The link was for a saved server, which kept its name and took the new connection. */ + updatedExisting?: true } | RemotePairingFailure diff --git a/src/shared/runtime-environment-host-key-match.ts b/src/shared/runtime-environment-host-key-match.ts new file mode 100644 index 00000000000..7c7caac60aa --- /dev/null +++ b/src/shared/runtime-environment-host-key-match.ts @@ -0,0 +1,36 @@ +import type { PairingOffer } from './pairing' +import type { KnownRuntimeEnvironment } from './runtime-environments' + +/** + * The saved server holding this host key. Why the key: it is the server's persisted identity, + * so two pairings carrying it are one server whatever endpoint or name each was saved under. + */ +export function findEnvironmentWithHostKey>( + environments: readonly T[], + publicKeyB64: string +): T | undefined { + const key = canonicalHostKeyB64(publicKeyB64) + return environments.find((environment) => + environment.endpoints.some((endpoint) => canonicalHostKeyB64(endpoint.publicKeyB64) === key) + ) +} + +/** Why: pairing links accept unpadded and URL-safe Base64, which decode to the same key. */ +function canonicalHostKeyB64(publicKeyB64: string): string { + return publicKeyB64.trim().replace(/-/g, '+').replace(/_/g, '/').replace(/=+$/, '') +} + +/** + * The spelling a saved server already holds for this offer's key. Why: the host-key fingerprint + * hashes the string, so a respelled key would read as another machine and retire its workspaces. + */ +export function savedHostKeySpelling( + environment: Pick, + offer: Pick +): string { + const key = canonicalHostKeyB64(offer.publicKeyB64) + return ( + environment.endpoints.find((endpoint) => canonicalHostKeyB64(endpoint.publicKeyB64) === key) + ?.publicKeyB64 ?? offer.publicKeyB64 + ) +} diff --git a/src/shared/runtime-environment-pairing-replace-guards.ts b/src/shared/runtime-environment-pairing-replace-guards.ts new file mode 100644 index 00000000000..d7844761742 --- /dev/null +++ b/src/shared/runtime-environment-pairing-replace-guards.ts @@ -0,0 +1,70 @@ +import { + isEphemeralVmRuntimeEnvironment, + type KnownRuntimeEnvironment, + type RuntimeEnvironmentSource +} from './runtime-environments' +import { RuntimeEnvironmentStoreError } from './runtime-environment-store-file' + +/** Refuses replacing or removing a pairing that SSH access or a managed deployment depends on. */ +export function assertNoIndependentSshAccess(environment: KnownRuntimeEnvironment): void { + if (environment.orcadDeployment) { + throw new RuntimeEnvironmentStoreError( + 'invalid_argument', + 'This server is managed by Orca over SSH; its pairing cannot be replaced or removed here. To decommission it, run `orca environment stop --environment --yes` or use Settings > Managed servers; for a host that is gone, run `orca environment forget --environment --yes`.' + ) + } + if (environment.pendingSshAccessOperation) { + throw new RuntimeEnvironmentStoreError( + 'invalid_argument', + "This server's SSH access change didn't finish. Retry or cancel it under Settings > Remote Orca Servers > SSH access for paired servers before replacing its pairing or removing it." + ) + } + if (environment.sshAccess) { + throw new RuntimeEnvironmentStoreError( + 'invalid_argument', + "Unlink this server's SSH access under Settings > Remote Orca Servers > SSH access for paired servers before replacing its pairing or removing it." + ) + } +} + +/** Refuses folding a new code into a saved server that another flow owns, naming that server. */ +export function assertPairingReplaceableByAdd( + environment: KnownRuntimeEnvironment, + source: RuntimeEnvironmentSource | undefined, + refreshable: boolean +): void { + const alreadyPaired = `This server is already paired as "${environment.name}".` + // Why: a cloud workspace pairs its own server on resume; a hand-made pairing would be swept with it. + if (isEphemeralVmRuntimeEnvironment(environment)) { + throw new RuntimeEnvironmentStoreError( + 'invalid_argument', + `${alreadyPaired} It belongs to a cloud workspace, which pairs it again when it resumes.` + ) + } + if (source === 'ephemeral-vm') { + throw new RuntimeEnvironmentStoreError( + 'invalid_argument', + `${alreadyPaired} The cloud workspace's server reuses that server's identity, so it was not saved.` + ) + } + if (environment.orcadDeployment?.kind === 'wsl') { + throw new RuntimeEnvironmentStoreError( + 'invalid_argument', + `${alreadyPaired} Orca manages it as a WSL host, so its pairing cannot be replaced here.` + ) + } + try { + assertNoIndependentSshAccess(environment) + } catch (error) { + throw error instanceof RuntimeEnvironmentStoreError + ? new RuntimeEnvironmentStoreError(error.code, `${alreadyPaired} ${error.message}`) + : error + } + // Why: only the app holding this server's connections can switch them to a new pairing. + if (!refreshable) { + throw new RuntimeEnvironmentStoreError( + 'invalid_argument', + `${alreadyPaired} To update its connection, add it again under Settings > Remote Orca Servers.` + ) + } +} diff --git a/src/shared/runtime-environment-sidecar.test.ts b/src/shared/runtime-environment-sidecar.test.ts index f28cb79b53a..dd40fd8ac3d 100644 --- a/src/shared/runtime-environment-sidecar.test.ts +++ b/src/shared/runtime-environment-sidecar.test.ts @@ -112,7 +112,10 @@ describe('runtime environment sidecar across a downgrade', () => { expect(() => removeEnvironment(userDataPath, access.id)).toThrow('Unlink') const plain = addEnvironmentFromPairingCode(userDataPath, { name: 'Plain', - pairingCode: encodePairingOffer(pairing) + pairingCode: encodePairingOffer({ + ...pairing, + publicKeyB64: Buffer.alloc(32, 2).toString('base64') + }) }) removeEnvironment(userDataPath, plain.id) expect(listEnvironments(userDataPath).map((entry) => entry.id)).toEqual([access.id]) diff --git a/src/shared/runtime-environment-ssh-access-store.test.ts b/src/shared/runtime-environment-ssh-access-store.test.ts index 814c14aa81e..025fc3e4936 100644 --- a/src/shared/runtime-environment-ssh-access-store.test.ts +++ b/src/shared/runtime-environment-ssh-access-store.test.ts @@ -174,9 +174,13 @@ describe('independent paired runtime SSH access persistence', () => { it('refuses a second registration of the same execution runtime', () => { const original = seed() + // Why another key: one key is one server, which the store would refresh instead of adding. const duplicate = addEnvironmentFromPairingCode(userDataPath, { name: 'Duplicate host', - pairingCode: encodePairingOffer(pairing) + pairingCode: encodePairingOffer({ + ...pairing, + publicKeyB64: Buffer.alloc(32, 2).toString('base64') + }) }) markEnvironmentUsed(userDataPath, duplicate.id, { runtimeId: 'host-runtime' }) const before = listEnvironments(userDataPath) diff --git a/src/shared/runtime-environment-store-same-server.test.ts b/src/shared/runtime-environment-store-same-server.test.ts new file mode 100644 index 00000000000..65e942057bf --- /dev/null +++ b/src/shared/runtime-environment-store-same-server.test.ts @@ -0,0 +1,197 @@ +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { encodePairingOffer } from './pairing' +import { + addEnvironmentFromPairingCode, + addOrUpdateEnvironmentFromPairingCode, + listEnvironments, + resolveEnvironmentPairingOffer +} from './runtime-environment-store' +import { addManagedOrcadEnvironment } from './runtime-environment-managed-orcad-store' + +const serverKey = Buffer.alloc(32, 7).toString('base64') + +function pairingCode(endpoint: string, deviceToken = 'first-token', publicKeyB64 = serverKey) { + return encodePairingOffer({ v: 2, endpoint, deviceToken, publicKeyB64 }) +} + +describe('pairing a server that is already saved', () => { + let userDataPath: string + const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') + beforeEach(() => { + // Why: this suite tests store contents, while secure-file tests cover Windows ACLs. + Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' }) + userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-env-same-server-')) + }) + afterEach(() => { + if (originalPlatform) { + Object.defineProperty(process, 'platform', originalPlatform) + } + rmSync(userDataPath, { recursive: true, force: true }) + }) + + it('refreshes the saved entry instead of saving the server twice under a new name', () => { + const first = addEnvironmentFromPairingCode(userDataPath, { + name: 'Studio', + pairingCode: pairingCode('ws://192.0.2.10:6768'), + now: 100 + }) + + const outcome = addOrUpdateEnvironmentFromPairingCode(userDataPath, { + name: 'Studio over relay', + pairingCode: pairingCode('wss://relay.example/runtime', 'second-token'), + runtimeId: 'runtime-after-restart', + now: 200 + }) + + expect(outcome.updatedExisting).toBe(true) + expect(outcome.environment).toMatchObject({ + id: first.id, + name: 'Studio', + createdAt: 100, + updatedAt: 200, + runtimeId: 'runtime-after-restart' + }) + expect(outcome.environment.pairingRevision).toBeGreaterThan(first.pairingRevision ?? 0) + expect(listEnvironments(userDataPath).map((entry) => entry.id)).toEqual([first.id]) + expect(resolveEnvironmentPairingOffer(userDataPath, first.id)).toMatchObject({ + endpoint: 'wss://relay.example/runtime', + deviceToken: 'second-token' + }) + }) + + it('refuses a second entry for the same server where it cannot switch connections over', () => { + const first = addEnvironmentFromPairingCode(userDataPath, { + name: 'Studio', + pairingCode: pairingCode('ws://192.0.2.10:6768') + }) + expect(() => + addEnvironmentFromPairingCode(userDataPath, { + name: 'Studio again', + pairingCode: pairingCode('ws://192.0.2.10:6768', 'second-token') + }) + ).toThrow( + 'This server is already paired as "Studio". To update its connection, add it again under Settings > Remote Orca Servers.' + ) + expect(listEnvironments(userDataPath)).toEqual([first]) + }) + + it("refreshes the saved server when the code is entered under the server's own name", () => { + const first = addEnvironmentFromPairingCode(userDataPath, { + name: 'Studio', + pairingCode: pairingCode('ws://192.0.2.10:6768') + }) + const outcome = addOrUpdateEnvironmentFromPairingCode(userDataPath, { + name: 'Studio', + pairingCode: pairingCode('ws://192.0.2.10:6768', 'second-token') + }) + expect(outcome).toMatchObject({ updatedExisting: true, environment: { id: first.id } }) + }) + + it('matches the same key spelled unpadded or URL-safe', () => { + const first = addEnvironmentFromPairingCode(userDataPath, { + name: 'Studio', + pairingCode: pairingCode('ws://192.0.2.10:6768') + }) + const urlSafeUnpadded = Buffer.alloc(32, 0xfb).toString('base64url') + addEnvironmentFromPairingCode(userDataPath, { + name: 'Wide', + pairingCode: pairingCode( + 'ws://192.0.2.12:6768', + 'token', + Buffer.alloc(32, 0xfb).toString('base64') + ) + }) + for (const spelling of [serverKey.replace(/=+$/, ''), urlSafeUnpadded]) { + const target = spelling === urlSafeUnpadded ? 'Wide' : 'Studio' + expect( + addOrUpdateEnvironmentFromPairingCode(userDataPath, { + name: `${target} again`, + pairingCode: pairingCode('ws://192.0.2.20:6768', 'other-token', spelling) + }) + ).toMatchObject({ updatedExisting: true, environment: { name: target } }) + } + // The saved spelling stays, so the host-key fingerprint the renderer compares is unchanged. + expect(resolveEnvironmentPairingOffer(userDataPath, first.id)).toMatchObject({ + publicKeyB64: serverKey, + deviceToken: 'other-token' + }) + expect(resolveEnvironmentPairingOffer(userDataPath, 'Wide').publicKeyB64).toBe( + Buffer.alloc(32, 0xfb).toString('base64') + ) + expect(listEnvironments(userDataPath).map((entry) => entry.id)).toContain(first.id) + expect(listEnvironments(userDataPath)).toHaveLength(2) + }) + + it('still saves a different server as its own entry', () => { + addEnvironmentFromPairingCode(userDataPath, { + name: 'Studio', + pairingCode: pairingCode('ws://192.0.2.10:6768') + }) + const outcome = addOrUpdateEnvironmentFromPairingCode(userDataPath, { + name: 'Laptop', + pairingCode: pairingCode( + 'ws://192.0.2.11:6768', + 'token', + Buffer.alloc(32, 8).toString('base64') + ) + }) + expect(outcome.updatedExisting).toBe(false) + expect(listEnvironments(userDataPath).map((entry) => entry.name)).toEqual(['Laptop', 'Studio']) + }) + + it('takes the tunnel dependency from the new pairing, not the replaced one', () => { + const first = addEnvironmentFromPairingCode(userDataPath, { + name: 'Studio', + pairingCode: pairingCode('ws://127.0.0.1:41000/runtime'), + connectionDependency: 'ssh-tunnel' + }) + expect(first.connectionDependency).toBe('ssh-tunnel') + + const outcome = addOrUpdateEnvironmentFromPairingCode(userDataPath, { + name: 'Studio on LAN', + pairingCode: pairingCode('ws://192.0.2.10:6768') + }) + expect(outcome.environment.connectionDependency).toBeUndefined() + }) + + it('refuses, naming the saved entry, when Orca manages that server over SSH', () => { + const managed = addManagedOrcadEnvironment(userDataPath, { + id: 'managed', + name: 'Managed box', + pairingCode: pairingCode('ws://127.0.0.1:41000/runtime'), + orcadDeployment: { + sshTargetId: 'target', + sshTargetGeneration: 1, + localPort: 41000, + remotePort: 6768 + }, + now: 100 + }) + + expect(() => + addOrUpdateEnvironmentFromPairingCode(userDataPath, { + name: 'Managed box by hand', + pairingCode: pairingCode('ws://192.0.2.10:6768', 'second-token') + }) + ).toThrow('This server is already paired as "Managed box".') + expect(listEnvironments(userDataPath)).toEqual([managed]) + }) + + it('refuses to fold a cloud workspace server into a saved entry', () => { + const first = addEnvironmentFromPairingCode(userDataPath, { + name: 'Studio', + pairingCode: pairingCode('ws://192.0.2.10:6768') + }) + expect(() => + addOrUpdateEnvironmentFromPairingCode(userDataPath, { + name: 'VM', + pairingCode: pairingCode('ws://192.0.2.20:6768', 'vm-token'), + source: 'ephemeral-vm' + }) + ).toThrow('This server is already paired as "Studio".') + expect(listEnvironments(userDataPath)).toEqual([first]) + }) +}) diff --git a/src/shared/runtime-environment-store-unreadable-records.test.ts b/src/shared/runtime-environment-store-unreadable-records.test.ts index 5fad48268ee..4efe44578f0 100644 --- a/src/shared/runtime-environment-store-unreadable-records.test.ts +++ b/src/shared/runtime-environment-store-unreadable-records.test.ts @@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { encodePairingOffer } from './pairing' import { addEnvironmentFromPairingCode, + addOrUpdateEnvironmentFromPairingCode, getEnvironmentStorePath, listEnvironments, markEnvironmentUsed, @@ -50,10 +51,12 @@ describe('runtime environment store with records this build cannot read', () => rmSync(userDataPath, { recursive: true, force: true }) }) + // Why a key per name: two names with one key are one server, which the store refreshes in place. function add(name: string) { + const publicKeyB64 = Buffer.alloc(32, name).toString('base64') return addEnvironmentFromPairingCode(userDataPath, { name, - pairingCode: encodePairingOffer(pairing), + pairingCode: encodePairingOffer({ ...pairing, publicKeyB64 }), now: 100 }) } @@ -188,6 +191,17 @@ describe('runtime environment store with records this build cannot read', () => expect(console.warn).toHaveBeenCalledWith(expect.stringContaining('Hiding 1 server')) }) + it('refuses a second entry for its server even though it is hidden', () => { + const { other } = setup() + expect(() => + addOrUpdateEnvironmentFromPairingCode(userDataPath, { + name: 'Pinned again', + pairingCode: encodePairingOffer({ ...pairing, endpoint: 'ws://192.0.2.10:6768' }) + }) + ).toThrow('already paired as "Pinned"') + expect(listEnvironments(userDataPath)).toEqual([other]) + }) + it('is never replaced, pruned, or orphaned by writes', () => { const { pinned, other, sidecarPath } = setup() const storeBefore = readFileSync(getEnvironmentStorePath(userDataPath), 'utf8') diff --git a/src/shared/runtime-environment-store.test.ts b/src/shared/runtime-environment-store.test.ts index d59d7870d49..a56d6c86984 100644 --- a/src/shared/runtime-environment-store.test.ts +++ b/src/shared/runtime-environment-store.test.ts @@ -13,12 +13,16 @@ import { updateEnvironmentFromPairingCode } from './runtime-environment-store' -function pairingCode(endpoint = 'ws://127.0.0.1:6768', pairedDeviceId?: string): string { +function pairingCode( + endpoint = 'ws://127.0.0.1:6768', + pairedDeviceId?: string, + keyFill = 1 +): string { return encodePairingOffer({ v: 2, endpoint, deviceToken: 'device-token', - publicKeyB64: Buffer.from(new Uint8Array(32).fill(1)).toString('base64'), + publicKeyB64: Buffer.from(new Uint8Array(32).fill(keyFill)).toString('base64'), ...(pairedDeviceId ? { pairedDeviceId } : {}) }) } @@ -104,7 +108,7 @@ describe('runtime environment store', () => { expect(() => addEnvironmentFromPairingCode(userDataPath, { name: 'dev box', - pairingCode: pairingCode('ws://192.0.2.10:6768') + pairingCode: pairingCode('ws://192.0.2.10:6768', undefined, 2) }) ).toThrow(RuntimeEnvironmentStoreError) expect(listEnvironments(userDataPath)).toEqual([first]) @@ -123,7 +127,7 @@ describe('runtime environment store', () => { expect(() => addEnvironmentFromPairingCode(userDataPath, { name, - pairingCode: pairingCode('ws://192.0.2.10:6768') + pairingCode: pairingCode('ws://192.0.2.10:6768', undefined, 2) }) ).toThrow('A server named "build" already exists.') } @@ -176,7 +180,7 @@ describe('runtime environment store', () => { const direct = addEnvironmentFromPairingCode(userDataPath, { name: 'direct box', - pairingCode: pairingCode('ws://192.0.2.11:6768'), + pairingCode: pairingCode('ws://192.0.2.11:6768', undefined, 2), connectionDependency: 'ssh-tunnel' }) expect(direct).not.toHaveProperty('connectionDependency') @@ -233,7 +237,7 @@ describe('runtime environment store', () => { }) const legacy = addEnvironmentFromPairingCode(userDataPath, { name: 'legacy box', - pairingCode: pairingCode('ws://192.0.2.10:6768'), + pairingCode: pairingCode('ws://192.0.2.10:6768', undefined, 2), now: 1_000 }) @@ -270,7 +274,7 @@ describe('runtime environment store', () => { expect(() => addEnvironmentFromPairingCode(userDataPath, { name: 'x'.repeat(MAX_RUNTIME_ENVIRONMENT_STORE_FILE_BYTES), - pairingCode: pairingCode('ws://192.0.2.10:6768') + pairingCode: pairingCode('ws://192.0.2.10:6768', undefined, 2) }) ).toThrow(RuntimeEnvironmentStoreError) expect(listEnvironments(userDataPath)).toEqual([first]) diff --git a/src/shared/runtime-environment-store.ts b/src/shared/runtime-environment-store.ts index bf4145e7d2b..c0a7d1c1c47 100644 --- a/src/shared/runtime-environment-store.ts +++ b/src/shared/runtime-environment-store.ts @@ -17,6 +17,14 @@ import { } from './runtime-environment-store-file' import { writeRuntimeEnvironmentSidecarEntry } from './runtime-environment-sidecar' import { findCollidingEnvironmentName } from './runtime-environment-name-collision' +import { + findEnvironmentWithHostKey, + savedHostKeySpelling +} from './runtime-environment-host-key-match' +import { + assertNoIndependentSshAccess, + assertPairingReplaceableByAdd +} from './runtime-environment-pairing-replace-guards' export { getEnvironmentStorePath, @@ -32,19 +40,73 @@ export function listEnvironments( return readEnvironmentStore(userDataPath, options).environments } +export type AddEnvironmentArgs = { + name: string + pairingCode: string + now?: number + source?: RuntimeEnvironmentSource + connectionDependency?: 'ssh-tunnel' + /** Recorded only when the code turns out to be for an already saved server. */ + runtimeId?: string +} + +export type AddEnvironmentOutcome = { + environment: KnownRuntimeEnvironment + /** The code was for a saved server, whose pairing it replaced instead of adding a second entry. */ + updatedExisting: boolean +} + +/** + * Saves a pairing, refusing one for a server already saved. Why never a second entry: two entries + * for one server publish the same workspaces, which makes every write to them ambiguous. + */ export function addEnvironmentFromPairingCode( userDataPath: string, - args: { - name: string - pairingCode: string - now?: number - source?: RuntimeEnvironmentSource - connectionDependency?: 'ssh-tunnel' - } + args: AddEnvironmentArgs ): KnownRuntimeEnvironment { + return saveEnvironmentFromPairingCode(userDataPath, args, false).environment +} + +/** + * Saves a pairing, or refreshes the saved server it is for. Only for callers that then retire the + * running app's connections to that server, which still authenticate with the replaced pairing. + */ +export function addOrUpdateEnvironmentFromPairingCode( + userDataPath: string, + args: AddEnvironmentArgs +): AddEnvironmentOutcome { + return saveEnvironmentFromPairingCode(userDataPath, args, true) +} + +function saveEnvironmentFromPairingCode( + userDataPath: string, + args: AddEnvironmentArgs, + refreshSameServer: boolean +): AddEnvironmentOutcome { const offer = parseRemotePairingCode(userDataPath, args.pairingCode) - const store = readPersistedEnvironmentStore(userDataPath) const now = args.now ?? Date.now() + const store = readPersistedEnvironmentStore(userDataPath) + const samePersisted = findEnvironmentWithHostKey(store.environments, offer.publicKeyB64) + // Why the overlaid entry: SSH access or a deployment lives only there, and the guard reads it. + const samePaired = + samePersisted && + readEnvironmentStore(userDataPath).environments.find((entry) => entry.id === samePersisted.id) + if (samePersisted && !samePaired) { + throw new RuntimeEnvironmentStoreError( + 'invalid_argument', + `This server is already paired as "${samePersisted.name}", whose saved settings this version of Orca cannot read.` + ) + } + if (samePaired) { + assertPairingReplaceableByAdd(samePaired, args.source, refreshSameServer) + const sameSpelling = { ...offer, publicKeyB64: savedHostKeySpelling(samePaired, offer) } + const environment = replaceEnvironmentPairing(userDataPath, samePaired.id, sameSpelling, { + now, + runtimeId: args.runtimeId, + connectionDependency: args.connectionDependency + }) + return { environment, updatedExisting: true } + } const existing = findCollidingEnvironmentName(store.environments, args.name) if (existing) { throw new RuntimeEnvironmentStoreError( @@ -69,7 +131,7 @@ export function addEnvironmentFromPairingCode( ].sort((a, b) => a.name.localeCompare(b.name)) } writeEnvironmentStore(userDataPath, next) - return environment + return { environment, updatedExisting: false } } function parseRemotePairingCode(userDataPath: string, pairingCode: string): PairingOffer { @@ -108,12 +170,24 @@ export function updateEnvironmentFromPairingCode( args: { pairingCode: string; now?: number; runtimeId?: string } ): KnownRuntimeEnvironment { const offer = parseRemotePairingCode(userDataPath, args.pairingCode) - assertNoIndependentSshAccess( - resolveEnvironmentFromStore(readEnvironmentStore(userDataPath), selector) - ) + const current = resolveEnvironmentFromStore(readEnvironmentStore(userDataPath), selector) + assertNoIndependentSshAccess(current) + return replaceEnvironmentPairing(userDataPath, selector, offer, { + now: args.now ?? Date.now(), + runtimeId: args.runtimeId, + connectionDependency: current.connectionDependency + }) +} + +/** Swaps a saved server's pairing in place, keeping its id, name and history. */ +function replaceEnvironmentPairing( + userDataPath: string, + selector: string, + offer: PairingOffer, + args: { now: number; runtimeId?: string; connectionDependency?: 'ssh-tunnel' } +): KnownRuntimeEnvironment { const store = readPersistedEnvironmentStore(userDataPath) const existing = resolveEnvironmentFromStore(store, selector) - const now = args.now ?? Date.now() const previousPairingRevision = existing.pairingRevision ?? existing.createdAt const environment = createEnvironmentFromPairingOffer({ id: existing.id, @@ -122,13 +196,13 @@ export function updateEnvironmentFromPairingCode( offer, runtimeId: args.runtimeId ?? existing.runtimeId, ...(existing.source ? { source: existing.source } : {}), - ...getPairingConnectionDependency(existing.connectionDependency, offer) + ...getPairingConnectionDependency(args.connectionDependency, offer) }) const next = { ...environment, createdAt: existing.createdAt, - updatedAt: now, - pairingRevision: Math.max(now, previousPairingRevision + 1), + updatedAt: args.now, + pairingRevision: Math.max(args.now, previousPairingRevision + 1), lastUsedAt: existing.lastUsedAt } writeEnvironmentStore(userDataPath, { @@ -262,24 +336,3 @@ export function resolveEnvironmentFromStore --yes` or use Settings > Managed servers; for a host that is gone, run `orca environment forget --environment --yes`.' - ) - } - if (environment.pendingSshAccessOperation) { - throw new RuntimeEnvironmentStoreError( - 'invalid_argument', - "This server's SSH access change didn't finish. Retry or cancel it under Settings > Remote Orca Servers > SSH access for paired servers before replacing its pairing or removing it." - ) - } - if (environment.sshAccess) { - throw new RuntimeEnvironmentStoreError( - 'invalid_argument', - "Unlink this server's SSH access under Settings > Remote Orca Servers > SSH access for paired servers before replacing its pairing or removing it." - ) - } -} diff --git a/src/shared/wsl-host-deployment-store.test.ts b/src/shared/wsl-host-deployment-store.test.ts index 844a44b0332..8ebb9b0947b 100644 --- a/src/shared/wsl-host-deployment-store.test.ts +++ b/src/shared/wsl-host-deployment-store.test.ts @@ -84,8 +84,10 @@ describe('WSL host deployment store', () => { function addWslManaged(record = wslRecord()) { const environment = addEnvironmentFromPairingCode(userDataPath, { name: 'Ubuntu', + // Why its own key: one key is one server, which the store refuses to save twice. pairingCode: encodePairingOffer({ ...pairing, + publicKeyB64: Buffer.alloc(32, 2).toString('base64'), endpoint: `ws://127.0.0.1:${record.localPort}/runtime` }), now: 150 @@ -276,6 +278,7 @@ describe('WSL host deployment store', () => { name: 'Remote', pairingCode: encodePairingOffer({ ...pairing, + publicKeyB64: Buffer.alloc(32, 3).toString('base64'), endpoint: 'wss://build.example.com/runtime' }), now: 150