Skip to content

Migrate site to Vite + React 19 + Tailwind v4#17

Open
itsthisjustin wants to merge 5 commits into
masterfrom
feat-migrate-to-react
Open

Migrate site to Vite + React 19 + Tailwind v4#17
itsthisjustin wants to merge 5 commits into
masterfrom
feat-migrate-to-react

Conversation

@itsthisjustin

@itsthisjustin itsthisjustin commented Jul 14, 2026

Copy link
Copy Markdown
Contributor
  • Replace Tailwind-CDN static HTML pages with a React SPA (React Router 7) built by Vite with @cloudflare/vite-plugin; worker moved to worker/

  • All 12 pages ported as routes with copy, images, and behavior preserved (WebSerial flasher, font builder, theme builder, debug console, admin, insider/login, kosync, sticky, unlocker, docs, roadmap)

  • Shared design system: brand-green tokens on Free-Ink patterns shared Header/Footer/Layout/Modal components - Legacy .html URLs 301-redirect to SPA routes; SPA fallback enabled

julie-is-late and others added 2 commits July 14, 2026 04:57
…esign language

- Replace Tailwind-CDN static HTML pages with a React SPA (React Router 7)
  built by Vite with @cloudflare/vite-plugin; worker moved to worker/
- All 12 pages ported as routes with copy, images, and behavior preserved
  (WebSerial flasher, font builder, theme builder, debug console, admin,
  insider/login, kosync, sticky, unlocker, docs, roadmap)
- Shared design system: brand-green tokens on Free-Ink patterns (Space
  Grotesk display, IBM Plex Mono labels, dot-field/blueprint textures),
  shared Header/Footer/Layout/Modal components
- Legacy .html URLs 301-redirect to SPA routes; SPA fallback enabled
@itsthisjustin itsthisjustin requested a review from uxjulia July 14, 2026 08:59
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 14, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
❌ Deployment failed
View logs
crosspoint-tools a043c0f Jul 14 2026, 02:22 PM

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The PR migrates CrossPoint Reader to a Vite-powered React SPA with routed pages, shared layout components, device flashing tools, documentation and font builders, an admin dashboard, and updated Cloudflare Worker asset handling. Legacy static HTML pages and JavaScript modules are removed.

Changes

React SPA migration

