diff --git a/lib/diagnostics-collectors.js b/lib/diagnostics-collectors.js index 01d5bea..f1bd6f2 100644 --- a/lib/diagnostics-collectors.js +++ b/lib/diagnostics-collectors.js @@ -16,18 +16,24 @@ function createCollectors(deps) { const { loadConfig, loadHistory, getAllLogPaths, support, stats, appInfo, systemInfo, agentInfo } = deps; function _secrets() { - try { return support.collectSecretValues(loadConfig()); } catch { return []; } + return support.collectSecretValues(loadConfig()); } function _currentHistory() { if (typeof loadHistory === 'function') { const history = loadHistory(); - return Array.isArray(history) ? history : []; + if (!Array.isArray(history)) throw new Error('History reader returned invalid data'); + return history; } const cfg = loadConfig(); return Array.isArray(cfg && cfg.history) ? cfg.history : []; } + function _historyContext() { + const secrets = _secrets(); + return { history: _currentHistory(), secrets }; + } + function _deepRedact(value, secrets) { return support.valueScrub(value, secrets || _secrets()); } @@ -139,10 +145,9 @@ function createCollectors(deps) { return { events: out.slice(-limit), truncated: out.length > limit }; } - function _historyErrors(history, opts) { + function _historyErrors(history, opts, secrets) { const o = opts || {}; const sinceMs = Number.isFinite(o.sinceMs) ? o.sinceMs : null; - const secrets = _secrets(); const errors = []; const byCategory = {}; for (const batch of (Array.isArray(history) ? history : [])) { @@ -171,16 +176,17 @@ function createCollectors(deps) { return { errors, byCategory }; } - function _listErrors(args, history) { + function _listErrors(args, history, secrets) { const a = args || {}; - const { errors, byCategory } = _historyErrors(history, a); + const { errors, byCategory } = _historyErrors(history, a, secrets); const limit = Math.min(Math.max(Number(a.limit) || 100, 1), 1000); const window = Number.isFinite(a.sinceMs) ? `since ${new Date(a.sinceMs).toISOString()}` : 'all history'; return { window, total: errors.length, byCategory, errors: errors.slice(-limit) }; } function listErrors(args) { - return _listErrors(args, _currentHistory()); + const { history, secrets } = _historyContext(); + return _listErrors(args, history, secrets); } function getQueueState(args) { @@ -212,12 +218,11 @@ function createCollectors(deps) { return result; } - function _getHistory(args, history) { + function _getHistory(args, history, secrets) { const a = args || {}; const limit = Math.min(Math.max(Number(a.limit) || 20, 1), 200); - const perHoster = stats.summarizePerHoster(history); + const perHoster = stats.summarizePerHoster([...history]); const recent = [...history].slice(-limit).reverse(); - const secrets = _secrets(); const batches = recent.map(b => { const out = { timestamp: b.timestamp || null, fileCount: Array.isArray(b.files) ? b.files.length : 0 }; if (a.includeFiles) { @@ -237,7 +242,8 @@ function createCollectors(deps) { } function getHistory(args) { - return _getHistory(args, _currentHistory()); + const { history, secrets } = _historyContext(); + return _getHistory(args, history, secrets); } function getRotationState() { @@ -259,10 +265,10 @@ function createCollectors(deps) { const a = args || {}; const errorLimit = Math.min(Math.max(Number(a.errorLimit) || 20, 1), 200); const errArgs = Number.isFinite(a.errorSinceMs) ? { sinceMs: a.errorSinceMs, limit: errorLimit } : { limit: errorLimit }; - const currentHistory = _currentHistory(); - const errors = _listErrors(errArgs, currentHistory); + const { history: currentHistory, secrets } = _historyContext(); + const errors = _listErrors(errArgs, currentHistory, secrets); const queue = getQueueState({ includeJobs: false }); - const history = _getHistory({ limit: 5 }, currentHistory); + const history = _getHistory({ limit: 5 }, currentHistory, secrets); const warnings = []; if (queue.source === 'persisted' && queue.stale) warnings.push('queue state is from the persisted snapshot (may lag live state; UploadManager not introspected in this build).'); if (errors.total > 0) warnings.push(`${errors.total} non-success result(s) in the error window.`); diff --git a/tests/diagnostics-collectors.test.js b/tests/diagnostics-collectors.test.js index e115e4b..b239563 100644 --- a/tests/diagnostics-collectors.test.js +++ b/tests/diagnostics-collectors.test.js @@ -6,6 +6,7 @@ const path = require('path'); const support = require('../lib/support-bundle'); const stats = require('../lib/stats'); const { createCollectors } = require('../lib/diagnostics-collectors'); +const { createAgent } = require('../lib/diagnostics-agent'); function makeFixture() { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'mhu-diag-')); @@ -116,30 +117,100 @@ test('getHistory, listErrors and serverHealth share loadHistory after migration' assert.equal(health.recentBatches[0].timestamp, health.errors.errors[0].ts, 'serverHealth must summarize one history snapshot'); }); -test('history commands fail closed instead of reading stale config history', () => { +test('getHistory, listErrors and serverHealth leave the supplied history snapshot unchanged', () => { + const original = [ + { timestamp: '2026-01-02T00:00:00.000Z', files: [{ name: 'newer.mkv', results: [{ hoster: 'voe.sx', status: 'done' }] }] }, + { timestamp: '2026-01-01T00:00:00.000Z', files: [{ name: 'older.mkv', results: [{ hoster: 'voe.sx', status: 'error', error: 'Not video file format' }] }] } + ]; + const sortingStats = { + ...stats, + summarizePerHoster: history => { + history.sort((a, b) => Date.parse(a.timestamp) - Date.parse(b.timestamp)); + return stats.summarizePerHoster(history); + } + }; + for (const [name, invoke] of [ + ['listErrors', c => c.listErrors({})], + ['getHistory', c => c.getHistory({ includeFiles: true })], + ['serverHealth', c => c.serverHealth({})] + ]) { + const snapshot = JSON.parse(JSON.stringify(original)); + const c = createCollectors({ + loadConfig: () => ({ hosters: {}, globalSettings: {}, history: [] }), + loadHistory: () => snapshot, + getAllLogPaths: () => ({ logDir: os.tmpdir() }), + support, stats: sortingStats, + appInfo: () => ({}), systemInfo: () => ({}), agentInfo: () => ({}) + }); + invoke(c); + assert.deepEqual(snapshot, original, `${name} must not mutate the supplied history snapshot`); + } +}); + +test('dedicated history reader failures never report healthy empty history or use stale config history', () => { + const sensitive = 'reader-private-value-38152'; + const config = { + hosters: { 'voe.sx': [{ apiKey: sensitive }] }, + globalSettings: {}, + history: [{ timestamp: '2025-12-31T00:00:00.000Z', files: [{ name: 'stale.mkv', results: [{ hoster: 'voe.sx', status: 'error', error: 'stale error' }] }] }] + }; + for (const [scenario, loadHistory] of [ + ['throwing reader', () => { throw new Error(`history reader failed with ${sensitive}`); }], + ['invalid reader result', () => ({ history: [] })] + ]) { + const c = createCollectors({ + loadConfig: () => JSON.parse(JSON.stringify(config)), + loadHistory, + getAllLogPaths: () => ({ logDir: os.tmpdir() }), + support, stats, + appInfo: () => ({}), systemInfo: () => ({}), agentInfo: () => ({}) + }); + for (const [name, invoke] of [ + ['getHistory', () => c.getHistory({ includeFiles: true })], + ['listErrors', () => c.listErrors({})], + ['serverHealth', () => c.serverHealth({})] + ]) { + assert.throws(invoke, /history/i, `${name} must reject a ${scenario}`); + } + const agent = createAgent(c); + for (const op of ['get_history', 'list_errors', 'server_health']) { + const response = agent.handle(op, { includeFiles: true }); + const json = JSON.stringify(response); + assert.equal(response.ok, false, `${op} must report the ${scenario} as unhealthy`); + assert.match(response.error, /history/i); + assert.ok(!json.includes(sensitive)); + assert.ok(!json.includes('stale.mkv')); + } + } +}); + +test('history operations fail closed when configured secrets cannot be loaded', () => { + const sensitive = 'history-private-value-27491'; + let historyReads = 0; const c = createCollectors({ - loadConfig: () => ({ - hosters: {}, - globalSettings: {}, - history: [{ timestamp: '2025-12-31T00:00:00.000Z', files: [{ name: 'stale.mkv', results: [{ hoster: 'voe.sx', status: 'error', error: 'stale error' }] }] }] - }), - loadHistory: () => null, + loadConfig: () => { throw new Error(`secret decryption failed near ${sensitive}`); }, + loadHistory: () => { + historyReads++; + return [{ timestamp: '2026-01-04T00:00:00.000Z', files: [{ name: `${sensitive}.mkv`, results: [{ hoster: 'voe.sx', status: 'error', error: `opaque ${sensitive}` }] }] }]; + }, getAllLogPaths: () => ({ logDir: os.tmpdir() }), support, stats, appInfo: () => ({}), systemInfo: () => ({}), agentInfo: () => ({}) }); - const health = c.serverHealth({ errorLimit: 5 }); - assert.deepEqual({ - historyBatches: c.getHistory({ limit: 5 }).totalBatches, - listedErrors: c.listErrors({ limit: 5 }).total, - healthBatches: health.recentBatches.length, - healthErrors: health.errors.total - }, { - historyBatches: 0, - listedErrors: 0, - healthBatches: 0, - healthErrors: 0 - }); + for (const [name, invoke] of [ + ['getHistory', () => c.getHistory({ includeFiles: true })], + ['listErrors', () => c.listErrors({})], + ['serverHealth', () => c.serverHealth({})] + ]) { + assert.throws(invoke, /secret decryption failed/, `${name} must fail without the configured redaction secrets`); + } + assert.equal(historyReads, 0, 'history must not be read before the redaction secrets are available'); + const agent = createAgent(c); + for (const op of ['get_history', 'list_errors', 'server_health']) { + const response = agent.handle(op, { includeFiles: true }); + assert.deepEqual(response, { ok: false, error: 'diagnostic response could not be safely returned' }); + assert.ok(!JSON.stringify(response).includes(sensitive)); + } }); test('readLog redacts a planted token and a Bearer line; doodstream is NOT readable; unknown name rejected', () => {