fix: verify checkout session metadata in team upgrade endpoint#28874
fix: verify checkout session metadata in team upgrade endpoint#28874pedroccastro wants to merge 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughA validation step was added to the team upgrade API route to verify that the team identifier stored in the Stripe checkout session metadata matches the team identifier from the URL route parameter. When a checkout session is confirmed as paid, the handler now extracts the team ID from the session metadata and compares it against the requested team ID. If the metadata contains a team ID that differs from the route parameter, the request fails with an HTTP 400 error response. The change adds six lines and does not modify other control flow logic. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/app/api/teams/`[team]/upgrade/route.ts:
- Around line 49-51: The current guard uses a truthy check on
checkoutSession.metadata?.teamId (sessionTeamId) which treats empty string as
absent and skips validation; update the logic in route.ts to check for the
presence of the key (e.g., metadata hasOwnProperty or sessionTeamId !==
undefined) and then validate that sessionTeamId is a valid integer (use
Number/parseInt and Number.isInteger or /^\d+$/) before comparing to id; if the
value is missing, non-numeric, or not equal to id, throw the existing
HttpError({ statusCode: 400, message: "Checkout session does not match team" })
so malformed values are rejected and only valid integer team IDs are accepted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3f8cb432-a70e-4119-8d6a-f02a4e51bd46
📒 Files selected for processing (1)
apps/web/app/api/teams/[team]/upgrade/route.ts
| const sessionTeamId = checkoutSession.metadata?.teamId; | ||
| if (sessionTeamId && Number(sessionTeamId) !== id) { | ||
| throw new HttpError({ statusCode: 400, message: "Checkout session does not match team" }); |
There was a problem hiding this comment.
Use presence check for teamId and reject malformed values.
On Line 50, the truthy guard (if (sessionTeamId && ...)) skips validation when teamId is an empty string, even though metadata is present. That weakens the new consistency check. Validate when the key is defined, and fail on non-integer values.
Suggested fix
- const sessionTeamId = checkoutSession.metadata?.teamId;
- if (sessionTeamId && Number(sessionTeamId) !== id) {
- throw new HttpError({ statusCode: 400, message: "Checkout session does not match team" });
- }
+ const sessionTeamId = checkoutSession.metadata?.teamId;
+ if (sessionTeamId !== undefined) {
+ const parsedSessionTeamId = Number.parseInt(sessionTeamId, 10);
+ if (!Number.isInteger(parsedSessionTeamId) || parsedSessionTeamId !== id) {
+ throw new HttpError({ statusCode: 400, message: "Checkout session does not match team" });
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sessionTeamId = checkoutSession.metadata?.teamId; | |
| if (sessionTeamId && Number(sessionTeamId) !== id) { | |
| throw new HttpError({ statusCode: 400, message: "Checkout session does not match team" }); | |
| const sessionTeamId = checkoutSession.metadata?.teamId; | |
| if (sessionTeamId !== undefined) { | |
| const parsedSessionTeamId = Number.parseInt(sessionTeamId, 10); | |
| if (!Number.isInteger(parsedSessionTeamId) || parsedSessionTeamId !== id) { | |
| throw new HttpError({ statusCode: 400, message: "Checkout session does not match team" }); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/app/api/teams/`[team]/upgrade/route.ts around lines 49 - 51, The
current guard uses a truthy check on checkoutSession.metadata?.teamId
(sessionTeamId) which treats empty string as absent and skips validation; update
the logic in route.ts to check for the presence of the key (e.g., metadata
hasOwnProperty or sessionTeamId !== undefined) and then validate that
sessionTeamId is a valid integer (use Number/parseInt and Number.isInteger or
/^\d+$/) before comparing to id; if the value is missing, non-numeric, or not
equal to id, throw the existing HttpError({ statusCode: 400, message: "Checkout
session does not match team" }) so malformed values are rejected and only valid
integer team IDs are accepted.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
2 participants
What does this PR do?
Adds a consistency check between the checkout session metadata and the team ID in the URL path for the team upgrade callback. Ensures the processed session was created for the specific team being upgraded.
Changes
checkoutSession.metadata.teamIdis present, verify it matches the team ID in the URL before processing billing mutationsteamIdis not set in session metadata (API v2 usespendingPaymentTeamId; legacy sessions predate the field)How should this be tested?
teamIdin session metadata continue to workMandatory Tasks