Conversation
- Add explicit display: flex to all div elements in channel OG image generation - Resolves 500 error when generating OpenGraph images for channel pages
…data API - Replace NextApiRequest/NextApiResponse with NextRequest for edge runtime - Use robust URL parsing instead of fragile string splitting - Add validation for required channelId parameter - Return proper Response objects with HTTP status codes - Addresses coderabbitai suggestions for type safety and runtime compatibility
Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…#1494) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
- Enhance commitSpaceTabToDatabase to handle new tabs renamed before commit - Remove registerSpaceTab (commitSpaceTabToDatabase now handles both) - Update API endpoint to use upload() with upsert:true for create/update - Improve error handling with proper types and statusCode checks
- Simplify commitAllSpaceChanges: remove categorization, commit all tabs - Make createSpaceTab and renameSpaceTab staged-only (no immediate commits) - Add deletedTabs tracking for explicit deletion management - Fix API status code comparison: use string '404' instead of number - Remove optimistic update pattern from renameSpaceTab (no-op commitFn) All tab operations now use consistent staged commit pattern, committed together via commitAllSpaceChanges for atomic batched updates.
…mplement domain->config routing, and add endpoint for registering community_configs (#1625) Co-authored-by: Jesse Paterson <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis change introduces bot ID protection infrastructure across the application by adding a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/api/iframely/route.ts (1)
59-73: Missing SSRF protection in embeddability check.The
checkEmbeddabilityfunction makes HEAD requests to user-supplied URLs without validating the scheme or blocking internal IPs. This could allow attackers to probe internal services.🔎 Suggested fix: Add URL validation
async function checkEmbeddability(url: string): Promise<boolean> { try { + // Validate URL scheme + const parsedUrl = new URL(url); + if (parsedUrl.protocol !== 'https:' && parsedUrl.protocol !== 'http:') { + return false; + } + + // Block internal URLs (reuse isInternalUrl from a shared utility) + if (isInternalUrl(url)) { + return false; + } + // Make a HEAD request to check headers and cache the result const response = await fetch(url, {Consider extracting the
isInternalUrlhelper fromrss/route.tsinto a shared utility module.
🧹 Nitpick comments (2)
src/common/utils/botIdProtection.ts (1)
31-53: Consider adding error handling for checkBotId failures.The
enforceBotIdProtectionfunction does not handle potential errors fromcheckBotId. If the bot detection service fails or throws an error, the API route will crash rather than gracefully degrading.🔎 Proposed enhancement with error handling
export async function enforceBotIdProtection( request: BotIdRequestLike, ): Promise<BotIdVerificationResult | NextResponse> { + try { const verification = await checkBotId({ advancedOptions: { headers: normalizeHeaders(request.headers), }, developmentOptions: { isDevelopment, }, }); if (verification.isBot) { const response = NextResponse.json( { success: false, error: "Access denied" }, { status: 403 }, ); applyBotIdHeaders(response, verification); return response; } return verification; + } catch (error) { + console.error("Bot ID verification failed:", error); + // In development, allow through; in production, you might want to block + if (isDevelopment) { + return { isBot: false, responseHeaders: {} } as BotIdVerificationResult; + } + // Or return a 500 error to be safe + return NextResponse.json( + { success: false, error: "Verification service unavailable" }, + { status: 500 }, + ); + } }src/app/api/miniapp-discovery/route.ts (1)
29-29: Type assertion with fallback works but is confusing.The expression
searchParams.get('timeWindow') as '...' || '7d'works because the cast happens first, but the null still evaluates as falsy. However, this doesn't validate that the input is actually one of the allowed values.🔎 Suggested improvement for type safety
- const timeWindow = searchParams.get('timeWindow') as '1h' | '6h' | '12h' | '24h' | '7d' || '7d'; + const timeWindowParam = searchParams.get('timeWindow'); + const validTimeWindows = ['1h', '6h', '12h', '24h', '7d'] as const; + const timeWindow = validTimeWindows.includes(timeWindowParam as any) + ? (timeWindowParam as typeof validTimeWindows[number]) + : '7d';
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
package.jsonsrc/app/api/frames/route.tssrc/app/api/iframely/route.tssrc/app/api/miniapp-discovery/route.tssrc/app/api/opengraph/route.tssrc/app/api/rss/route.tssrc/app/api/venice/background/route.tssrc/app/api/venice/route.tssrc/app/layout.tsxsrc/common/components/BotIdProtectionLoader.tsxsrc/common/utils/botIdProtection.ts
🧰 Additional context used
🧬 Code graph analysis (9)
src/app/api/venice/background/route.ts (2)
src/app/api/venice/config.ts (1)
VENICE_API_KEY(7-7)src/common/utils/botIdProtection.ts (2)
enforceBotIdProtection(31-53)applyBotIdHeaders(55-79)
src/app/api/miniapp-discovery/route.ts (2)
src/app/api/iframely/route.ts (1)
GET(9-57)src/common/utils/botIdProtection.ts (2)
enforceBotIdProtection(31-53)applyBotIdHeaders(55-79)
src/common/components/BotIdProtectionLoader.tsx (1)
src/common/utils/botIdProtection.ts (1)
botIdProtectedRoutes(19-29)
src/app/api/opengraph/route.ts (1)
src/common/utils/botIdProtection.ts (2)
enforceBotIdProtection(31-53)applyBotIdHeaders(55-79)
src/app/api/rss/route.ts (1)
src/common/utils/botIdProtection.ts (2)
enforceBotIdProtection(31-53)applyBotIdHeaders(55-79)
src/app/api/iframely/route.ts (1)
src/common/utils/botIdProtection.ts (2)
enforceBotIdProtection(31-53)applyBotIdHeaders(55-79)
src/app/api/venice/route.ts (2)
src/common/utils/botIdProtection.ts (2)
enforceBotIdProtection(31-53)applyBotIdHeaders(55-79)src/app/api/venice/config.ts (1)
VENICE_API_KEY(7-7)
src/app/layout.tsx (1)
src/common/components/BotIdProtectionLoader.tsx (1)
BotIdProtectionLoader(7-9)
src/app/api/frames/route.ts (1)
src/common/utils/botIdProtection.ts (2)
enforceBotIdProtection(31-53)applyBotIdHeaders(55-79)
⏰ 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: Agent
🔇 Additional comments (19)
src/common/components/BotIdProtectionLoader.tsx (1)
1-9: LGTM!The component correctly implements client-side bot protection by rendering
BotIdClientwith the configured protected routes. The"use client"directive is appropriate for this client-side component.src/common/utils/botIdProtection.ts (3)
5-29: LGTM!The type definitions and protected routes configuration are well-structured. The inclusion of both GET and POST methods for certain routes ensures comprehensive protection.
55-79: LGTM!The header application logic is defensive and correctly handles the three types of header operations (replace, add, append). The early returns prevent errors when response headers are missing.
81-104: LGTM!The header normalization logic correctly handles HTTP header semantics, including case-insensitivity (Line 85) and multiple values for the same header name (Lines 88-100).
src/app/api/venice/background/route.ts (2)
12-22: LGTM!The bot protection integration follows the correct pattern: early verification with immediate return if blocked, and a reusable
respondwrapper to ensure bot headers are consistently applied to all responses.
25-89: Consistent response handling.All response paths correctly use the
respondwrapper, ensuring bot protection headers are applied consistently across success and error responses.package.json (1)
96-96: [email protected] is valid and secure.Version 1.5.10 of the
botidpackage exists on npm and has no known vulnerabilities or deprecation warnings.src/app/layout.tsx (1)
131-131: BotIdClient placement in is correct per package documentation.The
BotIdProtectionLoadercomponent is correctly placed inside the document<head>. According to thebotidpackage documentation for Next.js, the<BotIdClient />component should be mounted in the root layout's<head>to ensure client-side bot protection runs early during page initialization.src/app/api/frames/route.ts (3)
3-6: Bot protection integration looks correct.The bot ID protection is properly integrated at the start of the GET handler, with early return on bot detection and consistent header application via the
respondwrapper. The pattern aligns with the shared utilities inbotIdProtection.ts.Also applies to: 238-248
331-341: POST handler bot protection correctly mirrors GET.The bot protection flow and
respondwrapper are consistently applied in the POST handler, ensuring all responses include bot ID headers.
16-56: SSRF protection preserved.The existing
isInternalUrlfunction continues to block internal IP ranges and localhost. The error responses now correctly route through therespondwrapper for consistent header application.src/app/api/rss/route.ts (1)
4-7: Bot protection integration is correct.The pattern is consistently applied:
enforceBotIdProtectionat the start, early return on bot detection, and all responses routed through therespondwrapper. The SSRF protection logic is preserved and more robust here (usingnode:netisIP).Also applies to: 77-87
src/app/api/opengraph/route.ts (1)
3-6: Bot protection integration is correct.The pattern is consistently applied with early return on bot detection and all responses routed through
respond.Also applies to: 9-19
src/app/api/venice/route.ts (2)
14-17: Bot protection integration is correct.The POST handler properly enforces bot protection and routes all responses through the
respondwrapper. Since this route only calls known external APIs (Venice, Neynar) rather than user-supplied URLs, SSRF protection isn't needed here.Also applies to: 36-46
171-179: Error details exposure is acceptable but verify for production.The error details are now included in the response. While helpful for debugging, ensure sensitive internal errors from the Venice API don't leak to clients in production. The current implementation looks reasonable for known API errors.
src/app/api/miniapp-discovery/route.ts (2)
3-6: Bot protection integration is correct for GET handler.The pattern is consistently applied with early return on bot detection and the
respondwrapper for all responses.Also applies to: 9-19
94-104: POST handler bot protection is correctly integrated.All action branches (
refresh,search,trending,discover,default) properly use therespondwrapper.src/app/api/iframely/route.ts (2)
2-5: Bot protection integration is correct.The pattern is consistently applied with early return on bot detection and all responses routed through
respond.Also applies to: 10-20
120-125: Verify API key exposure is intentional.
NEXT_PUBLIC_IFRAMELY_API_KEYuses theNEXT_PUBLIC_prefix, which exposes it to the client-side bundle. Since this code runs server-side in a route handler, consider using a non-prefixed env var (e.g.,IFRAMELY_API_KEY) to keep the key server-only, unless Iframely specifically requires client-side exposure.
| if (parsedUrl.protocol !== "https:") { | ||
| // Only allow https URLs for security; reject http and other protocols | ||
| return NextResponse.json({ | ||
| title: parsedUrl.hostname || url, | ||
| description: null, | ||
| image: null, | ||
| siteName: parsedUrl.hostname || url, | ||
| url, | ||
| error: `Only https URLs are allowed. Unsupported URL protocol: ${parsedUrl.protocol}` | ||
| }); | ||
| return respond( | ||
| { | ||
| title: parsedUrl.hostname || url, | ||
| description: null, | ||
| image: null, | ||
| siteName: parsedUrl.hostname || url, | ||
| url, | ||
| error: `Only https URLs are allowed. Unsupported URL protocol: ${parsedUrl.protocol}`, | ||
| }, | ||
| { status: 400 }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Consider adding SSRF protection.
This route only validates the https: protocol but doesn't block internal IP ranges or localhost like frames/route.ts and rss/route.ts do. An attacker could potentially request internal HTTPS services (e.g., https://10.0.0.1/... or https://localhost:8443/...).
🔎 Suggested fix: Add isInternalUrl check
You could reuse or extract the isInternalUrl helper from rss/route.ts into a shared utility and apply it here:
if (parsedUrl.protocol !== "https:") {
return respond(
{
title: parsedUrl.hostname || url,
description: null,
image: null,
siteName: parsedUrl.hostname || url,
url,
error: `Only https URLs are allowed. Unsupported URL protocol: ${parsedUrl.protocol}`,
},
{ status: 400 },
);
}
+ if (isInternalUrl(url)) {
+ return respond(
+ { error: "Invalid URL. Internal URLs are not allowed." },
+ { status: 400 },
+ );
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/api/opengraph/route.ts around lines 45 to 57, the handler only
enforces the https: protocol but does not block internal/localhost addresses,
leaving it open to SSRF; import or move the existing isInternalUrl helper (or
implement equivalent) used by frames/route.ts and rss/route.ts, run it after
parsing the URL and before fetching, and if it returns true respond with a 400
(or same error shape) indicating internal URLs are not allowed; ensure the
helper checks localhost, loopback, private RFC1918 ranges and IPv6 equivalents
and use the same error message format as the surrounding code for consistency.
There was a problem hiding this comment.
Pull request overview
This PR adds bot protection using the [email protected] package to secure API endpoints from automated bot traffic. The implementation includes both server-side protection via middleware and client-side protection through a global component loader.
Key Changes
- Installed
[email protected]package and integrated BotID protection for 7 API endpoints - Created centralized bot protection utilities with header normalization and response handling
- Added client-side BotIdProtectionLoader component to enforce protection globally
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
package.json |
Added [email protected] dependency |
yarn.lock |
Locked botid package version |
src/common/utils/botIdProtection.ts |
Implements core bot detection logic with header normalization and response utilities |
src/common/components/BotIdProtectionLoader.tsx |
Client-side component for global route protection |
src/app/layout.tsx |
Integrated BotIdProtectionLoader into application layout |
src/app/api/venice/route.ts |
Added bot protection and improved error handling with consistent JSON responses |
src/app/api/venice/background/route.ts |
Added bot protection and standardized error responses |
src/app/api/rss/route.ts |
Protected endpoint with bot detection and applied BotID headers to responses |
src/app/api/opengraph/route.ts |
Added bot protection and improved error responses with status codes |
src/app/api/miniapp-discovery/route.ts |
Protected both GET and POST methods with bot detection |
src/app/api/iframely/route.ts |
Added bot protection with consistent response pattern |
src/app/api/frames/route.ts |
Protected both GET and POST handlers with bot detection |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| title: parsedUrl.hostname || url, | ||
| description: null, | ||
| image: null, | ||
| siteName: parsedUrl.hostname || url, | ||
| url, | ||
| error: `Only https URLs are allowed. Unsupported URL protocol: ${parsedUrl.protocol}`, |
There was a problem hiding this comment.
The error response includes the full parsed URL in the response body when the protocol is not https. This could potentially expose internal server details or unexpected data to clients. Consider only returning a sanitized error message without the parsed URL details.
| title: parsedUrl.hostname || url, | |
| description: null, | |
| image: null, | |
| siteName: parsedUrl.hostname || url, | |
| url, | |
| error: `Only https URLs are allowed. Unsupported URL protocol: ${parsedUrl.protocol}`, | |
| title: null, | |
| description: null, | |
| image: null, | |
| siteName: null, | |
| url: null, | |
| error: "Only https URLs are allowed.", |
|
|
||
| if (verification.isBot) { | ||
| const response = NextResponse.json( | ||
| { success: false, error: "Access denied" }, |
There was a problem hiding this comment.
The error response when a bot is detected returns a generic "Access denied" message. Consider adding more context in the error message to help legitimate users understand why their request was denied, such as "Request blocked by bot protection" or providing a reference ID for troubleshooting.
| { success: false, error: "Access denied" }, | |
| { success: false, error: "Request blocked by bot protection" }, |
| return respond( | ||
| { error: "All models failed", details: lastError }, | ||
| { status: 500 }, | ||
| ); |
There was a problem hiding this comment.
The error response includes the lastError details which may contain sensitive information such as API keys, internal server paths, or stack traces if it's an Error object. This could leak internal implementation details to potential attackers. Consider sanitizing or omitting the details field in production, or only including safe error information.
src/app/layout.tsx
Outdated
| {validatedUiStylesheet && ( | ||
| <link rel="stylesheet" href={validatedUiStylesheet} /> | ||
| )} | ||
| <BotIdProtectionLoader /> |
There was a problem hiding this comment.
The BotIdProtectionLoader client component is placed inside the head tag, which is unusual and may cause hydration issues or unexpected behavior in React. Client-side components are typically placed in the body tag. According to React best practices, the head should contain metadata and static resources. Consider moving this component to the body tag, perhaps before the SpeedInsights component or within the Providers wrapper.
|
@willyogo I did what the issue asked for. Is there anything else that needs to be tested? I ran tests and implemented the changes. If there’s anything else, please let me know. |
…ting bundle analyzer with the BotID functionality.
|
@Jhonattan2121 good q. would like to test this and confirm that it both works and also does not break anything / negatively impact performance before merging. can you try and test both of the above and report back with results? |
…nt by moving it directly into the body and removing it from the head section.
BotID Protection - Test ReportExecutive SummaryThis report documents testing performed on PR #1654 implementing Vercel BotID protection. Testing confirms the implementation is working as intended, successfully blocking automated requests while allowing legitimate browser traffic. Test MethodologyEnvironments Tested
Test Approaches
Preview Environment (BotID Active)Result: 4/5 protected endpoints correctly blocked automated requests. Production Environment (No BotID)Result: All endpoints accessible without protection, confirming BotID is not yet deployed in production. Manual Browser TestingTest: Manual fetch via Chrome DevTools Console fetch('/api/rss?url=https://rss.nytimes.com/services/xml/rss/nyt/HomePage.xml')Result:
Conclusion: Legitimate browser requests are correctly allowed through BotID protection. Protected Endpoints ConfigurationThe following endpoints are protected per
All tested endpoints (except Implementation ReviewCode Changes
Verification Against DocumentationImplementation follows Vercel BotID documentation requirements:
|
|
thanks for the test report @Jhonattan2121! overall looks good. were you able to tell if there are any downsides to enabling this? ie. is response time slower for these endpoints when bot protection is enabled? i'm wondering if it's necessary or net-beneficial to implement this on all of the endpoints vs. just the frontend. |
|
@willyogo |
[email protected]and injectedBotIdProtectionLoaderintosrc/app/layout.tsxso the client guard runs globally.src/common/utils/botIdProtection.tsto normalize headers, callcheckBotId, and replay response headers consistently.BotIdProtectionLoadernow feedsbotIdProtectedRoutesintoBotIdClientas described in the Vercel docs./api/miniapp-discovery,/api/frames,/api/iframely,/api/opengraph,/api/rss,/api/venice, and/api/venice/backgroundwithenforceBotIdProtectionand always reapply BotID headers before returning JSON.botIdProtectedRoutescovers both GET and POST methods for those endpoints to keep client/server aligned.Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.