Fail closed on diagnostic config and history reads

Diagnostics previously reused the recovery-oriented ConfigStore readers. A missing, unreadable, or corrupt primary config could therefore be replaced by cached, backup, or default data, leaving diagnostics without a trustworthy decrypted secret set. Dedicated history failures and invalid payloads could likewise become a healthy empty result or stale config history.

Add explicit diagnostic config and history reader contracts. The config path bypasses caches and recovery fallbacks, validates the primary document, decrypts its current credentials, and propagates read, parse, validation, and decryption failures. The history path accepts a valid empty array, rejects unreadable or malformed dedicated data, and uses strict legacy config history only when no dedicated file exists. Keep the normal UI recovery readers unchanged and wire diagnostics to the strict contracts.

Cover primary recovery isolation, decryption failure propagation, valid empty history, corrupt and unreadable history, stale-history fallback prevention, pre-migration compatibility, and main-process wiring. Existing collector tests continue to prove successful responses, response-boundary redaction, shared history semantics, and snapshot non-mutation.
This commit is contained in:
Sucukdeluxe
2026-08-13 23:10:05 +02:00
parent c78160a521
commit f95e68bdb3
4 changed files with 135 additions and 15 deletions
+41 -4
View File
@@ -224,6 +224,15 @@ class ConfigStore {
} }
} }
_readHistoryFileStrict() {
const raw = fs.readFileSync(this.historyPath, 'utf-8');
if (!raw || raw.trim().length < 2) throw new Error('Die Diagnoseverlaufsdatei ist ungültig');
const parsed = JSON.parse(raw);
if (Array.isArray(parsed)) return parsed;
if (parsed && Array.isArray(parsed.history)) return parsed.history;
throw new Error('Die Diagnoseverlaufsdatei ist ungültig');
}
_writeHistoryFileDurable(arr) { _writeHistoryFileDurable(arr) {
const tmp = this.historyPath + '.tmp'; const tmp = this.historyPath + '.tmp';
const fd = fs.openSync(tmp, 'w'); const fd = fs.openSync(tmp, 'w');
@@ -380,7 +389,11 @@ class ConfigStore {
return r; return r;
} }
_loadImpl() { loadDiagnosticsConfig() {
return this._loadImpl(true);
}
_loadImpl(strict = false) {
try { try {
// In-memory cache keyed on the file's mtime+size. The processed config // In-memory cache keyed on the file's mtime+size. The processed config
// (merged + credential-decrypted) is reparsed/re-decrypted from disk ONLY // (merged + credential-decrypted) is reparsed/re-decrypted from disk ONLY
@@ -392,13 +405,24 @@ class ConfigStore {
// long-running main-thread drag. load() always returns a CLONE so callers // long-running main-thread drag. load() always returns a CLONE so callers
// can mutate the result without corrupting the cache. // can mutate the result without corrupting the cache.
let stat = null; let stat = null;
if (!strict) {
try { stat = fs.statSync(this.filePath); } catch {} try { stat = fs.statSync(this.filePath); } catch {}
}
const statKey = stat ? `${stat.mtimeMs}:${stat.size}` : ''; const statKey = stat ? `${stat.mtimeMs}:${stat.size}` : '';
if (stat && this._cache && this._cacheKey === statKey) { if (!strict && stat && this._cache && this._cacheKey === statKey) {
return this._clone(this._cache); return this._clone(this._cache);
} }
let data = null; let data = null;
if (strict) {
data = this._readAndParse(this.filePath);
if (!data || typeof data !== 'object' || Array.isArray(data) ||
!data.hosters || typeof data.hosters !== 'object' || Array.isArray(data.hosters) ||
!data.globalSettings || typeof data.globalSettings !== 'object' || Array.isArray(data.globalSettings) ||
(data.history !== undefined && !Array.isArray(data.history))) {
throw new Error('Die Diagnosekonfiguration ist ungültig');
}
} else {
// Try main config // Try main config
try { data = this._readAndParse(this.filePath); } catch {} try { data = this._readAndParse(this.filePath); } catch {}
// Fallback to backup if main is empty/corrupt // Fallback to backup if main is empty/corrupt
@@ -408,6 +432,7 @@ class ConfigStore {
if (!data) { if (!data) {
try { data = this._readAndParse(this.filePath + '.pre-history-split.bak'); } catch {} try { data = this._readAndParse(this.filePath + '.pre-history-split.bak'); } catch {}
} }
}
if (!data) { if (!data) {
const fresh = JSON.parse(JSON.stringify(DEFAULTS)); const fresh = JSON.parse(JSON.stringify(DEFAULTS));
fresh.globalSettings.logMode = normalizeLogMode(fresh.globalSettings); fresh.globalSettings.logMode = normalizeLogMode(fresh.globalSettings);
@@ -487,13 +512,13 @@ class ConfigStore {
// Decrypt credentials stored with safeStorage so the rest of the app // Decrypt credentials stored with safeStorage so the rest of the app
// keeps working with plaintext in memory. // keeps working with plaintext in memory.
secretStore.decryptCredentials(result); secretStore.decryptCredentials(result);
if (stat) { if (!strict && stat) {
this._cache = result; this._cache = result;
this._cacheKey = statKey; this._cacheKey = statKey;
} }
return this._clone(result); return this._clone(result);
} catch (error) { } catch (error) {
if (error instanceof secretStore.SecretStoreError) throw error; if (strict || error instanceof secretStore.SecretStoreError) throw error;
const fresh = JSON.parse(JSON.stringify(DEFAULTS)); const fresh = JSON.parse(JSON.stringify(DEFAULTS));
fresh.globalSettings.logMode = normalizeLogMode(fresh.globalSettings); fresh.globalSettings.logMode = normalizeLogMode(fresh.globalSettings);
return fresh; return fresh;
@@ -707,6 +732,18 @@ class ConfigStore {
return config.history || []; return config.history || [];
} }
loadDiagnosticsHistory() {
if (this._historyMigrated) return this._readHistoryFileStrict();
try {
return this._readHistoryFileStrict();
} catch (error) {
if (!error || error.code !== 'ENOENT') throw error;
}
const config = this.loadDiagnosticsConfig();
if (!Array.isArray(config.history)) throw new Error('Der Diagnoseverlauf ist ungültig');
return config.history;
}
_atomicWrite(data) { _atomicWrite(data) {
return new Promise((resolve, reject) => { return new Promise((resolve, reject) => {
const tmpPath = this.filePath + '.tmp'; const tmpPath = this.filePath + '.tmp';
+2 -2
View File
@@ -3246,8 +3246,8 @@ function _diagAgentInfo() {
function _buildDiagnosticHandler() { function _buildDiagnosticHandler() {
const collectors = createCollectors({ const collectors = createCollectors({
loadConfig: () => configStore.load(), loadConfig: () => configStore.loadDiagnosticsConfig(),
loadHistory: () => configStore.loadHistory(), loadHistory: () => configStore.loadDiagnosticsHistory(),
getAllLogPaths, getAllLogPaths,
support: { sanitizeConfig, collectSecretValues, redactLogText, valueScrub, collectFile, REDACTED }, support: { sanitizeConfig, collectSecretValues, redactLogText, valueScrub, collectFile, REDACTED },
stats, stats,
+81
View File
@@ -666,6 +666,47 @@ describe('ConfigStore', () => {
assert.equal(config.hosters['doodstream.com'][0].apiKey, 'from-backup'); assert.equal(config.hosters['doodstream.com'][0].apiKey, 'from-backup');
}); });
it('strict diagnostics config loading rejects primary failures while normal loading keeps recovery', () => {
assert.equal(typeof store.loadDiagnosticsConfig, 'function');
fs.writeFileSync(store.filePath + '.bak', JSON.stringify({
hosters: { 'doodstream.com': [{ id: 'bak-1', authType: 'api', apiKey: 'from-backup' }] },
hosterSettings: {},
globalSettings: {},
history: []
}), 'utf-8');
fs.writeFileSync(store.filePath, '{broken-config', 'utf-8');
assert.equal(store.load().hosters['doodstream.com'][0].apiKey, 'from-backup');
assert.throws(() => store.loadDiagnosticsConfig());
fs.rmSync(store.filePath);
assert.equal(store.load().globalSettings.language, 'en');
assert.throws(() => store.loadDiagnosticsConfig());
});
it('strict diagnostics config loading decrypts current credentials and propagates decryption failures', () => {
assert.equal(typeof store.loadDiagnosticsConfig, 'function');
const encrypted = `enc:v1:${Buffer.from('test-protected:diagnostic-secret').toString('base64')}`;
fs.writeFileSync(store.filePath, JSON.stringify({
hosters: { 'doodstream.com': [{ id: 'diag-1', authType: 'api', apiKey: encrypted }] },
hosterSettings: {},
globalSettings: {},
history: []
}), 'utf-8');
assert.equal(store.loadDiagnosticsConfig().hosters['doodstream.com'][0].apiKey, 'diagnostic-secret');
const originalDecryptString = safeStorage.decryptString;
safeStorage.decryptString = () => { throw new Error('diagnostic decrypt failure'); };
try {
assert.throws(() => store.loadDiagnosticsConfig(), /Gespeicherte Zugangsdaten konnten nicht entschlüsselt werden/);
} finally {
safeStorage.decryptString = originalDecryptString;
}
});
it('wipe-guard: a settings-only save recovers accounts from .bak when the live config validly has none', async () => { 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. // 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, JSON.stringify({ hosters: {}, hosterSettings: {}, globalSettings: {}, history: [] }), 'utf-8');
@@ -730,6 +771,46 @@ describe('ConfigStore history split (electron-history.json)', () => {
assert.equal(s.loadHistory().length, 30); assert.equal(s.loadHistory().length, 30);
}); });
it('strict diagnostics history accepts valid empty history and rejects failed or corrupt dedicated reads', () => {
assert.equal(typeof s.loadDiagnosticsHistory, 'function');
writeConfigWithHistory(7);
s._migrateHistory();
fs.writeFileSync(s.historyPath, '[]', 'utf-8');
assert.deepEqual(s.loadDiagnosticsHistory(), []);
for (const invalidHistory of ['null', '{}', '{"history":null}', '{broken-history']) {
fs.writeFileSync(s.historyPath, invalidHistory, 'utf-8');
assert.throws(() => s.loadDiagnosticsHistory());
}
fs.rmSync(s.historyPath);
fs.mkdirSync(s.historyPath);
assert.throws(() => s.loadDiagnosticsHistory());
});
it('strict diagnostics history never falls back to stale config history when a dedicated file exists', () => {
assert.equal(typeof s.loadDiagnosticsHistory, 'function');
writeConfigWithHistory(7);
fs.writeFileSync(s.historyPath, 'null', 'utf-8');
assert.equal(s._historyMigrated, false);
assert.equal(s.loadHistory().length, 7);
assert.throws(() => s.loadDiagnosticsHistory());
});
it('strict diagnostics history preserves the valid pre-migration history path', () => {
assert.equal(typeof s.loadDiagnosticsHistory, 'function');
writeConfigWithHistory(7);
assert.equal(s.loadDiagnosticsHistory().length, 7);
const config = JSON.parse(fs.readFileSync(s.filePath, 'utf-8'));
config.history = null;
fs.writeFileSync(s.filePath, JSON.stringify(config), 'utf-8');
assert.throws(() => s.loadDiagnosticsHistory());
});
it('appendHistory writes to history.json; the next config write strips stale history from the config file', async () => { it('appendHistory writes to history.json; the next config write strips stale history from the config file', async () => {
writeConfigWithHistory(10); writeConfigWithHistory(10);
s._migrateHistory(); s._migrateHistory();
+2
View File
@@ -107,4 +107,6 @@ test('main process keeps diagnostics local and fails closed at its final reply b
assert.match(source, /function _diagAllowlist\(\)\s*{\s*return \[\]/); assert.match(source, /function _diagAllowlist\(\)\s*{\s*return \[\]/);
assert.match(source, /bindMode: 'local'/); assert.match(source, /bindMode: 'local'/);
assert.match(source, /catch\s*{\s*result = { ok: false, error: 'diagnostic response could not be safely returned' }/); assert.match(source, /catch\s*{\s*result = { ok: false, error: 'diagnostic response could not be safely returned' }/);
assert.match(source, /loadConfig:\s*\(\)\s*=>\s*configStore\.loadDiagnosticsConfig\(\)/);
assert.match(source, /loadHistory:\s*\(\)\s*=>\s*configStore\.loadDiagnosticsHistory\(\)/);
}); });