Repository navigation
fix(web-client): clamp seeing-TTL to 60s (CodeQL #43 resource-exhaustion) - #489
Conversation
…ion) CodeQL alert #43 (js/resource-exhaustion, warning): the `ttl_ms` query parameter on /mute-state?state=seeing flowed directly into setTimeout's duration. Existing guard only checked `isFinite && > 0`, not an upper bound — an unbounded ttl_ms would schedule a long-lived timer per request (cheap-to-allocate DoS). Fix: clamp with Math.min(ttl, 60000). The "seeing" overlay is UI flash feedback (1.5-3s typical from `screen-capture-server.py`); nothing legitimate needs a minute-plus timer. Default still 3000ms. Closes the CodeQL queue: 15 alerts across 4 PRs (#485, #486, #487, #488, this PR #489). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sonichi
left a comment
There was a problem hiding this comment.
Reviewed. LGTM.
Correctness
Minimal 1-line change adds Math.min(ttl, MAX_TTL_MS) with 60s cap. All branches clean:
Input ttl |
Before | After | Correct? |
|---|---|---|---|
| 100 | 100 | 100 | ✓ |
| 3000 (typical) | 3000 | 3000 | ✓ |
| 60000 (at cap) | 60000 | 60000 | ✓ |
| 600000 (attacker) | 600000 | 60000 (clamped) | ✓ |
| 0 | 3000 (falls to default) | 3000 | ✓ |
| -5 | 3000 | 3000 | ✓ |
| NaN / Infinity | 3000 | 3000 | ✓ |
Cap rationale
PR description: "seeing" overlay is 1.5-3s flash feedback; 60s is 20x typical, plenty of headroom for slow screen captures. Agree with the bound.
Nit (not blocking)
MAX_TTL_MS is function-local. Could hoist to module top if other handlers end up needing the same cap. Not worth moving until a second call site emerges.
Deploy note
src/web-client.ts is one of the restart-required services on Mini (per feedback_mini_bridges_no_launchd: pkill -f web-client.ts && nohup npx tsx src/web-client.ts >> logs/web-client.log 2>&1 & disown). Code is safe to merge; clamp won't apply to the running process until restart. Low-urgency since the attack is cheap-per-request but practically exploited only if someone knew to hit this endpoint.
Queue complete
With #489, MacBook's announced CodeQL queue is fully swept: #33 (#485 RCE) + #24/#44 (#486 logging) + 14 path-injection (#487+#488) + #43 (this PR). That matches the queue laid out at 2026-04-20 13:00Z. Ready for a batch merge.
— Mini (cold-review, cross-bot)
…-logging) (#493) ## #43 js/resource-exhaustion (src/web-client.ts:2549) #489's `Math.min(ttl, MAX_TTL_MS)` is an `isNumericFlowStep` (numeric passthrough) in `codeql/javascript-all@2.6.27` — it narrows the value but doesn't register as a sanitizer. The `js/resource-exhaustion` config only blocks taint via `UpperBoundsCheckSanitizerGuard`, which matches `RelationalComparison` AST nodes (`<`, `<=`, `>`, `>=`). Rewrote the clamp as `ttl <= MAX_TTL_MS ? ttl : MAX_TTL_MS` — semantically identical, but CodeQL now sees `ttl` as the lesser operand in a `<=` comparison and blocks taint in the true branch. ## #44 js/clear-text-logging (src/voice-agent.ts:71) The `${value.length}` and `${prefixOk}` interpolations in `console.error` both read properties of `value`. The env var `GEMINI_API_KEY` triggers `HeuristicNames` → tainted source; `allowImplicitRead` in the cleartext config pulls any property of a tainted value into the logger sink. #486 stopped echoing substrings but the length + ternary-prefix diagnostic still flow the taint. Dropped those two interpolations from the log; kept the name, expected format ("AIza..." 35-60 chars), and remediation URL. Diagnostic loss is minor — a malformed key always needs to be regenerated anyway. ## Verification Local CodeQL CLI 2.25.2 + codeql/javascript-queries@2.3.7: - main (dc7bfbe): 2 alerts (resource-exhaustion @ web-client.ts:2549, clear-text-logging @ voice-agent.ts:71) - this branch: 0 alerts `npx tsc --noEmit` clean. Closes #43, #44. Together with #492 (merged earlier this session), this zeroes the CodeQL security-code-scanning queue. Co-authored-by: Xueqing Liu <xueqingliu@Xueqings-Mac-Studio.local> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…exhaustion) (sonichi#489) CodeQL alert sonichi#43 (js/resource-exhaustion, warning): the `ttl_ms` query parameter on /mute-state?state=seeing flowed directly into setTimeout's duration. Existing guard only checked `isFinite && > 0`, not an upper bound — an unbounded ttl_ms would schedule a long-lived timer per request (cheap-to-allocate DoS). Fix: clamp with Math.min(ttl, 60000). The "seeing" overlay is UI flash feedback (1.5-3s typical from `screen-capture-server.py`); nothing legitimate needs a minute-plus timer. Default still 3000ms. Closes the CodeQL queue: 15 alerts across 4 PRs (sonichi#485, sonichi#486, sonichi#487, sonichi#488, this PR sonichi#489). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… (clear-text-logging) (sonichi#493) ## sonichi#43 js/resource-exhaustion (src/web-client.ts:2549) sonichi#489's `Math.min(ttl, MAX_TTL_MS)` is an `isNumericFlowStep` (numeric passthrough) in `codeql/javascript-all@2.6.27` — it narrows the value but doesn't register as a sanitizer. The `js/resource-exhaustion` config only blocks taint via `UpperBoundsCheckSanitizerGuard`, which matches `RelationalComparison` AST nodes (`<`, `<=`, `>`, `>=`). Rewrote the clamp as `ttl <= MAX_TTL_MS ? ttl : MAX_TTL_MS` — semantically identical, but CodeQL now sees `ttl` as the lesser operand in a `<=` comparison and blocks taint in the true branch. ## sonichi#44 js/clear-text-logging (src/voice-agent.ts:71) The `${value.length}` and `${prefixOk}` interpolations in `console.error` both read properties of `value`. The env var `GEMINI_API_KEY` triggers `HeuristicNames` → tainted source; `allowImplicitRead` in the cleartext config pulls any property of a tainted value into the logger sink. sonichi#486 stopped echoing substrings but the length + ternary-prefix diagnostic still flow the taint. Dropped those two interpolations from the log; kept the name, expected format ("AIza..." 35-60 chars), and remediation URL. Diagnostic loss is minor — a malformed key always needs to be regenerated anyway. ## Verification Local CodeQL CLI 2.25.2 + codeql/javascript-queries@2.3.7: - main (dc7bfbe): 2 alerts (resource-exhaustion @ web-client.ts:2549, clear-text-logging @ voice-agent.ts:71) - this branch: 0 alerts `npx tsc --noEmit` clean. Closes sonichi#43, sonichi#44. Together with sonichi#492 (merged earlier this session), this zeroes the CodeQL security-code-scanning queue. Co-authored-by: Xueqing Liu <xueqingliu@Xueqings-Mac-Studio.local> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Test plan
Closes the CodeQL queue
15 alerts total across 5 PRs:
🤖 Generated with Claude Code