From 7a40afbe7eeb10656459645f5de8383f486e16a3 Mon Sep 17 00:00:00 2001 From: Administrator Date: Mon, 22 Jun 2026 03:53:01 +0200 Subject: [PATCH] fix(config): fsync config writes + guard against account-wipe on a corrupt read (v3.3.105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user's server crashed hard during an upload and lost all configured accounts. It was NOT the v3.3.104 update (a second server updated fine and kept its accounts) — it was a data-durability hole exposed by the crash: - Config writes were atomic (tmp + rename) but never fsync'd, so a hard crash could leave electron-config.json truncated/unflushed on disk. - On restart, load() reads the truncated file, falls back to .bak, and if that is also bad returns empty DEFAULTS. The next settings/queue save then persists EMPTY hosters — permanently wiping the accounts. Worse, the async _atomicWrite blindly copied the (now truncated) live file over .bak, so an empty live could clobber a good backup. Hardening (lib/config-store.js + main.js; no behavior change in the happy path): - fsync before rename in both write paths — _atomicWrite (openSync/writeSync/ fsyncSync/closeSync) and the synchronous save-global-settings-sync on window close. A hard crash can no longer leave a truncated config. - _atomicWrite only refreshes .bak when the current live file is non-trivial (trim length > 2), so an empty/truncated live can never overwrite a good backup (the sync-save path already did this). - Wipe-guard (_guardHosters): save(), saveRotationCursors() and the sync close-save never intend to change hosters; if after a load() the hosters are all empty and the write did not explicitly provide hosters, recover them from disk (_recoverHostersFromDisk: live -> .bak -> .pre-history-split.bak) instead of persisting the wipe. An explicit save({hosters: {}}) (user deleted all accounts) is still allowed. Restored hosters are already-encrypted on disk and encryptCredentials skips already-encrypted fields, so re-serializing is safe. - load() gained a third fallback tier — the permanent pre-history-split.bak snapshot (which still holds the accounts) — so load() itself recovers after corruption. Recovery for the already-affected server: copy %APPDATA%/multi-hoster-uploader/electron-config.json.pre-history-split.bak (or .bak) over electron-config.json with the app closed. 2 new regression tests (post-wipe valid-empty live + .bak → guard restores accounts; an explicit empty-hosters save is not blocked). 409 tests pass; clean boot. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/config-store.js | 58 ++++++++++++++++++++++++++++++-------- main.js | 4 ++- package.json | 2 +- tasks/todo.md | 31 ++++++++++++++++++++ tests/config-store.test.js | 21 ++++++++++++++ 5 files changed, 103 insertions(+), 13 deletions(-) diff --git a/lib/config-store.js b/lib/config-store.js index f58bc13..566372f 100644 --- a/lib/config-store.js +++ b/lib/config-store.js @@ -357,8 +357,10 @@ class ConfigStore { try { data = this._readAndParse(this.filePath); } catch {} // Fallback to backup if main is empty/corrupt if (!data) { - const backupPath = this.filePath + '.bak'; - try { data = this._readAndParse(backupPath); } catch {} + try { data = this._readAndParse(this.filePath + '.bak'); } catch {} + } + if (!data) { + try { data = this._readAndParse(this.filePath + '.pre-history-split.bak'); } catch {} } if (!data) { const fresh = JSON.parse(JSON.stringify(DEFAULTS)); @@ -481,12 +483,41 @@ class ConfigStore { return this._writeQueue; } + _anyHosters(cfg) { + const h = cfg && cfg.hosters; + return !!h && typeof h === 'object' && Object.values(h).some(a => Array.isArray(a) && a.length > 0); + } + + _recoverHostersFromDisk() { + for (const p of [this.filePath, this.filePath + '.bak', this.filePath + '.pre-history-split.bak']) { + try { + const raw = fs.readFileSync(p, 'utf-8'); + if (!raw || raw.trim().length < 2) continue; + const data = JSON.parse(raw); + if (this._anyHosters(data)) return data.hosters; + } catch {} + } + return null; + } + + _guardHosters(current, hostersIntentional) { + if (!hostersIntentional && !this._anyHosters(current)) { + const recovered = this._recoverHostersFromDisk(); + if (recovered) { + current.hosters = recovered; + if (this._perfLog) this._perfLog('config-guard: prevented account wipe — restored hosters from on-disk backup after a corrupt/empty read'); + } + } + return current; + } + save(config) { return this._enqueueWrite(() => { const current = this.load(); if (config.hosters) current.hosters = config.hosters; if (config.hosterSettings) current.hosterSettings = config.hosterSettings; if (config.globalSettings) current.globalSettings = config.globalSettings; + this._guardHosters(current, !!config.hosters); return this._commit(current); }); } @@ -503,18 +534,22 @@ class ConfigStore { return new Promise((resolve, reject) => { const tmpPath = this.filePath + '.tmp'; const backupPath = this.filePath + '.bak'; - fs.writeFile(tmpPath, data, 'utf-8', (err) => { - if (err) return reject(err); + let fd; + try { + fd = fs.openSync(tmpPath, 'w'); + fs.writeSync(fd, data); + fs.fsyncSync(fd); + } catch (e) { + try { if (fd !== undefined) fs.closeSync(fd); } catch {} + return reject(e); + } + try { fs.closeSync(fd); } catch {} + Promise.resolve().then(() => { try { - // Refresh .bak from the previous live file with a raw byte copy — - // no read+JSON.parse+write. The live file was itself written through - // this atomic path, so re-validating it by parsing the whole (growing) - // config on every write was pure waste. Wrapped in try/catch so an - // AV/indexer briefly locking the file doesn't fail the save — the - // rename to the live path is the part that matters. try { if (fs.existsSync(this.filePath)) { - fs.copyFileSync(this.filePath, backupPath); + const cur = fs.readFileSync(this.filePath, 'utf-8'); + if (cur && cur.trim().length > 2) fs.writeFileSync(backupPath, cur, 'utf-8'); } } catch {} fs.renameSync(tmpPath, this.filePath); @@ -604,6 +639,7 @@ class ConfigStore { return this._enqueueWrite(() => { const config = this.load(); config.rotationCursors = (cursors && typeof cursors === 'object' && !Array.isArray(cursors)) ? cursors : {}; + this._guardHosters(config, false); return this._commit(config); }); } diff --git a/main.js b/main.js index 77839b9..a0768fe 100644 --- a/main.js +++ b/main.js @@ -2476,10 +2476,12 @@ ipcMain.on('save-global-settings-sync', (event, globalSettings) => { const _diskDiag = current.globalSettings && current.globalSettings.diagnostics; current.globalSettings = globalSettings; if (_diskDiag) current.globalSettings.diagnostics = _diskDiag; + try { configStore._guardHosters(current, false); } catch {} _invalidateLogSettings(); const data = configStore._serializeForDisk(current); const backupPath = configStore.filePath + '.bak'; - fs.writeFileSync(tmpPath, data, 'utf-8'); + const _fd = fs.openSync(tmpPath, 'w'); + try { fs.writeSync(_fd, data); fs.fsyncSync(_fd); } finally { fs.closeSync(_fd); } if (fs.existsSync(configStore.filePath)) { // Use try/catch around the read so an AV/lock race doesn't fail the // whole save just because we couldn't refresh the .bak — the write to diff --git a/package.json b/package.json index 1131d91..0c4f2f0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "multi-hoster-uploader", - "version": "3.3.104", + "version": "3.3.105", "description": "Upload files to doodstream, voe, vidmoly, byse simultaneously", "main": "main.js", "scripts": { diff --git a/tasks/todo.md b/tasks/todo.md index 41ab0f7..72345bb 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -1,3 +1,34 @@ +# v3.3.105 — URGENT data-safety: config write fsync + account-wipe guard + +User report: after a server CRASHED during upload (NOT the v3.3.104 update — the other server updated fine and +kept its accounts), the accounts/credentials were gone. Root-cause chain (in code): config writes were atomic +(tmp+rename) but had NO fsync — a hard crash can leave electron-config.json truncated/unflushed → on restart +load() reads the corrupt/empty file, falls to .bak, and if that's also bad returns empty DEFAULTS → the next +settings/queue save persists EMPTY hosters → accounts permanently wiped (and the async _atomicWrite blindly +copyFileSync'd the live → .bak, so an empty live could clobber a good .bak). + +SHIPPED v3.3.105 (lib/config-store.js + main.js — data-safety, no behavior change): +- fsync before rename in BOTH write paths: _atomicWrite (openSync+writeSync+fsyncSync+closeSync, then + guarded-.bak + rename) and main.js save-global-settings-sync (openSync+writeSync+fsyncSync+closeSync). A hard + crash can no longer leave a truncated config. +- _atomicWrite .bak is now GUARDED: read the live file and only refresh .bak if it's non-trivial (trim>2) — + an empty/truncated live can never clobber a good .bak (matches what the sync-save already did). +- WIPE-GUARD (_guardHosters): in save()/saveRotationCursors()/the sync-save, when the write does NOT + intentionally set hosters (config.hosters absent) AND the resulting hosters are all-empty, recover the + hosters from disk (_recoverHostersFromDisk tries live → .bak → .pre-history-split.bak) instead of persisting + the wipe. An EXPLICIT save({hosters:{}}) (user deleted all) is still allowed (hostersIntentional=true). +- load() gained a 3rd fallback tier: .pre-history-split.bak (the permanent v3.3.99 snapshot with accounts) so + load() itself recovers after corruption. +2 new tests (post-wipe valid-empty live + .bak → guard restores; explicit empty NOT blocked). 409 tests pass. +RECOVERY for the affected server: %APPDATA%\multi-hoster-uploader\electron-config.json.pre-history-split.bak +(or .bak) → copy over electron-config.json with the app closed. + +DEFERRED (perf polish, workflow w5o2rpffx designs ready): history JSONL (append-only, kills per-batch 185MB +rewrite + first-open parse via loadHistoryRecent tail-read + meta sidecar) and the batch-start one-time +render/231ms residual. Do these AFTER the data-safety fix is confirmed stable. + +--- + # v3.3.104 — virtualize the Recent-uploads panel (the last non-virtual table) v3.3.103 log (real 2464-job, 4-hoster batch → 95 concurrent): the statSync fix HELD (batch-start main spike diff --git a/tests/config-store.test.js b/tests/config-store.test.js index 6bd9616..cf9c5ef 100644 --- a/tests/config-store.test.js +++ b/tests/config-store.test.js @@ -293,6 +293,27 @@ describe('ConfigStore', () => { const config = store.load(); assert.equal(config.hosters['doodstream.com'][0].apiKey, 'from-backup'); }); + + it('wipe-guard: a settings-only save recovers accounts from .bak when the live config validly has none', async () => { + // Post-wipe state: live config parses fine but has empty hosters; a backup still holds the accounts. + fs.writeFileSync(store.filePath, JSON.stringify({ hosters: {}, hosterSettings: {}, globalSettings: {}, history: [] }), 'utf-8'); + fs.writeFileSync(store.filePath + '.bak', JSON.stringify({ + hosters: { 'voe.sx': [{ id: 'v1', authType: 'api', apiKey: 'survive-key' }] }, + hosterSettings: {}, globalSettings: {}, history: [] + }), 'utf-8'); + await store.save({ globalSettings: { alwaysOnTop: true } }); + const cfg = store.load(); + assert.ok(cfg.hosters['voe.sx'] && cfg.hosters['voe.sx'].length === 1, 'guard must restore accounts from .bak, not persist the wipe'); + assert.equal(cfg.hosters['voe.sx'][0].apiKey, 'survive-key'); + assert.equal(cfg.globalSettings.alwaysOnTop, true); + }); + + it('wipe-guard: an explicit save({hosters:{}}) (user deleted all) is NOT blocked', async () => { + await store.save({ hosters: { 'doodstream.com': [{ id: 'd1', authType: 'api', apiKey: 'k' }] } }); + await store.save({ hosters: {} }); + const cfg = store.load(); + assert.equal((cfg.hosters['doodstream.com'] || []).length, 0, 'an intentional hosters write must be allowed to empty them'); + }); }); describe('ConfigStore history split (electron-history.json)', () => {