From f9e1d2f0738d5903987b6f0cf2283c4814e92912 Mon Sep 17 00:00:00 2001 From: byGalax Date: Tue, 12 May 2026 21:54:14 +0200 Subject: [PATCH] fix(secure-store): preserve original ciphertext on decrypt failure + startup path log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Critical hotfix for the 0.17.0 regression: users upgrading from 0.16.x were logged out, and their next login wrote a fresh empty secure-store on top of the original ciphertext — destroying device keys irrecoverably. Why it happened: loadState used a blanket `catch {}` that conflated "file doesn't exist (genuine new user)" with "file exists but can't be decrypted (DPAPI / OSCrypt quirk after the install rename)". Both paths returned an empty Map; the next scheduledSave then overwrote the original .bin file with a fresh blob. Fix: * Separate ENOENT from decrypt/parse failures. ENOENT → empty Map. Any other read error → log, empty Map (no quarantine, matches old behaviour for transient lock issues). * When decrypt/parse fails the original file is renamed to .broken- BEFORE returning empty Map. The next save writes to a fresh file; the original ciphertext is preserved on disk so a future build (or manual recovery) can still get at the bytes. * Loud console.error around the failure so future regressions surface in main-process logs. main.ts: move setPath('userData', appData/ChatApp) BEFORE setName so any productName-derived path caching inside setName can't beat us to it. Add a startup log of the resolved paths so future debugging has hard evidence instead of guessing. Affected users on 0.17.0 should still recover via Settings → Backup Wiederherstellen (account-level keys are unchanged); this fix prevents the data destruction for anyone who hasn't upgraded yet. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/desktop/electron/main.ts | 38 ++++++++--- apps/desktop/electron/modules/secure-store.ts | 64 ++++++++++++++++--- 2 files changed, 84 insertions(+), 18 deletions(-) diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 9a24c4f..c4caeef 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -37,23 +37,26 @@ const __dirnameSafe = path.dirname(__filenameSafe); const DEV_URL = 'http://localhost:1420'; const WINDOW_STATE_FILE = 'window-state.json'; +// Pin userData FIRST — before any other Electron call that might cache a +// productName-derived path. The 0.17.0 release saw users get logged out +// after upgrading from 0.16.x: the most likely culprit was an internal +// path resolution kicking off the moment `setName('Netralax')` ran, so +// 0.17.1 swaps the order so the explicit override wins regardless of +// what setName triggers internally. The literal 'ChatApp' here is the +// pre-rename product folder — installed users' SQLite, secrets, sounds, +// IndexedDB all live there and we never want to leave them stranded by +// a future rebrand. +app.setPath('userData', path.join(app.getPath('appData'), 'ChatApp')); + // App branding. productName in package.json drives the packaged exe name // (Netralax.exe) and electron-builder installer title. setName + the -// AppUserModelId below cover the live process: window title fallback, -// Windows taskbar grouping, notification source attribution. +// AppUserModelId cover the live process: window title fallback, Windows +// taskbar grouping, notification source attribution. app.setName('Netralax'); if (process.platform === 'win32') { app.setAppUserModelId('cloud.netralax.desktop'); } -// Pin userData to %APPDATA%\ChatApp regardless of productName so existing -// installs keep their profile, sounds, secrets, SQLite. Electron's default -// is %APPDATA%\, which after the Netralax rename would point -// at an empty fresh dir — same painful migration as the Tauri → Electron -// cut. Anchored to `appData` (the platform-AppData root) so productName -// changes can't drag it. -app.setPath('userData', path.join(app.getPath('appData'), 'ChatApp')); - // Run dev side-by-side with the installed packaged build by isolating the // renderer profile / secret-store / SQLite / IndexedDB / localStorage in // a separate userData dir. Without this both share `%APPDATA%\ChatApp`, @@ -64,6 +67,21 @@ if (!app.isPackaged) { app.setPath('userData', app.getPath('userData') + '-Dev'); } +// Startup diagnostics — the 0.17.0 logout regression was hard to debug +// because we had no record of the actual resolved paths. With this log +// any future user can paste their main-process output and we can tell +// at a glance whether userData ended up where we intended. +console.log( + '[main] resolved paths', + JSON.stringify({ + appName: app.getName(), + appData: app.getPath('appData'), + userData: app.getPath('userData'), + isPackaged: app.isPackaged, + platform: process.platform, + }), +); + let mainWindow: BrowserWindow | null = null; function resolvePreloadPath(): string { diff --git a/apps/desktop/electron/modules/secure-store.ts b/apps/desktop/electron/modules/secure-store.ts index 2df216d..4838374 100644 --- a/apps/desktop/electron/modules/secure-store.ts +++ b/apps/desktop/electron/modules/secure-store.ts @@ -40,20 +40,68 @@ function filePathFor(userId: string, encrypted: boolean): string { } async function loadState(filePath: string, encrypted: boolean): Promise> { + // Read step. Distinguish "no file yet" (genuinely new user — empty Map + // is correct) from "file exists but unreadable" (corruption / DPAPI + // breakage — we MUST NOT let the next write overwrite those bytes, + // because the original ciphertext is the only path back to the user's + // device keys if a future build can fix the read path). + let rawBuf: Buffer | null = null; + let rawStr: string | null = null; try { if (encrypted) { - const buf = await fs.readFile(filePath); - const json = safeStorage.decryptString(buf); + rawBuf = await fs.readFile(filePath); + } else { + rawStr = await fs.readFile(filePath, 'utf8'); + } + } catch (err: unknown) { + const code = (err as NodeJS.ErrnoException | null)?.code; + if (code === 'ENOENT') return new Map(); + console.warn('[secure-store] read failed (non-ENOENT)', filePath, err); + // For non-ENOENT read failures (EACCES, EBUSY, …) don't quarantine — + // the file might be transiently locked. Empty map + future writes + // will attempt to overwrite, matching the pre-0.17.1 behaviour for + // these rarer cases. + return new Map(); + } + + // Parse / decrypt step. + try { + if (encrypted && rawBuf) { + const json = safeStorage.decryptString(rawBuf); const parsed = JSON.parse(json) as { version?: number; entries?: Record }; return new Map(Object.entries(parsed.entries ?? {})); - } else { - const raw = await fs.readFile(filePath, 'utf8'); - const parsed = JSON.parse(raw) as { version?: number; entries?: Record }; + } + if (rawStr) { + const parsed = JSON.parse(rawStr) as { version?: number; entries?: Record }; return new Map(Object.entries(parsed.entries ?? {})); } - } catch { - // Missing file or malformed contents — start fresh. The next write - // will overwrite with a fresh blob. + return new Map(); + } catch (err: unknown) { + // CRITICAL: file existed but we couldn't decrypt or parse it. In the + // pre-0.17.1 build we silently started fresh — the next set() then + // scheduledSave() over-wrote the original ciphertext, destroying the + // user's device keys forever. Now we rename the original to + // `.broken-` BEFORE returning the empty map so the next + // write goes to a new file and the original bytes survive for + // forensics or a future decrypt-recovery path. + const ts = new Date().toISOString().replace(/[:.]/g, '-'); + const brokenPath = `${filePath}.broken-${ts}`; + try { + await fs.rename(filePath, brokenPath); + console.error( + `[secure-store] DECRYPT/PARSE FAILED for ${filePath} — preserved original at ${brokenPath}. Original error:`, + err, + ); + } catch (renameErr: unknown) { + // Even rename failed — fall back to the old behaviour (silent empty + // map) but log loudly so it's visible in the main-process output. + console.error( + '[secure-store] rename of broken file failed; original may be overwritten on next save', + renameErr, + 'original decrypt error:', + err, + ); + } return new Map(); } }