Commit Graph

2 Commits

Author SHA1 Message Date
Administrator
8d757a99dd fix(diagnostics): harden read-only agent — grep ReDoS, prototype-chain whitelist bypass, redaction gaps
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>
2026-06-19 18:40:57 +02:00
Administrator
ab7313f32c feat(diagnostics): read-only remote diagnostics agent over the existing WS transport (app side)
Adds the server/app half of a remote-diagnostics system so Claude (via a local
MCP gateway, added separately) can read a server's full state to diagnose
problems: logs, errors, queue/app state, redacted config, system health. All
read-only; security review folded in as hard requirements.

Architecture (design-verified):
- A SECOND, independent RemoteServer instance (diagnosticMode) on its own port
  (default 9110, bind 127.0.0.1) with NO capture/input callbacks, so screen
  capture and sendInputEvent are structurally unreachable from the diagnostic
  path. The existing screen-share server (port 9100) is byte-for-byte unchanged.
- lib/remote-server.js: a `host` bind option, a post-auth `diag-request` branch
  that delegates to onDiagnosticRequest and replies with a reqId-correlated
  `diag-response`, a `diagnosticMode` guard (capture never spawns), a
  timing-safe token compare (length-guarded), and getLastAccess().
- lib/diagnostics-agent.js: handle(op,args) enforces a HARDCODED read-only op
  whitelist as the sole authority — no write/exec op, no run_health_check.
- lib/diagnostics-collectors.js: pure, dependency-injected collectors
  (server_health one-shot hub, read_log, list_errors, get_queue_state,
  get_history, get_config_redacted, get_system_info, list_logs, get_app_events,
  get_rotation_state, get_health) reusing support-bundle + stats.

Security (all mandatory, implemented):
- Redaction gates every off-box payload. support-bundle now adds webhookUrl +
  diagToken to CRED_KEYS, and exports redactLogText (value-scrub of live secret
  strings + pattern-scrub of Discord webhooks / Bearer / api_key= / cookies /
  sess ids) + valueScrub + collectSecretValues. read_log takes a logical NAME
  (no path traversal); doodstream-debug.log is excluded from the readable set
  (it logs live api-key-bearing HTML). grep is length-capped (ReDoS guard).
- diagnostics config subtree (enabled/port/token/label/codeIssuedAt/bindAddress)
  defaults OFF, bind 127.0.0.1; deep-merge makes it migration-free. The
  server-owned token is preserved against renderer clobber in both
  save-global-settings handlers; the diagnostics:* IPC is the only mutator.
- app.requestSingleInstanceLock() (also the approved fix #6) so a relaunch can't
  EADDRINUSE-kill the agent and two instances can't clobber the config.

main.js: buildDiagnosticCode (mhu1_ base64url, no host embedded), start/stop +
auto-start-on-launch + stop-on-quit, the four diagnostics:* IPC handlers.
renderer: a "Diagnose-Zugriff" settings subtab (toggle, port, bind-address,
copyable connection code + regenerate, status, read-only security note).

382 tests pass (incl. 11 new: collectors+redaction, agent whitelist, and a live
RemoteServer protocol test proving auth->diag-response correlation, that a
diagnostic client never triggers the capture window, and brute-force lockout).
Lint clean (0 errors). Smoke boots identically to baseline. The standalone MCP
gateway package + operator setup docs land in a follow-up commit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-19 17:24:32 +02:00