fix(validation): correct Zod email schema and centralize authentication validation - #370
Conversation
|
@KrutagyaKaneria is attempting to deploy a commit to the vishnukothakapu's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRegistration now uses shared Zod email and password validation across the API and page. The API persists and sends verification data using the validated email, while unit tests cover schema and password-helper behavior. ChangesAuthentication validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant RegistrationAPI
participant ValidationSchema
participant Prisma
participant EmailService
Client->>RegistrationAPI: Submit email and password
RegistrationAPI->>ValidationSchema: Validate registration fields
ValidationSchema-->>RegistrationAPI: Return validated data
RegistrationAPI->>Prisma: Create user and verification token
RegistrationAPI->>EmailService: Send verification email
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.3)lib/validations/auth.tsFile contains syntax errors that prevent linting: Line 55: Expected a semicolon or an implicit semicolon after a statement, but found none 🔧 ESLint
lib/validations/auth.tsParsing error: ';' expected. 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 |
|
Hi @KrutagyaKaneria, please resolve conflicts |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/api/auth/register/route.ts (1)
35-48: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle
namevalidation errors if using the fullsignupSchema.If you update
registerSchemato validate thenamefield as well, ensure that you provide thenametosafeParseand handle its potential validation errors.♻️ Proposed fix
- const result = registerSchema.safeParse({ email, password }); + const result = registerSchema.safeParse({ name, email, password }); if (!result.success) { const fieldErrors = result.error.flatten().fieldErrors; + if (fieldErrors.name) { + return NextResponse.json( + { error: fieldErrors.name[0] }, + { status: 400 }, + ); + } if (fieldErrors.email) { return NextResponse.json( { error: "Invalid email address" },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/auth/register/route.ts` around lines 35 - 48, Update the registration validation around registerSchema.safeParse to include the name input when registerSchema validates it, then handle fieldErrors.name explicitly with the appropriate 400 response. Preserve the existing email and password validation responses for their respective errors.
🧹 Nitpick comments (1)
app/register/page.tsx (1)
142-149: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider adding HTML5
minLengthvalidation to the name input.To better align the client-side validation with the shared
signupSchema(which requires at least 2 characters for the name), consider addingminLength={2}to this input.♻️ Proposed fix
<Input name="name" placeholder="Full name" autoComplete="name" required + minLength={2} className="pl-10 transition-colors" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/register/page.tsx` around lines 142 - 149, Update the name Input in the registration form to include HTML5 minLength validation of 2 characters, matching the shared signupSchema requirement while preserving the existing required and other input properties.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/auth/register/route.ts`:
- Around line 86-92: Update the sendVerificationEmail error handler in the
registration route to stop logging result.data.email in plaintext. Remove the
email from the console.error call, while preserving the non-blocking
registration behavior and existing failure message.
- Line 12: Update registerSchema to use signupSchema directly instead of picking
only email and password, so registration validates name alongside the other
shared signup fields and remains aligned with frontend validation.
---
Outside diff comments:
In `@app/api/auth/register/route.ts`:
- Around line 35-48: Update the registration validation around
registerSchema.safeParse to include the name input when registerSchema validates
it, then handle fieldErrors.name explicitly with the appropriate 400 response.
Preserve the existing email and password validation responses for their
respective errors.
---
Nitpick comments:
In `@app/register/page.tsx`:
- Around line 142-149: Update the name Input in the registration form to include
HTML5 minLength validation of 2 characters, matching the shared signupSchema
requirement while preserving the existing required and other input properties.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bae5133a-4595-4b30-8fdb-dac5e55c1fff
📒 Files selected for processing (4)
app/api/auth/register/route.tsapp/register/page.tsxlib/validations/auth.test.tslib/validations/auth.ts
| const REGISTER_LIMIT = 5; | ||
| const REGISTER_WINDOW_MS = 60 * 60 * 1000; | ||
|
|
||
| const registerSchema = signupSchema.pick({ email: true, password: true }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate the name field to maintain consistency with the shared schema.
By picking only email and password, the API bypasses the name validation defined in signupSchema (which requires at least 2 characters). This means the API could accept registrations with an empty name even though the frontend enforces it.
Consider using signupSchema directly to validate all fields, ensuring the API and frontend rules remain perfectly aligned.
♻️ Proposed fix
-const registerSchema = signupSchema.pick({ email: true, password: true });
+const registerSchema = signupSchema;📝 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 registerSchema = signupSchema.pick({ email: true, password: true }); | |
| const registerSchema = signupSchema; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/auth/register/route.ts` at line 12, Update registerSchema to use
signupSchema directly instead of picking only email and password, so
registration validates name alongside the other shared signup fields and remains
aligned with frontend validation.
| // Send verification email — non-blocking in dev if SMTP not configured | ||
| try { | ||
| await sendVerificationEmail(normalizedEmail, token); | ||
| await sendVerificationEmail(result.data.email, token); | ||
| } catch { | ||
| // Email sending failure should not block registration | ||
| console.error("Failed to send verification email to", normalizedEmail); | ||
| console.error("Failed to send verification email to", result.data.email); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid logging sensitive user data (PII).
Logging the user's email address in plain text creates a privacy risk and violates best practices for log management. Consider masking the email or removing it entirely from the log message, as the sendVerificationEmail function already logs relevant error details.
🛡️ Proposed fix
// Send verification email — non-blocking in dev if SMTP not configured
try {
await sendVerificationEmail(result.data.email, token);
} catch {
// Email sending failure should not block registration
- console.error("Failed to send verification email to", result.data.email);
+ console.error("Failed to send verification email");
}📝 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.
| // Send verification email — non-blocking in dev if SMTP not configured | |
| try { | |
| await sendVerificationEmail(normalizedEmail, token); | |
| await sendVerificationEmail(result.data.email, token); | |
| } catch { | |
| // Email sending failure should not block registration | |
| console.error("Failed to send verification email to", normalizedEmail); | |
| console.error("Failed to send verification email to", result.data.email); | |
| } | |
| // Send verification email — non-blocking in dev if SMTP not configured | |
| try { | |
| await sendVerificationEmail(result.data.email, token); | |
| } catch { | |
| // Email sending failure should not block registration | |
| console.error("Failed to send verification email"); | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 90-90: Avoid logging sensitive data
Context: console.error("Failed to send verification email to", result.data.email)
Note: [CWE-532] Insertion of Sensitive Information into Log File.
(log-sensitive-data-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/auth/register/route.ts` around lines 86 - 92, Update the
sendVerificationEmail error handler in the registration route to stop logging
result.data.email in plaintext. Remove the email from the console.error call,
while preserving the non-blocking registration behavior and existing failure
message.
Source: Linters/SAST tools
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
2 participants
📌 Summary
This PR fixes an invalid Zod validation schema that could cause runtime crashes and centralizes authentication validation to ensure consistent behavior across the application.
✨ Changes Made
z.email()API withz.string().email()inlib/validations/auth.ts.🛠️ Bug Fixes
TypeError: z.email is not a function.🧪 Tests Added
✅ Acceptance Criteria
z.email()withz.string().email().lib/validations/auth.test.ts.npm testpasses successfully.Closes #365
#GSSoC
#ECSoC26
This PR is submitted as part of GirlScript Summer of Code (GSSoC) 2026.
Kindly review and consider approving it if it aligns with the project's testing and API quality standards. Thank you for your time and feedback!
Summary by CodeRabbit
New Features
Bug Fixes
Tests