From 19ae6a8198d54df76d4e393240a7ee3b9a15403a Mon Sep 17 00:00:00 2001 From: kipavy Date: Thu, 20 Aug 2026 10:02:32 +0000 Subject: [PATCH] fix(accounts): keep the switcher within the keychain's value cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The quick switcher kept every saved account, and each one's parked UI state, in a single keychain value. Windows Credential Manager refuses a credential blob over 2560 bytes once encoded as UTF-16, which two accounts' tokens exceed on their own — and every call site swallowed the rejection. Adding a second account wrote nothing, so it never joined the switcher; parking the outgoing account's workspace snapshot wrote nothing either, so switching back came up with no tabs at all. The switcher is now one entry per account plus an index of their ids, each well under the cap, and the UI state parks in localStorage, where the same data already lives while the account is signed in. A pre-0.29 single-value list migrates on first read. A failed read is no longer indistinguishable from an empty switcher: it blocks the write that would otherwise persist that emptiness over real accounts. Failures reach the user as a toast, and "Add another account" refuses to sign this one out when it could not be saved first. Restoring the workspace now waits for the replace-mode login sync. A switch wipes the config dir, so reconnecting before the cloud pull refills it finds no connection and errors every restored tab; a normal launch reads its cache from disk and never waits. --- .../SidebarAccountButton.switcher.test.tsx | 34 ++++ .../layout/SidebarAccountButton.tsx | 33 +++- src/components/layout/SplashScreen.tsx | 17 +- src/i18n/locales/en/layout.json | 1 + src/i18n/locales/fr/layout.json | 1 + src/i18n/locales/ru/layout.json | 1 + src/i18n/locales/zh/layout.json | 1 + src/plugins/runtime.ts | 19 +- src/services/loginSyncGate.ts | 35 ++++ src/services/savedAccounts.test.ts | 151 +++++++++++++--- src/services/savedAccounts.ts | 169 ++++++++++++++---- src/stores/persistedAccountUiState.test.ts | 60 +++++++ src/stores/persistedAccountUiState.ts | 56 ++++++ src/stores/workspaceRestore.test.ts | 88 +++++++++ src/stores/workspaceRestore.ts | 10 +- 15 files changed, 580 insertions(+), 96 deletions(-) create mode 100644 src/services/loginSyncGate.ts create mode 100644 src/stores/workspaceRestore.test.ts diff --git a/src/components/layout/SidebarAccountButton.switcher.test.tsx b/src/components/layout/SidebarAccountButton.switcher.test.tsx index 94fe935ea..dc8c30de0 100644 --- a/src/components/layout/SidebarAccountButton.switcher.test.tsx +++ b/src/components/layout/SidebarAccountButton.switcher.test.tsx @@ -38,6 +38,7 @@ vi.mock("@/services/savedAccounts", () => ({ import { SidebarAccountButton } from "./SidebarAccountButton"; import { useUIStore } from "@/stores/uiStore"; import { useSecurityStore } from "@/stores/securityStore"; +import { useNotificationStore } from "@/stores/notificationStore"; const CURRENT = { account_id: "current", mode: "server", master_password: ["master", "for", "current"].join("-"), @@ -50,6 +51,7 @@ beforeEach(() => { vi.clearAllMocks(); h.accountMode = "server"; h.keychain = { email: CURRENT.email, account_id: CURRENT.account_id }; + useNotificationStore.setState({ toasts: [] }); }); afterEach(cleanup); @@ -154,3 +156,35 @@ test("a local account is not offered the add-account route", async () => { await openMenu(); expect(screen.queryByText("layout.sidebarAccount.addAccount")).toBeNull(); }); + +/** + * A keychain that refuses the write is how an account went missing from the + * switcher without a word — the save failure was caught and dropped. + */ +test("a switcher save the keychain refuses is reported", async () => { + h.saveCurrentAccount.mockRejectedValueOnce(new Error("Keychain write error: too long")); + await openMenu(); + + await waitFor(() => + expect( + useNotificationStore.getState().toasts.some((toast) => + toast.message.startsWith("layout.sidebarAccount.saveFailed"), + ), + ).toBe(true), + ); +}); + +test("adding another account stops when the current one could not be saved", async () => { + h.signOutToAddAccount.mockRejectedValueOnce(new Error("Saved accounts could not be read")); + await openMenu(); + + await userEvent.click(await screen.findByText("layout.sidebarAccount.addAccount")); + + await waitFor(() => + expect( + useNotificationStore.getState().toasts.some((toast) => + toast.message.startsWith("layout.sidebarAccount.saveFailed"), + ), + ).toBe(true), + ); +}); diff --git a/src/components/layout/SidebarAccountButton.tsx b/src/components/layout/SidebarAccountButton.tsx index 96a5af02c..947483b17 100644 --- a/src/components/layout/SidebarAccountButton.tsx +++ b/src/components/layout/SidebarAccountButton.tsx @@ -73,12 +73,27 @@ export function SidebarAccountButton() { const rect = buttonRef.current.getBoundingClientRect(); setPos({ bottom: window.innerHeight - rect.bottom, left: rect.right + 8 }); } - await Promise.all([refreshAccountInfo(), saveCurrentAccount().catch(() => {})]); + await Promise.all([ + refreshAccountInfo(), + // A keychain that refuses this write is exactly how an account goes + // missing from the switcher, so it is said out loud rather than swallowed. + saveCurrentAccount().catch((e) => reportAccountError("saveFailed", e)), + ]); const accounts = await getSavedAccounts().catch(() => [] as SavedAccount[]); setSavedAccounts(accounts); setOpen(true); }; + const reportAccountError = (key: "switchFailed" | "saveFailed", e: unknown) => { + useNotificationStore.getState().addToast({ + source: { kind: "plugin", id: "system", name: "Voltius" }, + type: "toast", + message: t(`layout.sidebarAccount.${key}`, { error: e instanceof Error ? e.message : String(e) }), + severity: "error", + duration: 8000, + }); + }; + const handleLockVault = async () => { setOpen(false); await lockVaultSession(); @@ -93,7 +108,13 @@ export function SidebarAccountButton() { const handleAddAccount = async () => { setOpen(false); - await signOutToAddAccount(); + try { + await signOutToAddAccount(); + } catch (e) { + // It signs this account out to reach the auth screen, so it must not run + // when the switcher could not keep it: that is a one-way trip out. + reportAccountError("saveFailed", e); + } }; const handleSwitchAccount = async (account: SavedAccount) => { @@ -103,13 +124,7 @@ export function SidebarAccountButton() { } catch (e) { // The switch tears the session down before it rebuilds it; a silent // rejection would leave the user staring at an unchanged window. - useNotificationStore.getState().addToast({ - source: { kind: "plugin", id: "system", name: "Voltius" }, - type: "toast", - message: t("layout.sidebarAccount.switchFailed", { error: e instanceof Error ? e.message : String(e) }), - severity: "error", - duration: 8000, - }); + reportAccountError("switchFailed", e); } }; diff --git a/src/components/layout/SplashScreen.tsx b/src/components/layout/SplashScreen.tsx index 41ec10d37..baff60f77 100644 --- a/src/components/layout/SplashScreen.tsx +++ b/src/components/layout/SplashScreen.tsx @@ -11,7 +11,7 @@ import { usePortForwardingStore } from "@/stores/portForwardingStore"; import { autoLogin, consumeForceLockFlag, isServerMode } from "@/services/account"; import { saveCurrentAccount } from "@/services/savedAccounts"; import { syncOnLogin, syncOnLoginReplace, startRealtimeSync } from "@/services/sync"; -import { setLoginSyncPending, resolveLoginSync } from "@/plugins/runtime"; +import { setLoginSyncPending, resolveLoginSync } from "@/services/loginSyncGate"; import { loadSeededPlugins } from "@/plugins/seeded"; import { loadInstalledPlugins, loadPluginMeta, supersedeStaleFirstPartyShadows } from "@/stores/marketplaceStore"; import { usePluginRegistryStore } from "@/stores/pluginRegistryStore"; @@ -28,6 +28,15 @@ interface Props { onReady: () => void; } const STEP_IDS = ["init", "vault", "connections"] as const; +/** + * Keep the quick switcher's copy of this account current. Nothing on the splash + * can show a toast yet, so the failure is logged rather than dropped: a write + * refused in silence is how an account went missing from the switcher. + */ +function keepSwitcherFresh(): void { + saveCurrentAccount().catch((e) => console.warn("[splash] could not save this account to the switcher:", e)); +} + export default function SplashScreen({ onReady }: Props) { const { t } = useTranslation(); const [steps, setSteps] = useState(() => @@ -64,7 +73,7 @@ export default function SplashScreen({ onReady }: Props) { if (outcome === "ok") { setStep("vault", "done", t("layout.splash.sessionRestored")); setPhase("finishing"); - saveCurrentAccount().catch(() => {}); // keep saved accounts list fresh + keepSwitcherFresh(); await finishLoading(); return; } @@ -121,7 +130,7 @@ export default function SplashScreen({ onReady }: Props) { // Gate plugins behind this promise so they see post-merge data. // vault_reset (logout) wipes the config dir including plugin storage, // so plugins must not run their initial sync before server data lands. - setLoginSyncPending(); + setLoginSyncPending({ replace: useReplace }); (useReplace ? syncOnLoginReplace() : syncOnLogin()) .catch(() => {}) .finally(() => resolveLoginSync()); @@ -148,7 +157,7 @@ export default function SplashScreen({ onReady }: Props) { const handleAuthReady = async () => { setPhase("finishing"); - saveCurrentAccount().catch(() => {}); // keep saved accounts list fresh + keepSwitcherFresh(); await finishLoading(); }; diff --git a/src/i18n/locales/en/layout.json b/src/i18n/locales/en/layout.json index 0f0f1293f..efc597ac6 100644 --- a/src/i18n/locales/en/layout.json +++ b/src/i18n/locales/en/layout.json @@ -111,6 +111,7 @@ "savedAccountCloud": "Cloud", "savedAccountLocal": "Local", "switchFailed": "Account switch failed: {{error}}", + "saveFailed": "This account could not be saved to the switcher: {{error}}", "leaveLocalTitle": "Leave this local account?", "leaveLocalMessage": "This account's hosts, keys and snippets are stored only on this machine. Switching to {{account}} erases them and they cannot be recovered.", "leaveLocalConfirm": "Erase and switch", diff --git a/src/i18n/locales/fr/layout.json b/src/i18n/locales/fr/layout.json index e7d4c6c25..aa2246c79 100644 --- a/src/i18n/locales/fr/layout.json +++ b/src/i18n/locales/fr/layout.json @@ -111,6 +111,7 @@ "savedAccountCloud": "Cloud", "savedAccountLocal": "Local", "switchFailed": "Échec du changement de compte : {{error}}", + "saveFailed": "Ce compte n'a pas pu être enregistré dans le sélecteur : {{error}}", "leaveLocalTitle": "Quitter ce compte local ?", "leaveLocalMessage": "Les hôtes, clés et snippets de ce compte sont stockés uniquement sur cette machine. Passer à {{account}} les efface définitivement.", "leaveLocalConfirm": "Effacer et changer", diff --git a/src/i18n/locales/ru/layout.json b/src/i18n/locales/ru/layout.json index 6764b0648..8122031c4 100644 --- a/src/i18n/locales/ru/layout.json +++ b/src/i18n/locales/ru/layout.json @@ -111,6 +111,7 @@ "savedAccountCloud": "Облако", "savedAccountLocal": "Локально", "switchFailed": "Не удалось переключить аккаунт: {{error}}", + "saveFailed": "Не удалось сохранить аккаунт в переключателе: {{error}}", "leaveLocalTitle": "Выйти из локального аккаунта?", "leaveLocalMessage": "Хосты, ключи и сниппеты этого аккаунта хранятся только на этом устройстве. Переход на {{account}} удалит их безвозвратно.", "leaveLocalConfirm": "Удалить и переключиться", diff --git a/src/i18n/locales/zh/layout.json b/src/i18n/locales/zh/layout.json index bf882240e..a584755e5 100644 --- a/src/i18n/locales/zh/layout.json +++ b/src/i18n/locales/zh/layout.json @@ -111,6 +111,7 @@ "savedAccountCloud": "云", "savedAccountLocal": "本地", "switchFailed": "切换账户失败：{{error}}", + "saveFailed": "无法将此账户保存到切换器：{{error}}", "leaveLocalTitle": "离开此本地账户？", "leaveLocalMessage": "此账户的主机、密钥和代码片段仅存储在本机。切换到 {{account}} 将永久删除它们。", "leaveLocalConfirm": "删除并切换", diff --git a/src/plugins/runtime.ts b/src/plugins/runtime.ts index 14723a251..627ed733d 100644 --- a/src/plugins/runtime.ts +++ b/src/plugins/runtime.ts @@ -1,6 +1,7 @@ import { invoke } from "@tauri-apps/api/core"; import { openUrl } from "@tauri-apps/plugin-opener"; import { closePfTunnel, getPfState, openPfTunnel } from "@/services/portForwardingTunnels"; +import { whenLoginSyncSettled } from "@/services/loginSyncGate"; import { resolvePort } from "@/plugins/domains/ports"; import { runSnippetSequence, previewSnippetSequence } from "@/services/snippetSequence"; import type { RunTarget } from "@/services/sftpTarget"; @@ -148,22 +149,6 @@ const _exposedApis = new Map(); // opened. Without this a disabled plugin's sockets outlive it silently. const _sftpDisposers = new Map void>(); -// ─── Login-sync readiness gate ──────────────────────────────────────────── -// Resolves immediately for local/offline users; SplashScreen holds it pending -// while syncOnLogin / syncOnLoginReplace runs so plugins don't race the merge. - -let _loginSyncResolve: (() => void) | null = null; -let _loginSyncReady: Promise = Promise.resolve(); - -export function setLoginSyncPending(): void { - _loginSyncReady = new Promise((resolve) => { _loginSyncResolve = resolve; }); -} - -export function resolveLoginSync(): void { - _loginSyncResolve?.(); - _loginSyncResolve = null; -} - // ─── Per-plugin settings-change listeners ───────────────────────────────── const _settingsListeners = new Map void>>(); @@ -2311,7 +2296,7 @@ function createPluginAPI(manifest: PluginManifest): PluginAPI { _onBeforeQuit.add(cb); return () => _onBeforeQuit.delete(cb); }, - waitForLoginSync: () => _loginSyncReady, + waitForLoginSync: whenLoginSyncSettled, }, sync: { diff --git a/src/services/loginSyncGate.ts b/src/services/loginSyncGate.ts new file mode 100644 index 000000000..66fff1e1a --- /dev/null +++ b/src/services/loginSyncGate.ts @@ -0,0 +1,35 @@ +/** + * Login-sync readiness gate. + * + * Resolves immediately for local/offline users; SplashScreen holds it pending + * while syncOnLogin / syncOnLoginReplace runs, so nothing that needs the + * account's own data races the merge. + * + * Replace-mode is called out separately because it follows a wiped config dir: + * an account switch deletes every entity file, so until this settles the app + * knows of no connections at all — which is not the same as owning none. + */ + +let resolveReady: (() => void) | null = null; +let ready: Promise = Promise.resolve(); +let replacePending = false; + +export function setLoginSyncPending(options?: { replace?: boolean }): void { + replacePending = options?.replace ?? false; + ready = new Promise((resolve) => { resolveReady = resolve; }); +} + +export function resolveLoginSync(): void { + replacePending = false; + resolveReady?.(); + resolveReady = null; +} + +export function whenLoginSyncSettled(): Promise { + return ready; +} + +/** True while a wiped local cache is still waiting to be refilled from the cloud. */ +export function isReplaceSyncPending(): boolean { + return replacePending; +} diff --git a/src/services/savedAccounts.test.ts b/src/services/savedAccounts.test.ts index ed37b6e4c..f3c126ee1 100644 --- a/src/services/savedAccounts.test.ts +++ b/src/services/savedAccounts.test.ts @@ -7,10 +7,15 @@ const h = vi.hoisted(() => ({ push: vi.fn(async () => undefined), stopRealtimeSync: vi.fn(), clearPersistedAccountUiState: vi.fn(), - snapshotPersistedAccountUiState: vi.fn(() => ({ "voltius-vaults": "VAULTS_OF_CURRENT" })), - restorePersistedAccountUiState: vi.fn(), + parkAccountUiState: vi.fn(), + restoreAccountUiState: vi.fn(), + writeParkedUiState: vi.fn(), + dropAccountUiState: vi.fn(), reload: vi.fn(), store: {} as Record, + /** UTF-16 bytes a single keychain value may hold; 0 for an unbounded one. */ + valueCap: 0, + readFails: false, })); vi.mock("@tauri-apps/api/core", () => ({ invoke: h.invoke })); @@ -18,14 +23,24 @@ vi.mock("./vault", () => ({ lockVault: h.lockVault, wipeLocalConfig: h.wipeLocal vi.mock("@/services/sync", () => ({ push: h.push, stopRealtimeSync: h.stopRealtimeSync })); vi.mock("@/stores/persistedAccountUiState", () => ({ clearPersistedAccountUiState: h.clearPersistedAccountUiState, - snapshotPersistedAccountUiState: h.snapshotPersistedAccountUiState, - restorePersistedAccountUiState: h.restorePersistedAccountUiState, + parkAccountUiState: h.parkAccountUiState, + restoreAccountUiState: h.restoreAccountUiState, + writeParkedUiState: h.writeParkedUiState, + dropAccountUiState: h.dropAccountUiState, })); import { getSavedAccounts, saveCurrentAccount, removeSavedAccount, signOutToAddAccount, switchToAccount, type SavedAccount } from "./savedAccounts"; import { ACCOUNT_CACHE_KEYS } from "./accountCacheKeys"; -const LIST_KEY = "voltius.saved_accounts"; +const INDEX_KEY = "voltius.saved_accounts"; +const entryKey = (id: string) => `voltius.saved_account.${id}`; + +/** + * Windows Credential Manager's ceiling: CRED_MAX_CREDENTIAL_BLOB_SIZE, checked + * after the value is encoded as UTF-16, so 2560 bytes is 1280 ASCII characters. + * The switcher held every account in one value and blew straight through it. + */ +const WINDOWS_BLOB_CAP = 2560; /** Values are composed rather than written inline so secret scanners stay quiet. */ const fake = (kind: string, id: string) => [kind, "for", id].join("-"); @@ -48,10 +63,19 @@ const CLOUD_B = cloudAccount("b"); beforeEach(() => { vi.clearAllMocks(); h.store = {}; + h.valueCap = 0; + h.readFails = false; h.invoke.mockImplementation(async (cmd: string, args: Record = {}) => { switch (cmd) { - case "keychain_get": return h.store[args.key as string] ?? null; - case "keychain_set": h.store[args.key as string] = args.value as string; return undefined; + case "keychain_get": + if (h.readFails) throw new Error("Keychain read error"); + return h.store[args.key as string] ?? null; + case "keychain_set": { + const value = args.value as string; + if (h.valueCap && value.length * 2 > h.valueCap) throw new Error("Keychain write error: too long"); + h.store[args.key as string] = value; + return undefined; + } case "keychain_delete": delete h.store[args.key as string]; return undefined; default: throw new Error(`unexpected command ${cmd}`); } @@ -64,6 +88,12 @@ function activate(account: SavedAccount) { for (const [key, value] of Object.entries(account)) h.store[key] = value as string; } +/** Seed the switcher in its stored shape: an index of ids, one entry each. */ +function seed(...accounts: SavedAccount[]) { + h.store[INDEX_KEY] = JSON.stringify(accounts.map((a) => a.account_id)); + for (const account of accounts) h.store[entryKey(account.account_id)] = JSON.stringify(account); +} + test("saveCurrentAccount snapshots the active session and upserts by account_id", async () => { activate(CLOUD_A); await saveCurrentAccount(); @@ -93,19 +123,21 @@ test("a local account is never saved or listed — switching would wipe its only expect(await getSavedAccounts()).toEqual([]); // Entries written by an install from before the rule are filtered on read. - h.store[LIST_KEY] = JSON.stringify([{ ...CLOUD_A, mode: "local" }, CLOUD_B]); + seed({ ...CLOUD_A, mode: "local" }, CLOUD_B); expect((await getSavedAccounts()).map((a) => a.account_id)).toEqual(["b"]); }); test("getSavedAccounts survives a corrupt list", async () => { - h.store[LIST_KEY] = "{not json"; + h.store[INDEX_KEY] = "{not json"; expect(await getSavedAccounts()).toEqual([]); }); -test("removeSavedAccount drops only the named account", async () => { - h.store[LIST_KEY] = JSON.stringify([CLOUD_A, CLOUD_B]); +test("removeSavedAccount drops the account, its entry and its parked state", async () => { + seed(CLOUD_A, CLOUD_B); await removeSavedAccount("a"); expect((await getSavedAccounts()).map((a) => a.account_id)).toEqual(["b"]); + expect(h.store[entryKey("a")]).toBeUndefined(); + expect(h.dropAccountUiState).toHaveBeenCalledWith("a"); }); test("switchToAccount clears every account-scoped key before writing the target's", async () => { @@ -113,7 +145,7 @@ test("switchToAccount clears every account-scoped key before writing the target' // Keys the previous account cached that are not part of the session snapshot. h.store.handle = "alice"; h.store.wrapped_user_secrets = "WRAPPED_A"; - h.store[LIST_KEY] = JSON.stringify([CLOUD_A, CLOUD_B]); + seed(CLOUD_A, CLOUD_B); await switchToAccount(CLOUD_B); @@ -135,32 +167,26 @@ test("switchToAccount deletes session keys the target leaves empty", async () => test("switchToAccount parks the outgoing UI state and restores the incoming one", async () => { activate(CLOUD_A); - const bWithState = { ...CLOUD_B, ui_state: { "voltius-vaults": "VAULTS_OF_B" } }; - h.store[LIST_KEY] = JSON.stringify([CLOUD_A, bWithState]); + seed(CLOUD_A, CLOUD_B); - await switchToAccount(bWithState); + await switchToAccount(CLOUD_B); - const parked = (await getSavedAccounts()).find((a) => a.account_id === "a"); - expect(parked?.ui_state).toEqual({ "voltius-vaults": "VAULTS_OF_CURRENT" }); + expect(h.parkAccountUiState).toHaveBeenCalledWith("a"); expect(h.clearPersistedAccountUiState).toHaveBeenCalled(); - expect(h.restorePersistedAccountUiState).toHaveBeenCalledWith({ "voltius-vaults": "VAULTS_OF_B" }); + expect(h.restoreAccountUiState).toHaveBeenCalledWith("b"); }); test("parking the outgoing UI state keeps the rest of its saved entry", async () => { activate(CLOUD_A); - h.store[LIST_KEY] = JSON.stringify([CLOUD_A, CLOUD_B]); + seed(CLOUD_A, CLOUD_B); await switchToAccount(CLOUD_B); expect((await getSavedAccounts()).find((a) => a.account_id === "a")).toMatchObject(CLOUD_A); }); -test("saveCurrentAccount keeps the UI state already parked on the entry", async () => { - activate(CLOUD_A); - h.store[LIST_KEY] = JSON.stringify([{ ...CLOUD_A, ui_state: { "voltius-vaults": "PARKED" } }]); - h.store.jwt = fake("jwt", "a3"); - await saveCurrentAccount(); - const saved = (await getSavedAccounts())[0]; - expect(saved.ui_state).toEqual({ "voltius-vaults": "PARKED" }); - expect(saved.jwt).toBe(fake("jwt", "a3")); +test("only an account the switcher already holds gets its UI state parked", async () => { + activate(CLOUD_A); // signed in, never saved — parking it would strand its state + await switchToAccount(CLOUD_B); + expect(h.parkAccountUiState).not.toHaveBeenCalled(); }); test("switchToAccount tears the old session down before reloading", async () => { @@ -197,6 +223,73 @@ test("adding another account keeps the current one in the switcher", async () => test("adding another account parks the outgoing account's UI state", async () => { activate(CLOUD_A); await signOutToAddAccount(); - const parked = (await getSavedAccounts()).find((a) => a.account_id === "a"); - expect(parked?.ui_state).toEqual({ "voltius-vaults": "VAULTS_OF_CURRENT" }); + expect(h.parkAccountUiState).toHaveBeenCalledWith("a"); +}); + +/** + * The bug this layout exists for. Real tokens are ~350 characters each, so two + * accounts in one keychain value ran to ~3.8 KB UTF-16 — past the cap, and the + * write failed silently, leaving the second account out of the switcher for good. + */ +function realisticAccount(id: string): SavedAccount { + const token = (kind: string) => `${fake(kind, id)}.${"t".repeat(340)}`; + return { ...cloudAccount(id), jwt: token("jwt"), refresh_token: token("refresh") }; +} + +const utf16Bytes = (value: string) => value.length * 2; + +test("a second account survives a keychain that caps one value at the Windows blob size", async () => { + h.valueCap = WINDOWS_BLOB_CAP; + + activate(realisticAccount("a")); + await saveCurrentAccount(); + activate(realisticAccount("b")); + await saveCurrentAccount(); + + expect((await getSavedAccounts()).map((a) => a.account_id)).toEqual(["a", "b"]); + for (const [key, value] of Object.entries(h.store)) { + expect(utf16Bytes(value), `${key} would be refused by the keychain`).toBeLessThanOrEqual(WINDOWS_BLOB_CAP); + } +}); + +test("one account's entry leaves room under the Windows cap", async () => { + activate(realisticAccount("a")); + await saveCurrentAccount(); + // Headroom for a few more claims in the JWT before the cap bites again. + expect(utf16Bytes(h.store[entryKey("a")])).toBeLessThan(WINDOWS_BLOB_CAP * 0.8); +}); + +test("a keychain that cannot be read is never overwritten with an empty switcher", async () => { + seed(CLOUD_A, CLOUD_B); + activate(CLOUD_A); + const before = { ...h.store }; + h.readFails = true; + + await expect(saveCurrentAccount()).rejects.toThrow(); + + h.readFails = false; + expect(h.store).toEqual(before); + expect((await getSavedAccounts()).map((a) => a.account_id)).toEqual(["a", "b"]); +}); + +test("a pre-0.29 single-value list migrates to one entry per account", async () => { + h.store[INDEX_KEY] = JSON.stringify([ + { ...CLOUD_A, ui_state: { "voltius-vaults": "VAULTS_OF_A" } }, + CLOUD_B, + ]); + + expect(await getSavedAccounts()).toEqual([CLOUD_A, CLOUD_B]); + expect(JSON.parse(h.store[INDEX_KEY])).toEqual(["a", "b"]); + expect(JSON.parse(h.store[entryKey("a")])).toEqual(CLOUD_A); + // The UI state that used to ride along leaves the keychain for localStorage. + expect(h.writeParkedUiState).toHaveBeenCalledWith("a", { "voltius-vaults": "VAULTS_OF_A" }); +}); + +test("a migration the keychain refuses leaves the old list readable", async () => { + const legacy = JSON.stringify([CLOUD_A, CLOUD_B]); + h.store[INDEX_KEY] = legacy; + h.valueCap = 20; + + expect((await getSavedAccounts()).map((a) => a.account_id)).toEqual(["a", "b"]); + expect(h.store[INDEX_KEY]).toBe(legacy); }); diff --git a/src/services/savedAccounts.ts b/src/services/savedAccounts.ts index 564824ec3..193f46107 100644 --- a/src/services/savedAccounts.ts +++ b/src/services/savedAccounts.ts @@ -1,8 +1,10 @@ import { invoke } from "@tauri-apps/api/core"; import { clearPersistedAccountUiState, - restorePersistedAccountUiState, - snapshotPersistedAccountUiState, + dropAccountUiState, + parkAccountUiState, + restoreAccountUiState, + writeParkedUiState, type PersistedAccountUiState, } from "@/stores/persistedAccountUiState"; import { ACCOUNT_CACHE_KEYS } from "./accountCacheKeys"; @@ -28,14 +30,11 @@ export type SavedAccount = Record & { account_id: string; mode: string; master_password: string; - /** - * The account's persisted UI state, captured when it was last switched away - * from. The vault list lives only in localStorage, so without this a switch - * back would leave every host filed under a user-created vault invisible. - */ - ui_state?: PersistedAccountUiState; }; +/** Pre-0.29 shape: every account, and its UI state, inside one keychain value. */ +type LegacySavedAccount = SavedAccount & { ui_state?: PersistedAccountUiState }; + async function keychainGet(key: string): Promise { return invoke("keychain_get", { key }); } @@ -46,7 +45,18 @@ async function keychainDelete(key: string): Promise { return invoke("keychain_delete", { key }); } -const SAVED_ACCOUNTS_KEY = "voltius.saved_accounts"; +/** + * The switcher is one keychain entry per account plus an index of their ids, + * because keychains cap the size of a single value and the app has no way to + * see that coming: Windows Credential Manager refuses a blob over 2560 bytes + * once UTF-16 encoded, which two accounts' tokens exceed on their own. Held in + * one value, saving the second account failed, the failure was swallowed, and + * the account silently never joined the switcher. + */ +const INDEX_KEY = "voltius.saved_accounts"; +const ENTRY_PREFIX = "voltius.saved_account."; + +const entryKey = (account_id: string) => `${ENTRY_PREFIX}${account_id}`; /** * Only cloud accounts are switchable. Switching wipes the config dir and @@ -57,23 +67,95 @@ function isSwitchable(account: SavedAccount): boolean { return account.mode === "server"; } -export async function getSavedAccounts(): Promise { +function parseEntry(raw: string | null): SavedAccount | null { + if (!raw) return null; try { - const raw = await keychainGet(SAVED_ACCOUNTS_KEY); - if (!raw) return []; - // Filter on read too: installs from before the cloud-only rule may hold a - // local entry, and it must not surface as a switch target. - return (JSON.parse(raw) as SavedAccount[]).filter(isSwitchable); + const entry = JSON.parse(raw) as SavedAccount; + return entry?.account_id && isSwitchable(entry) ? entry : null; } catch { - return []; + return null; } } -async function setSavedAccounts(accounts: SavedAccount[]): Promise { - await keychainSet(SAVED_ACCOUNTS_KEY, JSON.stringify(accounts)); +/** + * Read the switcher. + * + * `ok` is false only when the keychain itself failed, and it is what keeps a + * bad read from becoming a bad write: an unreadable list must never be treated + * as an empty one, or the next save persists that emptiness over real accounts. + */ +async function loadSavedAccounts(): Promise<{ ok: boolean; accounts: SavedAccount[] }> { + let raw: string | null; + try { + raw = await keychainGet(INDEX_KEY); + } catch { + return { ok: false, accounts: [] }; + } + if (!raw) return { ok: true, accounts: [] }; + + let index: unknown; + try { + index = JSON.parse(raw); + } catch { + return { ok: true, accounts: [] }; // corrupt beyond repair — safe to replace + } + if (!Array.isArray(index)) return { ok: true, accounts: [] }; + + if (index.some((entry) => entry !== null && typeof entry === "object")) { + return migrateLegacyList(index as LegacySavedAccount[]); + } + + const accounts: SavedAccount[] = []; + for (const id of index) { + if (typeof id !== "string") continue; + let entry: string | null; + try { + entry = await keychainGet(entryKey(id)); + } catch { + return { ok: false, accounts: [] }; + } + const parsed = parseEntry(entry); + if (parsed) accounts.push(parsed); + } + return { ok: true, accounts }; } -/** Snapshot current active account and upsert it into the saved list. */ +/** + * Split a pre-0.29 single-value list into one entry per account, moving any + * parked UI state out of the keychain. Leaves the old value untouched if a + * write fails, so a keychain that refuses the migration today can still serve + * the accounts it already holds and retry on the next read. + */ +async function migrateLegacyList( + legacy: LegacySavedAccount[], +): Promise<{ ok: boolean; accounts: SavedAccount[] }> { + const accounts: SavedAccount[] = []; + for (const { ui_state, ...entry } of legacy) { + if (!entry?.account_id || !isSwitchable(entry)) continue; + if (ui_state) writeParkedUiState(entry.account_id, ui_state); + accounts.push(entry); + } + try { + for (const entry of accounts) { + await keychainSet(entryKey(entry.account_id), JSON.stringify(entry)); + } + await keychainSet(INDEX_KEY, JSON.stringify(accounts.map((a) => a.account_id))); + } catch { + return { ok: false, accounts }; + } + return { ok: true, accounts }; +} + +export async function getSavedAccounts(): Promise { + return (await loadSavedAccounts()).accounts; +} + +/** + * Snapshot the active account and upsert it into the switcher. + * + * Rejects when the keychain does — the caller decides whether that is worth + * telling the user about. Swallowing it here is what hid the size cap. + */ export async function saveCurrentAccount(): Promise { const values = await Promise.all(SESSION_KEYS.map((key) => keychainGet(key))); const session = Object.fromEntries( @@ -89,33 +171,48 @@ export async function saveCurrentAccount(): Promise { await upsertSavedAccount(entry); } -/** Merge an entry into the saved list, keeping fields the caller did not supply. */ +/** Merge an entry into the switcher, keeping fields the caller did not supply. */ async function upsertSavedAccount(entry: SavedAccount): Promise { - const existing = await getSavedAccounts(); - const idx = existing.findIndex((a) => a.account_id === entry.account_id); - if (idx >= 0) { - existing[idx] = { ...existing[idx], ...entry }; - } else { - existing.push(entry); - } - await setSavedAccounts(existing); + const { ok, accounts } = await loadSavedAccounts(); + if (!ok) throw new Error("Saved accounts could not be read"); + + const existing = accounts.find((a) => a.account_id === entry.account_id); + await keychainSet(entryKey(entry.account_id), JSON.stringify({ ...existing, ...entry })); + if (existing) return; + await keychainSet( + INDEX_KEY, + JSON.stringify([...accounts.map((a) => a.account_id), entry.account_id]), + ); } /** - * Park the outgoing account's persisted UI state on its saved entry, so that - * switching back restores the vaults and teams it was last showing. + * Park the outgoing account's persisted UI state, so that switching back + * restores the vaults and teams it was last showing. + * + * It stays in localStorage, where it already lives while the account is signed + * in: it runs to kilobytes — workspace snapshot, command history, snippet + * variables — which is far past what a keychain value holds. */ async function stashUiStateForCurrentAccount(): Promise { const account_id = await keychainGet("account_id"); if (!account_id) return; - const current = (await getSavedAccounts()).find((a) => a.account_id === account_id); - if (!current) return; - await upsertSavedAccount({ ...current, ui_state: snapshotPersistedAccountUiState() }); + const { accounts } = await loadSavedAccounts(); + if (!accounts.some((a) => a.account_id === account_id)) return; + parkAccountUiState(account_id); } export async function removeSavedAccount(account_id: string): Promise { - const existing = await getSavedAccounts(); - await setSavedAccounts(existing.filter((a) => a.account_id !== account_id)); + const { ok, accounts } = await loadSavedAccounts(); + // Index first: a failed entry delete then leaves a dangling id, which reads + // skip, rather than an account the switcher still offers. + if (ok) { + await keychainSet( + INDEX_KEY, + JSON.stringify(accounts.map((a) => a.account_id).filter((id) => id !== account_id)), + ); + } + await keychainDelete(entryKey(account_id)).catch(() => {}); + dropAccountUiState(account_id); } /** @@ -165,7 +262,7 @@ export async function switchToAccount(account: SavedAccount): Promise { const value = account[key]; if (value) await keychainSet(key, value); } - restorePersistedAccountUiState(account.ui_state); + restoreAccountUiState(account.account_id); window.location.reload(); } diff --git a/src/stores/persistedAccountUiState.test.ts b/src/stores/persistedAccountUiState.test.ts index 02c891dc7..af5741ced 100644 --- a/src/stores/persistedAccountUiState.test.ts +++ b/src/stores/persistedAccountUiState.test.ts @@ -6,6 +6,10 @@ import { clearPersistedAccountUiState, snapshotPersistedAccountUiState, restorePersistedAccountUiState, + parkAccountUiState, + restoreAccountUiState, + dropAccountUiState, + writeParkedUiState, } from "./persistedAccountUiState"; const storeSources = import.meta.glob("./*.ts", { query: "?raw", import: "default", eager: true }) as Record; @@ -89,3 +93,59 @@ test("no key is classified twice", () => { const all = [...ACCOUNT_SCOPED_STORAGE_KEYS, ...ACCOUNT_SCOPED_RESET_KEYS, ...DEVICE_SCOPED_STORAGE_KEYS]; expect(all).toHaveLength(new Set(all).size); }); + +/** + * The round trip an account switch makes. It runs through localStorage because + * a keychain value cannot hold it: 2560 bytes on Windows, against a workspace + * snapshot and a command history that run to kilobytes. + */ +test("an account gets back the state it was parked with", () => { + const storage = fakeStorage({ ...ACCOUNT_STATE, ...DEVICE_STATE }); + + parkAccountUiState("acct-a", storage); + clearPersistedAccountUiState(storage); + restorePersistedAccountUiState({ "voltius-teams": "the other account's teams" }, storage); + clearPersistedAccountUiState(storage); + restoreAccountUiState("acct-a", storage); + + expect(storage.data).toMatchObject(ACCOUNT_STATE); + // Handed back, so nothing stays parked to go stale behind the live keys. + expect(storage.data["voltius.parked-ui-state.acct-a"]).toBeUndefined(); +}); + +test("parked state belongs to one account only", () => { + const storage = fakeStorage({ "voltius-vaults": "vaults of a" }); + parkAccountUiState("acct-a", storage); + clearPersistedAccountUiState(storage); + + restoreAccountUiState("acct-b", storage); + + expect(storage.data["voltius-vaults"]).toBeUndefined(); +}); + +test("signing an account out drops what it parked", () => { + const storage = fakeStorage({ "voltius-host-command-vars": "remembered secrets" }); + parkAccountUiState("acct-a", storage); + + dropAccountUiState("acct-a", storage); + + expect(storage.data["voltius.parked-ui-state.acct-a"]).toBeUndefined(); +}); + +test("an unparseable park restores nothing and clears itself", () => { + const storage = fakeStorage({}); + storage.data["voltius.parked-ui-state.acct-a"] = "{not json"; + + restoreAccountUiState("acct-a", storage); + + expect(storage.data).toEqual({}); +}); + +test("state migrated out of a keychain entry restores like any other park", () => { + const storage = fakeStorage({}); + writeParkedUiState("acct-a", { "voltius-vaults": "vaults from the old blob" }, storage); + + restoreAccountUiState("acct-a", storage); + + expect(storage.data["voltius-vaults"]).toBe("vaults from the old blob"); +}); diff --git a/src/stores/persistedAccountUiState.ts b/src/stores/persistedAccountUiState.ts index 1a8300896..3addb912d 100644 --- a/src/stores/persistedAccountUiState.ts +++ b/src/stores/persistedAccountUiState.ts @@ -103,3 +103,59 @@ export function restorePersistedAccountUiState( if (value !== undefined) storage.setItem(key, value); } } + +/** + * Where a signed-out account's state waits for it to come back. + * + * It stays in localStorage rather than riding along in the account's keychain + * entry: it is kilobytes (a workspace snapshot, command history, remembered + * snippet variables) and keychain values are capped — 2560 bytes on Windows — + * so parking it there failed, silently, and the account came back with no tabs. + * The same data sits in these very keys while the account is signed in, so + * holding it under a per-account name adds no exposure it did not already have; + * signing an account out drops its parked copy with the rest of its session. + */ +const PARKED_PREFIX = "voltius.parked-ui-state."; + +const parkedKey = (accountId: string) => `${PARKED_PREFIX}${accountId}`; + +export function writeParkedUiState( + accountId: string, + state: PersistedAccountUiState, + storage: PersistedAccountStorage | undefined = globalThis.localStorage, +): void { + if (!storage) return; + storage.setItem(parkedKey(accountId), JSON.stringify(state)); +} + +/** Park what the account is showing now, to be handed back on the way in. */ +export function parkAccountUiState( + accountId: string, + storage: PersistedAccountStorage | undefined = globalThis.localStorage, +): void { + if (!storage) return; + writeParkedUiState(accountId, snapshotPersistedAccountUiState(storage), storage); +} + +/** Hand an account its parked state back; the live keys hold it from here. */ +export function restoreAccountUiState( + accountId: string, + storage: PersistedAccountStorage | undefined = globalThis.localStorage, +): void { + if (!storage) return; + const raw = storage.getItem(parkedKey(accountId)); + if (!raw) return; + try { + restorePersistedAccountUiState(JSON.parse(raw) as PersistedAccountUiState, storage); + } catch { + // Unparseable park: nothing to restore, and dropping it below clears it. + } + storage.removeItem(parkedKey(accountId)); +} + +export function dropAccountUiState( + accountId: string, + storage: PersistedAccountStorage | undefined = globalThis.localStorage, +): void { + storage?.removeItem(parkedKey(accountId)); +} diff --git a/src/stores/workspaceRestore.test.ts b/src/stores/workspaceRestore.test.ts new file mode 100644 index 000000000..d4b459dd6 --- /dev/null +++ b/src/stores/workspaceRestore.test.ts @@ -0,0 +1,88 @@ +import { test, expect, vi, beforeEach, afterEach } from "vitest"; + +const h = vi.hoisted(() => ({ + reconnect: vi.fn(async () => undefined), + restoreSessions: vi.fn(), + removeSession: vi.fn(), + hydrate: vi.fn(), + snapshot: { + version: 1, + sessions: [{ id: "s1", connectionId: "c1", connectionName: "host", type: "ssh", persist: true }], + layout: { splitTabs: [], activeSplitTabId: null, splitTabActive: false, titlebarOrder: [] }, + activeSessionId: "s1", + }, +})); + +vi.mock("./workspaceSnapshotStore", () => ({ + readWorkspaceSnapshot: () => h.snapshot, + clearWorkspaceSnapshot: vi.fn(), + startWorkspaceSnapshotSync: vi.fn(), +})); +vi.mock("./toggleSettingsStore", () => ({ getToggle: () => true })); +vi.mock("./liveSessionManifestCore", () => ({ resolveRemoteSessions: () => ({ closedIds: [] }) })); +vi.mock("./crossDeviceSessionsStore", () => ({ + useCrossDeviceSessionsStore: { getState: () => ({ manifests: {}, tombstones: {} }) }, +})); +vi.mock("./sessionStore", () => ({ + useSessionStore: { + getState: () => ({ + sessions: [], + restoreSessions: h.restoreSessions, + removeSession: h.removeSession, + reconnect: h.reconnect, + markConnected: vi.fn(), + markError: vi.fn(), + }), + }, +})); +vi.mock("./layoutStore", () => ({ + useLayoutStore: { getState: () => ({ splitTabs: [], hydrate: h.hydrate, removeSession: h.removeSession }) }, + getPaneSessionIds: () => [], +})); +vi.mock("./uiStore", () => ({ + useUIStore: { getState: () => ({ setActiveNav: vi.fn(), setSidebarOpen: vi.fn() }) }, +})); +vi.mock("@/services/local", () => ({ localConnect: vi.fn(async () => undefined) })); +vi.mock("@/hooks/useTerminal", () => ({ setRestoreScrollOffset: vi.fn() })); + +const flush = () => new Promise((resolve) => setTimeout(resolve, 250)); + +beforeEach(() => { + vi.clearAllMocks(); + vi.resetModules(); // workspaceRestore is one-shot per module instance +}); + +afterEach(async () => { + const { resolveLoginSync } = await import("@/services/loginSyncGate"); + resolveLoginSync(); +}); + +/** + * A switch lands here with the config dir wiped: reconnecting before the cloud + * pull refills it finds no connection and errors every restored tab. + */ +test("reconnect waits for the replace-sync that refills the wiped config dir", async () => { + const { setLoginSyncPending, resolveLoginSync } = await import("@/services/loginSyncGate"); + const { restoreWorkspaceOnLaunch } = await import("./workspaceRestore"); + setLoginSyncPending({ replace: true }); + + const restored = restoreWorkspaceOnLaunch(); + await flush(); + + expect(h.restoreSessions).toHaveBeenCalled(); // tabs paint immediately + expect(h.reconnect).not.toHaveBeenCalled(); + + resolveLoginSync(); + await restored; + expect(h.reconnect).toHaveBeenCalledWith("s1", { restore: true }); +}); + +test("a normal launch reconnects without waiting on sync", async () => { + const { setLoginSyncPending } = await import("@/services/loginSyncGate"); + const { restoreWorkspaceOnLaunch } = await import("./workspaceRestore"); + setLoginSyncPending(); // merge-mode: the local cache is already on disk + + await restoreWorkspaceOnLaunch(); + + expect(h.reconnect).toHaveBeenCalledWith("s1", { restore: true }); +}); diff --git a/src/stores/workspaceRestore.ts b/src/stores/workspaceRestore.ts index fe5e40c35..35297beb1 100644 --- a/src/stores/workspaceRestore.ts +++ b/src/stores/workspaceRestore.ts @@ -10,6 +10,7 @@ import { useSessionStore } from "./sessionStore"; import { useLayoutStore, getPaneSessionIds, type SplitTab } from "./layoutStore"; import { useUIStore } from "./uiStore"; import { localConnect } from "@/services/local"; +import { isReplaceSyncPending, whenLoginSyncSettled } from "@/services/loginSyncGate"; import { setRestoreScrollOffset } from "@/hooks/useTerminal"; import type { SerialConnectParams, TerminalSession } from "@/types"; import type { SnapshotSession } from "./workspaceSnapshotCore"; @@ -90,7 +91,14 @@ export async function restoreWorkspaceOnLaunch(): Promise { await waitForTerminalMount(); startWorkspaceSnapshotSync(); - // 3. Reconnect everything in parallel. Persistent SSH re-attaches its tmux + // 3. An account switch reaches this point with a wiped config dir: the + // connections and secrets these sessions need are still on their way down + // from the cloud. Reconnecting now would find no connection at all and error + // every restored tab, so wait for the pull that refills them. A normal launch + // reads its cache from disk and never waits here. + if (isReplaceSyncPending()) await whenLoginSyncSettled(); + + // 4. Reconnect everything in parallel. Persistent SSH re-attaches its tmux // (same session id → same key) and replays history (restore flag). Vault // unlock happens lazily inside credential resolution; failures land in the // existing per-session error overlay (retry affordances included).