fix(secure-store): preserve original ciphertext on decrypt failure + startup path log
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
<file>.broken-<iso-ts> 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) <noreply@anthropic.com>
This commit is contained in:
@@ -37,23 +37,26 @@ const __dirnameSafe = path.dirname(__filenameSafe);
|
|||||||
const DEV_URL = 'http://localhost:1420';
|
const DEV_URL = 'http://localhost:1420';
|
||||||
const WINDOW_STATE_FILE = 'window-state.json';
|
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
|
// App branding. productName in package.json drives the packaged exe name
|
||||||
// (Netralax.exe) and electron-builder installer title. setName + the
|
// (Netralax.exe) and electron-builder installer title. setName + the
|
||||||
// AppUserModelId below cover the live process: window title fallback,
|
// AppUserModelId cover the live process: window title fallback, Windows
|
||||||
// Windows taskbar grouping, notification source attribution.
|
// taskbar grouping, notification source attribution.
|
||||||
app.setName('Netralax');
|
app.setName('Netralax');
|
||||||
if (process.platform === 'win32') {
|
if (process.platform === 'win32') {
|
||||||
app.setAppUserModelId('cloud.netralax.desktop');
|
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%\<productName>, 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
|
// Run dev side-by-side with the installed packaged build by isolating the
|
||||||
// renderer profile / secret-store / SQLite / IndexedDB / localStorage in
|
// renderer profile / secret-store / SQLite / IndexedDB / localStorage in
|
||||||
// a separate userData dir. Without this both share `%APPDATA%\ChatApp`,
|
// a separate userData dir. Without this both share `%APPDATA%\ChatApp`,
|
||||||
@@ -64,6 +67,21 @@ if (!app.isPackaged) {
|
|||||||
app.setPath('userData', app.getPath('userData') + '-Dev');
|
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;
|
let mainWindow: BrowserWindow | null = null;
|
||||||
|
|
||||||
function resolvePreloadPath(): string {
|
function resolvePreloadPath(): string {
|
||||||
|
|||||||
@@ -40,20 +40,68 @@ function filePathFor(userId: string, encrypted: boolean): string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async function loadState(filePath: string, encrypted: boolean): Promise<Map<string, string>> {
|
async function loadState(filePath: string, encrypted: boolean): Promise<Map<string, string>> {
|
||||||
|
// 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 {
|
try {
|
||||||
if (encrypted) {
|
if (encrypted) {
|
||||||
const buf = await fs.readFile(filePath);
|
rawBuf = await fs.readFile(filePath);
|
||||||
const json = safeStorage.decryptString(buf);
|
} 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<string, string> };
|
const parsed = JSON.parse(json) as { version?: number; entries?: Record<string, string> };
|
||||||
return new Map(Object.entries(parsed.entries ?? {}));
|
return new Map(Object.entries(parsed.entries ?? {}));
|
||||||
} else {
|
}
|
||||||
const raw = await fs.readFile(filePath, 'utf8');
|
if (rawStr) {
|
||||||
const parsed = JSON.parse(raw) as { version?: number; entries?: Record<string, string> };
|
const parsed = JSON.parse(rawStr) as { version?: number; entries?: Record<string, string> };
|
||||||
return new Map(Object.entries(parsed.entries ?? {}));
|
return new Map(Object.entries(parsed.entries ?? {}));
|
||||||
}
|
}
|
||||||
} catch {
|
return new Map();
|
||||||
// Missing file or malformed contents — start fresh. The next write
|
} catch (err: unknown) {
|
||||||
// will overwrite with a fresh blob.
|
// 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
|
||||||
|
// `<file>.broken-<iso-ts>` 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();
|
return new Map();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user