Conversation
|
Warning Rate limit exceeded@Blaumaus has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 6 minutes and 57 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughReplaces manual CAPTCHA with a PoW-based flow: adds a PoW web worker, updates build to bundle the worker, simplifies iframe height handling, refactors captcha logic to request challenges, solve via worker (with main-thread fallback), report progress, verify solutions, and update UI for a computing state. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Main as Main Thread (captcha.ts)
participant Worker as PoW Worker (pow-worker.js)
participant API as Backend API
User->>Main: request CAPTCHA / open frame
Main->>API: POST /generateChallenge
API-->>Main: { challenge, difficulty, maxIterations? }
Main->>Main: show "Computing..." UI
Main->>Worker: postMessage(PowChallenge)
Worker->>Worker: solveChallenge loop (sha256 + check prefix)
alt every 10k attempts
Worker-->>Main: { type: "progress", attempts, hashRate }
Main->>Main: update `#pow-progress`
end
Worker-->>Main: { type: "result", nonce, solution }
Main->>Worker: terminate
Main->>API: POST /verifySolution { nonce, solution }
API-->>Main: { token, lifetime } or error
Main->>Main: update UI -> success or error
Main-->>User: reveal token / success state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/captcha-loader.ts (1)
70-72: Consider addressing the TODO for origin validation.The
postMessagecallback lacks origin validation, which could allow malicious iframes to spoof messages. While not a regression from this PR, it's worth addressing as part of this security-sensitive captcha flow.const postMessageCallback = (pmEvent: MessageEvent) => { - // TODO: Validate origin + const allowedOrigins = isDevelopment + ? ['http://localhost:5005'] + : ['https://cap.swetrix.com'] + if (!allowedOrigins.includes(pmEvent.origin)) { + return + } const { data } = pmEventsrc/pow-worker.ts (1)
5-22: Consider sharing types between worker and main thread.The
PowChallenge,PowResult, andPowProgressinterfaces are duplicated in bothpow-worker.tsandcaptcha.ts. Extract these to a shared types file to prevent drift.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
rollup.config.js(2 hunks)src/captcha-loader.ts(2 hunks)src/captcha.ts(5 hunks)src/pages/dark.html(2 hunks)src/pages/light.html(2 hunks)src/pow-worker.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/captcha-loader.ts (1)
dist/esnext/captcha-loader.js (1)
captchaFrame(123-123)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (9)
src/captcha-loader.ts (1)
24-24: LGTM! Frame height simplification aligns with the new PoW flow.The removal of height mapping for manual states and use of a single constant is appropriate since the PoW mechanism no longer requires expandable manual input UI.
Also applies to: 142-142
rollup.config.js (1)
53-66: LGTM! Worker build configuration is appropriate.The IIFE format is correct for web workers. Consider adding
sourceMaps()plugin for easier debugging in development, similar to the captcha-loader build.src/captcha.ts (2)
117-139: LGTM! Challenge generation with proper error handling.The function correctly handles failures by notifying the loader and updating the UI state. The error handling pattern is consistent.
176-230: Worker lifecycle management looks correct.Good pattern: terminating existing workers before creating new ones, proper cleanup on success/error, and fallback to main thread when workers aren't available.
src/pages/light.html (2)
74-83: LGTM! Clean UI implementation for the computing state.The new
#status-computingelement with progress indicator provides good user feedback during PoW computation. The flex column layout and smaller progress text create a nice visual hierarchy.Also applies to: 190-193
70-72: No changes needed — dark.html already has the matching style changes with appropriate theme adjustments. The #status font-size is consistently set to 14px in both files, and the #status-computing and #pow-progress rules are properly mirrored in dark.html with theme-appropriate color values.Likely an incorrect or invalid review comment.
src/pages/dark.html (3)
68-70: Verify font size reduction aligns with new UI layout.The #status font size is reduced from 16px to 14px. While this is a minor change, please confirm this sizing works well with the new computing state UI and the nested progress indicator below it.
72-81: CSS styling for computing state is appropriate.The flex-based layout correctly positions the "Verifying..." label and progress text vertically, with appropriate color (#9ca3af) and spacing (2px margin-top) to differentiate the progress indicator from the main status text.
185-192: HTML structure for PoW computing state is well-formed.The status state machine is correctly implemented with three mutually exclusive spans (status-default, status-failure, status-computing). The nested structure inside #status-computing provides clear visual hierarchy: "Verifying..." label with an empty #pow-progress container for dynamic progress updates from the worker. This aligns well with the worker-based PoW mechanism described in the PR.
Deploying captchacdn with
|
| Latest commit: |
87ad75b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1bbca6a3.captchacdn.pages.dev |
| Branch Preview URL: | https://captcha-pow.captchacdn.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/captcha.ts (2)
121-144: Consider adding a timeout to the fetch request.The
fetchcall has no timeout mechanism. If the server is slow or unresponsive, the request could hang indefinitely, leaving users stuck on the loading state.const generateChallenge = async (): Promise<PowChallenge | null> => { try { + const controller = new AbortController() + const timeoutId = setTimeout(() => controller.abort(), 10000) // 10s timeout + const response = await fetch(`${API_URL}${ENDPOINTS.GENERATE}`, { method: 'POST', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify({ // @ts-ignore pid: window.__SWETRIX_PROJECT_ID, }), + signal: controller.signal, }) + + clearTimeout(timeoutId) if (!response.ok) { throw new Error('Failed to generate challenge') } return await response.json() } catch (e) { sendMessageToLoader(IFRAME_MESSAGE_TYPES.FAILURE) activateAction(ACTION.failure) return null } }
146-178: Same timeout concern applies here.Similar to
generateChallenge, this fetch call could hang indefinitely. Consider adding anAbortControllertimeout for consistency and resilience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/captcha.ts(5 hunks)src/pow-worker.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pow-worker.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/captcha.ts (1)
dist/esnext/captcha.js (11)
isDevelopment(3-3)API_URL(4-4)CAPTCHA_TOKEN_LIFETIME(7-7)setLifetimeTimeout(70-75)sendMessageToLoader(30-38)activateAction(43-69)response(106-117)ENDPOINTS(10-13)data(121-121)captchaComponent(131-131)branding(132-132)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (7)
src/captcha.ts (7)
1-18: LGTM!The configuration setup with environment-based URLs and reasonable iteration/timeout limits for PoW is well-structured.
33-51: LGTM!Well-defined TypeScript interfaces for PoW communication types and proper state management for the worker.
70-105: LGTM!The UI state management correctly integrates the new
statusComputingelement for the PoW loading state.
107-119: LGTM!Progress display and token lifetime management are implemented correctly.
180-270: LGTM!The worker lifecycle management is well-implemented with:
- Proper cleanup of existing workers before creating new ones
- Comprehensive handling of all message types (
progress,timeout,result,error, and unknown)- Graceful fallback to main thread on worker errors
- Consistent termination and state cleanup in all code paths
The previous review concerns about handling unexpected message types have been addressed.
272-334: LGTM!The main-thread fallback correctly implements:
- Iteration limit (
MAX_ITERATIONS) and timeout (TIMEOUT_MS) as requested in previous reviews- Progress updates every 10k iterations with event loop yielding
- Proper failure handling when limits are exceeded
Note:
crypto.subtlerequires a secure context (HTTPS), which should be satisfied by the captcha iframe deployment.
336-363: LGTM!The event handler correctly:
- Prevents duplicate submissions during loading/completed states
- Allows users to retry after failure
- Chains the challenge generation and solving flow properly
Depends on:
Summary by CodeRabbit
New Features
Refactor
UI Updates
✏️ Tip: You can customize this high-level summary in your review settings.