Skip to content

fix(review): bound theme storage writes - #4177

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-vulnerability-in-themestoragekey-usage
Jul 8, 2026
Merged

fix(review): bound theme storage writes#4177
JSONbored merged 1 commit into
mainfrom
codex/fix-vulnerability-in-themestoragekey-usage

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The new theme-storage write path executed page.evaluate without a local timeout, allowing a hostile page to monkey-patch localStorage.setItem and hang screenshot/scroll-render workers (availability DoS).
  • The intent is to preserve the theme-forcing fallback behavior while preventing an attacker-controlled page-realm from blocking the renderer indefinitely.

Description

  • Add a bounded helper forceThemeStorage(page, storageKey, storageValue) that races the page.evaluate localStorage write against a 2s timeout and logs render_theme_storage_write_timeout when it times out.
  • Wire the helper into both captureShot and captureScrollFrames so the code degrades (returns null frames/png) instead of awaiting a potentially stalled evaluate before page.reload.
  • Add the THEME_STORAGE_WRITE_TIMEOUT_MS constant and broaden the ScreenshotPage.evaluate type signature to accept argumented callbacks, and include regression tests exercising the timed-out write path for both capture modes.

Testing

  • Ran unit tests with npx vitest run test/unit/visual-shot.test.ts; all tests passed (58/58) and the new regression tests exercised the timeout behavior.
  • Ran npm run typecheck and git diff --check, both succeeded locally.
  • npm audit --audit-level=moderate could not complete in this environment (registry returned 403).
  • Attempted npm run test:coverage ... but the coverage reporter crashed with TypeError: jsTokens is not a function despite the tests themselves passing; the functional behavior guarded by the new tests is validated.

Codex Task

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.72%. Comparing base (97f087b) to head (b570ec7).
⚠️ Report is 24 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4177   +/-   ##
=======================================
  Coverage   93.72%   93.72%           
=======================================
  Files         385      385           
  Lines       36243    36249    +6     
  Branches    13281    13284    +3     
=======================================
+ Hits        33968    33974    +6     
  Misses       1618     1618           
  Partials      657      657           
Files with missing lines Coverage Δ
src/review/visual/shot.ts 92.82% <100.00%> (+0.21%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 8, 2026
@JSONbored JSONbored self-assigned this Jul 8, 2026
@loopover-orb

loopover-orb Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Important

🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪

🔍 Gittensory is reviewing…

AI analysis is in progress. This comment will update when the review is complete.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟪 Reviewing

@JSONbored
JSONbored merged commit 7794970 into main Jul 8, 2026
10 checks passed
@JSONbored
JSONbored deleted the codex/fix-vulnerability-in-themestoragekey-usage branch July 8, 2026 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Development

Successfully merging this pull request may close these issues.

1 participant