Layer / File(s) Summary
SPA runtime and shared foundation
index.html, src/main.jsx, src/App.jsx, src/components/*, src/index.css, vite.config.js, package.json, worker/*, wrangler.jsonc
Vite, React Router, shared layout components, global styling, Worker routing, typed Worker contracts, and webhook helpers are added.
Site shell and homepage flows
src/pages/HomePage.jsx, src/pages/home/*, src/components/DownloadModal.jsx, src/components/BuyModal.jsx
Homepage sections, responsive navigation, firmware downloads, purchase links, banners, and shared markdown rendering are implemented.
Flashing and device diagnostics
src/lib/esptool.bundle.js, src/lib/flasher.js, src/pages/home/FlashTools.jsx, src/pages/DebugPage.jsx, src/pages/StickyPage.jsx, src/pages/InsiderPage.jsx
WebSerial transport, ESP flashing support, firmware installation flows, Sticky flashing, Insider builds, and diagnostic operations are implemented.
Content and management pages
src/pages/DocsPage.jsx, src/pages/FontsPage.jsx, src/pages/ThemeBuilderPage.jsx, src/pages/AdminPage.jsx, src/pages/LoginPage.jsx, src/pages/KosyncPage.jsx, src/pages/RoadmapPage.jsx, src/pages/UnlockerPage.jsx
Documentation, font building, theme editing, authentication, KOReader registration, roadmap, unlocker, and admin workflows are migrated to React.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

Suggested reviewers: uxjulia

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main migration to a Vite + React + Tailwind setup.
Description check ✅ Passed The description is directly related and accurately summarizes the SPA migration and route/component work.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch feat-migrate-to-react

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@itsthisjustin

Copy link
Copy Markdown
Contributor Author

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
❌ Deployment failed
View logs crosspoint-tools a0af79c Jul 14 2026, 08:58 AM

What to change in Workers & Pages → crosspoint-tools → Settings → Builds:

Build command: npm run build - Deploy command: leave as npx wrangler deploy (it finds the generated config via the redirect file)

Introduce short labels for navigation items that display on medium viewports, with full labels shown on extra-large screens. Add NavLabel component to handle label switching and make FundButton text responsive ("Fund" on mobile, "Fund CrossPoint" on larger screens). Adjust spacing and add whitespace-nowrap to prevent text wrapping.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 19

🧹 Nitpick comments (9)
src/pages/LoginPage.jsx (1)

49-75: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add a timeout to the magic-link fetch.

handleSubmit has no timeout/AbortController around the fetch; a hung request leaves sending true indefinitely and the submit button permanently disabled with no feedback to the user.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/LoginPage.jsx` around lines 49 - 75, Update handleSubmit’s
magic-link fetch to use an AbortController with a timeout, aborting requests
that exceed the chosen limit and surfacing the existing failure status to the
user. Ensure cleanup clears the timeout and setSending(false) runs even when the
request hangs or is aborted.
src/components/SiteBanner.jsx (1)

5-17: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Replace dangerouslySetInnerHTML with real React elements.

The escape-then-linkify ordering currently prevents attribute-breakout and javascript: URLs, but it's a fragile, hand-rolled pattern flagged by static analysis for both the manual entity-escaping and the dangerouslySetInnerHTML sink. A future edit to this parsing logic could easily reopen an XSS hole with no compiler/test safety net. Since the only need is to render plain text plus a handful of <a> links, build an array of React nodes instead of an HTML string, removing the sink entirely.

♻️ Proposed refactor
-function escapeHtml(s) {
-  return String(s).replace(/[&<>"']/g, (c) => ({ '&': '&amp;', '<': '&lt;', '>': '&gt;', '"': '&quot;', "'": '&`#39`;' }[c]))
-}
-
-// Minimal markdown: [label](url) links plus bare https:// URLs.
-function renderBannerHtml(text) {
-  let html = escapeHtml(text)
-  html = html.replace(/\[([^\]]+)\]\((https?:\/\/[^\s)]+|\/[^\s)]*)\)/g,
-    (_, label, url) => `<a href="${url}" class="${LINK_CLASS}">${label}</a>`)
-  html = html.replace(/(^|\s)(https?:\/\/[^\s<]+)/g,
-    (_, pre, url) => `${pre}<a href="${url}" class="${LINK_CLASS}">${url}</a>`)
-  return html
-}
+// Minimal markdown: [label](url) links plus bare https:// URLs, returned as React nodes.
+function renderBannerNodes(text) {
+  const linkRe = /\[([^\]]+)\]\((https?:\/\/[^\s)]+|\/[^\s)]*)\)|(^|\s)(https?:\/\/[^\s<]+)/g
+  const nodes = []
+  let last = 0
+  let match
+  while ((match = linkRe.exec(text))) {
+    if (match.index > last) nodes.push(text.slice(last, match.index))
+    if (match[1]) {
+      nodes.push(<a key={nodes.length} href={match[2]} className={LINK_CLASS}>{match[1]}</a>)
+    } else {
+      if (match[3]) nodes.push(match[3])
+      nodes.push(<a key={nodes.length} href={match[4]} className={LINK_CLASS}>{match[4]}</a>)
+    }
+    last = linkRe.lastIndex
+  }
+  if (last < text.length) nodes.push(text.slice(last))
+  return nodes
+}
-  const [html, setHtml] = useState('')
+  const [nodes, setNodes] = useState(null)

   useEffect(() => {
     let cancelled = false
     fetch('/api/banner')
       .then((res) => (res.ok ? res.json() : null))
       .then((banner) => {
-        if (!cancelled && banner?.enabled && banner?.text) setHtml(renderBannerHtml(banner.text))
+        if (!cancelled && banner?.enabled && banner?.text) setNodes(renderBannerNodes(banner.text))
       })
       .catch(() => {})
     return () => {
       cancelled = true
     }
   }, [])

-  if (!html) return null
+  if (!nodes) return null
   return (
-    <div
-      className="bg-brand-600 px-6 py-2.5 text-center text-sm font-medium text-white"
-      dangerouslySetInnerHTML={{ __html: html }}
-    />
+    <div className="bg-brand-600 px-6 py-2.5 text-center text-sm font-medium text-white">
+      {nodes}
+    </div>
   )

Also applies to: 38-41

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/SiteBanner.jsx` around lines 5 - 17, Refactor renderBannerHtml
in SiteBanner.jsx to return React nodes rather than an escaped HTML string, and
replace the associated dangerouslySetInnerHTML usage with direct child
rendering. Preserve support for markdown-style links and bare https:// URLs,
allow only http(s) or relative link targets, and render unmatched content as
plain text without manual entity escaping.

Source: Linters/SAST tools

package.json (1)

22-30: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

vite pinned to a version now two majors behind and past its security-patch window.

Vite has since released v7 (June 2025) and v8 (~Dec 2025/Jan 2026); per Vite's own support policy only the latest two majors receive security patches, so ^6.0.0 is now unsupported for patches. Worth bumping and confirming @cloudflare/vite-plugin@^1.40.0's supported Vite range before upgrading.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@package.json` around lines 22 - 30, Update the devDependency version for vite
from ^6.0.0 to a currently supported major, and verify that
`@cloudflare/vite-plugin` remains compatible with the selected Vite version before
finalizing the dependency change.
src/index.css (1)

4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Stylelint config doesn't recognize Tailwind v4's @theme/@utility at-rules.

These are correct Tailwind v4 CSS-first directives, but the current stylelint setup (SCSS-oriented at-rule-no-unknown) flags them as unknown, producing recurring false-positive noise. Worth adding a Tailwind-aware stylelint config/allowlist for these at-rules.

Also applies to: 35-60

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/index.css` at line 4, Update the Stylelint configuration’s at-rule
validation to recognize Tailwind v4 CSS-first directives, specifically `@theme`
and `@utility`, so these rules are not reported as unknown. Preserve the existing
SCSS at-rule checks and add the Tailwind directives through the project’s
established configuration or allowlist mechanism.

Source: Linters/SAST tools

src/pages/fonts/fontBuilder.js (1)

120-137: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Consider bundling JSZip via npm instead of runtime CDN injection.

loadJSZip injects a <script> tag pointing at jsDelivr with no integrity/crossorigin attribute. Now that the project has real npm/Vite bundling, importing jszip statically avoids the unpinned third-party script-injection trust boundary (and any future CSP friction) entirely.

♻️ Suggested approach
-let jszipPromise = null
-export function loadJSZip() {
-  if (typeof window !== 'undefined' && window.JSZip) return Promise.resolve(window.JSZip)
-  if (!jszipPromise) {
-    jszipPromise = new Promise((resolve) => {
-      const s = document.createElement('script')
-      s.src = 'https://cdn.jsdelivr.net/npm/jszip@3.10.1/dist/jszip.min.js'
-      s.defer = true
-      s.onload = () => resolve(window.JSZip || null)
-      s.onerror = () => resolve(null)
-      document.head.appendChild(s)
-    })
-  }
-  return jszipPromise
-}
+import JSZip from 'jszip'
+
+export function loadJSZip() {
+  return Promise.resolve(JSZip)
+}

Requires adding "jszip": "^3.10.1" to package.json dependencies (the caller in FontsPage.jsx already falls back to individual downloads if loadJSZip() resolves falsy, so the fallback path stays intact if you'd rather keep the graceful-degradation behavior with a try/catch around the static import instead).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/fonts/fontBuilder.js` around lines 120 - 137, Replace the runtime
CDN script injection in loadJSZip with the project-bundled jszip dependency,
adding jszip at the required version and importing it through the existing Vite
module flow. Preserve loadJSZip’s promise-based API and its caller’s graceful
fallback behavior, including resolving a falsy value if the bundled module
cannot be loaded.
src/components/Modal.jsx (1)

4-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Modal lacks initial focus and aria-labelledby.

No focus is moved into the dialog on open (keyboard users can Tab straight through into background content) and the dialog isn't programmatically associated with its title for screen readers.

♻️ Proposed fix
-import { useEffect } from 'react'
+import { useEffect, useRef } from 'react'

 export default function Modal({ open, onClose, title, children }) {
+  const dialogRef = useRef(null)
   useEffect(() => {
     if (!open) return
+    dialogRef.current?.focus()
     const onKey = (e) => {
       if (e.key === 'Escape') onClose()
     }
     ...
   }, [open, onClose])

   if (!open) return null
   return (
     <div className="fixed inset-0 z-[100] flex items-center justify-center px-4 py-8" role="dialog" aria-modal="true">
       <div className="absolute inset-0 bg-stone-900/40 backdrop-blur-sm" onClick={onClose} />
-      <div className="relative max-h-[calc(100vh-4rem)] w-full max-w-lg overflow-y-auto rounded-2xl bg-white shadow-xl ring-1 ring-stone-950/5">
+      <div
+        ref={dialogRef}
+        tabIndex={-1}
+        aria-labelledby="modal-title"
+        className="relative max-h-[calc(100vh-4rem)] w-full max-w-lg overflow-y-auto rounded-2xl bg-white shadow-xl ring-1 ring-stone-950/5"
+      >
         <div className="flex items-start justify-between gap-4 border-b border-stone-100 px-6 py-4">
-          <h2 className="font-display text-xl font-semibold tracking-tight text-stone-900">{title}</h2>
+          <h2 id="modal-title" className="font-display text-xl font-semibold tracking-tight text-stone-900">{title}</h2>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/Modal.jsx` around lines 4 - 38, Update the Modal component to
move focus into the dialog when it opens, using a ref on the dialog container
and focusing it after render while preserving normal cleanup behavior. Add a
stable id to the title rendered by the h2 and set the dialog’s aria-labelledby
to that id so assistive technologies associate the dialog with its title.
src/pages/AdminPage.jsx (1)

156-209: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Adopt readJsonResponse/describeFailure consistently instead of raw res.json().

These helpers (already imported at line 5) were specifically written to tolerate non-JSON error bodies (edge/WAF challenge pages, per the comment in admin/api.js), but only StickyCard uses them. TriggerBuildCard.triggerBuild, BannerCard.saveBanner, and BetaCard.uploadBeta/saveBetaEdit still call res.json() directly (or .catch(() => ({}))), which surfaces a generic "Connection error" or a raw parse-error message instead of the more informative describeFailure output when the server returns a non-JSON error page.

♻️ Example fix for TriggerBuildCard
       const res = await fetch('/api/build/trigger', {
         method: 'POST',
         headers: { Authorization: `Bearer ${secret}` },
       })
-      const data = await res.json()
+      const r = await readJsonResponse(res)
 
-      if (res.ok) {
-        setResult({ kind: 'ok', text: data.commit })
-        log(`Build triggered: ${data.commit}`)
+      if (r.ok) {
+        setResult({ kind: 'ok', text: r.data?.commit })
+        log(`Build triggered: ${r.data?.commit}`)
         setTimeout(() => refreshRef.current?.(), 3000)
       } else {
-        setResult({ kind: 'error', text: data.error })
-        log(`Trigger failed: ${data.error}`)
+        setResult({ kind: 'error', text: describeFailure(r) })
+        log(`Trigger failed: ${describeFailure(r)}`)
       }

Also applies to: 213-263, 336-392, 429-469

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/AdminPage.jsx` around lines 156 - 209, Update
TriggerBuildCard.triggerBuild, BannerCard.saveBanner, and
BetaCard.uploadBeta/saveBetaEdit to use the imported readJsonResponse and
describeFailure helpers instead of direct res.json() calls or parse-error
fallbacks. Route non-OK responses through describeFailure so non-JSON server
bodies produce the informative error text, while preserving existing success
handling and UI state updates.
src/pages/InsiderPage.jsx (1)

126-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace blocking alert() validation with the page's existing inline banners.

handleFlashNightly/handleCfFlash (and the catch blocks at lines 346 and 420) use native alert() for a very reachable user mistake (no device model selected, or a flasher error), while the rest of the page already has styled inline error/warning banners. alert() is jarring, blocks the JS thread, and is inconsistent with the rest of the UX. Consider disabling the flash buttons until deviceModel is set, or rendering an inline message instead.

Also applies to: 363-375

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/InsiderPage.jsx` around lines 126 - 138, Replace the native alert
calls in handleFlashNightly, handleCfFlash, and the referenced flasher-error
catch blocks with the page’s existing inline error or warning banner state.
Render the no-device-model validation and non-NotFoundError messages through
that banner mechanism, and preserve the existing early returns and silent
handling of NotFoundError.
src/pages/ThemeBuilderPage.jsx (1)

19-23: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add unmount cleanup for the theme builder Return a teardown from useEffect to clear the icon-build polling timer; leaving the page mid-build can keep background polling/fetches alive.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/ThemeBuilderPage.jsx` around lines 19 - 23, Update the useEffect
that calls initThemeBuilder to return an unmount cleanup function that invokes
the theme builder’s existing teardown mechanism for the icon-build polling timer
and related fetch activity. Preserve the document title assignment and one-time
initialization behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/App.jsx`:
- Around line 35-48: Update ScrollManager so hash navigation retries locating
the element after lazy-loaded route content mounts instead of immediately
falling back to window.scrollTo(0, 0) when getElementById returns null. Use a
bounded or cancellable retry mechanism, clean it up when pathname or hash
changes/unmounts, and preserve immediate scrolling when the target already
exists.

In `@src/components/DownloadModal.jsx`:
- Around line 48-69: Update the catalog-fetch error path in the useEffect for
DownloadModal so failed requests do not leave catalog set to a truthy value that
blocks future retries. Preserve setLoadError(err.message) and ensure closing and
reopening the modal can trigger /api/catalog again after a transient failure.

In `@src/lib/esptool.bundle.js`:
- Around line 7082-7092: In the detecting branch of detectChip(), validate the
local chip result returned by magic2Chip(chipMagicValue) before assigning it to
this.chip. Throw the existing ESPError for a null/unsupported result, and only
set this.chip when chip is valid.

In `@src/lib/theme-builder.js`:
- Around line 2294-2303: Update initThemeBuilder and the icon-build polling flow
around pollIconBuild so unmounting the theme-builder page cancels any in-flight
polling and prevents further /api/theme-build/status requests. Return or expose
a cleanup function that clears iconPollTimer and invokes it from the page
teardown path, while preserving the existing per-canvas initialization guard.

In `@src/pages/AdminPage.jsx`:
- Line 1008: Remove the client-side persistence of the admin secret in the
AdminPage session-storage flow, specifically the sessionStorage.setItem call
using STORAGE_KEY. Replace this bearer-token storage with an HttpOnly, Secure,
SameSite cookie issued and managed by the worker, updating the associated
authentication flow to use that cookie without exposing the secret to page
JavaScript.
- Around line 980-1013: Update authenticate() to reject every non-2xx response
from /api/build/trigger before calling setSecret or setAuthed, while preserving
the existing “Invalid secret” message for HTTP 401. For other failed statuses,
set an appropriate connection/request error, clear auth busy state, and return;
only allow the existing authenticated flow when res.ok is true.

In `@src/pages/DocsPage.jsx`:
- Around line 15-44: Add an afterSanitizeAttributes hook to the marked
configuration in DocsPage.jsx that detects anchor elements with target="_blank"
and guarantees rel includes both noopener and noreferrer, while preserving any
existing rel values. Apply this to the HTML sanitization flow used for rendered
docs content.

In `@src/pages/FontsPage.jsx`:
- Around line 66-71: Remove the familyNameRef declaration and the unconditional
familyNameRef.current assignment from the FontsPage component; update
downloadAll and any other references to use the current familyName value from
its render closure instead, including the additional usage noted near lines
328-336.
- Around line 407-647: Add htmlFor attributes to every file-input label in the
FontsPage JSX, matching each label to its corresponding input id: fontFolder,
fontRegular, fontBold, fontItalic, fontBoldItalic, fallbackFontFolder,
fallbackFontRegular, fallback2FontFolder, and fallback2FontRegular. Preserve the
existing labels and input behavior.

In `@src/pages/home/FlashTools.jsx`:
- Around line 286-334: Move the setRestart call from the initial setup into the
successful completion path after flasher.flashFirmware resolves. Keep it unset
or false while firmware fetching/flashing is in progress and when the try block
fails, and preserve the existing unplug condition based on skipReset and model
!== 'x4'.

In `@src/pages/home/markdown.js`:
- Around line 14-18: Update safeUrl so the root-relative URL branch accepts a
single leading slash but rejects protocol-relative values beginning with “//”.
Preserve the existing handling for http(s), hash, and mailto URLs, and continue
returning “#” for disallowed inputs.

In `@src/pages/insider/buildShared.jsx`:
- Around line 30-35: Update cleanCommitMessage to handle nullish or missing
message values before calling split, returning a safe empty or established
fallback string while preserving the current formatting behavior for valid
messages.

In `@src/pages/InsiderPage.jsx`:
- Around line 205-231: Re-check cfPollingRef.current immediately after each
fetchCustomBuildStatus() await in both poll() and the visibilitychange handler,
returning without applying results when polling was cleared during the request.
Ensure stale responses cannot call showCustomBuildReady, setCfError, or
setCfState after clearCustomBuild().
- Around line 88-94: Update loadBuildInfo in InsiderPage to wrap fetchBuildMeta
and the success state updates in try/catch/finally, ensuring setLoading(false)
always executes when the request rejects so the refresh UI remains available.
Preserve the existing metadata reset and successful setMeta behavior, and follow
the sibling effect’s established error-handling approach where applicable.
- Around line 733-752: Update the size input handler in the sizes.map callback
to prevent empty or invalid values from storing NaN in cfSizes. Parse the
entered value and retain the previous valid size, or apply an appropriate
existing fallback, when parsing fails; preserve valid numeric updates so
handleCfBuild receives only usable font sizes.

In `@src/pages/LoginPage.jsx`:
- Around line 41-47: Update toggleLogin so it derives the next visibility value
from the current showLogin state, schedules the emailRef focus outside the
setShowLogin updater when opening the login view, and then applies the next
value without side effects inside the updater.

In `@src/pages/StickyPage.jsx`:
- Around line 16-51: Update safeUrl() to reject protocol-relative URLs beginning
with //, and add attribute-safe escaping for URLs before interpolating them into
href values in renderMarkdown(). Apply this to both markdown links and bare URL
links, preserving the existing allowed schemes and rendering behavior; reuse the
established marked/DOMPurify pipeline only if it is already available for this
page.

In `@worker/types.ts`:
- Line 9: Restrict ALLOW_INSECURE_DEV_WEBHOOKS to development or preview
environments in isAuthorizedWebhookRequest, ensuring production cannot disable
webhook signature validation. Alternatively, remove this variable from
production bindings while preserving its intended dev-only behavior.

In `@worker/webhook.ts`:
- Around line 3-25: Update verifyGitHubSignature to use crypto.subtle.verify()
with the imported HMAC key and decoded x-hub-signature-256 value instead of
constructing expected and comparing with ===. Preserve the existing sha256=
prefix validation and return the request body alongside the verification result.

---

Nitpick comments:
In `@package.json`:
- Around line 22-30: Update the devDependency version for vite from ^6.0.0 to a
currently supported major, and verify that `@cloudflare/vite-plugin` remains
compatible with the selected Vite version before finalizing the dependency
change.

In `@src/components/Modal.jsx`:
- Around line 4-38: Update the Modal component to move focus into the dialog
when it opens, using a ref on the dialog container and focusing it after render
while preserving normal cleanup behavior. Add a stable id to the title rendered
by the h2 and set the dialog’s aria-labelledby to that id so assistive
technologies associate the dialog with its title.

In `@src/components/SiteBanner.jsx`:
- Around line 5-17: Refactor renderBannerHtml in SiteBanner.jsx to return React
nodes rather than an escaped HTML string, and replace the associated
dangerouslySetInnerHTML usage with direct child rendering. Preserve support for
markdown-style links and bare https:// URLs, allow only http(s) or relative link
targets, and render unmatched content as plain text without manual entity
escaping.

In `@src/index.css`:
- Line 4: Update the Stylelint configuration’s at-rule validation to recognize
Tailwind v4 CSS-first directives, specifically `@theme` and `@utility`, so these
rules are not reported as unknown. Preserve the existing SCSS at-rule checks and
add the Tailwind directives through the project’s established configuration or
allowlist mechanism.

In `@src/pages/AdminPage.jsx`:
- Around line 156-209: Update TriggerBuildCard.triggerBuild,
BannerCard.saveBanner, and BetaCard.uploadBeta/saveBetaEdit to use the imported
readJsonResponse and describeFailure helpers instead of direct res.json() calls
or parse-error fallbacks. Route non-OK responses through describeFailure so
non-JSON server bodies produce the informative error text, while preserving
existing success handling and UI state updates.

In `@src/pages/fonts/fontBuilder.js`:
- Around line 120-137: Replace the runtime CDN script injection in loadJSZip
with the project-bundled jszip dependency, adding jszip at the required version
and importing it through the existing Vite module flow. Preserve loadJSZip’s
promise-based API and its caller’s graceful fallback behavior, including
resolving a falsy value if the bundled module cannot be loaded.

In `@src/pages/InsiderPage.jsx`:
- Around line 126-138: Replace the native alert calls in handleFlashNightly,
handleCfFlash, and the referenced flasher-error catch blocks with the page’s
existing inline error or warning banner state. Render the no-device-model
validation and non-NotFoundError messages through that banner mechanism, and
preserve the existing early returns and silent handling of NotFoundError.

In `@src/pages/LoginPage.jsx`:
- Around line 49-75: Update handleSubmit’s magic-link fetch to use an
AbortController with a timeout, aborting requests that exceed the chosen limit
and surfacing the existing failure status to the user. Ensure cleanup clears the
timeout and setSending(false) runs even when the request hangs or is aborted.

In `@src/pages/ThemeBuilderPage.jsx`:
- Around line 19-23: Update the useEffect that calls initThemeBuilder to return
an unmount cleanup function that invokes the theme builder’s existing teardown
mechanism for the icon-build polling timer and related fetch activity. Preserve
the document title assignment and one-time initialization behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: df1c3835-82db-42f4-87b5-9e12820539a8

📥 Commits

Reviewing files that changed from the base of the PR and between e341837 and a0af79c.

⛔ Files ignored due to path filters (2)
  • package-lock.json is excluded by !**/package-lock.json
  • public/screenshots/hero-home.png is excluded by !**/*.png
📒 Files selected for processing (63)
  • .gitignore
  • index.html
  • package.json
  • public/admin.html
  • public/debug.html
  • public/docs.html
  • public/fonts.html
  • public/index.html
  • public/insider.html
  • public/js/buy-modal.js
  • public/js/download-modal.js
  • public/js/site-nav.js
  • public/kosync.html
  • public/login.html
  • public/roadmap.html
  • public/sticky.html
  • public/theme-builder.html
  • public/unlocker.html
  • scripts/test-theme-builder.cjs
  • src/App.jsx
  • src/components/BuyModal.jsx
  • src/components/DownloadModal.jsx
  • src/components/Footer.jsx
  • src/components/Header.jsx
  • src/components/Layout.jsx
  • src/components/Modal.jsx
  • src/components/SiteBanner.jsx
  • src/components/ui.jsx
  • src/index.css
  • src/lib/esptool.bundle.js
  • src/lib/flasher.js
  • src/lib/theme-builder.js
  • src/main.jsx
  • src/pages/AdminPage.jsx
  • src/pages/DebugPage.jsx
  • src/pages/DocsPage.jsx
  • src/pages/FontsPage.jsx
  • src/pages/HomePage.jsx
  • src/pages/InsiderPage.jsx
  • src/pages/KosyncPage.jsx
  • src/pages/LoginPage.jsx
  • src/pages/RoadmapPage.jsx
  • src/pages/StickyPage.jsx
  • src/pages/ThemeBuilderPage.jsx
  • src/pages/UnlockerPage.jsx
  • src/pages/admin/api.js
  • src/pages/debug/helpers.js
  • src/pages/docs/docs-prose.css
  • src/pages/fonts/fontBuilder.js
  • src/pages/home/Community.jsx
  • src/pages/home/Features.jsx
  • src/pages/home/FlashTools.jsx
  • src/pages/home/GetInTouch.jsx
  • src/pages/home/Hero.jsx
  • src/pages/home/UnlockSection.jsx
  • src/pages/home/markdown.js
  • src/pages/insider/buildShared.jsx
  • tsconfig.json
  • vite.config.js
  • worker/index.ts
  • worker/types.ts
  • worker/webhook.ts
  • wrangler.jsonc
💤 Files with no reviewable changes (15)
  • public/js/site-nav.js
  • public/sticky.html
  • public/theme-builder.html
  • public/js/buy-modal.js
  • public/index.html
  • public/js/download-modal.js
  • public/unlocker.html
  • public/docs.html
  • public/roadmap.html
  • public/kosync.html
  • public/debug.html
  • public/login.html
  • public/insider.html
  • public/fonts.html
  • public/admin.html

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

🛑 Comments failed to post (19)
src/App.jsx (1)

35-48: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Hash-anchor scroll races with lazy-loaded routes.

ScrollManager lives outside the <Suspense> boundary, so its effect runs as soon as the route changes, not once the lazy page has actually mounted. On a first visit to a lazy route with a hash (e.g. /roadmap#scope, or #unlock-tool on the home page while it's still loading), document.getElementById(hash.slice(1)) returns null because the target isn't in the DOM yet, and the code silently falls back to window.scrollTo(0, 0) — defeating the very behavior the comment above it promises.

🩹 Proposed fix: retry until the target mounts
 function ScrollManager() {
   const { pathname, hash } = useLocation()
   useEffect(() => {
     if (hash) {
-      const el = document.getElementById(hash.slice(1))
-      if (el) {
-        el.scrollIntoView()
-        return
-      }
+      const id = hash.slice(1)
+      let attempts = 0
+      const tryScroll = () => {
+        const el = document.getElementById(id)
+        if (el) {
+          el.scrollIntoView()
+        } else if (attempts++ < 30) {
+          requestAnimationFrame(tryScroll)
+        }
+      }
+      tryScroll()
+      return
     }
     window.scrollTo(0, 0)
   }, [pathname, hash])
   return null
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

function ScrollManager() {
  const { pathname, hash } = useLocation()
  useEffect(() => {
    if (hash) {
      const id = hash.slice(1)
      let attempts = 0
      const tryScroll = () => {
        const el = document.getElementById(id)
        if (el) {
          el.scrollIntoView()
        } else if (attempts++ < 30) {
          requestAnimationFrame(tryScroll)
        }
      }
      tryScroll()
      return
    }
    window.scrollTo(0, 0)
  }, [pathname, hash])
  return null
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/App.jsx` around lines 35 - 48, Update ScrollManager so hash navigation
retries locating the element after lazy-loaded route content mounts instead of
immediately falling back to window.scrollTo(0, 0) when getElementById returns
null. Use a bounded or cancellable retry mechanism, clean it up when pathname or
hash changes/unmounts, and preserve immediate scrolling when the target already
exists.
src/components/DownloadModal.jsx (1)

48-69: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Failed catalog fetch permanently blocks retries for the session.

On error, catalog is set to { releases: [] } (truthy), which forever satisfies the if (!open || catalog) return guard — closing and reopening the modal never retries the fetch. A single transient network blip permanently degrades the download feature until a full page reload.

🔧 Proposed fix
+  // Allow a fresh attempt the next time the modal is reopened after a failed load.
+  useEffect(() => {
+    if (!open && loadError) {
+      setCatalog(null)
+      setLoadError(null)
+    }
+  }, [open, loadError])
+
   useEffect(() => {
     if (!open || catalog) return
     let cancelled = false
     fetch('/api/catalog')
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  // Allow a fresh attempt the next time the modal is reopened after a failed load.
  useEffect(() => {
    if (!open && loadError) {
      setCatalog(null)
      setLoadError(null)
    }
  }, [open, loadError])

  useEffect(() => {
    if (!open || catalog) return
    let cancelled = false
    fetch('/api/catalog')
      .then((res) => {
        if (!res.ok) throw new Error(`Catalog request failed: ${res.status}`)
        return res.json()
      })
      .then((data) => {
        if (!cancelled) setCatalog(data)
      })
      .catch((err) => {
        console.error(err)
        if (!cancelled) {
          setCatalog({ releases: [] })
          setLoadError(err.message)
        }
      })
    return () => {
      cancelled = true
    }
  }, [open, catalog])
🧰 Tools
🪛 ast-grep (0.44.1)

[warning] 56-56: Avoid using the initial state variable in setState
Context: setCatalog(data)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(setstate-same-var)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/DownloadModal.jsx` around lines 48 - 69, Update the
catalog-fetch error path in the useEffect for DownloadModal so failed requests
do not leave catalog set to a truthy value that blocks future retries. Preserve
setLoadError(err.message) and ensure closing and reopening the modal can trigger
/api/catalog again after a transient failure.
src/lib/esptool.bundle.js (1)

7082-7092: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n "esptool-js" package.json

Repository: crosspoint-reader/crosspoint-tools

Length of output: 172


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant section and nearby definitions.
sed -n '7068,7100p' src/lib/esptool.bundle.js

printf '\n---\n'

# Look for magic2Chip and chip initialization paths in the bundle.
rg -n "magic2Chip|this\.chip === null|this\.chip = chip|CHIP_DETECT_MAGIC_REG_ADDR" src/lib/esptool.bundle.js

printf '\n---\n'

# Look for any vendoring metadata in the repo.
git ls-files | rg '(^|/)(package\.json|package-lock\.json|pnpm-lock\.yaml|yarn\.lock|LICENSE|NOTICE|README)'

Repository: crosspoint-reader/crosspoint-tools

Length of output: 2150


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find where the chip field is initialized and how connect() reaches this branch.
rg -n "this\.chip\s*=|class .*EspTool|constructor\(" src/lib/esptool.bundle.js | head -n 80

printf '\n---\n'

# Print the connect() prologue and a bit of surrounding context.
sed -n '7000,7088p' src/lib/esptool.bundle.js

Repository: crosspoint-reader/crosspoint-tools

Length of output: 4223


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the constructor around the chip field initialization.
sed -n '6600,6665p' src/lib/esptool.bundle.js

printf '\n---\n'

# Show the chip-mapping helper.
sed -n '6535,6575p' src/lib/esptool.bundle.js

Repository: crosspoint-reader/crosspoint-tools

Length of output: 3492


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find every chip field reference in the bundle.
rg -n "this\.chip\b" src/lib/esptool.bundle.js

printf '\n---\n'

# Show the detectChip logic after connect().
sed -n '7090,7135p' src/lib/esptool.bundle.js

Repository: crosspoint-reader/crosspoint-tools

Length of output: 3726


Check the detected chip value here
this.chip is still unset on the first connect, so this guard never throws for an unsupported magic value. Check chip === null before assigning it; otherwise detectChip() falls through to unknown! and later dereferences this.chip.

Proposed fix
      const chip = await magic2Chip(chipMagicValue);
-      if (this.chip === null) {
+      if (chip === null) {
         throw new ESPError(`Unexpected CHIP magic value ${chipMagicValue}. Failed to autodetect chip type.`);
       } else {
         this.chip = chip;
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    if (detecting) {
      const chipMagicValue = await this.readReg(this.CHIP_DETECT_MAGIC_REG_ADDR) >>> 0;
      this.debug("Chip Magic " + chipMagicValue.toString(16));
      const chip = await magic2Chip(chipMagicValue);
      if (chip === null) {
        throw new ESPError(`Unexpected CHIP magic value ${chipMagicValue}. Failed to autodetect chip type.`);
      } else {
        this.chip = chip;
      }
    }
  }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/esptool.bundle.js` around lines 7082 - 7092, In the detecting branch
of detectChip(), validate the local chip result returned by
magic2Chip(chipMagicValue) before assigning it to this.chip. Throw the existing
ESPError for a null/unsupported result, and only set this.chip when chip is
valid.
src/lib/theme-builder.js (1)

2294-2303: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -A 8 "initThemeBuilder" src/pages/ThemeBuilderPage.jsx
rg -n -B2 -A 15 "function pollIconBuild" src/lib/theme-builder.js

Repository: crosspoint-reader/crosspoint-tools

Length of output: 1517


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant implementation around the poll and any teardown helpers.
sed -n '1540,1625p' src/lib/theme-builder.js
printf '\n----\n'
sed -n '2285,2315p' src/lib/theme-builder.js
printf '\n----\n'
sed -n '1,60p' src/pages/ThemeBuilderPage.jsx

# Also search for any destroy/cleanup export usage.
printf '\n---- SEARCH ----\n'
rg -n "destroyThemeBuilder|cleanup|tbInit|iconPollTimer|renderIconStatus" src/lib/theme-builder.js src/pages/ThemeBuilderPage.jsx

Repository: crosspoint-reader/crosspoint-tools

Length of output: 8656


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Programmatically inspect whether renderIconStatus touches potentially removed DOM,
# and whether any cleanup path exists from the page component.
python3 - <<'PY'
from pathlib import Path
import re

theme = Path('src/lib/theme-builder.js').read_text()
page = Path('src/pages/ThemeBuilderPage.jsx').read_text()

def extract(name):
    m = re.search(rf'function {name}\(\)\s*\{{', theme)
    if not m:
        return None
    start = m.start()
    depth = 0
    i = m.end() - 1
    while i < len(theme):
        ch = theme[i]
        if ch == '{':
            depth += 1
        elif ch == '}':
            depth -= 1
            if depth == 0:
                return theme[start:i+1]
        i += 1
    return None

for fn in ['renderIconStatus', 'pollIconBuild', 'initThemeBuilder']:
    body = extract(fn)
    print(f'=== {fn} ===')
    print(body[:2000] if body else 'NOT FOUND')
    print()

print('=== page cleanup mentions ===')
for pat in ['return () =>', 'destroyThemeBuilder', 'initThemeBuilder']:
    print(pat, pat in page)
PY

Repository: crosspoint-reader/crosspoint-tools

Length of output: 1850


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the exact call graph around initThemeBuilder and the page effect cleanup.
rg -n -A 6 -B 4 "useEffect\(\(\) => \{|initThemeBuilder\(|destroyThemeBuilder|pollIconBuild\(" src/pages/ThemeBuilderPage.jsx src/lib/theme-builder.js

Repository: crosspoint-reader/crosspoint-tools

Length of output: 5446


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1548,1610p' src/lib/theme-builder.js
printf '\n----\n'
sed -n '2288,2308p' src/lib/theme-builder.js
printf '\n----\n'
sed -n '1,40p' src/pages/ThemeBuilderPage.jsx

Repository: crosspoint-reader/crosspoint-tools

Length of output: 5296


Add cleanup for the icon-build poll timer

initThemeBuilder() only clears iconPollTimer on re-entry; it doesn’t stop an in-flight pollIconBuild() loop when /theme-builder unmounts. Add a cleanup path so the page stops polling /api/theme-build/status after navigation away.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/theme-builder.js` around lines 2294 - 2303, Update initThemeBuilder
and the icon-build polling flow around pollIconBuild so unmounting the
theme-builder page cancels any in-flight polling and prevents further
/api/theme-build/status requests. Return or expose a cleanup function that
clears iconPollTimer and invokes it from the page teardown path, while
preserving the existing per-canvas initialization guard.
src/pages/AdminPage.jsx (2)

980-1013: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

authenticate() only rejects on exactly HTTP 401 — any other failure status is treated as success.

If /api/build/trigger returns anything other than 401 or a 2xx (e.g. a 5xx from an upstream hiccup, a WAF/edge challenge, 429), the code skips straight to setSecret(value); setAuthed(true) without checking res.ok, granting the dashboard UI even though the secret was never actually confirmed valid. Every subsequent API call in the dashboard would then 401 independently, producing a confusing "logged in but nothing works" state instead of a clear auth error.

🐛 Proposed fix
       if (res.status === 401) {
         setAuthError('Invalid secret')
         setAuthBusy(false)
         return
       }
 
+      if (!res.ok) {
+        setAuthError(`Server error (${res.status})`)
+        setAuthBusy(false)
+        return
+      }
+
       const data = await res.json()
       setSecret(value)
       setAuthed(true)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  async function authenticate() {
    const value = (secretInputRef.current?.value || '').trim()
    if (!value) return

    setAuthBusy(true)
    setAuthError('')

    // Test the secret by hitting the build trigger endpoint; a 401 means the
    // secret is wrong. Auth success triggers a build as a side effect (that's ok).
    try {
      const res = await fetch('/api/build/trigger', {
        method: 'POST',
        headers: { Authorization: `Bearer ${value}` },
      })

      if (res.status === 401) {
        setAuthError('Invalid secret')
        setAuthBusy(false)
        return
      }

      if (!res.ok) {
        setAuthError(`Server error (${res.status})`)
        setAuthBusy(false)
        return
      }

      const data = await res.json()
      setSecret(value)
      setAuthed(true)
      log(`Authenticated. Build triggered: ${data.commit || 'unknown'}`)

      // Save to session. Cards load their own data (status, beta list,
      // sticky info, banner) when the dashboard mounts.
      sessionStorage.setItem(STORAGE_KEY, value)
    } catch {
      setAuthError('Connection error')
      setAuthBusy(false)
    }
  }
🧰 Tools
🪛 ast-grep (0.44.1)

[error] 983-983: React's useState should not be directly called
Context: setAuthBusy(true)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)


[error] 984-984: React's useState should not be directly called
Context: setAuthError('')
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)


[error] 995-995: React's useState should not be directly called
Context: setAuthError('Invalid secret')
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)


[error] 996-996: React's useState should not be directly called
Context: setAuthBusy(false)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)


[error] 1001-1001: React's useState should not be directly called
Context: setSecret(value)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)


[error] 1002-1002: React's useState should not be directly called
Context: setAuthed(true)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)


[error] 1009-1009: React's useState should not be directly called
Context: setAuthError('Connection error')
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)


[error] 1010-1010: React's useState should not be directly called
Context: setAuthBusy(false)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)


[warning] 1001-1001: Avoid using the initial state variable in setState
Context: setSecret(value)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(setstate-same-var)

🪛 React Doctor (0.7.6)

[error] 1008-1008: Storing an auth token in localStorage/sessionStorage exposes it to any XSS on the page: JavaScript can read web storage and exfiltrate the token. Keep tokens in an HttpOnly, Secure, SameSite cookie instead.

Don't persist auth tokens (JWTs, access/refresh tokens, secrets) in localStorage/sessionStorage; they're readable by any XSS. Use an HttpOnly cookie set by the server.

(auth-token-in-web-storage)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/AdminPage.jsx` around lines 980 - 1013, Update authenticate() to
reject every non-2xx response from /api/build/trigger before calling setSecret
or setAuthed, while preserving the existing “Invalid secret” message for HTTP
401. For other failed statuses, set an appropriate connection/request error,
clear auth busy state, and return; only allow the existing authenticated flow
when res.ok is true.

1008-1008: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Admin secret persisted in sessionStorage is readable by any XSS on the page.

Static analysis flags this correctly: any script injected via XSS anywhere on this origin can read sessionStorage and exfiltrate the admin secret, which grants firmware-build/upload control. This mirrors the legacy public/admin.html pattern (per the comment in admin/api.js), so it's likely not a regression from this migration, but it's worth tracking as a follow-up to move to an HttpOnly/Secure/SameSite cookie issued by the worker instead of a client-readable bearer token.

🧰 Tools
🪛 React Doctor (0.7.6)

[error] 1008-1008: Storing an auth token in localStorage/sessionStorage exposes it to any XSS on the page: JavaScript can read web storage and exfiltrate the token. Keep tokens in an HttpOnly, Secure, SameSite cookie instead.

Don't persist auth tokens (JWTs, access/refresh tokens, secrets) in localStorage/sessionStorage; they're readable by any XSS. Use an HttpOnly cookie set by the server.

(auth-token-in-web-storage)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/AdminPage.jsx` at line 1008, Remove the client-side persistence of
the admin secret in the AdminPage session-storage flow, specifically the
sessionStorage.setItem call using STORAGE_KEY. Replace this bearer-token storage
with an HttpOnly, Secure, SameSite cookie issued and managed by the worker,
updating the associated authentication flow to use that cookie without exposing
the secret to page JavaScript.

Source: Linters/SAST tools

src/pages/DocsPage.jsx (1)

15-44: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== Files ==\n'
git ls-files 'src/pages/DocsPage.jsx' 'src/pages/*' | sed -n '1,20p'

printf '\n== DocsPage.jsx (relevant slice) ==\n'
nl -ba src/pages/DocsPage.jsx | sed -n '1,220p'

printf '\n== Search DOMPurify / target / rel usage ==\n'
rg -n "DOMPurify|ADD_ATTR|target=|noopener|noreferrer|afterSanitizeAttributes|marked.parse|walkTokens" src -S

Repository: crosspoint-reader/crosspoint-tools

Length of output: 792


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== DocsPage.jsx (top 220 lines) ==\n'
sed -n '1,220p' src/pages/DocsPage.jsx

printf '\n== Search for raw HTML handling and link attributes ==\n'
rg -n "DOMPurify|ADD_ATTR|target|rel=|afterSanitizeAttributes|marked\.parse|sanitize\(" src/pages/DocsPage.jsx src -S

Repository: crosspoint-reader/crosspoint-tools

Length of output: 25112


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,220p' src/pages/DocsPage.jsx
rg -n "DOMPurify|ADD_ATTR|target|rel=|afterSanitizeAttributes|marked\.parse|sanitize\(" src/pages/DocsPage.jsx src -S

Repository: crosspoint-reader/crosspoint-tools

Length of output: 25020


Prevent reverse tabnabbing in docs links ADD_ATTR: ['id', 'target'] lets raw docs HTML keep target="_blank" without a guaranteed rel="noopener noreferrer". Since this page renders content from the public docs repo, a compromised docs PR can inject a link that can control the opener tab. Add an afterSanitizeAttributes hook to set rel on any <a target="_blank">.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/DocsPage.jsx` around lines 15 - 44, Add an afterSanitizeAttributes
hook to the marked configuration in DocsPage.jsx that detects anchor elements
with target="_blank" and guarantees rel includes both noopener and noreferrer,
while preserving any existing rel values. Apply this to the HTML sanitization
flow used for rendered docs content.
src/pages/FontsPage.jsx (2)

66-71: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Unnecessary ref, mutated during render.

familyNameRef.current = familyName runs unconditionally on every render (flagged by React Doctor's no-ref-current-in-render rule — writes to .current should happen in an effect/handler, not render). It's also unneeded here: downloadAll is a plain function recreated each render, so it already closes over the current familyName state value directly.

🐛 Proposed fix
   const pollTimerRef = useRef(null)
   const builtFilesRef = useRef([])
-  const familyNameRef = useRef('')
-  familyNameRef.current = familyName
   const logPanelRef = useRef(null)
       const fam = sanitizeFamilyName(familyNameRef.current)
+      const fam = sanitizeFamilyName(familyName)

Also applies to: 328-336

🧰 Tools
🪛 React Doctor (0.7.6)

[error] 69-69: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.

Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.

(no-ref-current-in-render)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/FontsPage.jsx` around lines 66 - 71, Remove the familyNameRef
declaration and the unconditional familyNameRef.current assignment from the
FontsPage component; update downloadAll and any other references to use the
current familyName value from its render closure instead, including the
additional usage noted near lines 328-336.

Source: Linters/SAST tools


407-647: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

File-input labels missing htmlFor (broken label/control association).

Every text-input label in this file correctly sets htmlFor matching the input's id (e.g. familyName, customIntervals, size${i}), but the file-input labels (fontFolder, fontRegular, fontBold, fontItalic, fontBoldItalic, fallbackFontFolder, fallbackFontRegular, fallback2FontFolder, fallback2FontRegular) omit it. Screen readers won't announce these labels when the corresponding input receives focus, and clicking the label text won't activate the file picker.

🛠️ Example fix (repeat for each file-input label)
-              <label className="block text-sm/6 font-medium text-stone-700">
+              <label htmlFor="fontFolder" className="block text-sm/6 font-medium text-stone-700">
                 Quick add from folder
               </label>
-                <label className="block text-sm/6 font-medium text-stone-700">
+                <label htmlFor="fontRegular" className="block text-sm/6 font-medium text-stone-700">
                   Regular <span className="text-red-500">*</span>
                 </label>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/FontsPage.jsx` around lines 407 - 647, Add htmlFor attributes to
every file-input label in the FontsPage JSX, matching each label to its
corresponding input id: fontFolder, fontRegular, fontBold, fontItalic,
fontBoldItalic, fallbackFontFolder, fallbackFontRegular, fallback2FontFolder,
and fallback2FontRegular. Preserve the existing labels and input behavior.
src/pages/home/FlashTools.jsx (1)

286-334: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Restart instructions shown before flashing succeeds.

setRestart(...) fires unconditionally before the firmware is even downloaded or flashed. Any failure in the try block (missing file, fetch error, flasher error) still leaves the "After flashing" unplug/reset card visible, misleading the user into power-cycling a device that was never flashed.

🐛 Proposed fix
     setRunning(true)
-    setRestart({ unplug: skipReset && model !== 'x4' })
     setPercent(0)
     setProgress({ title, steps, states: [...states], status: downloadMsg ? { kind: 'info', text: downloadMsg } : null })

     try {
       ...
       setProgress((p) => ({
         ...p,
         status: {
           kind: 'ok',
           text: skipReset
             ? 'Flash complete! Unplug and replug the USB cable to restart your device.'
             : 'Flash complete! Your device will restart with the new firmware.',
         },
       }))
+      setRestart({ unplug: skipReset && model !== 'x4' })
     } catch (err) {
       setProgress((p) => ({ ...p, status: { kind: 'err', text: err.message } }))
     }
🧰 Tools
🪛 ast-grep (0.44.1)

[error] 286-286: React's useState should not be directly called
Context: setRestart({ unplug: skipReset && model !== 'x4' })
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)


[error] 287-287: React's useState should not be directly called
Context: setPercent(0)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)


[error] 288-288: React's useState should not be directly called
Context: setProgress({ title, steps, states: [...states], status: downloadMsg ? { kind: 'info', text: downloadMsg } : null })
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)


[error] 307-307: React's useState should not be directly called
Context: setProgress((p) => ({ ...p, status: null }))
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)


[error] 321-329: React's useState should not be directly called
Context: setProgress((p) => ({
...p,
status: {
kind: 'ok',
text: skipReset
? 'Flash complete! Unplug and replug the USB cable to restart your device.'
: 'Flash complete! Your device will restart with the new firmware.',
},
}))
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)


[error] 331-331: React's useState should not be directly called
Context: setProgress((p) => ({ ...p, status: { kind: 'err', text: err.message } }))
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)


[error] 334-334: React's useState should not be directly called
Context: setRunning(false)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/home/FlashTools.jsx` around lines 286 - 334, Move the setRestart
call from the initial setup into the successful completion path after
flasher.flashFirmware resolves. Keep it unset or false while firmware
fetching/flashing is in progress and when the try block fails, and preserve the
existing unplug condition based on skipReset and model !== 'x4'.
src/pages/home/markdown.js (1)

14-18: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

safeUrl accepts protocol-relative URLs (//evil.com).

The \/ alternative in the allowlist regex matches any string starting with a single /, including //host protocol-relative URLs, which browsers resolve as absolute links to an external origin. This defeats the intent of allowlisting only same-origin root-relative paths. This helper's output feeds href attributes rendered via dangerouslySetInnerHTML (e.g. beta build notes in src/pages/home/FlashTools.jsx), so closing this gap matters for the whole call chain.

🛡️ Proposed fix
 function safeUrl(url) {
   const trimmed = url.trim()
-  if (/^(https?:\/\/|\/|#|mailto:)/i.test(trimmed)) return trimmed
+  if (/^(https?:\/\/|\/(?!\/)|#|mailto:)/i.test(trimmed)) return trimmed
   return '#'
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

function safeUrl(url) {
  const trimmed = url.trim()
  if (/^(https?:\/\/|\/(?!\/)|#|mailto:)/i.test(trimmed)) return trimmed
  return '#'
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/home/markdown.js` around lines 14 - 18, Update safeUrl so the
root-relative URL branch accepts a single leading slash but rejects
protocol-relative values beginning with “//”. Preserve the existing handling for
http(s), hash, and mailto URLs, and continue returning “#” for disallowed
inputs.
src/pages/insider/buildShared.jsx (1)

30-35: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Guard against missing message to avoid a hard crash.

message.split('\n') throws if message is undefined/null. Any malformed changelog entry from the build-meta API would crash the whole ChangelogList render.

🛡️ Proposed guard
 export function cleanCommitMessage(message) {
-  const msg = message
+  const msg = (message || '')
     .split('\n')[0]
     .replace(/^(feat|fix|chore|refactor|docs|style|test|perf|ci|build|revert)(\(.+?\))?:\s*/i, '')
   return msg.charAt(0).toUpperCase() + msg.slice(1)
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

