From 76ad81a0d319ecaa6e5e510f85c7a37cdb57bd36 Mon Sep 17 00:00:00 2001 From: Sucukdeluxe <259325684+Sucukdeluxe@users.noreply.github.com> Date: Thu, 13 Aug 2026 21:07:28 +0200 Subject: [PATCH] fix: harden audit durability and fallback config writes --- lib/config-store.js | 13 +++++ lib/upload-audit.js | 24 +++++++++- tests/config-store.test.js | 45 ++++++++++++++++++ tests/upload-audit.test.js | 97 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 177 insertions(+), 2 deletions(-) diff --git a/lib/config-store.js b/lib/config-store.js index 8831d4a..67593ee 100644 --- a/lib/config-store.js +++ b/lib/config-store.js @@ -611,6 +611,19 @@ class ConfigStore { }); } + saveFallbackLogPath(logFilePath) { + const snapshot = String(logFilePath || '').trim(); + return this._enqueueWrite(() => { + const current = this.load(); + current.globalSettings = { + ...(current.globalSettings || {}), + logFilePath: snapshot + }; + this._guardHosters(current, false); + return this._commit(current); + }); + } + saveRendererGlobalSettings(globalSettings) { const snapshot = this._clone(globalSettings || {}); return this._enqueueWrite(() => { diff --git a/lib/upload-audit.js b/lib/upload-audit.js index 3819f46..9fa1f6c 100644 --- a/lib/upload-audit.js +++ b/lib/upload-audit.js @@ -6,6 +6,26 @@ function getUploadAuditLogPath(uploadLogPath, pathApi = nodePath) { return pathApi.join(pathApi.dirname(uploadLogPath), 'upload-audit.log'); } +async function appendDurably(fs, targetPath, line) { + const handle = await fs.promises.open(targetPath, 'a'); + let appendError = null; + let closeError = null; + try { + await handle.appendFile(line, 'utf-8'); + await handle.sync(); + } catch (error) { + appendError = error; + } + try { + await handle.close(); + } catch (error) { + closeError = error; + } + if (appendError && closeError) throw new AggregateError([appendError, closeError], 'Audit append and close failed'); + if (appendError) throw appendError; + if (closeError) throw closeError; +} + function createUploadAuditWriter(options) { const source = options && typeof options === 'object' ? options : {}; const fs = source.fs; @@ -20,7 +40,7 @@ function createUploadAuditWriter(options) { const maxBackups = Number.isFinite(source.maxBackups) ? source.maxBackups : 2; let activePath = null; - if (!fs || !fs.promises || typeof fs.promises.appendFile !== 'function' || typeof resolveUploadLogTarget !== 'function') { + if (!fs || !fs.promises || typeof fs.promises.open !== 'function' || typeof resolveUploadLogTarget !== 'function') { throw new TypeError('createUploadAuditWriter requires fs and resolveUploadLogTarget'); } @@ -49,7 +69,7 @@ function createUploadAuditWriter(options) { } } rotateLogFile(targetPath, maxBytes, maxBackups); - await fs.promises.appendFile(targetPath, line, 'utf-8'); + await appendDurably(fs, targetPath, line); activePath = targetPath; return true; } catch (error) { diff --git a/tests/config-store.test.js b/tests/config-store.test.js index 0f4c97d..07bb5b4 100644 --- a/tests/config-store.test.js +++ b/tests/config-store.test.js @@ -513,6 +513,51 @@ describe('ConfigStore', () => { assert.equal(store.load().globalSettings.lastBrowseDirectory, selectedDirectory); }); + it('merges the fallback log path after earlier queued settings without reverting them', async () => { + assert.equal(typeof store.saveFallbackLogPath, 'function'); + await store.save({ + globalSettings: { + alwaysOnTop: false, + webhookUrl: 'https://before.invalid', + logFilePath: '' + } + }); + + const originalAtomicWrite = store._atomicWrite.bind(store); + let releaseSettingsWrite; + let signalSettingsWriteStarted; + const settingsWriteStarted = new Promise(resolve => { signalSettingsWriteStarted = resolve; }); + store._atomicWrite = (data) => { + const settings = JSON.parse(data).globalSettings; + if (!releaseSettingsWrite && settings.webhookUrl === 'https://concurrent.invalid') { + signalSettingsWriteStarted(); + return new Promise((resolve, reject) => { + releaseSettingsWrite = () => originalAtomicWrite(data).then(resolve, reject); + }); + } + return originalAtomicWrite(data); + }; + + const current = store.load(); + const settingsSave = store.save({ + globalSettings: { + ...current.globalSettings, + alwaysOnTop: true, + webhookUrl: 'https://concurrent.invalid' + } + }); + await settingsWriteStarted; + const fallbackPath = path.join(tmpDir, 'fallback', 'fileuploader.log'); + const fallbackSave = store.saveFallbackLogPath(fallbackPath); + releaseSettingsWrite(); + await Promise.all([settingsSave, fallbackSave]); + + const saved = store.load().globalSettings; + assert.equal(saved.alwaysOnTop, true); + assert.equal(saved.webhookUrl, 'https://concurrent.invalid'); + assert.equal(saved.logFilePath, fallbackPath); + }); + it('merges remote settings in the write queue and returns the canonical token', async () => { await store.save({ globalSettings: { diff --git a/tests/upload-audit.test.js b/tests/upload-audit.test.js index 2a12a50..14c69a0 100644 --- a/tests/upload-audit.test.js +++ b/tests/upload-audit.test.js @@ -69,6 +69,103 @@ test('audit writer reports the actual fallback file after a failed primary write fs.rmSync(directory, { recursive: true, force: true }); }); +test('audit writer retries a safe target after sync or close durability failures', async (t) => { + const { createUploadAuditWriter } = require('../lib/upload-audit'); + for (const failedStage of ['sync', 'close']) { + await t.test(failedStage, async () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), `mhu-upload-audit-${failedStage}-`)); + const primaryLog = path.join(directory, 'primary', 'fileuploader.log'); + const fallbackLog = path.join(directory, 'fallback', 'fileuploader.log'); + const primaryAudit = path.join(directory, 'primary', 'upload-audit.log'); + const fallbackAudit = path.join(directory, 'fallback', 'upload-audit.log'); + const syncCalls = []; + const closeCalls = []; + const reports = []; + const durabilityFs = { + ...fs, + promises: { + ...fs.promises, + open: async (targetPath, flags) => { + const handle = await fs.promises.open(targetPath, flags); + return { + appendFile: handle.appendFile.bind(handle), + sync: async () => { + syncCalls.push(targetPath); + if (targetPath === primaryAudit && failedStage === 'sync') throw new Error('controlled sync failure'); + return handle.sync(); + }, + close: async () => { + closeCalls.push(targetPath); + await handle.close(); + if (targetPath === primaryAudit && failedStage === 'close') throw new Error('controlled close failure'); + } + }; + } + } + }; + const writer = createUploadAuditWriter({ + fs: durabilityFs, + path, + resolveUploadLogTarget: excluded => { + if (!excluded.has(primaryLog)) return { path: primaryLog, isFallback: false }; + if (!excluded.has(fallbackLog)) return { path: fallbackLog, isFallback: true }; + return null; + }, + rotateLogFile: () => {}, + invalidateUploadLogTarget: () => {}, + persistFallbackLogPath: async () => true, + reportError: (label, error) => reports.push({ label, message: error.message }), + retryDelays: [0, 0] + }); + + assert.equal(await writer.append('# UPLOAD-PLAN {}\r\n', 'upload-plan'), true); + assert.equal(writer.getActivePath(), fallbackAudit); + assert.deepEqual(syncCalls, [primaryAudit, fallbackAudit]); + assert.deepEqual(closeCalls, [primaryAudit, fallbackAudit]); + assert.equal(fs.readFileSync(fallbackAudit, 'utf8'), '# UPLOAD-PLAN {}\r\n'); + assert.equal(reports.some(report => report.label === 'upload-plan' && report.message.includes(failedStage)), true); + fs.rmSync(directory, { recursive: true, force: true }); + }); + } +}); + +test('audit writer fails closed when no target can sync the appended bytes', async () => { + const { createUploadAuditWriter } = require('../lib/upload-audit'); + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'mhu-upload-audit-sync-exhausted-')); + const targets = ['first', 'second'].map(name => path.join(directory, name, 'fileuploader.log')); + const durabilityFs = { + ...fs, + promises: { + ...fs.promises, + open: async (targetPath, flags) => { + const handle = await fs.promises.open(targetPath, flags); + return { + appendFile: handle.appendFile.bind(handle), + sync: async () => { throw new Error(`controlled sync failure: ${targetPath}`); }, + close: handle.close.bind(handle) + }; + } + } + }; + const writer = createUploadAuditWriter({ + fs: durabilityFs, + path, + resolveUploadLogTarget: excluded => { + const targetPath = targets.find(candidate => !excluded.has(candidate)); + return targetPath ? { path: targetPath, isFallback: true } : null; + }, + rotateLogFile: () => {}, + invalidateUploadLogTarget: () => {}, + persistFallbackLogPath: async () => true, + reportError: () => {}, + retryDelays: [0, 0] + }); + + assert.equal(await writer.append('# UPLOAD-PLAN {}\r\n', 'upload-plan'), false); + assert.equal(writer.getActivePath(), null); + fs.rmSync(directory, { recursive: true, force: true }); +}); + test('audit writer rejects false and thrown fallback persistence before trying the next safe target', async (t) => { const { createUploadAuditWriter } = require('../lib/upload-audit'); for (const rejection of ['false', 'throw']) {