From 7b5420eeaa2cc05047ece99f7de089428d6fec82 Mon Sep 17 00:00:00 2001 From: Administrator Date: Fri, 19 Jun 2026 17:44:59 +0200 Subject: [PATCH] =?UTF-8?q?fix(diagnostics):=20deep-redact=20queue=20jobs?= =?UTF-8?q?=20+=20rotation=20state=20=E2=80=94=20one=20collector=20still?= =?UTF-8?q?=20leaked?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit get_queue_state with includeJobs:true (the DEFAULT path) scrubbed the job list with value-scrub only, so an opaque token a hoster returns inside a job error (token=... that is NOT one of the user's stored credentials) survived in the response. Same leak class already fixed for get_config_redacted and server_health, still open on the queue collector's default path. The first end-to-end gate missed it on two coincidences: server_health calls getQueueState with includeJobs:false (no job error ever serialized), and the fixture's queue error used a value that WAS a config secret (so value-scrub caught it anyway). The direct get_queue_state{includeJobs:true} path with a non-config token was never exercised. - getQueueState job list and getRotationState now go through _deepRedact (per-leaf pattern + value scrub), matching the other collectors. - e2e-verify.mjs now plants a non-config token in a queue job error and asserts both get_queue_state{includeJobs:true} and the default-args call leak nothing. - Added a main-suite regression test for the default includeJobs path. 386 app tests + 9 gateway tests + e2e gate pass; lint 0 errors. Co-Authored-By: Claude Opus 4.8 (1M context) --- gateway/test/e2e-verify.mjs | 11 ++++++++++- lib/diagnostics-collectors.js | 8 ++------ tests/diagnostics-collectors.test.js | 19 +++++++++++++++++++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/gateway/test/e2e-verify.mjs b/gateway/test/e2e-verify.mjs index 014e0ef..4e6af15 100644 --- a/gateway/test/e2e-verify.mjs +++ b/gateway/test/e2e-verify.mjs @@ -38,7 +38,7 @@ const fakeConfig = { globalSettings: { webhookUrl: WEBHOOK, diagnostics: { enabled: true, port: 9110, token: 'x'.repeat(64) }, - pendingQueue: { savedAt: '2026-06-19T09:00:00Z', queueJobs: [{ file: 'a.mp4', fileName: 'a.mp4', hoster: 'doodstream', status: 'error', error: `failed with apiKey=${SECRET_API}` }], selectedUploadHosters: ['doodstream'], selectedFiles: ['a.mp4'] }, + pendingQueue: { savedAt: '2026-06-19T09:00:00Z', queueJobs: [{ file: 'a.mp4', fileName: 'a.mp4', hoster: 'doodstream', status: 'error', error: `failed with apiKey=${SECRET_API}` }, { file: 'b.mp4', fileName: 'b.mp4', hoster: 'streamtape', status: 'error', error: `upload rejected: token=${SECRET_TOKEN}` }], selectedUploadHosters: ['doodstream'], selectedFiles: ['a.mp4', 'b.mp4'] }, }, rotationCursors: { doodstream: 1 }, history: [{ timestamp: '2026-06-19T08:00:00Z', files: [{ name: 'a.mp4', results: [{ hoster: 'doodstream', status: 'error', error: `boom token=${SECRET_TOKEN}` }] }] }], @@ -114,6 +114,15 @@ function assertNoLeak(label, payload) { assert.equal(cfg.ok, true, 'get_config_redacted must succeed'); assertNoLeak('get_config_redacted', cfg); + const queue = await client.request('get_queue_state', { includeJobs: true }); + assert.equal(queue.ok, true, 'get_queue_state must succeed'); + assert.ok(Array.isArray(queue.data.jobs) && queue.data.jobs.length >= 2, 'jobs present'); + assertNoLeak('get_queue_state:includeJobs', queue); + + const queueDefault = await client.request('get_queue_state', {}); + assert.equal(queueDefault.ok, true, 'get_queue_state (default args) must succeed'); + assertNoLeak('get_queue_state:default', queueDefault); + const writeAttempt = await client.request('save_config', { x: 1 }); assert.equal(writeAttempt.ok, false, 'unknown/write op must be rejected by whitelist'); diff --git a/lib/diagnostics-collectors.js b/lib/diagnostics-collectors.js index 7afbc9f..9e29d51 100644 --- a/lib/diagnostics-collectors.js +++ b/lib/diagnostics-collectors.js @@ -17,10 +17,6 @@ function createCollectors(deps) { try { return support.collectSecretValues(loadConfig()); } catch { return []; } } - function _scrub(value, secrets) { - try { return support.valueScrub(value, secrets || _secrets()); } catch { return value; } - } - function _deepRedact(value, secrets) { const s = secrets || _secrets(); const walk = (v) => { @@ -197,7 +193,7 @@ function createCollectors(deps) { }; if (a.includeJobs !== false) { const maxJobs = Math.min(Math.max(Number(a.maxJobs) || 200, 1), 2000); - result.jobs = _scrub(jobs.slice(0, maxJobs).map(j => ({ + result.jobs = _deepRedact(jobs.slice(0, maxJobs).map(j => ({ file: j.file, fileName: j.fileName, hoster: j.hoster, status: j.status, error: j.error || null }))); result.jobsTruncated = jobs.length > maxJobs; @@ -233,7 +229,7 @@ function createCollectors(deps) { function getRotationState() { const cfg = loadConfig(); - return { rotationCursors: _scrub(cfg.rotationCursors || {}) }; + return { rotationCursors: _deepRedact(cfg.rotationCursors || {}) }; } function getHealth() { diff --git a/tests/diagnostics-collectors.test.js b/tests/diagnostics-collectors.test.js index ae88ef8..a4f90d8 100644 --- a/tests/diagnostics-collectors.test.js +++ b/tests/diagnostics-collectors.test.js @@ -71,6 +71,25 @@ test('getQueueState flags stale=true for the persisted snapshot and counts by st assert.equal(q.counts.error, 1); }); +test('getQueueState (includeJobs default) pattern-scrubs an opaque token in a job error that is NOT a config secret', () => { + const config = { + hosters: {}, hosterSettings: {}, + globalSettings: { pendingQueue: { savedAt: 1, selectedUploadHosters: [], selectedFiles: [], queueJobs: [ + { file: 'C:/b.mkv', fileName: 'b.mkv', hoster: 'streamtape', status: 'error', error: 'upload rejected: token=OPAQUE_NONconfig_TOKEN_9988' } + ] } }, + history: [], rotationCursors: {} + }; + const collectors = createCollectors({ + loadConfig: () => JSON.parse(JSON.stringify(config)), + getAllLogPaths: () => ({ logDir: os.tmpdir() }), + support, stats, + appInfo: () => ({}), systemInfo: () => ({}), agentInfo: () => ({}) + }); + const q = collectors.getQueueState({}); + const json = JSON.stringify(q); + assert.ok(!json.includes('OPAQUE_NONconfig_TOKEN_9988'), 'opaque token in a job error must be pattern-scrubbed even on the default includeJobs path'); +}); + test('listErrors classifies via stats.classifyErrorCategory and redacts error text', () => { const { collectors } = makeFixture(); const e = collectors.listErrors({});