Skip to content

Dashboard/webhook/Slack hardening (B14, B19, B21) - #371

Merged
andrewklingelhofer merged 3 commits into
mainfrom
fix/audit-dashboard-webhook-hardening
Jul 18, 2026
Merged

Dashboard/webhook/Slack hardening (B14, B19, B21)#371
andrewklingelhofer merged 3 commits into
mainfrom
fix/audit-dashboard-webhook-hardening

Conversation

@andrewklingelhofer

Copy link
Copy Markdown
Contributor

Summary

Resource-leak and untrusted-input hardening across the HTTP/notification surface.

# Problem Fix
B14 SSE heartbeat interval leaked on normal critter completion (timers + closures retained forever) Clear the heartbeat + mark closed on the done path
B19 HMAC verifiers length-checked UTF-16 units before a byte-wise timingSafeEqualRangeError → HTTP 500 instead of 401 on a multi-byte header Compare decoded bytes, never throw; one shared verifyHmacSignature
B21 Issue title/error interpolated into Slack mrkdwn unescaped → <!channel>/<@U>/<url|text> injection Escape &<> in free-text fields (URL fields left intact so query strings aren't corrupted)
+ Dashboard token compared with non-constant-time === crypto.timingSafeEqual with a length guard (header + cookie)

Tests

audit-web-hardening.test.ts — a 64-UTF-16-unit/65-byte header that threw on the old code now returns false without throwing; valid sig accepted; Slack escaping neutralizes <!channel> while URLs pass through; timing-safe accept/reject.

Verification

typecheck clean · lint exit 0 · bun test 861 pass / 0 fail.

Scoped out (deliberately): trimming /healthz disclosure — the clean CLI reads activeCritterDetails[].workDir from it, so the response shape must stay. Tracked for a follow-up.

🤖 Generated with Claude Code

andrewklingelhofer and others added 3 commits June 15, 2026 21:47
- B14 (health.ts): live-log SSE 'done' branch now sets closed=true and clears
  the heartbeat timer (not just the poll timer), so the 15s heartbeat stops
  firing once a critter finishes instead of leaking forever.
- B19 (webhook.ts): de-duplicate the Linear/Jira HMAC verifiers into a single
  verifyHmacSignature() that compares decoded bytes (not UTF-16 string length)
  and never throws — a multi-byte/odd-length header now yields a clean 401
  instead of a RangeError -> HTTP 500.
- B21 (slack.ts): add escapeSlackText() and apply it to every untrusted field
  (title/error/reason/repoUrl/prUrl/phase) so issue content can't inject Slack
  control sequences like <!channel>, <@u123>, or <url|text>.
- auth.ts: compare the dashboard token with crypto.timingSafeEqual behind a
  byte-length guard (header and cookie) to remove the timing oracle.

Adds src/__tests__/audit-web-hardening.test.ts covering all four.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
- webhook.ts: drop the dead try/catch around Buffer.from(hex); in Bun it
  never throws (malformed/odd hex truncates), so the byte-length compare is
  what rejects bad input. Behavior unchanged; removes unreachable code.
- slack.ts: stop escaping URL fields (prUrl, repoUrl). Escaping '&' to
  '&amp;' corrupts query strings and breaks Slack auto-linking. Free-text
  fields (title, error, reason) are still escaped.
- audit-web-hardening.test.ts: strengthen the B19 regression to reproduce the
  original bug with a 64-code-unit / 65-byte header that threw on the old
  string-.length guard; assert the verifier returns false without throwing.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
resolvePhaseMcpConfig expands ~ via node:os homedir(); the test compared it to
process.env.HOME. Bun caches homedir() on first call and ignores later
process.env.HOME mutations, so when a sibling test changes HOME they diverge on
Linux CI and this test fails (works on macOS by execution-order luck). Assert
against the same homedir() the implementation uses.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@andrewklingelhofer
andrewklingelhofer merged commit 3c00a5f into main Jul 18, 2026
1 check passed
@andrewklingelhofer
andrewklingelhofer deleted the fix/audit-dashboard-webhook-hardening branch July 18, 2026 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant