Skip to content

fix: resolve critical build, security, and billing issues for production readiness#4

Merged
projectamazonph merged 1 commit into
mainfrom
fix/critical-security-and-build-issues
Jul 17, 2026
Merged

fix: resolve critical build, security, and billing issues for production readiness#4
projectamazonph merged 1 commit into
mainfrom
fix/critical-security-and-build-issues

Conversation

@projectamazonph

@projectamazonph projectamazonph commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Summary

This PR addresses the critical and high-severity findings from the Interview Lab codebase audit. The fixes target the blockers preventing production deployment and the most urgent security vulnerabilities.

Changes

Build integrity

  • FieldButton outline variant: Added missing outline variant to fieldButtonVariants in glass-button.tsx — fixes the TypeScript compilation failure on main that blocked Vercel deployment
  • Fixed pre-existing type errors in FieldBadge (added secondary, destructive, muted variants) and Button (fixed invalid default variant/size usage)

Billing lockdown

  • Subscription checkout disabled: /api/subscription/checkout now returns 503 with "Paid plans are not currently available." instead of performing direct unpaid upgrades
  • Plan change disabled: The change action in /api/subscription/manage is blocked for paid tier upgrades

Authentication hardening

  • JWT secret validation: Removed the publicly-known fallback interviewlab-dev-secret-change-in-production. The app now throws at startup if JWT_SECRET is absent or shorter than 32 characters

Server-side authorization

  • Questions API: Requires authentication + subscription tier check for non-beginner difficulties; strips premium fields (whyEmployersAsk, strongAnswerPoints, weakAnswerWarnings, sampleAnswer, advancedQuestions) for free-tier users; clamps pagination to 1–100
  • Guides API: Requires authentication + subscription check for intermediate/advanced levels; returns content: null with locked: true for inaccessible guides

Security

  • Verification token logging removed: Removed console.log that exposed raw verification URLs with tokens; createVerificationToken now properly awaits DB calls
  • Rate limiting: Wrapped counter read-check-increment in db.$transaction for atomicity; changed fail-open to fail-closed (deny on DB errors)

Data integrity

  • Fabricated rating removed: Removed the aggregateRating (4.8 stars, 1247 count) from structured data since there is no corresponding review system

Validation

  • tsc --noEmit passes with zero errors
  • Key changed files: 14 files (src/ changes + package-lock.json)

Remaining known issues (not addressed in this PR)

  • Analytics route fails at build time due to database dependency
  • Several other findings from the audit (AI adapter, rate limiter storage, transaction wrapping, etc.) are scoped for follow-up PRs

Closes the critical and high-severity findings from the Interview Lab audit.

Summary by CodeRabbit

  • New Features

    • Added subscription-based access controls for guides and interview questions.
    • Premium content is now locked or limited for users without the required access.
    • Added additional badge and button style options.
  • Bug Fixes

    • Improved email verification token persistence.
    • Strengthened rate-limit handling and session security.
  • Changes

    • Paid subscription checkout and management are temporarily unavailable.
    • Removed aggregate rating information from structured site data.
    • Updated download and learning-path button styling for clearer actions.

- Add outline variant to FieldButton to fix production build failure
- Disable subscription checkout and plan change (paid tiers not available)
- Remove JWT fallback secret; require JWT_SECRET at startup
- Protect questions and guides APIs with server-side auth + tier checks
- Remove raw verification token logging; await async DB calls
- Make rate limiter atomic with db.$transaction; fail closed on error
- Remove fabricated aggregate rating from structured data
- Fix pre-existing type errors in FieldBadge, Button variants
@vercel

vercel Bot commented Jul 17, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
interview-lab Error Error Jul 17, 2026 5:22am

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The change adds subscription-aware guide and question responses, hardens JWT, verification-token, and rate-limit handling, disables paid-plan mutations with HTTP 503 responses, updates component variants, and removes aggregate rating metadata from JSON-LD.

Changes

Access and request protection

Layer / File(s) Summary
Authentication and request protection
src/lib/session.ts, src/lib/email-verification.ts, src/lib/rate-limit.ts, src/app/api/auth/register/route.ts
JWT secrets now require 32 characters, verification tokens persist before resolution, rate limits use transactions and fail closed, and registration awaits token creation without logging verification URLs.
Gated content responses
src/app/api/guides/route.ts, src/app/api/questions/route.ts
Guide and question endpoints enforce authentication and subscription access, while restricted guide content and question fields are sanitized.

Paid-plan availability

Layer / File(s) Summary
Paid-plan mutation shutdown
src/app/api/subscription/checkout/route.ts, src/app/api/subscription/manage/route.ts
Checkout and subscription changes now return HTTP 503 without processing billing or subscription updates.

Presentation and metadata

Layer / File(s) Summary
Component variant updates
src/components/ui/glass-badge.tsx, src/components/ui/glass-button.tsx, src/components/interview-lab/DownloadCenter.tsx, src/components/interview-lab/LearningPaths.tsx
New badge and button variants are available, and restricted or incomplete actions use updated styling and sizing.
Structured metadata update
src/app/layout.tsx
The JSON-LD data no longer emits aggregate rating fields.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is detailed, but it does not follow the required template and is missing Problem, Solution, Scope, Risk and rollback, and Documentation sections. Add the missing template sections and include actual validation results, scope exclusions, risk/rollback, and documentation notes.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main production-readiness fixes across build, security, billing, and access control.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/critical-security-and-build-issues

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 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 `@src/app/api/auth/register/route.ts`:
- Line 127: Update the registration flow containing db.user.create and
createVerificationToken so both writes are atomic: execute them within one
database transaction, or remove the newly created user when token issuance
fails. Preserve the existing success and error responses while ensuring no user
row remains if createVerificationToken fails.

In `@src/app/api/questions/route.ts`:
- Around line 34-35: Update the pagination parsing for limit and offset in the
questions route to detect non-numeric parseInt results before applying Math.min,
Math.max, or passing values to Prisma. Fall back to the existing defaults of 50
for limit and 0 for offset, while preserving the current 1–100 limit and
non-negative offset bounds for valid numeric inputs.
- Around line 49-50: Update the premium-difficulty condition in the questions
route so difficulty="all" bypasses the authentication and subscription gate
alongside omitted difficulty and beginner. Preserve the existing gate for other
non-free difficulty values.
- Around line 76-86: Update the question sanitization in the questions mapping
to evaluate access per record with checkQuestionBankAccess(userTier,
q.difficulty), rather than using the shared canAccessPremium flag. Return the
full question only when that check passes; otherwise copy only
QUESTION_SAFE_FIELDS into the sanitized record, preserving the existing
free-tier field filtering.

In `@src/app/api/subscription/manage/route.ts`:
- Around line 179-182: Move the `change` action handling in the route’s
action-parsing flow to return the 503 unavailable response before authentication
and subscription database access. Then remove the later redundant `case
'change'` branch, preserving existing behavior for all other actions.

In `@src/lib/email-verification.ts`:
- Around line 10-11: Update the old-token cleanup in the email verification flow
to let deleteMany errors propagate instead of swallowing them with catch(() =>
{}). Ensure token creation or replacement does not continue when cleanup fails,
while preserving the existing deletion criteria for the email.

In `@src/lib/rate-limit.ts`:
- Around line 18-52: The transaction in the rate-limit flow must prevent stale
existing.count values from dropping concurrent increments. Update the
existing-count branch around tx.rateLimitEntry.update to use a database-side
atomic increment together with a conditional limit check, or configure
serializable isolation with retry, while preserving the allowed/remaining
results and max enforcement.
🪄 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: 887929b3-a24b-4944-bd9b-05febb37c65c

📥 Commits

Reviewing files that changed from the base of the PR and between 10b70d2 and 190b3be.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (13)
  • src/app/api/auth/register/route.ts
  • src/app/api/guides/route.ts
  • src/app/api/questions/route.ts
  • src/app/api/subscription/checkout/route.ts
  • src/app/api/subscription/manage/route.ts
  • src/app/layout.tsx
  • src/components/interview-lab/DownloadCenter.tsx
  • src/components/interview-lab/LearningPaths.tsx
  • src/components/ui/glass-badge.tsx
  • src/components/ui/glass-button.tsx
  • src/lib/email-verification.ts
  • src/lib/rate-limit.ts
  • src/lib/session.ts


// Create email verification token
const verifyToken = createVerificationToken(sanitizedEmail);
await createVerificationToken(sanitizedEmail);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the register route and related helpers
sed -n '1,220p' src/app/api/auth/register/route.ts

printf '\n--- createVerificationToken references ---\n'
rg -n "createVerificationToken|sanitizedEmail|duplicate-email|409|catch" src/app/api/auth -S

printf '\n--- verification token implementation ---\n'
fd -a "verification" src || true
rg -n "function createVerificationToken|const createVerificationToken|export .*createVerificationToken" src -S

Repository: projectamazonph/Interview-lab

Length of output: 6884


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,220p' src/lib/email-verification.ts

printf '\n--- prisma schema user/token tables ---\n'
rg -n "model (User|EmailVerificationToken|VerificationToken|Profile)" prisma src -S

Repository: projectamazonph/Interview-lab

Length of output: 1818


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the Prisma schema or equivalent model definitions
fd -a "schema.prisma" .
fd -a "*.prisma" .
rg -n "model EmailVerificationToken|emailVerificationToken" . -S

Repository: projectamazonph/Interview-lab

Length of output: 495


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,130p' prisma/schema.prisma

Repository: projectamazonph/Interview-lab

Length of output: 3730


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,220p' src/app/api/auth/login/route.ts
printf '\n--- verify-email route ---\n'
sed -n '1,220p' src/app/api/auth/verify-email/route.ts

Repository: projectamazonph/Interview-lab

Length of output: 4245


Make user creation and verification-token issuance atomic. If createVerificationToken fails after db.user.create, the request returns 500 but the user row stays committed; a retry then hits the duplicate-email check and there’s no recovery path. Wrap both writes in one transaction or delete the user on token failure.

🤖 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 `@src/app/api/auth/register/route.ts` at line 127, Update the registration flow
containing db.user.create and createVerificationToken so both writes are atomic:
execute them within one database transaction, or remove the newly created user
when token issuance fails. Preserve the existing success and error responses
while ensuring no user row remains if createVerificationToken fails.

Comment on lines +34 to +35
const limit = Math.min(Math.max(parseInt(searchParams.get('limit') || '50'), 1), 100);
const offset = Math.max(parseInt(searchParams.get('offset') || '0'), 0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Handle non-numeric pagination values before clamping.

Values such as limit=abc produce NaN, which survives both clamps and can make the Prisma query fail with 500.

Proposed fix
-    const limit = Math.min(Math.max(parseInt(searchParams.get('limit') || '50'), 1), 100);
-    const offset = Math.max(parseInt(searchParams.get('offset') || '0'), 0);
+    const parsedLimit = Number.parseInt(searchParams.get('limit') ?? '50', 10);
+    const parsedOffset = Number.parseInt(searchParams.get('offset') ?? '0', 10);
+    const limit = Number.isFinite(parsedLimit)
+      ? Math.min(Math.max(parsedLimit, 1), 100)
+      : 50;
+    const offset = Number.isFinite(parsedOffset)
+      ? Math.max(parsedOffset, 0)
+      : 0;
📝 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.

Suggested change
const limit = Math.min(Math.max(parseInt(searchParams.get('limit') || '50'), 1), 100);
const offset = Math.max(parseInt(searchParams.get('offset') || '0'), 0);
const parsedLimit = Number.parseInt(searchParams.get('limit') ?? '50', 10);
const parsedOffset = Number.parseInt(searchParams.get('offset') ?? '0', 10);
const limit = Number.isFinite(parsedLimit)
? Math.min(Math.max(parsedLimit, 1), 100)
: 50;
const offset = Number.isFinite(parsedOffset)
? Math.max(parsedOffset, 0)
: 0;
🤖 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 `@src/app/api/questions/route.ts` around lines 34 - 35, Update the pagination
parsing for limit and offset in the questions route to detect non-numeric
parseInt results before applying Math.min, Math.max, or passing values to
Prisma. Fall back to the existing defaults of 50 for limit and 0 for offset,
while preserving the current 1–100 limit and non-negative offset bounds for
valid numeric inputs.

Comment on lines +49 to +50
// For non-free difficulties, require auth and subscription check
if (difficulty && difficulty !== 'beginner') {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Exclude all from the premium-difficulty gate.

difficulty=all returns 401 to anonymous clients, while omitting the parameter returns the same mixed dataset with redaction.

-    if (difficulty && difficulty !== 'beginner') {
+    if (difficulty && difficulty !== 'beginner' && difficulty !== 'all') {
📝 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.

Suggested change
// For non-free difficulties, require auth and subscription check
if (difficulty && difficulty !== 'beginner') {
// For non-free difficulties, require auth and subscription check
if (difficulty && difficulty !== 'beginner' && difficulty !== 'all') {
🤖 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 `@src/app/api/questions/route.ts` around lines 49 - 50, Update the
premium-difficulty condition in the questions route so difficulty="all" bypasses
the authentication and subscription gate alongside omitted difficulty and
beginner. Preserve the existing gate for other non-free difficulty values.

Comment on lines +76 to +86
// Strip premium fields for free-tier users
const userTier = user?.subscriptionTier ?? 'free';
const canAccessPremium = (userTier === 'starter' || userTier === 'pro');

const sanitized = questions.map((q: Record<string, unknown>) => {
if (canAccessPremium) return q;
const safe: Record<string, unknown> = {};
for (const key of Object.keys(QUESTION_SAFE_FIELDS)) {
safe[key] = q[key];
}
return safe;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Apply subscription access per question difficulty.

Treating both starter and pro as universally premium lets starter users receive full advanced records through an unfiltered query. Sanitize each record using checkQuestionBankAccess(userTier, q.difficulty).

Proposed fix
-    const canAccessPremium = (userTier === 'starter' || userTier === 'pro');
-
     const sanitized = questions.map((q: Record<string, unknown>) => {
-      if (canAccessPremium) return q;
+      const access = checkQuestionBankAccess(userTier, q.difficulty as string);
+      if (access.allowed) return q;
       const safe: Record<string, unknown> = {};
       for (const key of Object.keys(QUESTION_SAFE_FIELDS)) {
         safe[key] = q[key];
       }
-      return safe;
+      return { ...safe, locked: 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.

Suggested change
// Strip premium fields for free-tier users
const userTier = user?.subscriptionTier ?? 'free';
const canAccessPremium = (userTier === 'starter' || userTier === 'pro');
const sanitized = questions.map((q: Record<string, unknown>) => {
if (canAccessPremium) return q;
const safe: Record<string, unknown> = {};
for (const key of Object.keys(QUESTION_SAFE_FIELDS)) {
safe[key] = q[key];
}
return safe;
// Strip premium fields for free-tier users
const userTier = user?.subscriptionTier ?? 'free';
const sanitized = questions.map((q: Record<string, unknown>) => {
const access = checkQuestionBankAccess(userTier, q.difficulty as string);
if (access.allowed) return q;
const safe: Record<string, unknown> = {};
for (const key of Object.keys(QUESTION_SAFE_FIELDS)) {
safe[key] = q[key];
}
return { ...safe, locked: true };
🧰 Tools
🪛 ast-grep (0.44.1)

[error] 82-84: Recursive/iterative merge copies attacker-controllable keys from a source object into a target via a computed property assignment without rejecting dangerous keys, allowing prototype pollution. Skip or block "proto", "constructor", and "prototype" keys (e.g. if (key === "__proto__" || key === "constructor" || key === "prototype") continue;), use a null-prototype object (Object.create(null)), or use a safe merge utility instead.
Context: for (const key of Object.keys(QUESTION_SAFE_FIELDS)) {
safe[key] = q[key];
}
Note: [CWE-1321] Improperly Controlled Modification of Object Prototype Attributes ('Prototype Pollution').

(prototype-pollution-recursive-merge-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 `@src/app/api/questions/route.ts` around lines 76 - 86, Update the question
sanitization in the questions mapping to evaluate access per record with
checkQuestionBankAccess(userTier, q.difficulty), rather than using the shared
canAccessPremium flag. Return the full question only when that check passes;
otherwise copy only QUESTION_SAFE_FIELDS into the sanitized record, preserving
the existing free-tier field filtering.

Comment on lines +179 to +182
return NextResponse.json(
{ error: 'Paid plans are not currently available.' },
{ status: 503 }
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Return the unavailable response before authentication and database access.

This branch runs only after Lines 104–114 authenticate and query the subscription. Consequently, unauthenticated requests return 401 and database failures return 500 instead of the intended 503. Parse the action first and short-circuit change before those dependencies.

Proposed ordering
 export async function POST(request: Request) {
   try {
+    const body: ManagePostBody = await request.json();
+    const { action, tier, billing = 'monthly' } = body;
+
+    if (action === 'change') {
+      return NextResponse.json(
+        { error: 'Paid plans are not currently available.' },
+        { status: 503 }
+      );
+    }
+
     const user = await getUserFromRequest(request);
     if (!user) {
       return NextResponse.json({ error: 'Unauthorized' }, { status: 401 });
     }
 
-    const body: ManagePostBody = await request.json();
-    const { action, tier, billing = 'monthly' } = body;
-
     const subscription = await db.subscription.findUnique({
       where: { userId: user.id },
     });

Then remove the redundant case 'change' implementation.

📝 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.

Suggested change
return NextResponse.json(
{ error: 'Paid plans are not currently available.' },
{ status: 503 }
);
export async function POST(request: Request) {
try {
const body: ManagePostBody = await request.json();
const { action, tier, billing = 'monthly' } = body;
if (action === 'change') {
return NextResponse.json(
{ error: 'Paid plans are not currently available.' },
{ status: 503 }
);
}
const user = await getUserFromRequest(request);
if (!user) {
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 });
}
const subscription = await db.subscription.findUnique({
where: { userId: user.id },
});
🤖 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 `@src/app/api/subscription/manage/route.ts` around lines 179 - 182, Move the
`change` action handling in the route’s action-parsing flow to return the 503
unavailable response before authentication and subscription database access.
Then remove the later redundant `case 'change'` branch, preserving existing
behavior for all other actions.

Comment on lines +10 to +11
// Clean up old tokens for this email first
await db.verificationToken.deleteMany({ where: { email } }).catch(() => {});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the target file first
ast-grep outline src/lib/email-verification.ts --view expanded

# Read the file with line numbers
cat -n src/lib/email-verification.ts

# Find related verification-token usage
rg -n "verificationToken|deleteMany\\(|create\\(|findFirst\\(|findUnique\\(" src lib prisma . -g '!node_modules' -g '!dist' -g '!build'

Repository: projectamazonph/Interview-lab

Length of output: 11824


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the Prisma model for verification tokens
rg -n "model VerificationToken|verificationToken" prisma src -g '!node_modules' -g '!dist' -g '!build'

# Read the schema section if present
if [ -f prisma/schema.prisma ]; then
  rg -n -A20 -B5 "model VerificationToken" prisma/schema.prisma
fi

# Find call sites for token creation
rg -n "createVerificationToken\\(" src __tests__ -g '!node_modules' -g '!dist' -g '!build'

# Read any surrounding auth/register or resend flow that may depend on invalidation
rg -n -A40 -B20 "createVerificationToken\\(" src/app src/lib __tests__ -g '!node_modules' -g '!dist' -g '!build'

Repository: projectamazonph/Interview-lab

Length of output: 7955


Let token cleanup fail closed.
deleteMany is what invalidates previously issued links; swallowing its error can leave older verification tokens usable alongside the new one. Let the error bubble so token replacement aborts on cleanup failure.

🤖 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 `@src/lib/email-verification.ts` around lines 10 - 11, Update the old-token
cleanup in the email verification flow to let deleteMany errors propagate
instead of swallowing them with catch(() => {}). Ensure token creation or
replacement does not continue when cleanup fails, while preserving the existing
deletion criteria for the email.

Comment thread src/lib/rate-limit.ts
Comment on lines +18 to 52
// Use a transaction for atomic read-check-update to prevent race conditions
const result = await db.$transaction(async (tx) => {
const existing = await tx.rateLimitEntry.findUnique({
where: { key: compositeKey },
update: {
count: 1,
resetTime: new Date(now.getTime() + windowMs),
},
create: {
key: compositeKey,
count: 1,
resetTime: new Date(now.getTime() + windowMs),
},
});
return { allowed: true, remaining: max - 1 };
}

if (existing.count >= max) {
return { allowed: false, remaining: 0 };
}
if (!existing || now > existing.resetTime) {
// New window: create or reset
await tx.rateLimitEntry.upsert({
where: { key: compositeKey },
update: {
count: 1,
resetTime: new Date(now.getTime() + windowMs),
},
create: {
key: compositeKey,
count: 1,
resetTime: new Date(now.getTime() + windowMs),
},
});
return { allowed: true, remaining: max - 1 };
}

if (existing.count >= max) {
return { allowed: false, remaining: 0 };
}

// Atomic increment within the transaction
await tx.rateLimitEntry.update({
where: { key: compositeKey },
data: { count: existing.count + 1 },
});

await db.rateLimitEntry.update({
where: { key: compositeKey },
data: { count: existing.count + 1 },
return { allowed: true, remaining: max - existing.count - 1 };
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the database provider, rate-limit schema, and all implementations.
fd -a 'schema.prisma' -x sh -c 'echo "=== $1 ==="; sed -n "/datasource /,/^}/p; /model RateLimitEntry/,/^}/p" sh {}
ast-grep outline src/lib/rate-limit.ts --view expanded
rg -n -C3 '\bcheckRateLimit\s*\(' .

Repository: projectamazonph/Interview-lab

Length of output: 227


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '== files ==\n'
git ls-files | rg '(^|/)(schema\.prisma|src/lib/rate-limit\.ts|prisma/.*\.prisma)$'

printf '\n== schema.prisma excerpts ==\n'
for f in $(git ls-files | rg '(^|/)(schema\.prisma|prisma/.*\.prisma)$'); do
  echo "--- $f ---"
  wc -l "$f"
  # show datasource and model blocks if present
  sed -n '/datasource /,/^}/p;/model RateLimitEntry/,/^}/p' "$f"
done

printf '\n== src/lib/rate-limit.ts ==\n'
wc -l src/lib/rate-limit.ts
cat -n src/lib/rate-limit.ts

printf '\n== call sites / related names ==\n'
rg -n -C 2 '\b(checkRateLimit|rateLimitEntry|RateLimitEntry)\b' src .

Repository: projectamazonph/Interview-lab

Length of output: 13494


Stale read can drop increments under concurrency.
existing.count is read once, then the row is updated with a literal existing.count + 1. Two concurrent requests can both pass the limit check and write the same value, so the counter undercounts and the window can allow more than max. Use a DB-side increment/conditional update, or serializable isolation with retry.

🤖 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 `@src/lib/rate-limit.ts` around lines 18 - 52, The transaction in the
rate-limit flow must prevent stale existing.count values from dropping
concurrent increments. Update the existing-count branch around
tx.rateLimitEntry.update to use a database-side atomic increment together with a
conditional limit check, or configure serializable isolation with retry, while
preserving the allowed/remaining results and max enforcement.

@projectamazonph
projectamazonph merged commit 556f8ff into main Jul 17, 2026
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant