fix(review): bound screenshot height probe

[JSONbored]

Motivation

• The full-page height probe used page.evaluate() in the page's own JS realm without a timeout, which a hostile page can hang and thus cause the public screenshot route to block or exhaust rendering resources.
• The intent is to preserve the post-capture PNG IHDR validation while removing the new availability / render-hang DoS path introduced by the unbounded evaluate.

Description

• Add a short, explicit timeout constant SCREENSHOT_HEIGHT_PROBE_TIMEOUT_MS and use it to time-bound the height probe before rasterization via Promise.race around page.evaluate().
• Fail closed when the height probe times out, logging a structured event (render_screenshot_height_probe_timeout) and returning null to avoid reaching page.screenshot().
• Preserve the existing screenshot timeout (SCREENSHOT_TIMEOUT_MS) and the post-capture PNG dimension/byte checks (readPngDimensions) so Chromium-rasterized output continues to be the ultimate enforcement point.
• Add a regression unit test that simulates a non-resolving page.evaluate() and asserts the probe times out, no rasterization is attempted, and the browser is closed.

Testing

• Ran the unit suite for the visual shot tests with npx vitest run test/unit/visual-shot.test.ts, and the tests passed locally.
• Ran a focused run npx vitest run test/unit/visual-shot.test.ts -t "height probe|screenshot rasterization|bounded review|attacker-controlled", which passed and emitted the new render_screenshot_height_probe_timeout log event.
• Ran npm run typecheck and git diff --check, both of which succeeded locally.
• Coverage remapping failed in npm run test:coverage due to a local dependency tree resolving js-tokens@4.0.0 while ast-v8-to-istanbul expects ^10.0.0, and npm audit --audit-level=moderate returned a 403 from the registry; these are infra/dependency issues unrelated to the source change itself.

---

Codex Task

[loopover-orb]

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

_{Review updated: 2026-07-07 07:57:01 UTC}

2 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · clean

⏸️ Suggested Action - Manual Review

• No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
• Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This PR wraps the pre-rasterization height probe (`page.evaluate()`) in a `Promise.race` against a 2s timeout, failing closed (returns `null`, logs a structured event) if the hostile page hangs the evaluate call, while leaving the authoritative post-capture PNG dimension check (`readPngDimensions`) untouched. The fix correctly targets the actual DoS vector — an unbounded `evaluate()` in the page's own JS realm — and the regression test faithfully simulates a non-resolving `evaluate()` via fake timers, asserting no rasterization occurs and the browser is closed. The `clearTimeout` cast and race construction are correct: the timer id is always assigned synchronously before the race is awaited, so there's no undefined-timer edge case.

Nits — 5 non-blocking

• The `console.log(JSON.stringify(...))` at shot.ts:161 flagged by the external brief as a 'debug leftover' is actually consistent with this file's existing structured-logging convention (e.g. the adjacent `render_screenshot_too_large` log) — not a stray debug statement, but worth confirming it routes to the same log sink/level as the other `ev:` events.
• SCREENSHOT_HEIGHT_PROBE_TIMEOUT_MS is hardcoded at 2000ms with no override; if this ever needs tuning per-deployment it'd be worth aligning with how SCREENSHOT_TIMEOUT_MS is configured, though as a fixed conservative bound it's fine for now.
• The losing `page.evaluate()` promise isn't cancelled when the timeout wins the race — if the hostile page later resolves it, the result is simply discarded, which is correct behavior but worth a one-line comment noting the orphaned promise is intentional.
• Consider a brief comment near the `Promise.race` noting that the timeout winning leaves the `evaluate()` call still pending in the page's realm (harmless since the page/browser gets closed by the caller on `null`), to preempt future readers wondering about the dangling promise.
• If other probes/evaluates exist elsewhere in shot.ts with similar hostile-page exposure, consider a shared `withTimeout(promise, ms)` helper to avoid re-deriving this race pattern per call site.

Concerns raised — review before merging

• No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
• Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Signal	Result	Evidence
Code review	❌ 2 blockers	1 reviewer
Linked issue	⚠️ Missing	No linked issue or no-issue rationale found.
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (no linked issue context).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 51 registered-repo PR(s), 43 merged, 343 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 51 PR(s), 343 issue(s).
Gate result	❌ Blocking	Repo-configured hard blocker found.

Review context

• Author: JSONbored
• Role context: owner (maintainer lane)
• Public audience mode: oss maintainer
• Lane context: Repository is configured for direct PR review.
• Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
• Official Gittensor activity: 51 PR(s), 343 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Treat this as maintainer-lane context rather than normal contributor-lane activity.
• Explain no-issue PR.
• Link the issue being solved, or explicitly explain why this is a no-issue PR.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

• Re-run Gittensory review

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.66%. Comparing base (1ce4364) to head (a810f1e).
⚠️ Report is 15 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3959   +/-   ##
=======================================
  Coverage   93.66%   93.66%           
=======================================
  Files         372      372           
  Lines       34856    34862    +6     
  Branches    12743    12744    +1     
=======================================
+ Hits        32649    32655    +6     
  Misses       1588     1588           
  Partials      619      619           

Files with missing lines	Coverage Δ
src/review/visual/shot.ts	91.80% <100.00%> (+0.27%)	⬆️

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.