8d757a99dd
Intensive end-to-end testing (a live gateway-MCP <-> agent integration harness +
an adversarial redaction/abuse probe + an independent security audit) surfaced
three real issues in the shipped read-only diagnostic agent. All run in lib/**,
which is packaged in the app.
1. grep ReDoS froze the Electron main process. read_log compiled the
client-supplied grep into `new RegExp(grep, 'i')` and ran it synchronously over
the log tail IN the main process. A catastrophic pattern (e.g. "(a+)+$" against
a long line) hangs the whole app — empirically confirmed (8s timeout, killed).
JS regex is synchronous and uncancellable, so grep is now a case-insensitive
literal substring filter with "|" alternation ("error|timeout|502"). Provably
linear-time; covers the real diagnostic need.
2. Prototype-chain whitelist bypass. The op table was a plain object literal, so
handle("constructor" | "toString" | "valueOf", ...) resolved an inherited
Object.prototype function, passed the `typeof fn === 'function'` guard and
returned {ok:true}. Harmless functions today, but a whitelist-integrity hole.
Now guarded with a string check + Object.prototype.hasOwnProperty.
3. Redaction defense-in-depth gaps. redactLogText now also scrubs: basic-auth URL
passwords (scheme://user:pass@host), Authorization: Basic, JWTs (eyJ...x.y.z),
and bare/JSON session= values. Mostly theoretical in today's readable logs
(secret-bearing bodies go to the excluded doodstream-debug.log; other hosters
throw static strings) but matters as the verbose-logging surface grows.
Verified: 383 app tests (incl. new regression tests for all three), the live
gateway-MCP integration harness (all 14 tools, zero leaks, error paths), the
adversarial probe (14/14+ secret shapes scrubbed, ReDoS 1ms, lockout, malformed
args), e2e gate, lint 0 errors. Only residual: a standalone high-entropy blob with
zero key/Bearer/URL context — inherent to any denylist, acknowledged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
63 lines
2.8 KiB
JavaScript
63 lines
2.8 KiB
JavaScript
const { test } = require('node:test');
|
|
const assert = require('node:assert');
|
|
const { createAgent } = require('../lib/diagnostics-agent');
|
|
|
|
function stubCollectors() {
|
|
const calls = [];
|
|
const mk = (name) => (a) => { calls.push([name, a]); return { name, a }; };
|
|
return {
|
|
calls,
|
|
getSystemInfo: mk('getSystemInfo'),
|
|
serverHealth: mk('serverHealth'),
|
|
getConfigRedacted: mk('getConfigRedacted'),
|
|
listLogs: mk('listLogs'),
|
|
readLog: mk('readLog'),
|
|
getAppEvents: mk('getAppEvents'),
|
|
listErrors: mk('listErrors'),
|
|
getQueueState: mk('getQueueState'),
|
|
getHistory: mk('getHistory'),
|
|
getRotationState: mk('getRotationState'),
|
|
getHealth: mk('getHealth')
|
|
};
|
|
}
|
|
|
|
test('agent rejects unknown ops and any write/exec-shaped op', () => {
|
|
const agent = createAgent(stubCollectors());
|
|
for (const bad of ['delete_log', 'write_config', 'run_health_check', 'exec', 'eval', '__proto__', 'set_setting', 'restart']) {
|
|
const r = agent.handle(bad, {});
|
|
assert.equal(r.ok, false, `${bad} must be rejected`);
|
|
assert.match(r.error, /unknown or non-readonly/);
|
|
}
|
|
});
|
|
|
|
test('agent rejects inherited Object.prototype members (no whitelist bypass via the prototype chain)', () => {
|
|
const agent = createAgent(stubCollectors());
|
|
for (const proto of ['constructor', 'toString', 'valueOf', 'hasOwnProperty', 'isPrototypeOf', 'toLocaleString']) {
|
|
const r = agent.handle(proto, {});
|
|
assert.equal(r.ok, false, `${proto} (inherited) must NOT be treated as an op`);
|
|
}
|
|
for (const bad of [null, undefined, 42, {}, ['read_log']]) {
|
|
assert.equal(agent.handle(bad, {}).ok, false, `non-string op ${JSON.stringify(bad)} must be rejected`);
|
|
}
|
|
});
|
|
|
|
test('agent maps each whitelisted op to its collector and is read-only only', () => {
|
|
const stub = stubCollectors();
|
|
const agent = createAgent(stub);
|
|
assert.equal(agent.handle('server_health', { errorLimit: 5 }).ok, true);
|
|
assert.equal(agent.handle('read_log', { name: 'debug' }).ok, true);
|
|
assert.equal(agent.handle('tail_log', { name: 'debug' }).ok, true, 'tail_log aliases read_log');
|
|
assert.equal(agent.handle('get_config_redacted', {}).ok, true);
|
|
const ops = new Set(agent.ops);
|
|
assert.ok(!ops.has('run_health_check'), 'no live probe op in this build');
|
|
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'); } });
|
|
assert.equal(agent.handle('read_log', { name: 'x' }).ok, false);
|
|
const thrown = agent.handle('get_system_info', {});
|
|
assert.equal(thrown.ok, false);
|
|
assert.match(thrown.error, /boom/);
|
|
});
|