From 4c48044a9584a1c126d3a04a2167c0fcaccd5f4c Mon Sep 17 00:00:00 2001 From: Sucukdeluxe <259325684+Sucukdeluxe@users.noreply.github.com> Date: Thu, 13 Aug 2026 21:08:26 +0200 Subject: [PATCH] Harden diagnostic response redaction Redact every diagnostic response at the agent boundary, fail closed when sanitization cannot complete, and remove Windows, UNC, and slash-UNC paths from returned data. Preserve benign text while removing complete configured secret values, including nested JSON escapes and quoted HTML credential fields. Add focused regression coverage for collector errors, successful responses, support bundles, path variants, and punctuation secrets. --- lib/diagnostics-agent.js | 23 ++++-- lib/diagnostics-collectors.js | 19 ++--- lib/support-bundle.js | 102 ++++++++++++++++++++++----- tests/diagnostics-agent.test.js | 41 ++++++++++- tests/diagnostics-collectors.test.js | 16 ++++- tests/support-bundle.test.js | 82 +++++++++++++++++++++ 6 files changed, 244 insertions(+), 39 deletions(-) diff --git a/lib/diagnostics-agent.js b/lib/diagnostics-agent.js index f5c2239..520fc52 100644 --- a/lib/diagnostics-agent.js +++ b/lib/diagnostics-agent.js @@ -1,3 +1,5 @@ +const { valueScrub } = require('./support-bundle'); + function createAgent(collectors) { const OPS = { get_system_info: (a) => collectors.getSystemInfo(a), @@ -14,15 +16,28 @@ function createAgent(collectors) { get_health: () => collectors.getHealth() }; + function redactResponse(value) { + try { + const redacted = typeof collectors.redactResponse === 'function' + ? collectors.redactResponse(value) + : valueScrub(value, []); + const response = valueScrub(redacted, []); + if (!response || typeof response !== 'object' || Array.isArray(response)) throw new Error('invalid redaction result'); + return response; + } catch { + return { ok: false, error: 'diagnostic response could not be safely returned' }; + } + } + function handle(op, args) { const fn = (typeof op === 'string' && Object.prototype.hasOwnProperty.call(OPS, op)) ? OPS[op] : null; - if (typeof fn !== 'function') return { ok: false, error: `unknown or non-readonly op: ${op}` }; + if (typeof fn !== 'function') return redactResponse({ ok: false, error: `unknown or non-readonly op: ${op}` }); try { const data = fn(args || {}); - if (data && data.ok === false) return data; - return { ok: true, data }; + if (data && data.ok === false) return redactResponse(data); + return redactResponse({ ok: true, data }); } catch (e) { - return { ok: false, error: String((e && e.message) || e) }; + return redactResponse({ ok: false, error: String((e && e.message) || e) }); } } diff --git a/lib/diagnostics-collectors.js b/lib/diagnostics-collectors.js index 7c4a169..bce9e38 100644 --- a/lib/diagnostics-collectors.js +++ b/lib/diagnostics-collectors.js @@ -20,18 +20,11 @@ function createCollectors(deps) { } function _deepRedact(value, secrets) { - const s = secrets || _secrets(); - const walk = (v) => { - if (typeof v === 'string') return support.redactLogText(v, s); - if (Array.isArray(v)) return v.map(walk); - if (v && typeof v === 'object') { - const o = {}; - for (const k of Object.keys(v)) o[k] = walk(v[k]); - return o; - } - return v; - }; - try { return walk(value); } catch { return value; } + return support.valueScrub(value, secrets || _secrets()); + } + + function redactResponse(value) { + return _deepRedact(value); } function _resolveLogPath(name, backup) { @@ -274,7 +267,7 @@ function createCollectors(deps) { return { getSystemInfo, getConfigRedacted, listLogs, readLog, getAppEvents, listErrors, getQueueState, getHistory, getRotationState, getHealth, serverHealth, - READABLE_LOGS + redactResponse, READABLE_LOGS }; } diff --git a/lib/support-bundle.js b/lib/support-bundle.js index b646d26..c1f0898 100644 --- a/lib/support-bundle.js +++ b/lib/support-bundle.js @@ -35,24 +35,28 @@ function collectSecretValues(config) { function redactConfiguredSecrets(text, secrets) { if (!Array.isArray(secrets)) return text; - const values = Array.from(new Set(secrets.filter(value => typeof value === 'string' && value.length > 0))) + const values = Array.from(new Set(secrets + .filter(value => typeof value === 'string' && value.length > 0) + .flatMap(value => { + const variants = [value]; + for (let index = 0; index < 3; index++) { + const escaped = JSON.stringify(variants[variants.length - 1]).slice(1, -1); + if (escaped === variants[variants.length - 1]) break; + variants.push(escaped); + } + return variants; + }))) .sort((a, b) => b.length - a.length); let out = text; for (const value of values) { - if (value.length >= 6) { - out = out.split(value).join(REDACTED); - continue; - } let offset = 0; while (offset < out.length) { const index = out.indexOf(value, offset); if (index < 0) break; - const first = value[0]; - const last = value[value.length - 1]; const before = index > 0 ? out[index - 1] : ''; const after = index + value.length < out.length ? out[index + value.length] : ''; - const identifier = character => /[A-Za-z0-9_]/.test(character); - if ((!identifier(first) || !identifier(before)) && (!identifier(last) || !identifier(after))) { + const continuation = character => /[A-Za-z0-9_.]/.test(character); + if (!continuation(before) && !continuation(after)) { out = `${out.slice(0, index)}${REDACTED}${out.slice(index + value.length)}`; offset = index + REDACTED.length; } else { @@ -64,23 +68,83 @@ function redactConfiguredSecrets(text, secrets) { } function redactHtmlCredentialFields(text) { - return text.replace(/]*>/gi, input => { + let out = ''; + let offset = 0; + const lower = text.toLowerCase(); + while (offset < text.length) { + const start = lower.indexOf('') { + end++; + break; + } + } + const input = text.slice(start, end); const sensitive = /\btype\s*=\s*["']?password\b/i.test(input) || /\b(?:name|id)\s*=\s*["']?(?:password|passwd|api[_-]?(?:key|token)|token|secret|authorization|cookie|session(?:[_-]?id)?)\b/i.test(input); - if (!sensitive) return input; - return input - .replace(/(\bvalue\s*=\s*)(["'])(.*?)\2/gi, `$1$2${REDACTED}$2`) - .replace(/(\bvalue\s*=\s*)(?!["'])([^\s>]+)/gi, `$1${REDACTED}`); - }); + out += sensitive + ? input + .replace(/(\bvalue\s*=\s*)(["'])([\s\S]*?)\2/gi, `$1$2${REDACTED}$2`) + .replace(/(\bvalue\s*=\s*)(?!["'])([^\s>]+)/gi, `$1${REDACTED}`) + : input; + offset = end; + } + return out; +} + +function redactAbsolutePaths(text) { + const isDriveStart = (value, index) => /[A-Za-z]/.test(value[index] || '') + && !/[A-Za-z0-9]/.test(value[index - 1] || '') + && value[index + 1] === ':' + && /[\\/]/.test(value[index + 2] || ''); + const isBackslashUncStart = (value, index) => { + if (value[index] !== '\\' || value[index + 1] !== '\\' || value[index - 1] === '\\') return false; + let cursor = index + 2; + while (value[cursor] === '\\') cursor++; + if (value[cursor] === '?') return true; + const separator = value.indexOf('\\', cursor); + return separator > cursor; + }; + const isSlashUncStart = (value, index) => value[index] === '/' + && value[index + 1] === '/' + && !/[:/]/.test(value[index - 1] || '') + && !/[\/]/.test(value[index + 2] || '') + && value.indexOf('/', index + 2) > index + 2; + let out = ''; + let index = 0; + while (index < text.length) { + if (!isDriveStart(text, index) && !isBackslashUncStart(text, index) && !isSlashUncStart(text, index)) { + out += text[index]; + index++; + continue; + } + let end = index; + while (end < text.length && !/[\r\n"'<>|]/.test(text[end])) end++; + const candidate = text.slice(index, end).replace(/\s+(?:trigger|error|outcome|hoster|attempt|status|code)=.*$/i, ''); + out += ''; + index += candidate.length; + } + return out; } function redactLogText(text, secrets) { if (typeof text !== 'string' || !text) return text; let out = redactConfiguredSecrets(text, secrets); out = redactHtmlCredentialFields(out) - .replace(/("(?:file|fileName|stagedFile|sourceFile|targetFile|path|[A-Za-z0-9_]*Path)"\s*:\s*")[^"]*(")/gi, '$1$2') - .replace(/\b[A-Za-z]:(?:\\+|\/+)[^\r\n"'<>|]*?(?=\s+(?:trigger|error|outcome|hoster|attempt|status|code)=|\r?\n|$|["'])/gi, '') - .replace(/\\{2,}[A-Za-z0-9._$-]+\\+[^\r\n"'<>|]*?(?=\s+(?:trigger|error|outcome|hoster|attempt|status|code)=|\r?\n|$|["'])/g, '') + .replace(/("(?:file|fileName|stagedFile|sourceFile|targetFile|path|[A-Za-z0-9_]*Path)"\s*:\s*")[^"]*(")/gi, '$1$2'); + out = redactAbsolutePaths(out) .replace(/https?:\/\/(?:ptb\.|canary\.)?discord(?:app)?\.com\/api\/webhooks\/\d+\/[\w-]+/gi, 'https://discord.com/api/webhooks/' + REDACTED) .replace(/(\/\/[^\s/:@]+:)[^\s/@]+(@)/g, '$1' + REDACTED + '$2') .replace(/(\b(?:proxy-)?authorization\s*:\s*)[^\r\n]*/gi, '$1' + REDACTED) @@ -99,7 +163,7 @@ function valueScrub(value, secrets) { if (Array.isArray(value)) return value.map(entry => valueScrub(entry, secrets)); if (typeof value === 'object') { const out = {}; - for (const [key, entry] of Object.entries(value)) out[key] = valueScrub(entry, secrets); + for (const [key, entry] of Object.entries(value)) out[redactLogText(key, secrets)] = valueScrub(entry, secrets); return out; } return value; diff --git a/tests/diagnostics-agent.test.js b/tests/diagnostics-agent.test.js index e728d6a..71e3908 100644 --- a/tests/diagnostics-agent.test.js +++ b/tests/diagnostics-agent.test.js @@ -1,6 +1,7 @@ const { test } = require('node:test'); const assert = require('node:assert'); const { createAgent } = require('../lib/diagnostics-agent'); +const { valueScrub } = require('../lib/support-bundle'); function stubCollectors() { const calls = []; @@ -28,6 +29,9 @@ test('agent rejects unknown ops and any write/exec-shaped op', () => { assert.equal(r.ok, false, `${bad} must be rejected`); assert.match(r.error, /unknown or non-readonly/); } + const pathShaped = agent.handle('C:\\Users\\PrivateProfile\\operation', {}); + assert.ok(!pathShaped.error.includes('PrivateProfile')); + assert.match(pathShaped.error, //); }); test('agent rejects inherited Object.prototype members (no whitelist bypass via the prototype chain)', () => { @@ -53,10 +57,43 @@ test('agent maps each whitelisted op to its collector and is read-only only', () for (const op of agent.ops) assert.ok(!/write|delete|set_|exec|restart|cancel|retry/.test(op), `${op} must be read-only`); }); -test('agent surfaces a collector ok:false verbatim and never throws', () => { - const agent = createAgent({ readLog: () => ({ ok: false, error: 'unknown or non-readable log: x' }), getSystemInfo: () => { throw new Error('boom'); } }); +test('agent redacts collector failures and thrown errors at the response boundary', () => { + const drivePath = 'C:\\Users\\PrivateProfile\\secret.log'; + const uncPath = '\\\\?\\UNC\\private-server\\secret-share\\secret.log'; + const agent = createAgent({ + readLog: () => ({ ok: false, error: `cannot read ${drivePath}` }), + getSystemInfo: () => { throw new Error(`boom at ${uncPath}`); } + }); assert.equal(agent.handle('read_log', { name: 'x' }).ok, false); + assert.ok(!JSON.stringify(agent.handle('read_log', { name: 'x' })).includes('PrivateProfile')); const thrown = agent.handle('get_system_info', {}); assert.equal(thrown.ok, false); assert.match(thrown.error, /boom/); + assert.ok(!thrown.error.includes('private-server')); + assert.match(thrown.error, //); +}); + +test('agent redacts every successful response with configured secrets at the boundary', () => { + const secret = 'configured-secret-123'; + const slashUnc = '//private-server/secret-share/secret.log'; + const collectors = stubCollectors(); + collectors.getSystemInfo = () => ({ nested: { message: `token ${secret}`, path: slashUnc } }); + collectors.redactResponse = value => valueScrub(value, [secret]); + const result = createAgent(collectors).handle('get_system_info', {}); + const json = JSON.stringify(result); + assert.equal(result.ok, true); + assert.ok(!json.includes(secret)); + assert.ok(!json.includes('private-server')); + assert.match(json, //); + assert.match(json, //); +}); + +test('agent fails closed when response redaction fails', () => { + const agent = createAgent({ + getSystemInfo: () => ({ token: 'must-not-leak' }), + redactResponse: () => { throw new Error('redactor unavailable'); } + }); + const result = agent.handle('get_system_info', {}); + assert.deepEqual(result, { ok: false, error: 'diagnostic response could not be safely returned' }); + assert.ok(!JSON.stringify(result).includes('must-not-leak')); }); diff --git a/tests/diagnostics-collectors.test.js b/tests/diagnostics-collectors.test.js index 8cd626f..b083da0 100644 --- a/tests/diagnostics-collectors.test.js +++ b/tests/diagnostics-collectors.test.js @@ -6,7 +6,6 @@ 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-')); @@ -192,3 +191,18 @@ test('serverHealth assembles the one-shot hub without leaking secrets', () => { assert.ok(!json.includes('HUNTER2SECRET') && !json.includes('SECRETTOKEN123456') && !json.includes('WBHOOKSECRETTOKEN'), 'no secret leaks in server_health'); assert.ok(!json.includes(dir) && !json.includes(paths.debug), 'server_health must not expose absolute log paths'); }); + +test('redactResponse scrubs configured secrets and absolute paths from arbitrary nested output', () => { + const { collectors, fixtureAlpha } = makeFixture(); + const value = { + error: `token ${fixtureAlpha} at C:\\Users\\PrivateProfile\\secret.log`, + nested: [{ source: '\\\\?\\UNC\\private-server\\secret-share\\secret.log' }] + }; + const out = collectors.redactResponse(value); + const json = JSON.stringify(out); + assert.ok(!json.includes(fixtureAlpha)); + assert.ok(!json.includes('PrivateProfile')); + assert.ok(!json.includes('private-server')); + assert.match(json, //); + assert.match(json, //); +}); diff --git a/tests/support-bundle.test.js b/tests/support-bundle.test.js index c5d73ec..3713be6 100644 --- a/tests/support-bundle.test.js +++ b/tests/support-bundle.test.js @@ -117,6 +117,42 @@ test('redactLogText masks a one-character configured secret only as a complete s assert.ok(out.includes(`marker=${REDACTED}`)); }); +test('redactLogText replaces configured secrets only as complete values', () => { + const out = redactLogText([ + 'password=orange', + 'configured token orange accepted', + 'file=orangejuice', + 'file=orange.mkv', + 'password=.', + 'version=2.1.20', + 'sentence finished.' + ].join('\n'), ['orange', '.']); + assert.ok(!out.includes('password=orange')); + assert.ok(!out.includes('token orange')); + assert.ok(!out.includes('password=.')); + assert.ok(out.includes('file=orangejuice')); + assert.ok(out.includes('file=orange.mkv')); + assert.ok(out.includes('version=2.1.20')); + assert.ok(out.includes('sentence finished.')); +}); + +test('redactLogText removes JSON-escaped configured secrets and quoted HTML credential values', () => { + const jsonSecret = 'alpha"beta\\gamma'; + const password = 'abc>secret'; + const token = 'token>quoted'; + const input = [ + JSON.stringify({ note: jsonSecret, token: jsonSecret }), + ``, + `` + ].join('\n'); + const out = redactLogText(input, [jsonSecret]); + assert.ok(!out.includes(jsonSecret)); + assert.ok(!out.includes('alpha\\"beta\\\\gamma')); + assert.ok(!out.includes(password)); + assert.ok(!out.includes(token)); + assert.ok((out.match(//g) || []).length >= 3); +}); + test('redactLogText removes complete local paths from structured and free-form log text', () => { const profilePath = ['C:', 'Users', 'ProfileFixture', 'Private Folder', 'episode.mkv'].join('\\'); const drivePath = ['D:', 'Archive', 'Private Folder', 'source.mkv'].join('\\'); @@ -135,6 +171,23 @@ test('redactLogText removes complete local paths from structured and free-form l assert.ok((out.match(//g) || []).length >= 4); }); +test('redactLogText removes extended UNC, extended drive, UNC and slash-UNC paths', () => { + const extendedUnc = '\\\\?\\UNC\\private-server\\secret-share\\hidden.log'; + const extendedDrive = '\\\\?\\C:\\Users\\PrivateProfile\\hidden.log'; + const unc = '\\\\private-server\\secret-share\\hidden.log'; + const slashUnc = '//private-server/secret-share/hidden.log'; + const out = redactLogText([ + `extended UNC failure: ${extendedUnc}`, + `extended drive failure: ${extendedDrive}`, + `UNC failure: ${unc}`, + `slash UNC failure: ${slashUnc}` + ].join('\n'), []); + for (const fragment of ['private-server', 'secret-share', 'PrivateProfile', 'hidden.log']) { + assert.ok(!out.includes(fragment), `private path fragment survived: ${fragment}`); + } + assert.equal((out.match(//g) || []).length, 4); +}); + test('sanitizeConfig does not mutate input', () => { const input = { hosters: { 'voe.sx': [{ password: 'secret' }] } }; const clone = JSON.parse(JSON.stringify(input)); @@ -285,3 +338,32 @@ test('buildSupportBundleText removes a one-character configured secret', () => { fs.unlinkSync(tmp); } }); + +test('buildSupportBundleText contains no escaped secrets, credential HTML or absolute path variants', () => { + const tmp = path.join(os.tmpdir(), `mhu-bundle-hard-redaction-${Date.now()}.log`); + const secret = 'alpha"beta\\gamma'; + const paths = [ + '\\\\?\\UNC\\private-server\\secret-share\\hidden.log', + '\\\\private-server\\secret-share\\hidden.log', + '//private-server/secret-share/hidden.log', + 'C:\\Users\\PrivateProfile\\hidden.log' + ]; + fs.writeFileSync(tmp, [ + JSON.stringify({ token: secret, path: paths[0] }), + '', + ...paths + ].join('\n')); + try { + const text = buildSupportBundleText({ + header: { Source: paths[3] }, + sanitizedConfig: { marker: JSON.stringify(secret), path: paths[1] }, + secrets: [secret], + files: [{ label: paths[2], path: tmp }] + }); + for (const value of ['alpha', 'beta', 'gamma', 'abc>secret', 'private-server', 'secret-share', 'PrivateProfile', 'hidden.log', tmp]) { + assert.ok(!text.includes(value), `support bundle leak survived: ${value}`); + } + } finally { + fs.unlinkSync(tmp); + } +});