Conversation
…kout .default() only applies when value is undefined, but .catch() handles any invalid value (e.g. 'monthly/' with trailing slash from Safari URL handling) by falling back to 'monthly'. Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
✅ Deploy Preview for hyprnote-storybook canceled.
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
yujonglee
deleted the
devin/1771075009-fix-checkout-period-validation
branch
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const validateSearch = z.object({ | ||
| period: z.enum(["monthly", "yearly"]).default("monthly"), | ||
| period: z.enum(["monthly", "yearly"]).catch("monthly"), | ||
| scheme: z.enum(VALID_SCHEMES).optional(), |
There was a problem hiding this comment.
🔴 scheme field has the same trailing-slash vulnerability that was just fixed for period
The scheme field on line 15 uses .optional() which only handles undefined values, not invalid strings. If Safari (or any browser) sends a URL like ?scheme=hyprnote/ (with a trailing slash from URL normalization — the exact same issue that motivated this PR for period), the Zod enum validation will throw instead of gracefully falling back to undefined.
Root Cause and Suggested Fix
The period field was changed from .default("monthly") to .catch("monthly") to handle invalid values like monthly/. The scheme field at apps/web/src/routes/_view/app/checkout.tsx:15 has the identical vulnerability:
scheme: z.enum(VALID_SCHEMES).optional(), // throws on "hyprnote/"Should be:
scheme: z.enum(VALID_SCHEMES).optional().catch(undefined), // falls back to undefined on invalid valuesImpact: An invalid scheme value (e.g. with a trailing slash) will cause a Zod validation error, which cascades into the same kind of React concurrent rendering recovery error described in the Sentry issue HYPRNOTE2-6TA.
| scheme: z.enum(VALID_SCHEMES).optional(), | |
| scheme: z.enum(VALID_SCHEMES).optional().catch(undefined), |
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| const validateSearch = z.object({ | ||
| period: z.enum(["monthly", "yearly"]).default("monthly"), | ||
| period: z.enum(["monthly", "yearly"]).catch("monthly"), |
There was a problem hiding this comment.
🚩 .catch("monthly") silently normalizes ALL invalid period values, including yearly/
The .catch("monthly") on line 14 will silently convert any invalid period value to "monthly", including values like period=yearly/ where the user clearly intended yearly. This is a behavioral tradeoff worth noting: a user who intended yearly billing but whose browser appended a trailing slash will silently be redirected to a monthly checkout session instead. An alternative approach would be to strip trailing slashes before validation (e.g., via .transform(v => v.replace(/\/$/, "")) before the enum check), which would preserve the user's actual intent. The PR description flags this as a review item, and the team should decide whether silent fallback is acceptable or if normalization is preferred.
Was this helpful? React with 👍 or 👎 to provide feedback.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| const validateSearch = z.object({ | ||
| period: z.enum(["monthly", "yearly"]).default("monthly"), | ||
| period: z.enum(["monthly", "yearly"]).catch("monthly"), | ||
| scheme: z.enum(VALID_SCHEMES).optional(), |
There was a problem hiding this comment.
The scheme field has the same trailing-slash vulnerability mentioned in the PR description but wasn't fixed. If Safari URL normalization adds a trailing slash to the scheme parameter (e.g., scheme=hyprnote/), .optional() will throw a validation error instead of gracefully handling it.
Fix: Apply the same pattern:
scheme: z.enum(VALID_SCHEMES).catch(undefined).optional(),Or alternatively:
scheme: z.enum(VALID_SCHEMES).optional().catch(undefined),This ensures invalid scheme values fall back to undefined instead of throwing, preventing the same Sentry errors that affected the period field.
| scheme: z.enum(VALID_SCHEMES).optional(), | |
| scheme: z.enum(VALID_SCHEMES).catch(undefined).optional(), |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
1 participant
Summary
Fixes a Sentry error (
HYPRNOTE2-6TA) where Safari 26.2 sendsperiod=monthly/(with a trailing slash from URL normalization), causing the Zod enum validation to throw since.default()only handlesundefinedvalues, not invalid strings.Changed
.default("monthly")→.catch("monthly")so any invalidperiodvalue gracefully falls back to"monthly"instead of throwing a validation error that cascades into React concurrent rendering recovery errors.Review & Testing Checklist for Human
.catch()silently falling back to"monthly"for any bad value (includingperiod=yearly/→ becomesmonthly) is acceptable behavior, rather than attempting to strip/normalize the valuescheme: z.enum(VALID_SCHEMES).optional()has the same trailing-slash vulnerability — an invalid scheme value likehyprnote/would also throw rather than becomingundefinedNotes