Conversation
|
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. WalkthroughAdds a hackathon coupon redemption feature: frontend UI and settings route, a redeem-coupon hook, backend redeem endpoint and status exposure with Clerk privateMetadata, shared schemas for request/response and offer shape, env config for codes, an in-memory rate limiter, and runtime/test updates to sanitize/process.env protections. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant UI as RedeemCouponSection (Frontend)
participant Hook as useRedeemCoupon
participant API as BubbleLab API (/subscription/redeem)
participant Clerk as Clerk (identity store)
participant Status as /subscription/status
User->>UI: Enter coupon code & submit
UI->>Hook: mutate(code)
Hook->>API: POST /subscription/redeem { code }
API->>API: normalize & validate code vs HACKATHON_COUPON_CODES
alt invalid
API-->>Hook: 400 { success: false, message }
else valid
API->>API: calculate expiresAt (now + 1 day)
API->>Clerk: update privateMetadata.hackathonOffer { code, redeemedAt, expiresAt }
alt Clerk success
API-->>Hook: 200 { success: true, message, expiresAt }
Hook->>UI: mutation success (invalidate subscription)
UI->>User: show success & offer banner
else Clerk failure
API-->>Hook: error response
Hook->>UI: show error
end
end
Note right of Hook: subsequent subscription status fetch includes hackathonOffer
UI->>Status: GET /subscription/status (after invalidate)
Status-->>UI: subscription + hackathonOffer
UI->>User: display active offer with formatted expiration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Suggested PR title from PearlTitle: Body: Frontend Changes
Backend Changes
Features
Schema Updates
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/bubble-studio/src/components/RedeemCouponSection.tsx (1)
59-68: Consider extracting date formatting to a shared utility.The
formatExpirationDatefunction appears to duplicate similar logic mentioned in the AI summary forMonthlyUsageBar.tsx(formatOfferExpiration). Consider extracting this to a shared utility module to maintain consistency and reduce duplication.🔎 Suggested refactor
Create a shared utility file (e.g.,
apps/bubble-studio/src/utils/date-formatting.ts):export function formatOfferExpiration(dateString: string): string { const date = new Date(dateString); return date.toLocaleString('en-US', { month: 'short', day: 'numeric', hour: 'numeric', minute: '2-digit', hour12: true, }); }Then import and use in both components:
+import { formatOfferExpiration } from '../utils/date-formatting'; + export const RedeemCouponSection: React.FC = () => { // ... state and hooks ... - - const formatExpirationDate = (dateString: string): string => { - const date = new Date(dateString); - return date.toLocaleString('en-US', { - month: 'short', - day: 'numeric', - hour: 'numeric', - minute: '2-digit', - hour12: true, - }); - }; return ( // ... JSX using formatOfferExpiration instead ...
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/bubble-studio/src/components/MonthlyUsageBar.tsxapps/bubble-studio/src/components/RedeemCouponSection.tsxapps/bubble-studio/src/components/Sidebar.tsxapps/bubble-studio/src/hooks/useRedeemCoupon.tsapps/bubble-studio/src/pages/SettingsPage.tsxapps/bubble-studio/src/routeTree.gen.tsapps/bubble-studio/src/routes/settings.tsxapps/bubblelab-api/.env.exampleapps/bubblelab-api/src/config/env.tsapps/bubblelab-api/src/middleware/auth.tsapps/bubblelab-api/src/routes/subscription.tsapps/bubblelab-api/src/schemas/subscription.tspackages/bubble-shared-schemas/src/subscription-status-schema.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/packages/bubble-shared-schemas/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/api.mdc)
**/packages/bubble-shared-schemas/**/*.{ts,tsx}: Write shared schemas between frontend and backend in/packages/bubble-shared-schemasdirectory
Runpnpm build:coreafter modifying shared schemas since it is a separate package and types need to be regenerated
Files:
packages/bubble-shared-schemas/src/subscription-status-schema.ts
🧠 Learnings (5)
📚 Learning: 2025-12-19T03:16:48.793Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/api.mdc:0-0
Timestamp: 2025-12-19T03:16:48.793Z
Learning: Applies to **/packages/bubble-shared-schemas/**/*.{ts,tsx} : Write shared schemas between frontend and backend in `/packages/bubble-shared-schemas` directory
Applied to files:
apps/bubblelab-api/src/schemas/subscription.tspackages/bubble-shared-schemas/src/subscription-status-schema.ts
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: Applies to packages/bubble-core/src/index.ts : All new types and classes added to bubble-core must be exported from `packages/bubble-core/src/index.ts` to ensure they are included in the generated bundle
Applied to files:
apps/bubble-studio/src/pages/SettingsPage.tsxapps/bubble-studio/src/components/RedeemCouponSection.tsx
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: Applies to apps/bubble-studio/src/components/MonacoEditor.tsx : The Monaco Editor integration in `apps/bubble-studio/src/components/MonacoEditor.tsx` must fetch the bundled types from `/bubble-types.txt`, wrap them in a module declaration for `bubblelab/bubble-core`, and add them to Monaco's TypeScript type system
Applied to files:
apps/bubble-studio/src/components/RedeemCouponSection.tsxapps/bubble-studio/src/routeTree.gen.ts
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: When adding new types to `bubblelab/shared-schemas`, they are automatically included in the bundle without requiring manual configuration changes
Applied to files:
packages/bubble-shared-schemas/src/subscription-status-schema.ts
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: Applies to apps/bubble-studio/public/bubble-types.txt : The bundle file at `apps/bubble-studio/public/bubble-types.txt` must be kept synchronized with `packages/bubble-core/dist/bubble-bundle.d.ts` after each build
Applied to files:
apps/bubble-studio/src/routeTree.gen.ts
🧬 Code graph analysis (6)
apps/bubblelab-api/src/schemas/subscription.ts (1)
packages/bubble-shared-schemas/src/subscription-status-schema.ts (2)
redeemCouponRequestSchema(25-32)redeemCouponResponseSchema(37-52)
apps/bubble-studio/src/pages/SettingsPage.tsx (1)
apps/bubble-studio/src/components/RedeemCouponSection.tsx (1)
RedeemCouponSection(6-139)
apps/bubble-studio/src/components/RedeemCouponSection.tsx (2)
apps/bubble-studio/src/hooks/useRedeemCoupon.ts (1)
useRedeemCoupon(18-73)apps/bubble-studio/src/services/analytics.ts (1)
reset(107-117)
apps/bubble-studio/src/routes/settings.tsx (1)
apps/bubble-studio/src/pages/SettingsPage.tsx (1)
SettingsPage(4-31)
apps/bubblelab-api/src/middleware/auth.ts (1)
apps/bubblelab-api/src/services/subscription-validation.ts (1)
PLAN_TYPE(11-11)
apps/bubblelab-api/src/routes/subscription.ts (4)
apps/bubblelab-api/src/middleware/auth.ts (1)
getAppType(368-371)apps/bubblelab-api/src/config/env.ts (1)
env(67-95)packages/bubble-shared-schemas/src/subscription-status-schema.ts (1)
HackathonOffer(22-22)apps/bubblelab-api/src/schemas/subscription.ts (2)
redeemCouponRoute(33-66)getSubscriptionStatusRoute(9-30)
⏰ 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). (3)
- GitHub Check: Agent
- GitHub Check: test
- GitHub Check: Cloudflare Pages: bubblelab-documentation
🔇 Additional comments (22)
apps/bubble-studio/src/components/MonthlyUsageBar.tsx (1)
194-202: LGTM!The hackathon offer banner implementation is clean and follows the existing UI patterns. The conditional rendering logic is sound.
apps/bubble-studio/src/components/Sidebar.tsx (1)
206-243: LGTM!The Settings navigation item follows the exact same pattern as existing navigation items (Home, My Flows, Credentials). The implementation is consistent and includes proper accessibility attributes.
packages/bubble-shared-schemas/src/subscription-status-schema.ts (2)
4-54: Well-structured schemas for the hackathon feature.The new schemas are comprehensive and well-documented with OpenAPI annotations. The design appropriately uses nullable types for the offer's expiration fields.
1-139: Reminder: Build the shared schemas package.After modifying shared schemas, you need to regenerate types by running
pnpm build:corefrom the repository root.Based on coding guidelines.
apps/bubblelab-api/.env.example (1)
31-31: LGTM!The new environment variables are properly documented and placed in appropriate sections.
Also applies to: 40-41
apps/bubble-studio/src/pages/SettingsPage.tsx (1)
1-31: LGTM!Clean and straightforward implementation of the Settings page. The layout structure and styling are consistent with the application's design patterns.
apps/bubble-studio/src/hooks/useRedeemCoupon.ts (1)
1-73: LGTM!The hook implementation is well-structured with:
- Proper error handling that extracts structured responses from API errors
- Query invalidation on successful redemption to refresh subscription data
- Good logging for debugging
- Clean TypeScript types
apps/bubble-studio/src/routes/settings.tsx (1)
1-26: LGTM!The route implementation properly handles authentication by checking
isSignedInand redirecting unauthenticated users to the home page with a sign-in prompt. The redirect pattern follows TanStack Router conventions.apps/bubblelab-api/src/schemas/subscription.ts (2)
2-6: LGTM! Shared schema imports follow established patterns.The addition of
redeemCouponRequestSchemaandredeemCouponResponseSchemafrom the shared schemas package aligns with the project's architecture for sharing types between frontend and backend.
32-66: LGTM! Route definition is complete and follows best practices.The
redeemCouponRoutedefinition:
- Follows the existing pattern established by
getSubscriptionStatusRoute- Includes proper authentication via
BearerAuth- Has comprehensive OpenAPI metadata (summary, description, tags)
- Defines appropriate response codes (200, 400, 401)
- Uses validated request/response schemas
apps/bubblelab-api/src/config/env.ts (2)
91-91: LGTM! Environment variable follows established patterns.The
HACKATHON_COUPON_CODESenvironment variable:
- Uses an empty string as a safe default
- Follows the same pattern as other optional configuration variables
- Allows the feature to be disabled by default when not configured
134-136: LGTM! Logging follows security best practices.The logging statement appropriately shows only whether the variable is configured, without exposing the actual coupon codes. This follows the same secure pattern used for other sensitive configuration values.
apps/bubble-studio/src/components/RedeemCouponSection.tsx (6)
1-16: LGTM! Component structure and imports are well-organized.The component properly imports necessary dependencies and sets up state management with appropriate hooks for redemption logic and subscription data.
18-31: LGTM! Loading state improves user experience.The loading state prevents user interaction until subscription data is available and provides clear visual feedback.
33-36: LGTM! Safe offer detection with optional chaining.The active offer detection properly handles undefined values with optional chaining, preventing potential runtime errors.
37-57: LGTM! Form submission handler is robust.The handler includes:
- Proper form event handling
- Input validation
- Error state management
- Success flow (clearing input)
- Error handling with user-friendly messages
76-86: LGTM! Active offer display provides clear feedback.The success banner appropriately shows the active offer status with a clear expiration time, using visual cues (green color, CheckCircle icon) for positive reinforcement.
88-136: LGTM! Redemption form has excellent UX.The form implementation includes several user-friendly features:
- Automatic uppercase conversion for codes
- Appropriate disabled states during loading
- Clear success/error messaging with icons
- Helpful instructional text
- Proper visual feedback for all states
apps/bubblelab-api/src/routes/subscription.ts (3)
6-6: LGTM! Import additions support the new redemption feature.All new imports are necessary for the hackathon coupon functionality and follow the project's module structure.
Also applies to: 19-22, 26-29
36-78: LGTM! Helper functions are well-structured.The helper functions provide clear separation of concerns:
getValidCouponCodes: Safely parses and normalizes comma-separated codescalculateOfferExpiration: Simple 1-day expiration calculationgetActiveHackathonOffer: Properly determines offer status with safe null handlingThe date parsing in
getActiveHackathonOfferhandles invalid date strings safely (invalid dates would result inisActive: false).
181-181: LGTM! Status endpoint enhancement has proper error handling.The hackathon offer integration:
- Uses appropriate error handling with try-catch
- Gracefully degrades if Clerk is unavailable (offer simply won't appear)
- Doesn't impact the core subscription status functionality
- Properly logs errors for debugging
Also applies to: 246-264, 292-292
apps/bubble-studio/src/routeTree.gen.ts (1)
1-189: LGTM! Generated route tree properly integrates the settings route.This is an auto-generated file from TanStack Router. The changes consistently integrate the new
/settingsroute following the same patterns as existing routes. The type definitions, interfaces, and module augmentations are all properly structured.
| // Format hackathon offer expiration date | ||
| const formatOfferExpiration = (dateString: string): string => { | ||
| const date = new Date(dateString); | ||
| return date.toLocaleString(undefined, { | ||
| month: 'short', | ||
| day: 'numeric', | ||
| hour: 'numeric', | ||
| minute: '2-digit', | ||
| hour12: true, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Inconsistent locale usage in date formatting.
The formatOfferExpiration function uses undefined for the locale parameter, which falls back to the browser's default locale. However, formatResetDate (lines 91-94) explicitly uses 'en-US'. This inconsistency could result in dates being formatted differently across the UI.
🔎 Proposed fix
- const formatOfferExpiration = (dateString: string): string => {
- const date = new Date(dateString);
- return date.toLocaleString(undefined, {
+ const formatOfferExpiration = (dateString: string): string => {
+ const date = new Date(dateString);
+ return date.toLocaleString('en-US', {
month: 'short',
day: 'numeric',
hour: 'numeric',
minute: '2-digit',
hour12: 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.
| // Format hackathon offer expiration date | |
| const formatOfferExpiration = (dateString: string): string => { | |
| const date = new Date(dateString); | |
| return date.toLocaleString(undefined, { | |
| month: 'short', | |
| day: 'numeric', | |
| hour: 'numeric', | |
| minute: '2-digit', | |
| hour12: true, | |
| }); | |
| }; | |
| // Format hackathon offer expiration date | |
| const formatOfferExpiration = (dateString: string): string => { | |
| const date = new Date(dateString); | |
| return date.toLocaleString('en-US', { | |
| month: 'short', | |
| day: 'numeric', | |
| hour: 'numeric', | |
| minute: '2-digit', | |
| hour12: true, | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In apps/bubble-studio/src/components/MonthlyUsageBar.tsx around lines 96 to 106,
formatOfferExpiration currently calls toLocaleString with undefined (browser
default) causing inconsistent formatting compared to formatResetDate which uses
'en-US'; change the locale argument to 'en-US' so both functions use the same
explicit locale (preserve the existing options for month, day, hour, minute and
hour12).
There was a problem hiding this comment.
Pull request overview
This PR adds a hackathon promotional code redemption feature that allows users to redeem coupon codes for temporary pro access. The implementation includes backend API endpoints for code validation and redemption, schema definitions for the feature, authentication middleware integration to enforce the promotional offers, and a frontend settings page where users can enter and redeem their codes.
- Backend API endpoint (
POST /subscription/redeem) for validating and applying hackathon coupon codes - Hackathon offer status integrated into subscription status response and authentication middleware
- New Settings page in the frontend with a coupon redemption UI component
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/bubble-shared-schemas/src/subscription-status-schema.ts | Adds schemas for hackathon offers, coupon redemption requests/responses, and extends subscription status to include offer information |
| apps/bubblelab-api/src/schemas/subscription.ts | Defines OpenAPI route schema for the coupon redemption endpoint |
| apps/bubblelab-api/src/routes/subscription.ts | Implements coupon redemption business logic and integrates offer status into subscription endpoint |
| apps/bubblelab-api/src/middleware/auth.ts | Adds hackathon offer checking to grant pro_plus access when offers are active |
| apps/bubblelab-api/src/config/env.ts | Adds HACKATHON_COUPON_CODES environment variable configuration |
| apps/bubblelab-api/.env.example | Documents the new hackathon coupon codes environment variable |
| apps/bubble-studio/src/routes/settings.tsx | Creates new settings route with authentication guard |
| apps/bubble-studio/src/pages/SettingsPage.tsx | Implements settings page layout with promotions section |
| apps/bubble-studio/src/hooks/useRedeemCoupon.ts | Provides React hook for coupon redemption with mutation handling |
| apps/bubble-studio/src/components/Sidebar.tsx | Adds settings navigation link to sidebar |
| apps/bubble-studio/src/components/RedeemCouponSection.tsx | Implements UI component for entering and redeeming coupon codes |
| apps/bubble-studio/src/components/MonthlyUsageBar.tsx | Displays active hackathon offer status banner |
| apps/bubble-studio/src/routeTree.gen.ts | Auto-generated route tree update for new settings route |
| }), | ||
| message: z.string().openapi({ | ||
| description: 'Human-readable message about the redemption result', | ||
| example: 'Coupon redeemed successfully! You now have Pro access.', |
There was a problem hiding this comment.
The error message describes "Pro access" but the code grants "pro_plus" plan with unlimited access. The message should accurately reflect what is being granted - either change the message to "unlimited access" or clarify what "Pro access" means in this context to match the actual plan type being granted.
| example: 'Coupon redeemed successfully! You now have Pro access.', | |
| example: 'Coupon redeemed successfully! You now have unlimited access.', |
| success: false, | ||
| message: `You already have an active promotional offer until ${expirationDate.toLocaleString()}.`, | ||
| }, | ||
| 409 |
There was a problem hiding this comment.
Using 409 (Conflict) for an already active offer is semantically incorrect. HTTP 409 is typically used for resource conflicts during creation/modification, not for business rule violations. Consider using 400 (Bad Request) instead, which is more appropriate for client errors when attempting an invalid operation.
| 409 | |
| 400 |
| // Helper: Check if hackathon offer is active | ||
| function getActiveHackathonOffer( | ||
| privateMetadata: Record<string, unknown> | ||
| ): HackathonOffer | null { | ||
| const hackathonOffer = privateMetadata?.hackathonOffer as | ||
| | HackathonOfferMetadata | ||
| | undefined; | ||
|
|
||
| if (!hackathonOffer?.expiresAt) { | ||
| return null; | ||
| } | ||
|
|
||
| const expiresAt = new Date(hackathonOffer.expiresAt); | ||
| const isActive = expiresAt > new Date(); | ||
|
|
||
| return { | ||
| isActive, | ||
| expiresAt: hackathonOffer.expiresAt, | ||
| redeemedAt: hackathonOffer.redeemedAt || null, | ||
| }; | ||
| } |
There was a problem hiding this comment.
The function getActiveHackathonOffer guarantees that expiresAt is non-null when it returns a non-null object (due to the check on line 66), but it returns the HackathonOffer type which has expiresAt as nullable. Consider creating a more specific return type like ActiveHackathonOffer that has expiresAt as a required non-null string, which would eliminate the need for the non-null assertion on line 135 and better represent the actual guarantee.
| return c.json( | ||
| { | ||
| success: false, | ||
| message: `You already have an active promotional offer until ${expirationDate.toLocaleString()}.`, |
There was a problem hiding this comment.
Using toLocaleString() without specifying a locale makes the date format unpredictable across different servers/environments. This could result in inconsistent date formatting in error messages shown to users. Consider using a consistent locale like 'en-US' or using a standardized format like ISO string with custom formatting.
| message: `You already have an active promotional offer until ${expirationDate.toLocaleString()}.`, | |
| message: `You already have an active promotional offer until ${expirationDate.toLocaleString('en-US')}.`, |
| app.openapi(redeemCouponRoute, async (c) => { | ||
| const userId = getUserId(c); | ||
| const appType = getAppType(c); | ||
| const { code } = c.req.valid('json'); | ||
|
|
||
| // Normalize the code | ||
| const normalizedCode = code.trim().toUpperCase(); | ||
|
|
||
| // Validate the coupon code | ||
| const validCodes = getValidCouponCodes(); | ||
| if (validCodes.length === 0) { | ||
| return c.json( | ||
| { | ||
| success: false, | ||
| message: 'Coupon redemption is not available at this time.', | ||
| }, | ||
| 503 | ||
| ); | ||
| } | ||
|
|
||
| if (!validCodes.includes(normalizedCode)) { | ||
| return c.json( | ||
| { | ||
| success: false, | ||
| message: 'Invalid coupon code. Please check the code and try again.', | ||
| }, | ||
| 400 | ||
| ); | ||
| } | ||
|
|
||
| // Calculate expiration | ||
| const expiresAt = calculateOfferExpiration(); | ||
| const redeemedAt = new Date(); | ||
|
|
||
| // Update Clerk private metadata | ||
| const clerkClient = getClerkClient(appType); | ||
| if (!clerkClient) { | ||
| return c.json( | ||
| { | ||
| success: false, | ||
| message: 'Unable to process redemption. Please try again later.', | ||
| }, | ||
| 500 | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| const clerkUser = await clerkClient.users.getUser(userId); | ||
|
|
||
| // Check if user already has an active hackathon offer | ||
| const existingOffer = getActiveHackathonOffer( | ||
| clerkUser.privateMetadata as Record<string, unknown> | ||
| ); | ||
| if (existingOffer?.isActive) { | ||
| const expirationDate = new Date(existingOffer.expiresAt!); | ||
| return c.json( | ||
| { | ||
| success: false, | ||
| message: `You already have an active promotional offer until ${expirationDate.toLocaleString()}.`, | ||
| }, | ||
| 409 | ||
| ); | ||
| } | ||
|
|
||
| // Set the hackathon offer in private metadata | ||
| await clerkClient.users.updateUserMetadata(userId, { | ||
| privateMetadata: { | ||
| ...clerkUser.privateMetadata, | ||
| hackathonOffer: { | ||
| expiresAt: expiresAt.toISOString(), | ||
| redeemedAt: redeemedAt.toISOString(), | ||
| code: normalizedCode, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| console.log( | ||
| `[subscription/redeem] User ${userId} redeemed coupon ${normalizedCode}, expires at ${expiresAt.toISOString()}` | ||
| ); | ||
|
|
||
| return c.json({ | ||
| success: true, | ||
| message: 'Coupon redeemed successfully! You now have unlimited access.', | ||
| expiresAt: expiresAt.toISOString(), | ||
| }); | ||
| } catch (err) { | ||
| console.error('[subscription/redeem] Error redeeming coupon:', err); | ||
| return c.json( | ||
| { | ||
| success: false, | ||
| message: 'Failed to redeem coupon. Please try again later.', | ||
| }, | ||
| 500 | ||
| ); | ||
| } | ||
| }); |
There was a problem hiding this comment.
There is no rate limiting on the coupon redemption endpoint. This allows attackers to brute-force coupon codes by making unlimited redemption attempts. Consider implementing rate limiting (e.g., max 5 attempts per minute per user) to prevent abuse and protect the coupon codes from being discovered through brute force attacks.
| } catch (error) { | ||
| // Handle API errors and extract the response data | ||
| if (error instanceof ApiHttpError && error.data) { | ||
| const errorData = error.data as RedeemCouponResponse; | ||
| // If the error response has a valid structure, return it instead of throwing | ||
| if ( | ||
| typeof errorData === 'object' && | ||
| 'success' in errorData && | ||
| 'message' in errorData | ||
| ) { | ||
| console.log('[useRedeemCoupon] Error response:', errorData); | ||
| return errorData; | ||
| } | ||
| } | ||
| throw error; | ||
| } | ||
| }, | ||
| onSuccess: (data) => { | ||
| if (data.success) { | ||
| // Invalidate subscription query to refresh data with new offer | ||
| queryClient.invalidateQueries({ queryKey: ['subscription'] }); | ||
| console.log( | ||
| '[useRedeemCoupon] Coupon redeemed, subscription data invalidated' |
There was a problem hiding this comment.
The error handling is inconsistent. When the API returns an error response with a valid structure (lines 39-46), it's returned as data rather than thrown as an error. This means errors appear in the data state, not the error state. Then on line 54, there's a catch block that sets a generic error message, which will never execute for API errors since they're not thrown. This makes error handling confusing - consider either throwing all errors consistently or handling all responses (success and error) in the data state.
|
|
||
| export default RedeemCouponSection; |
There was a problem hiding this comment.
There's a redundant default export on line 141 when the component is already exported as a named export on line 6. This is unnecessary and could cause confusion about which export style to use. Remove the default export and use only the named export for consistency.
| export default RedeemCouponSection; |
| // Helper: Calculate expiration (1 day from now) | ||
| function calculateOfferExpiration(): Date { | ||
| const expiresAt = new Date(); | ||
| expiresAt.setDate(expiresAt.getDate() + 1); // 1 day from now |
There was a problem hiding this comment.
The calculation comment states "1 day from now" but this hardcoded duration should be configurable via environment variable. Hackathon offers might need different durations (e.g., 3 days, 1 week), and hardcoding this makes it inflexible. Consider adding a HACKATHON_OFFER_DURATION_HOURS environment variable.
| // Helper: Calculate expiration (1 day from now) | |
| function calculateOfferExpiration(): Date { | |
| const expiresAt = new Date(); | |
| expiresAt.setDate(expiresAt.getDate() + 1); // 1 day from now | |
| // Helper: Calculate expiration based on configured duration (default: 24 hours) | |
| function calculateOfferExpiration(): Date { | |
| const expiresAt = new Date(); | |
| const durationHoursEnv = env.HACKATHON_OFFER_DURATION_HOURS; | |
| const parsedDuration = durationHoursEnv ? Number(durationHoursEnv) : NaN; | |
| const durationHours = | |
| Number.isFinite(parsedDuration) && parsedDuration > 0 ? parsedDuration : 24; | |
| expiresAt.setTime( | |
| expiresAt.getTime() + durationHours * 60 * 60 * 1000 | |
| ); |
| success: false, | ||
| message: 'Coupon redemption is not available at this time.', | ||
| }, | ||
| 503 |
There was a problem hiding this comment.
Returning a 503 (Service Unavailable) status code is inappropriate when coupon codes are simply not configured. This status code typically indicates temporary server issues, not missing configuration. Consider using 404 (Not Found) or removing this check entirely and letting the validation fail naturally with an invalid code message.
| 503 | |
| 404 |
| // Format hackathon offer expiration date | ||
| const formatOfferExpiration = (dateString: string): string => { | ||
| const date = new Date(dateString); | ||
| return date.toLocaleString(undefined, { | ||
| month: 'short', | ||
| day: 'numeric', | ||
| hour: 'numeric', | ||
| minute: '2-digit', | ||
| hour12: true, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
There's code duplication between this formatting function and the one in RedeemCouponSection.tsx (lines 59-68). Both format expiration dates with the same logic. Consider extracting this to a shared utility function to maintain consistency and reduce duplication.
zhubzy
force-pushed
the
feat/redeption-code
branch
from
5386bd5 to
6aca24f
Compare
Deploying bubblelab-documentation with
|
| Latest commit: |
5ab85e3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2e35d2fd.bubblelab-documentation.pages.dev |
| Branch Preview URL: | https://feat-redeption-code.bubblelab-documentation.pages.dev |
Deploying bubble-studio with
|
| Latest commit: |
5ab85e3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e660ec3a.bubble-studio.pages.dev |
| Branch Preview URL: | https://feat-redeption-code.bubble-studio.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (10)
apps/bubble-studio/src/components/MonthlyUsageBar.tsx (2)
96-106: Extract duplicate date formatting logic to a shared utility.This
formatOfferExpirationfunction duplicates the same formatting logic inRedeemCouponSection.tsx(lines 59-68). Extracting this to a shared utility function would reduce duplication and ensure consistent formatting across components.🔎 Proposed refactor
Create a new utility file
apps/bubble-studio/src/utils/date-formatting.ts:export function formatOfferExpiration(dateString: string): string { const date = new Date(dateString); return date.toLocaleString('en-US', { month: 'short', day: 'numeric', hour: 'numeric', minute: '2-digit', hour12: true, }); }Then import and use it in both components:
+import { formatOfferExpiration } from '@/utils/date-formatting'; + - const formatOfferExpiration = (dateString: string): string => { - const date = new Date(dateString); - return date.toLocaleString(undefined, { - month: 'short', - day: 'numeric', - hour: 'numeric', - minute: '2-digit', - hour12: true, - }); - };
96-106: Inconsistent locale usage in date formatting.The
formatOfferExpirationfunction usesundefinedfor the locale parameter, which falls back to the browser's default locale. However,formatResetDate(lines 91-94) explicitly uses'en-US'. This inconsistency could result in dates being formatted differently across the UI.🔎 Proposed fix
const formatOfferExpiration = (dateString: string): string => { const date = new Date(dateString); - return date.toLocaleString(undefined, { + return date.toLocaleString('en-US', { month: 'short', day: 'numeric', hour: 'numeric', minute: '2-digit', hour12: true, }); };apps/bubblelab-api/src/routes/subscription.ts (7)
44-49: Make the offer duration configurable via environment variable.The expiration duration is hardcoded to 1 day (line 47). Different hackathon offers might require different durations (e.g., 3 days, 1 week). Consider adding a
HACKATHON_OFFER_DURATION_HOURSenvironment variable for flexibility.🔎 Proposed refactor
In
apps/bubblelab-api/src/config/env.ts, add:HACKATHON_OFFER_DURATION_HOURS: process.env.HACKATHON_OFFER_DURATION_HOURS || '24',Then update this function:
-// Helper: Calculate expiration (1 day from now) +// Helper: Calculate expiration based on configured duration (default: 24 hours) function calculateOfferExpiration(): Date { const expiresAt = new Date(); - expiresAt.setDate(expiresAt.getDate() + 1); // 1 day from now + const durationHoursEnv = env.HACKATHON_OFFER_DURATION_HOURS; + const parsedDuration = durationHoursEnv ? Number(durationHoursEnv) : NaN; + const durationHours = + Number.isFinite(parsedDuration) && parsedDuration > 0 ? parsedDuration : 24; + expiresAt.setTime( + expiresAt.getTime() + durationHours * 60 * 60 * 1000 + ); return expiresAt; }
58-78: Consider a more specific return type for active offers.The function
getActiveHackathonOfferguarantees thatexpiresAtis non-null when it returns a non-null object (due to the check on line 66), but it returns theHackathonOffertype which hasexpiresAtas nullable. A more specific return type likeActiveHackathonOfferwith required non-nullexpiresAtwould eliminate the need for the non-null assertion on line 135 and better represent the actual guarantee.🔎 Proposed refactor
+// Helper: Active hackathon offer with guaranteed non-null expiresAt +interface ActiveHackathonOffer extends HackathonOffer { + expiresAt: string; // non-null override +} + // Helper: Check if hackathon offer is active function getActiveHackathonOffer( privateMetadata: Record<string, unknown> -): HackathonOffer | null { +): ActiveHackathonOffer | null { const hackathonOffer = privateMetadata?.hackathonOffer as | HackathonOfferMetadata | undefined;
80-176: Add rate limiting to prevent coupon code brute-force attacks.The redemption endpoint lacks rate limiting, which could allow attackers to brute-force coupon codes by making unlimited redemption attempts. Consider implementing rate limiting (e.g., max 5 attempts per minute per user) using middleware or a library like
hono-rate-limiterto protect against abuse.
91-99: Use a more appropriate HTTP status code for missing configuration.Returning 503 (Service Unavailable) when coupon codes are not configured is inappropriate. This status code typically indicates temporary server issues, not missing configuration. Consider using 404 (Not Found) to indicate the feature is unavailable, or let the validation fail naturally with an invalid code message.
🔎 Proposed fix
if (validCodes.length === 0) { return c.json( { success: false, message: 'Coupon redemption is not available at this time.', }, - 503 + 404 ); }
134-143: Use 400 (Bad Request) for business rule violations.Using 409 (Conflict) for an already active offer is semantically incorrect. HTTP 409 is typically used for resource conflicts during creation/modification (e.g., duplicate resource), not for business rule violations. Use 400 (Bad Request) instead, which is more appropriate for client errors when attempting an invalid operation.
🔎 Proposed fix
return c.json( { success: false, message: `You already have an active promotional offer until ${expirationDate.toLocaleString()}.`, }, - 409 + 400 );
139-139: Use a consistent locale for date formatting.Using
toLocaleString()without specifying a locale makes the date format unpredictable across different servers/environments. This could result in inconsistent date formatting in error messages shown to users. Consider using a consistent locale like 'en-US' or a standardized ISO format with custom formatting.🔎 Proposed fix
- message: `You already have an active promotional offer until ${expirationDate.toLocaleString()}.`, + message: `You already have an active promotional offer until ${expirationDate.toLocaleString('en-US')}.`,
163-163: Clarify the duration in the success message.The message "You now have unlimited access" could be misleading since the offer expires in 1 day. Be more specific about the duration to set proper user expectations.
🔎 Suggested improvement
return c.json({ success: true, - message: 'Coupon redeemed successfully! You now have unlimited access.', + message: 'Coupon redeemed successfully! You now have 1 day of unlimited access.', expiresAt: expiresAt.toISOString(), });packages/bubble-shared-schemas/src/subscription-status-schema.ts (1)
36-54: Clarify the success message to accurately reflect what is granted.The message on line 45 states "You now have Pro access" but the actual implementation grants "pro_plus" plan with unlimited access for a limited time (24 hours). The message should accurately reflect what is being granted to avoid user confusion.
🔎 Proposed fix
message: z.string().openapi({ description: 'Human-readable message about the redemption result', - example: 'Coupon redeemed successfully! You now have Pro access.', + example: 'Coupon redeemed successfully! You now have unlimited access.', }),
🧹 Nitpick comments (2)
apps/bubblelab-api/.env.example (1)
40-41: Consider section placement and add example format for HACKATHON_COUPON_CODES.
HACKATHON_COUPON_CODESis consumed by the backend/subscription/redeemendpoint for coupon validation, not by Python scripts. The current placement under "For python scripts / pdf operations" may be slightly misleading. Consider either:
- Moving it to a new dedicated section for hackathon/promotion configuration, or
- Moving it under "Optional environmental variables for default integration support"
Additionally, include an example format to guide developers on the expected comma-separated structure.
🔎 Proposed refactor for improved organization and clarity
- # For python scripts / pdf operations + # For python scripts / pdf operations PYTHON_PATH= + # Hackathon / Promotions + HACKATHON_COUPON_CODES=CODE1,CODE2,CODE3 # Comma-separated list of valid hackathon coupon codesAlternatively, move it under the optional environmental variables section:
# Optional environmental variables for default integration support OPENAI_API_KEY= RESEND_API_KEY= FIRE_CRAWL_API_KEY= CLOUDFLARE_R2_ACCESS_KEY= CLOUDFLARE_R2_SECRET_KEY= CLOUDFLARE_R2_ACCOUNT_ID= WISPR_API_KEY= + HACKATHON_COUPON_CODES= # Comma-separated list of valid hackathon coupon codes (e.g., CODE1,CODE2,CODE3)packages/bubble-runtime/src/runtime/BubbleRunner.test.ts (1)
450-457: Consider consistency withexpectValidScriptor document the intent.This test uses
validateBubbleFlowdirectly withrequireLintErrors=true, whereasexpectValidScriptusesrequireLintErrors=false. If the stricter lint validation is intentional for this specific test case, consider adding a brief comment explaining why. Otherwise, consider usingexpectValidScriptfor consistency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
apps/bubble-studio/src/components/MonthlyUsageBar.tsxapps/bubble-studio/src/components/RedeemCouponSection.tsxapps/bubble-studio/src/components/Sidebar.tsxapps/bubble-studio/src/hooks/useRedeemCoupon.tsapps/bubble-studio/src/pages/SettingsPage.tsxapps/bubble-studio/src/routeTree.gen.tsapps/bubble-studio/src/routes/settings.tsxapps/bubblelab-api/.env.exampleapps/bubblelab-api/src/config/env.tsapps/bubblelab-api/src/middleware/auth.tsapps/bubblelab-api/src/routes/subscription.tsapps/bubblelab-api/src/schemas/subscription.tspackages/bubble-runtime/src/runtime/BubbleRunner.test.tspackages/bubble-runtime/src/runtime/BubbleRunner.tspackages/bubble-runtime/src/utils/sanitize-script.tspackages/bubble-runtime/tests/fixtures/index.tspackages/bubble-runtime/tests/fixtures/legitimate-process-env-string.tspackages/bubble-runtime/tests/fixtures/malicious-process-env-bracket.tspackages/bubble-runtime/tests/fixtures/malicious-process-env-standalone.tspackages/bubble-runtime/tests/fixtures/malicious-process-env.tspackages/bubble-shared-schemas/src/subscription-status-schema.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/bubble-studio/src/components/Sidebar.tsx
- apps/bubble-studio/src/hooks/useRedeemCoupon.ts
- apps/bubblelab-api/src/middleware/auth.ts
- apps/bubblelab-api/src/config/env.ts
- apps/bubble-studio/src/components/RedeemCouponSection.tsx
- apps/bubblelab-api/src/schemas/subscription.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/packages/bubble-shared-schemas/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/api.mdc)
**/packages/bubble-shared-schemas/**/*.{ts,tsx}: Write shared schemas between frontend and backend in/packages/bubble-shared-schemasdirectory
Runpnpm build:coreafter modifying shared schemas since it is a separate package and types need to be regenerated
Files:
packages/bubble-shared-schemas/src/subscription-status-schema.ts
🧠 Learnings (13)
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: Applies to packages/bubble-core/scripts/bubble-bundler.ts : The bundler script at `packages/bubble-core/scripts/bubble-bundler.ts` must use absolute file paths as cache keys to prevent collisions between packages with identically named files
Applied to files:
packages/bubble-runtime/src/runtime/BubbleRunner.tspackages/bubble-runtime/src/utils/sanitize-script.tspackages/bubble-runtime/src/runtime/BubbleRunner.test.ts
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: Applies to packages/bubble-core/dist/**/*.d.ts : All external imports (except relative imports) must be removed from the bundled output to prevent circular processing and external dependency issues
Applied to files:
packages/bubble-runtime/src/runtime/BubbleRunner.tspackages/bubble-runtime/src/runtime/BubbleRunner.test.ts
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: Applies to packages/bubble-core/dist/**/*.d.ts : The type bundling system for Monaco Editor must process TypeScript declaration files (.d.ts) and inline all dependencies into a single self-contained bundle in `packages/bubble-core/dist/bubble-bundle.d.ts`
Applied to files:
packages/bubble-runtime/src/runtime/BubbleRunner.tspackages/bubble-runtime/tests/fixtures/malicious-process-env-standalone.tspackages/bubble-runtime/tests/fixtures/legitimate-process-env-string.tspackages/bubble-runtime/src/runtime/BubbleRunner.test.ts
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: The bundle process must run the TypeScript compiler (`tsc`) before executing the bundler script (`tsx scripts/bubble-bundler.ts`) to ensure all .d.ts files are generated
Applied to files:
packages/bubble-runtime/src/runtime/BubbleRunner.tspackages/bubble-runtime/src/runtime/BubbleRunner.test.ts
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: Applies to packages/bubble-core/src/index.ts : All new types and classes added to bubble-core must be exported from `packages/bubble-core/src/index.ts` to ensure they are included in the generated bundle
Applied to files:
packages/bubble-runtime/src/runtime/BubbleRunner.tsapps/bubble-studio/src/pages/SettingsPage.tsxpackages/bubble-runtime/tests/fixtures/legitimate-process-env-string.tspackages/bubble-runtime/src/runtime/BubbleRunner.test.ts
📚 Learning: 2025-12-22T09:55:47.864Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.864Z
Learning: Applies to packages/bubble-runtime/src/extraction/README.md : Refer to packages/bubble-runtime/src/extraction/README.md for information about bubble parsing, dependency graphs, and per-invocation cloning
Applied to files:
packages/bubble-runtime/src/runtime/BubbleRunner.ts
📚 Learning: 2025-12-19T03:16:48.793Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/api.mdc:0-0
Timestamp: 2025-12-19T03:16:48.793Z
Learning: Applies to **/bubblelab-api/**/*.test.{js,ts} : Reference webhook.test as the example for how backend tests should be written
Applied to files:
packages/bubble-runtime/tests/fixtures/malicious-process-env-standalone.tspackages/bubble-runtime/tests/fixtures/malicious-process-env.tspackages/bubble-runtime/tests/fixtures/legitimate-process-env-string.tspackages/bubble-runtime/src/runtime/BubbleRunner.test.tspackages/bubble-runtime/tests/fixtures/malicious-process-env-bracket.ts
📚 Learning: 2025-12-19T03:16:48.793Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/api.mdc:0-0
Timestamp: 2025-12-19T03:16:48.793Z
Learning: Applies to **/bubblelab-api/**/*.test.{js,ts} : Use `pnpm bun test` command to run backend tests to ensure setup files are properly loaded
Applied to files:
packages/bubble-runtime/tests/fixtures/malicious-process-env-standalone.tspackages/bubble-runtime/tests/fixtures/legitimate-process-env-string.tspackages/bubble-runtime/src/runtime/BubbleRunner.test.ts
📚 Learning: 2025-12-22T09:55:47.864Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.864Z
Learning: Applies to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts : Refer to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts for documentation on how bubble flow is supposed to be generated
Applied to files:
packages/bubble-runtime/tests/fixtures/legitimate-process-env-string.tspackages/bubble-runtime/src/runtime/BubbleRunner.test.ts
📚 Learning: 2025-12-19T03:16:48.793Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/api.mdc:0-0
Timestamp: 2025-12-19T03:16:48.793Z
Learning: Applies to **/packages/bubble-shared-schemas/**/*.{ts,tsx} : Write shared schemas between frontend and backend in `/packages/bubble-shared-schemas` directory
Applied to files:
packages/bubble-shared-schemas/src/subscription-status-schema.ts
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: When adding new types to `bubblelab/shared-schemas`, they are automatically included in the bundle without requiring manual configuration changes
Applied to files:
packages/bubble-shared-schemas/src/subscription-status-schema.ts
📚 Learning: 2025-12-19T03:16:48.793Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/api.mdc:0-0
Timestamp: 2025-12-19T03:16:48.793Z
Learning: Applies to **/packages/bubble-shared-schemas/**/*.{ts,tsx} : Run `pnpm build:core` after modifying shared schemas since it is a separate package and types need to be regenerated
Applied to files:
packages/bubble-shared-schemas/src/subscription-status-schema.ts
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: Applies to apps/bubble-studio/public/bubble-types.txt : The bundle file at `apps/bubble-studio/public/bubble-types.txt` must be kept synchronized with `packages/bubble-core/dist/bubble-bundle.d.ts` after each build
Applied to files:
apps/bubble-studio/src/routeTree.gen.ts
🧬 Code graph analysis (10)
packages/bubble-runtime/src/runtime/BubbleRunner.ts (2)
packages/bubble-runtime/src/utils/sanitize-script.ts (1)
sanitizeScript(10-53)packages/bubble-runtime/src/index.ts (1)
sanitizeScript(8-8)
apps/bubble-studio/src/pages/SettingsPage.tsx (1)
apps/bubble-studio/src/components/RedeemCouponSection.tsx (1)
RedeemCouponSection(6-139)
packages/bubble-runtime/tests/fixtures/malicious-process-env-standalone.ts (2)
packages/bubble-shared-schemas/src/trigger.ts (1)
WebhookEvent(64-66)packages/bubble-core/src/index.ts (1)
BubbleFlow(19-19)
packages/bubble-runtime/src/utils/sanitize-script.ts (1)
packages/bubble-runtime/src/index.ts (1)
sanitizeScript(8-8)
packages/bubble-runtime/tests/fixtures/malicious-process-env.ts (2)
packages/bubble-shared-schemas/src/trigger.ts (1)
WebhookEvent(64-66)packages/bubble-core/src/index.ts (1)
BubbleFlow(19-19)
packages/bubble-runtime/tests/fixtures/legitimate-process-env-string.ts (2)
packages/bubble-shared-schemas/src/trigger.ts (1)
WebhookEvent(64-66)packages/bubble-core/src/index.ts (1)
BubbleFlow(19-19)
packages/bubble-runtime/src/runtime/BubbleRunner.test.ts (3)
packages/bubble-runtime/src/runtime/BubbleRunner.ts (1)
BubbleRunner(47-791)packages/bubble-runtime/src/validation/index.ts (1)
validateBubbleFlow(37-102)packages/bubble-runtime/tests/fixtures/index.ts (1)
getFixture(88-90)
apps/bubblelab-api/src/routes/subscription.ts (4)
apps/bubblelab-api/src/middleware/auth.ts (2)
getUserId(320-329)getAppType(368-371)apps/bubblelab-api/src/config/env.ts (1)
env(67-95)packages/bubble-shared-schemas/src/subscription-status-schema.ts (1)
HackathonOffer(22-22)apps/bubblelab-api/src/schemas/subscription.ts (1)
redeemCouponRoute(33-66)
packages/bubble-runtime/tests/fixtures/malicious-process-env-bracket.ts (2)
packages/bubble-shared-schemas/src/trigger.ts (1)
WebhookEvent(64-66)packages/bubble-core/src/index.ts (1)
BubbleFlow(19-19)
apps/bubble-studio/src/routes/settings.tsx (1)
apps/bubble-studio/src/pages/SettingsPage.tsx (1)
SettingsPage(4-31)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: Cloudflare Pages: bubblelab-documentation
🔇 Additional comments (18)
apps/bubble-studio/src/components/MonthlyUsageBar.tsx (1)
194-202: LGTM!The hackathon offer banner is correctly implemented with proper conditional rendering and uses the formatted expiration date effectively.
packages/bubble-shared-schemas/src/subscription-status-schema.ts (3)
4-22: LGTM!The
hackathonOfferSchemais well-structured with appropriate OpenAPI metadata and clear field descriptions.
24-34: LGTM!The
redeemCouponRequestSchemacorrectly validates coupon codes with a minimum length constraint.
131-133: LGTM! Don't forget to rebuild the shared schemas.The
hackathonOfferfield is correctly integrated into the subscription status response schema.As per coding guidelines: Run
pnpm build:coreafter modifying shared schemas since it is a separate package and types need to be regenerated.apps/bubblelab-api/src/routes/subscription.ts (2)
36-42: LGTM!The
getValidCouponCodesfunction correctly parses and normalizes coupon codes from the environment variable.
246-264: LGTM!The hackathon offer status retrieval is correctly implemented with proper error handling and logging.
packages/bubble-runtime/src/runtime/BubbleRunner.ts (1)
426-427: LGTM!The call to
sanitizeScripthas been correctly updated to match the new signature that accepts only the script code. This aligns with the security improvements to blockprocess.envaccess.apps/bubble-studio/src/routes/settings.tsx (1)
9-26: LGTM!The authentication check and redirect logic are correctly implemented. The use of
replace: trueprevents back-button issues, and returningnullduring the redirect is appropriate for React 19.apps/bubble-studio/src/pages/SettingsPage.tsx (1)
1-31: LGTM!The
SettingsPagecomponent is well-structured with clean layout and proper integration of theRedeemCouponSection. The styling is consistent with the application's design patterns.packages/bubble-runtime/tests/fixtures/malicious-process-env-standalone.ts (1)
1-26: LGTM!This test fixture correctly demonstrates the malicious patterns that the
sanitizeScriptfunction should block. The code attempts to accessprocess.envdirectly (line 16) and enumerate its keys (line 17), which are appropriate test cases for validating security controls.packages/bubble-runtime/tests/fixtures/index.ts (1)
66-69: LGTM!The new test fixtures for
process.envsecurity testing are correctly integrated. The naming is clear and follows the existing pattern, making it easy to identify malicious vs. legitimate test cases.packages/bubble-runtime/tests/fixtures/malicious-process-env-bracket.ts (1)
1-26: LGTM!The test fixture correctly demonstrates the bracket notation attack vector for
process.envaccess. The structure is consistent with the other malicious fixtures, and it properly tests both single and multiple key accesses via bracket notation.packages/bubble-runtime/src/runtime/BubbleRunner.test.ts (2)
7-30: LGTM! Good helper extraction.The
expectValidScripthelper consolidates repeated validation logic into a single reusable function with optional error logging. This improves test maintainability and reduces duplication.
565-652: LGTM! Comprehensive security test coverage.The test suite thoroughly covers all
process.envaccess vectors:
- Dot notation (
process.env.KEY)- Bracket notation (
process.env['KEY'])- Standalone object access (
process.env)- Legitimate string/comment mentions (should pass)
The tests properly validate script syntax before execution and assert the expected security error message.
packages/bubble-runtime/src/utils/sanitize-script.ts (1)
10-53: LGTM! Well-designed two-pass sanitization.The placeholder-based approach correctly protects string literals and comments from false positive matches. The split error message (
'process' + '.env') cleverly avoids self-matching during the regex replacement.Minor edge case consideration: If user code contains
__SANITIZE_PLACEHOLDER_followed by a number and__, the restoration loop could cause unexpected behavior. Consider using a more unique/random prefix if this becomes a concern in practice.packages/bubble-runtime/tests/fixtures/malicious-process-env.ts (1)
1-25: LGTM!The test fixture correctly demonstrates the dot notation attack vector (
process.env.SECRET_KEY). The structure is consistent with the other test fixtures in this PR.packages/bubble-runtime/tests/fixtures/legitimate-process-env-string.ts (1)
1-28: LGTM!Excellent positive test case that validates the sanitizer correctly preserves legitimate mentions of
process.envin:
- Single-line comments (lines 1, 13, 16)
- String literals (line 18)
- Template strings (line 19)
This ensures the two-pass placeholder approach works correctly without false positives.
apps/bubble-studio/src/routeTree.gen.ts (1)
12-12: LGTM! Auto-generated route integration is correct.The SettingsRoute has been correctly integrated into the generated route tree. All additions follow the established pattern for TanStack Router route registration, including:
- Import declaration
- Route constant creation
- Type declarations across all route interfaces
- Module augmentation for type safety
- Route tree wiring
As noted in the file header, this is an auto-generated file and should not be manually modified.
Also applies to: 20-24, 62-62, 71-71, 81-81, 92-92, 95-102, 110-110, 120-120, 126-132, 184-184
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
apps/bubblelab-api/src/routes/subscription.ts (3)
149-158: Inconsistent date formatting: use locale for consistency.
formatExpirationDate(line 39) uses'en-US'locale, but line 154 usestoLocaleString()without a locale, making the date format environment-dependent.🔎 Proposed fix
if (existingOffer?.isActive) { const expirationDate = new Date(existingOffer.expiresAt!); return c.json( { success: false, - message: `You already have an active promotional offer until ${expirationDate.toLocaleString()}.`, + message: `You already have an active promotional offer until ${formatExpirationDate(expirationDate)}.`, }, 409 ); }
106-114: Consider using 404 instead of 503 for unconfigured coupon codes.503 (Service Unavailable) typically indicates temporary server issues. If coupon codes aren't configured, this is more of a "feature not available" scenario. A 404 or disabling the endpoint entirely might be more semantically correct, though 503 is acceptable if this is meant to be a temporary state during hackathon setup.
70-90: Consider tightening the return type to avoid non-null assertion.The function guarantees
expiresAtis non-null when returning a non-null object (due to the check on line 78), but the return type usesHackathonOfferwhich may have nullableexpiresAt. This forces the non-null assertion on line 150.🔎 Suggested improvement
+// Return type guarantees expiresAt is present +interface ActiveHackathonOffer extends Omit<HackathonOffer, 'expiresAt'> { + expiresAt: string; // non-null guaranteed +} // Helper: Check if hackathon offer is active function getActiveHackathonOffer( privateMetadata: Record<string, unknown> -): HackathonOffer | null { +): ActiveHackathonOffer | null {
🧹 Nitpick comments (3)
packages/bubble-runtime/src/runtime/BubbleRunner.test.ts (1)
7-30: Clarify the intended usage pattern for expectValidScript.The docstring states "This should be called after runAll()," but the new security tests (lines 579, 602, 625, 648, 671) call it before runAll(), while other tests (lines 431, 444, 468, etc.) follow the docstring and call it after. This inconsistency creates confusion about the correct usage pattern.
If calling it before runAll() is intentional for security tests (e.g., to validate that sanitization produces syntactically valid code before execution), consider updating the docstring to clarify that both patterns are valid depending on the test scenario.
Suggested docstring update
/** * Utility function to validate that a BubbleRunner's script is valid after execution. - * This should be called after runAll() to ensure the injected code didn't break the script. + * Can be called before runAll() to validate sanitization transforms, or after runAll() + * to ensure the injected code didn't break the script. */apps/bubblelab-api/src/middleware/rate-limit.test.ts (1)
6-152: Good test coverage with room for edge case improvement.The tests cover the core rate limiting behaviors well with proper isolation via unique
keyGeneratorkeys. Consider adding a test for window reset behavior to verify that requests are allowed again afterwindowMsexpires:🔎 Suggested additional test case
it('should reset counter after window expires', async () => { const shortWindow = 100; // 100ms for testing app.use( '/test', rateLimit({ windowMs: shortWindow, maxAttempts: 1, keyGenerator: () => 'test-user-reset', }) ); app.post('/test', (c) => c.json({ success: true })); // First request succeeds const res1 = await app.request('/test', { method: 'POST' }); expect(res1.status).toBe(200); // Second request blocked const res2 = await app.request('/test', { method: 'POST' }); expect(res2.status).toBe(429); // Wait for window to expire await new Promise((resolve) => setTimeout(resolve, shortWindow + 10)); // Third request should succeed (window reset) const res3 = await app.request('/test', { method: 'POST' }); expect(res3.status).toBe(200); });Based on learnings, tests should reference webhook.test as the example pattern. Run tests with
pnpm bun testto ensure setup files are properly loaded.apps/bubblelab-api/src/middleware/rate-limit.ts (1)
58-64: First request doesn't receive rate limit headers.When a key has no existing entry, the middleware sets the count and calls
next()without addingX-RateLimit-*headers. This creates inconsistency where clients only see rate limit headers after their second request (or when blocked).🔎 Proposed fix to add headers on first request
if (!entry) { // First request in window - rateLimitStore.set(key, { + const newEntry = { count: 1, resetAt: now + windowMs, - }); + }; + rateLimitStore.set(key, newEntry); + + // Add rate limit headers for first request + c.header('X-RateLimit-Limit', maxAttempts.toString()); + c.header('X-RateLimit-Remaining', (maxAttempts - 1).toString()); + c.header('X-RateLimit-Reset', Math.ceil(newEntry.resetAt / 1000).toString()); + return next(); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/bubblelab-api/src/middleware/rate-limit.test.tsapps/bubblelab-api/src/middleware/rate-limit.tsapps/bubblelab-api/src/routes/subscription.tspackages/bubble-runtime/src/runtime/BubbleRunner.test.tspackages/bubble-runtime/src/utils/sanitize-script.tspackages/bubble-runtime/tests/fixtures/index.tspackages/bubble-runtime/tests/fixtures/malicious-process-bracket-env.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bubble-runtime/src/utils/sanitize-script.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/bubblelab-api/**/*.test.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/api.mdc)
**/bubblelab-api/**/*.test.{js,ts}: Usepnpm bun testcommand to run backend tests to ensure setup files are properly loaded
Reference webhook.test as the example for how backend tests should be written
Files:
apps/bubblelab-api/src/middleware/rate-limit.test.ts
🧠 Learnings (4)
📚 Learning: 2025-12-19T03:16:48.793Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/api.mdc:0-0
Timestamp: 2025-12-19T03:16:48.793Z
Learning: Applies to **/bubblelab-api/**/*.test.{js,ts} : Use `pnpm bun test` command to run backend tests to ensure setup files are properly loaded
Applied to files:
packages/bubble-runtime/src/runtime/BubbleRunner.test.tsapps/bubblelab-api/src/middleware/rate-limit.test.ts
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: Applies to packages/bubble-core/scripts/bubble-bundler.ts : The bundler script at `packages/bubble-core/scripts/bubble-bundler.ts` must use absolute file paths as cache keys to prevent collisions between packages with identically named files
Applied to files:
packages/bubble-runtime/src/runtime/BubbleRunner.test.ts
📚 Learning: 2025-12-19T03:16:48.793Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/api.mdc:0-0
Timestamp: 2025-12-19T03:16:48.793Z
Learning: Applies to **/bubblelab-api/**/*.test.{js,ts} : Reference webhook.test as the example for how backend tests should be written
Applied to files:
packages/bubble-runtime/src/runtime/BubbleRunner.test.tsapps/bubblelab-api/src/middleware/rate-limit.test.tspackages/bubble-runtime/tests/fixtures/malicious-process-bracket-env.ts
📚 Learning: 2025-12-22T09:55:47.864Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.864Z
Learning: Applies to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts : Refer to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts for documentation on how bubble flow is supposed to be generated
Applied to files:
packages/bubble-runtime/src/runtime/BubbleRunner.test.ts
🧬 Code graph analysis (4)
packages/bubble-runtime/src/runtime/BubbleRunner.test.ts (2)
packages/bubble-runtime/src/runtime/BubbleRunner.ts (1)
BubbleRunner(47-791)packages/bubble-runtime/src/validation/index.ts (1)
validateBubbleFlow(37-102)
apps/bubblelab-api/src/middleware/rate-limit.test.ts (1)
apps/bubblelab-api/src/middleware/rate-limit.ts (1)
rateLimit(36-96)
apps/bubblelab-api/src/routes/subscription.ts (4)
apps/bubblelab-api/src/middleware/auth.ts (2)
getUserId(320-329)getAppType(368-371)apps/bubblelab-api/src/config/env.ts (1)
env(67-95)packages/bubble-shared-schemas/src/subscription-status-schema.ts (1)
HackathonOffer(22-22)apps/bubblelab-api/src/middleware/rate-limit.ts (1)
couponRedemptionRateLimit(102-106)
packages/bubble-runtime/tests/fixtures/malicious-process-bracket-env.ts (2)
packages/bubble-shared-schemas/src/trigger.ts (1)
WebhookEvent(64-66)packages/bubble-core/src/index.ts (1)
BubbleFlow(19-19)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: Cloudflare Pages: bubblelab-documentation
🔇 Additional comments (7)
packages/bubble-runtime/tests/fixtures/malicious-process-bracket-env.ts (1)
1-26: LGTM! Well-structured security test fixture.The fixture correctly demonstrates the malicious pattern it's designed to test, using both pure bracket notation (
process['env']['SECRET_KEY']) and mixed bracket/dot notation (process['env'].API_TOKEN). This complements the other security fixtures to ensure comprehensive coverage of process.env access patterns.packages/bubble-runtime/src/runtime/BubbleRunner.test.ts (1)
568-678: Excellent security test coverage!The new test suite comprehensively validates process.env access prevention across multiple attack vectors:
- Dot notation, bracket notation, standalone object access, and mixed patterns
- A positive test case ensuring legitimate string/comment mentions aren't blocked
The consistent error assertion pattern and thorough coverage strengthen the security posture of the runtime.
packages/bubble-runtime/tests/fixtures/index.ts (1)
66-70: All new fixture files are properly created and registered.The five new security test fixtures are in place:
- malicious-process-env.ts
- malicious-process-env-bracket.ts
- malicious-process-env-standalone.ts
- malicious-process-bracket-env.ts
- legitimate-process-env-string.ts
apps/bubblelab-api/src/middleware/rate-limit.ts (1)
15-29: In-memory rate limiting won't scale across multiple server instances.The
rateLimitStoreMap is local to each process. In a multi-instance deployment, rate limits won't be shared, allowing users to exceed limits by hitting different instances. For the hackathon feature this is likely acceptable, but consider documenting this limitation or using Redis for production-grade rate limiting.apps/bubblelab-api/src/routes/subscription.ts (3)
37-46: LGTM: Good use of explicit locale for consistent date formatting.Using
'en-US'locale ensures consistent date formatting across different server environments.
160-170: Good practice: Preserving existing metadata when updating.The spread of
clerkUser.privateMetadatabefore addinghackathonOfferensures other private metadata fields aren't overwritten.
261-279: LGTM: Good error handling for Clerk access in status endpoint.The try/catch with logging ensures the status endpoint doesn't fail if hackathon offer retrieval fails.
| /** | ||
| * Pre-configured rate limiter for coupon redemption | ||
| * 5 attempts per hour per user | ||
| */ | ||
| export const couponRedemptionRateLimit = rateLimit({ | ||
| windowMs: 60 * 60 * 1000, // 1 hour | ||
| maxAttempts: 10, | ||
| message: 'Too many redemption attempts.', | ||
| }); |
There was a problem hiding this comment.
Comment/code mismatch: documentation says 5 attempts but code configures 10.
The JSDoc comment states "5 attempts per hour" but maxAttempts is set to 10.
🔎 Proposed fix
/**
* Pre-configured rate limiter for coupon redemption
- * 5 attempts per hour per user
+ * 10 attempts per hour per user
*/
export const couponRedemptionRateLimit = rateLimit({
windowMs: 60 * 60 * 1000, // 1 hour
maxAttempts: 10,
message: 'Too many redemption attempts.',
});📝 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.
| /** | |
| * Pre-configured rate limiter for coupon redemption | |
| * 5 attempts per hour per user | |
| */ | |
| export const couponRedemptionRateLimit = rateLimit({ | |
| windowMs: 60 * 60 * 1000, // 1 hour | |
| maxAttempts: 10, | |
| message: 'Too many redemption attempts.', | |
| }); | |
| /** | |
| * Pre-configured rate limiter for coupon redemption | |
| * 10 attempts per hour per user | |
| */ | |
| export const couponRedemptionRateLimit = rateLimit({ | |
| windowMs: 60 * 60 * 1000, // 1 hour | |
| maxAttempts: 10, | |
| message: 'Too many redemption attempts.', | |
| }); |
🤖 Prompt for AI Agents
In apps/bubblelab-api/src/middleware/rate-limit.ts around lines 98 to 106, the
JSDoc says "5 attempts per hour" but the rate limiter is configured with
maxAttempts: 10; update them to be consistent by setting maxAttempts to 5 to
match the comment (or if 10 is intended, change the JSDoc comment to reflect "10
attempts per hour"); ensure the chosen value is applied to the rateLimit config
and the JSDoc text is updated accordingly.
| // Apply rate limiting to redeem endpoint (5 attempts per hour per user) | ||
| app.use('/redeem', couponRedemptionRateLimit); |
There was a problem hiding this comment.
Comment states "5 attempts per hour" but rate limiter is configured for 10.
This comment is inconsistent with the actual couponRedemptionRateLimit configuration (10 attempts per hour).
🔎 Proposed fix
-// Apply rate limiting to redeem endpoint (5 attempts per hour per user)
+// Apply rate limiting to redeem endpoint (10 attempts per hour per user)
app.use('/redeem', couponRedemptionRateLimit);📝 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.
| // Apply rate limiting to redeem endpoint (5 attempts per hour per user) | |
| app.use('/redeem', couponRedemptionRateLimit); | |
| // Apply rate limiting to redeem endpoint (10 attempts per hour per user) | |
| app.use('/redeem', couponRedemptionRateLimit); |
🤖 Prompt for AI Agents
In apps/bubblelab-api/src/routes/subscription.ts around lines 92-93, the inline
comment states "5 attempts per hour per user" but the actual
couponRedemptionRateLimit is configured for 10 attempts per hour; update the
comment to accurately reflect the configured limit (change "5" to "10") or, if
the intended limit is 5, modify the rate limiter configuration to 5 attempts per
hour so code and comment match.
2 participants
Summary
Related Issues
Type of Change
Checklist
pnpm checkand all tests passScreenshots (if applicable)
Additional Context
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.