Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughImplements QR code redirect handling in middleware: async lookup to PEANUT_API_URL/redirect/lookup for paths under /qr/, redirects on 200 with targetUrl, otherwise falls through. Adds timeout via AbortController, request-scoped logging, guards for missing code/env, and updates matcher/config to include /qr/:path*. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 1
🧹 Nitpick comments (1)
src/middleware.ts (1)
53-68: Ensure timeout is cleared on all paths (use finally).If fetch throws, the timeout isn’t cleared. Move clearTimeout to a finally and hoist timeoutId.
Apply this diff:
- if (code && PEANUT_API_URL) { - try { - const controller = new AbortController() - const timeoutId = setTimeout(() => controller.abort(), 3000) + if (code && PEANUT_API_URL) { + let timeoutId: ReturnType<typeof setTimeout> | undefined + try { + const controller = new AbortController() + timeoutId = setTimeout(() => controller.abort(), 3000) const lookupUrl = `${PEANUT_API_URL}/redirect/lookup?input=${encodeURIComponent(`qr/${code}`)}` console.log('[QR Redirect] Looking up URL:', lookupUrl) const res = await fetch(lookupUrl, { method: 'GET', cache: 'no-store', // important to not cache this so the live update works fast (or should we?) signal: controller.signal, }) - - clearTimeout(timeoutId) console.log('[QR Redirect] Response status:', res.status) + } catch (e) { + console.error('[QR Redirect] Error during redirect lookup:', e) + // Network error/timeout -> fall through to normal routing + } finally { + if (timeoutId) clearTimeout(timeoutId) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/middleware.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#1190
File: src/app/(mobile-ui)/qr-pay/page.tsx:156-176
Timestamp: 2025-09-08T03:13:09.111Z
Learning: In the peanut-ui mobile app, the `/qr-pay` route is only accessed through the DirectSendQR component which always includes the qrCode parameter in the URL when redirecting users to the QR pay page.
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#1190
File: src/app/(mobile-ui)/qr-pay/page.tsx:156-176
Timestamp: 2025-09-08T03:13:09.111Z
Learning: In the peanut-ui mobile app, the `/qr-pay` route is only accessed through the DirectSendQR component which always includes the qrCode parameter in the URL when redirecting users to the QR pay page after scanning MERCADO_PAGO or PIX QR codes.
📚 Learning: 2025-09-08T03:13:09.111Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#1190
File: src/app/(mobile-ui)/qr-pay/page.tsx:156-176
Timestamp: 2025-09-08T03:13:09.111Z
Learning: In the peanut-ui mobile app, the `/qr-pay` route is only accessed through the DirectSendQR component which always includes the qrCode parameter in the URL when redirecting users to the QR pay page.
Applied to files:
src/middleware.ts
📚 Learning: 2025-09-12T09:06:22.741Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1207
File: src/components/Global/PostSignupActionManager/post-signup-action.consts.ts:0-0
Timestamp: 2025-09-12T09:06:22.741Z
Learning: In the peanut-ui codebase, redirect URLs are always stored as relative paths, never as absolute paths. This means regex patterns can safely anchor to `^\/` when matching stored redirect URLs.
Applied to files:
src/middleware.ts
🧬 Code graph analysis (1)
src/middleware.ts (1)
src/constants/general.consts.ts (1)
PEANUT_API_URL(43-47)
⏰ 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: Deploy-Preview
🔇 Additional comments (3)
src/middleware.ts (3)
4-4: LGTM: Centralized API base import.Good reuse of PEANUT_API_URL (w/ trailing-slash trim).
6-6: LGTM: Async middleware.Necessary for the awaited QR lookup.
130-131: LGTM: Matcher includes /qr/:path.*Middleware will run for QR paths as intended.
| if (res.status === 200) { | ||
| const data = await res.json().catch(() => null) | ||
| const targetUrl = data?.targetUrl | ||
| if (typeof targetUrl === 'string' && targetUrl.length > 0) { | ||
| console.log('[QR Redirect] Redirecting to:', targetUrl) | ||
| return NextResponse.redirect(new URL(targetUrl, request.url)) | ||
| } | ||
| console.log('[QR Redirect] Invalid target URL in response') |
There was a problem hiding this comment.
Prevent open redirects: restrict targetUrl to same-origin relative paths.
Currently any absolute URL from the API will be redirected. Enforce a leading “/” (no “//”) to keep redirects within the app.
Apply this diff:
- if (res.status === 200) {
- const data = await res.json().catch(() => null)
- const targetUrl = data?.targetUrl
- if (typeof targetUrl === 'string' && targetUrl.length > 0) {
- console.log('[QR Redirect] Redirecting to:', targetUrl)
- return NextResponse.redirect(new URL(targetUrl, request.url))
- }
- console.log('[QR Redirect] Invalid target URL in response')
- }
+ if (res.status === 200) {
+ const data = await res.json().catch(() => null)
+ const targetUrl = data?.targetUrl
+ // Only allow relative app paths to avoid open redirects
+ if (typeof targetUrl === 'string' && /^\/(?!\/)/.test(targetUrl)) {
+ console.log('[QR Redirect] Redirecting to:', targetUrl)
+ return NextResponse.redirect(new URL(targetUrl, request.url))
+ }
+ console.log('[QR Redirect] Invalid or external target URL in response')
+ }📝 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 (res.status === 200) { | |
| const data = await res.json().catch(() => null) | |
| const targetUrl = data?.targetUrl | |
| if (typeof targetUrl === 'string' && targetUrl.length > 0) { | |
| console.log('[QR Redirect] Redirecting to:', targetUrl) | |
| return NextResponse.redirect(new URL(targetUrl, request.url)) | |
| } | |
| console.log('[QR Redirect] Invalid target URL in response') | |
| if (res.status === 200) { | |
| const data = await res.json().catch(() => null) | |
| const targetUrl = data?.targetUrl | |
| // Only allow relative app paths to avoid open redirects | |
| if (typeof targetUrl === 'string' && /^\/(?!\/)/.test(targetUrl)) { | |
| console.log('[QR Redirect] Redirecting to:', targetUrl) | |
| return NextResponse.redirect(new URL(targetUrl, request.url)) | |
| } | |
| console.log('[QR Redirect] Invalid or external target URL in response') | |
| } |
🤖 Prompt for AI Agents
In src/middleware.ts around lines 70 to 77, the code currently redirects to any
absolute URL returned by the API; change the validation so targetUrl is allowed
only as a same-origin relative path by requiring it to start with a single '/'
and not with '//' (e.g. ensure typeof targetUrl==='string' &&
targetUrl.startsWith('/') && !targetUrl.startsWith('//') && targetUrl.length>1),
then perform the redirect using the existing relative URL resolution; reject or
log any values that fail this check to prevent open redirects.
See api PR here https://github.com/peanutprotocol/peanut-api-ts/pull/359 for more info