export function cleanCommitMessage(message) {
  const msg = (message || '')
    .split('\n')[0]
    .replace(/^(feat|fix|chore|refactor|docs|style|test|perf|ci|build|revert)(\(.+?\))?:\s*/i, '')
  return msg.charAt(0).toUpperCase() + msg.slice(1)
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/insider/buildShared.jsx` around lines 30 - 35, Update
cleanCommitMessage to handle nullish or missing message values before calling
split, returning a safe empty or established fallback string while preserving
the current formatting behavior for valid messages.
src/pages/InsiderPage.jsx (3)

88-94: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Wrap loadBuildInfo in try/catch to avoid a stuck loading spinner.

Unlike the sibling effect at lines 100-115, this has no error handling. If fetchBuildMeta() rejects, setLoading(false) is skipped and the page is stuck showing the loading spinner indefinitely — the only retry UI (the "Refresh" button) is gated behind !loading, so there's no way to recover without a full page reload.

🛡️ Proposed fix
   const loadBuildInfo = useCallback(async () => {
     setLoading(true)
     setMeta(null)
-    const data = await fetchBuildMeta()
-    setLoading(false)
-    setMeta(data || null)
+    try {
+      const data = await fetchBuildMeta()
+      setMeta(data || null)
+    } catch {
+      setMeta(null)
+    } finally {
+      setLoading(false)
+    }
   }, [])
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  const loadBuildInfo = useCallback(async () => {
    setLoading(true)
    setMeta(null)
    try {
      const data = await fetchBuildMeta()
      setMeta(data || null)
    } catch {
      setMeta(null)
    } finally {
      setLoading(false)
    }
  }, [])
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/InsiderPage.jsx` around lines 88 - 94, Update loadBuildInfo in
InsiderPage to wrap fetchBuildMeta and the success state updates in
try/catch/finally, ensuring setLoading(false) always executes when the request
rejects so the refresh UI remains available. Preserve the existing metadata
reset and successful setMeta behavior, and follow the sibling effect’s
established error-handling approach where applicable.

205-231: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

TOCTOU race: cfPollingRef.current is checked before the await, not re-checked after.

Both poll() (lines 205-231) and the visibilitychange handler (lines 281-300) guard on cfPollingRef.current before calling fetchCustomBuildStatus(), but not after the await resolves. If clearCustomBuild() sets the ref to false while a request from either path is in flight, the stale response still runs showCustomBuildReady() / setCfError / setCfState('failed'), silently reviving state the user just cleared.

🛡️ Proposed fix (apply to both call sites)
     async function poll() {
       if (!cfPollingRef.current) return
       try {
         const build = await fetchCustomBuildStatus()
+        if (!cfPollingRef.current) return // re-check after the await
         if (build) {
       fetchCustomBuildStatus()
         .then((build) => {
-          if (!build) return
+          if (!cfPollingRef.current || !build) return // re-check after the await
           if (build.status === 'success') {

Also applies to: 281-300

🧰 Tools
🪛 ast-grep (0.44.1)

[warning] 227-227: Avoid using the initial state variable in setState
Context: setTimeout(poll, 10000)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(setstate-same-var)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/InsiderPage.jsx` around lines 205 - 231, Re-check
cfPollingRef.current immediately after each fetchCustomBuildStatus() await in
both poll() and the visibilitychange handler, returning without applying results
when polling was cleared during the request. Ensure stale responses cannot call
showCustomBuildReady, setCfError, or setCfState after clearCustomBuild().

733-752: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Guard parseInt against NaN in font-size inputs.

parseInt(e.target.value) with no fallback lets a momentarily-empty field push NaN into cfSizes. In handleCfBuild, vals.some((v, i) => v !== defaults[i]) (line 337) is always true for a NaN entry, so an invalid size can be uploaded to the build backend.

🛡️ Proposed fix
                                           onChange={(e) =>
                                             setCfSizes((prev) => {
                                               const next = [...(prev[family] || sizes)]
-                                              next[i] = parseInt(e.target.value)
+                                              const parsed = parseInt(e.target.value, 10)
+                                              next[i] = Number.isNaN(parsed) ? sizes[i] : parsed
                                               return { ...prev, [family]: next }
                                             })
                                           }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

                                    {sizes.map((sz, i) => (
                                      <div key={i} className="flex-1">
                                        <label className="text-[10px] text-stone-400">
                                          {SIZE_LABELS[i]}
                                        </label>
                                        <input
                                          type="number"
                                          min="6"
                                          max="30"
                                          value={sz}
                                          onChange={(e) =>
                                            setCfSizes((prev) => {
                                              const next = [...(prev[family] || sizes)]
                                              const parsed = parseInt(e.target.value, 10)
                                              next[i] = Number.isNaN(parsed) ? sizes[i] : parsed
                                              return { ...prev, [family]: next }
                                            })
                                          }
                                          className="w-full rounded border border-stone-200 bg-white px-1.5 py-1 text-center text-xs text-stone-700 tabular-nums focus:border-brand-300 focus:ring-1 focus:ring-brand-500/20 focus:outline-none"
                                        />
                                      </div>
                                    ))
🧰 Tools
🪛 ast-grep (0.44.1)

[warning] 734-736: A list component should have a key to prevent re-rendering
Context:
{SIZE_LABELS[i]}

Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(list-component-needs-key)


[warning] 737-750: A list component should have a key to prevent re-rendering
Context: <input
type="number"
min="6"
max="30"
value={sz}
onChange={(e) =>
setCfSizes((prev) => {
const next = [...(prev[family] || sizes)]
next[i] = parseInt(e.target.value)
return { ...prev, [family]: next }
})
}
className="w-full rounded border border-stone-200 bg-white px-1.5 py-1 text-center text-xs text-stone-700 tabular-nums focus:border-brand-300 focus:ring-1 focus:ring-brand-500/20 focus:outline-none"
/>
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(list-component-needs-key)


[warning] 732-752: Do not use array indexes for a list component's key
Context: sizes.map((sz, i) => (



{SIZE_LABELS[i]}

<input
type="number"
min="6"
max="30"
value={sz}
onChange={(e) =>
setCfSizes((prev) => {
const next = [...(prev[family] || sizes)]
next[i] = parseInt(e.target.value)
return { ...prev, [family]: next }
})
}
className="w-full rounded border border-stone-200 bg-white px-1.5 py-1 text-center text-xs text-stone-700 tabular-nums focus:border-brand-300 focus:ring-1 focus:ring-brand-500/20 focus:outline-none"
/>

))
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(list-component-no-index)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/InsiderPage.jsx` around lines 733 - 752, Update the size input
handler in the sizes.map callback to prevent empty or invalid values from
storing NaN in cfSizes. Parse the entered value and retain the previous valid
size, or apply an appropriate existing fallback, when parsing fails; preserve
valid numeric updates so handleCfBuild receives only usable font sizes.
src/pages/LoginPage.jsx (1)

41-47: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Move the setTimeout side effect out of the state updater.

setShowLogin's functional updater performs a side effect (setTimeout); React may invoke updaters more than once, so this side effect isn't guaranteed to run exactly once. Since the current showLogin value is already in scope, a functional updater isn't needed here.

🔧 Proposed fix
 function toggleLogin() {
-  setShowLogin((prev) => {
-    const next = !prev
-    if (next) setTimeout(() => emailRef.current?.focus(), 0)
-    return next
-  })
+  const next = !showLogin
+  setShowLogin(next)
+  if (next) setTimeout(() => emailRef.current?.focus(), 0)
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  function toggleLogin() {
    const next = !showLogin
    setShowLogin(next)
    if (next) setTimeout(() => emailRef.current?.focus(), 0)
  }
🧰 Tools
🪛 ast-grep (0.44.1)

[error] 41-45: React's useState should not be directly called
Context: setShowLogin((prev) => {
const next = !prev
if (next) setTimeout(() => emailRef.current?.focus(), 0)
return next
})
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)

🪛 React Doctor (0.7.6)

[error] 42-42: This state updater performs setTimeout(). React may run updater functions more than once, so side effects here can repeat or observe inconsistent external state.

Keep state updater callbacks pure and return only the next state. Move notifications, storage, timers, ref writes, and other external work into the event or effect that queues the update.

(no-impure-state-updater)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/LoginPage.jsx` around lines 41 - 47, Update toggleLogin so it
derives the next visibility value from the current showLogin state, schedules
the emailRef focus outside the setShowLogin updater when opening the login view,
and then applies the next value without side effects inside the updater.

Source: Linters/SAST tools

src/pages/StickyPage.jsx (1)

16-51: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Escape quotes before interpolating URLs into HTML

escapeHtml() only handles text-node escaping, but renderMarkdown() later drops the URL directly into href="...". A markdown link containing " can break out of the attribute and inject arbitrary attributes; safeUrl() also lets protocol-relative URLs like //evil.com through. Block // URLs and consider reusing the marked + DOMPurify pipeline already used in DocsPage.jsx.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/StickyPage.jsx` around lines 16 - 51, Update safeUrl() to reject
protocol-relative URLs beginning with //, and add attribute-safe escaping for
URLs before interpolating them into href values in renderMarkdown(). Apply this
to both markdown links and bare URL links, preserving the existing allowed
schemes and rendering behavior; reuse the established marked/DOMPurify pipeline
only if it is already available for this page.
worker/types.ts (1)

9-9: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n "ALLOW_INSECURE_DEV_WEBHOOKS" worker wrangler.jsonc

Repository: crosspoint-reader/crosspoint-tools

Length of output: 482


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant implementation and deployment config.
sed -n '280,340p' worker/index.ts
printf '\n---\n'
sed -n '1,220p' wrangler.jsonc
printf '\n---\n'
rg -n "production|dev|preview|ALLOW_INSECURE_DEV_WEBHOOKS|webhook" worker/index.ts wrangler.jsonc

Repository: crosspoint-reader/crosspoint-tools

Length of output: 5693


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for any runtime environment gating around the webhook bypass flag.
ast-grep outline worker/index.ts --view expanded
printf '\n---\n'
rg -n "env\.(NODE_ENV|ENVIRONMENT|WRANGLER|preview|production|dev)|process\.env|import\.meta\.env|ALLOW_INSECURE_DEV_WEBHOOKS" worker/index.ts worker/types.ts wrangler.jsonc

Repository: crosspoint-reader/crosspoint-tools

Length of output: 6737


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the relevant section plus any deployment/env config that might restrict the flag.
sed -n '300,325p' worker/index.ts
printf '\n---\n'
sed -n '1,220p' wrangler.jsonc
printf '\n---\n'
rg -n "ALLOW_INSECURE_DEV_WEBHOOKS|environment|vars|preview|production|dev" -g 'wrangler*' -g '*.json' -g '*.jsonc' -g '*.toml' .

Repository: crosspoint-reader/crosspoint-tools

Length of output: 33608


Restrict ALLOW_INSECURE_DEV_WEBHOOKS to dev-only. worker/index.ts reads this env var directly in isAuthorizedWebhookRequest, so a production secret can disable webhook signature checks. Gate it behind a dev/preview-only check or remove it from production bindings.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@worker/types.ts` at line 9, Restrict ALLOW_INSECURE_DEV_WEBHOOKS to
development or preview environments in isAuthorizedWebhookRequest, ensuring
production cannot disable webhook signature validation. Alternatively, remove
this variable from production bindings while preserving its intended dev-only
behavior.
worker/webhook.ts (1)

3-25: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Use constant-time HMAC verification here instead of ===.

signature === expected compares the hex strings with short-circuiting string equality, which leaks timing on a security-sensitive check. Switch to crypto.subtle.verify() to keep the comparison constant-time.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@worker/webhook.ts` around lines 3 - 25, Update verifyGitHubSignature to use
crypto.subtle.verify() with the imported HMAC key and decoded
x-hub-signature-256 value instead of constructing expected and comparing with
===. Preserve the existing sha256= prefix validation and return the request body
alongside the verification result.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/index.css (1)

1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use string notation for @import.

To align with standard Stylelint conventions (import-notation), prefer using string notation directly instead of the url() function for @import.

♻️ Proposed fix
-@import url('https://fonts.googleapis.com/css2?family=Lora:ital,wght@0,400;0,500;0,600;0,700;1,400&family=Geist+Mono:wght@400;500;600&family=Noto+Serif:wght@400;500;700&family=Noto+Serif+Hebrew:wght@400;700&family=Noto+Sans:wght@400;500;600;700&display=swap');
+@import 'https://fonts.googleapis.com/css2?family=Lora:ital,wght@0,400;0,500;0,600;0,700;1,400&family=Geist+Mono:wght@400;500;600&family=Noto+Serif:wght@400;500;700&family=Noto+Serif+Hebrew:wght@400;700&family=Noto+Sans:wght@400;500;600;700&display=swap';
 `@import` "tailwindcss";
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/index.css` around lines 1 - 2, Update the font `@import` in the
stylesheet to use direct quoted-string notation instead of wrapping the URL in
`url()`. Keep the imported Google Fonts URL and the existing Tailwind import
unchanged.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/index.css`:
- Around line 1-2: Update the font `@import` in the stylesheet to use direct
quoted-string notation instead of wrapping the URL in `url()`. Keep the imported
Google Fonts URL and the existing Tailwind import unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9cc56132-e84a-448b-a391-5f6c9fce3bc9

📥 Commits

Reviewing files that changed from the base of the PR and between a0af79c and a043c0f.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (6)
  • package.json
  • src/components/Header.jsx
  • src/index.css
  • src/pages/docs/docs-prose.css
  • src/pages/home/Hero.jsx
  • vite.config.js
🚧 Files skipped from review as they are similar to previous changes (4)
  • vite.config.js
  • src/pages/docs/docs-prose.css
  • package.json
  • src/pages/home/Hero.jsx
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: Workers Builds: crosspoint-tools
⚠️ CI failures not shown inline (1)

GitHub Check: Workers Builds: crosspoint-tools: Workers Builds: crosspoint-tools

Conclusion: failure

View job details

Build ID: [90fb49f2-c319-4811-8227-97bbb8c6719f](https://dash.cloudflare.com/73f82799694e2fad048f544e0be28c1c/workers/services/view/crosspoint-tools/production/builds/90fb49f2-c319-4811-8227-97bbb8c6719f)
Script: [crosspoint-tools](https://dash.cloudflare.com/73f82799694e2fad048f544e0be28c1c/workers/services/view/crosspoint-tools/production)
🧰 Additional context used
🪛 ast-grep (0.44.1)
src/components/Header.jsx

[warning] 65-65: A list component should have a key to prevent re-rendering
Context:
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(list-component-needs-key)


[warning] 69-69: A list component should have a key to prevent re-rendering
Context:
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(list-component-needs-key)

🪛 Stylelint (17.14.0)
src/index.css

[error] 1-1: Expected "url('https://fonts.googleapis.com/css2?family=Lora:ital,wght@0,400;0,500;0,600;0,700;1,400&family=Geist+Mono:wght@400;500;600&family=Noto+Serif:wght@400;500;700&family=Noto+Serif+Hebrew:wght@400;700&family=Noto+Sans:wght@400;500;600;700&display=swap')" to be "'https://fonts.googleapis.com/css2?family=Lora:ital,wght@0,400;0,500;0,600;0,700;1,400&family=Geist+Mono:wght@400;500;600&family=Noto+Serif:wght@400;500;700&family=Noto+Serif+Hebrew:wght@400;700&family=Noto+Sans:wght@400;500;600;700&display=swap'" (import-notation)

(import-notation)


[error] 4-4: Unexpected unknown at-rule "@theme" (scss/at-rule-no-unknown)

(scss/at-rule-no-unknown)

🔇 Additional comments (5)
src/components/Header.jsx (3)

4-44: LGTM!


46-93: LGTM!


95-117: LGTM!

src/index.css (2)

4-11: LGTM!


110-120: LGTM!

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.

3 participants