fix: harden audit durability and fallback config writes
This commit is contained in:
@@ -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) {
|
saveRendererGlobalSettings(globalSettings) {
|
||||||
const snapshot = this._clone(globalSettings || {});
|
const snapshot = this._clone(globalSettings || {});
|
||||||
return this._enqueueWrite(() => {
|
return this._enqueueWrite(() => {
|
||||||
|
|||||||
+22
-2
@@ -6,6 +6,26 @@ function getUploadAuditLogPath(uploadLogPath, pathApi = nodePath) {
|
|||||||
return pathApi.join(pathApi.dirname(uploadLogPath), 'upload-audit.log');
|
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) {
|
function createUploadAuditWriter(options) {
|
||||||
const source = options && typeof options === 'object' ? options : {};
|
const source = options && typeof options === 'object' ? options : {};
|
||||||
const fs = source.fs;
|
const fs = source.fs;
|
||||||
@@ -20,7 +40,7 @@ function createUploadAuditWriter(options) {
|
|||||||
const maxBackups = Number.isFinite(source.maxBackups) ? source.maxBackups : 2;
|
const maxBackups = Number.isFinite(source.maxBackups) ? source.maxBackups : 2;
|
||||||
let activePath = null;
|
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');
|
throw new TypeError('createUploadAuditWriter requires fs and resolveUploadLogTarget');
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -49,7 +69,7 @@ function createUploadAuditWriter(options) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
rotateLogFile(targetPath, maxBytes, maxBackups);
|
rotateLogFile(targetPath, maxBytes, maxBackups);
|
||||||
await fs.promises.appendFile(targetPath, line, 'utf-8');
|
await appendDurably(fs, targetPath, line);
|
||||||
activePath = targetPath;
|
activePath = targetPath;
|
||||||
return true;
|
return true;
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
|
|||||||
@@ -513,6 +513,51 @@ describe('ConfigStore', () => {
|
|||||||
assert.equal(store.load().globalSettings.lastBrowseDirectory, selectedDirectory);
|
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 () => {
|
it('merges remote settings in the write queue and returns the canonical token', async () => {
|
||||||
await store.save({
|
await store.save({
|
||||||
globalSettings: {
|
globalSettings: {
|
||||||
|
|||||||
@@ -69,6 +69,103 @@ test('audit writer reports the actual fallback file after a failed primary write
|
|||||||
fs.rmSync(directory, { recursive: true, force: true });
|
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) => {
|
test('audit writer rejects false and thrown fallback persistence before trying the next safe target', async (t) => {
|
||||||
const { createUploadAuditWriter } = require('../lib/upload-audit');
|
const { createUploadAuditWriter } = require('../lib/upload-audit');
|
||||||
for (const rejection of ['false', 'throw']) {
|
for (const rejection of ['false', 'throw']) {
|
||||||
|
|||||||
Reference in New Issue
Block a user