Phase 1: server auth, ESLint cleanup, AI adapter, free-tier cleanup#5
Merged
Conversation
Documents the remaining 15 items across three priority phases after the initial critical fixes in PR #4.
P1.1 — Server AI adapter: - Replaced BrowserLLMIntegration in 3 AI routes (cover-letter, resume-review, assessment-score) with z-ai-web-dev-sdk - Removed fake fallback data that fabricated experience claims - Added input length limits and JSON schema validation P1.2 — Client auth from server session: - Added GET /api/auth/session endpoint (validates JWT cookie against DB) - AuthProvider now validates session via server on mount instead of trusting localStorage blindly - localStorage kept as write-through cache with server re-validation P1.3 — ESLint fixes & rule re-enablement: - Fixed 4 ESLint errors (impure render calls, unescaped entities) - Re-enabled no-unused-vars, prefer-const, no-debugger, no-empty, no-fallthrough, no-unreachable - Cleaned up unused imports in 5 modified files Free companion cleanup: - Subscription-guard returns allowed: true unconditionally - All subscription API endpoints removed - PricingPage, UpgradeModal, SubscriptionBanner stubbed as no-ops - README pricing table replaced with Free Companion notice
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe change migrates three AI routes to server-side ZAI calls, adds server-backed session restoration, removes tier-based subscription enforcement and UI, adjusts linting and auth behavior, and updates product documentation with a remediation plan. ChangesPlatform transition
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant NextRoute
participant ZAI
Client->>NextRoute: Submit AI task data
NextRoute->>ZAI: Send system and user messages
ZAI-->>NextRoute: Return generated content
NextRoute->>NextRoute: Extract and parse JSON
NextRoute-->>Client: Return result or error
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 10
🧹 Nitpick comments (1)
src/lib/auth-context.tsx (1)
48-51: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid fetching the profile twice during restoration.
setUsertriggers the effect at Lines 81–85, while Line 51 performs the same request directly. Let the centralized user-change effect own this fetch.🤖 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/auth-context.tsx` around lines 48 - 51, Remove the direct fetchUserProfile call from the restoration branch that handles data.user in the auth initialization flow. Keep setUser(data.user) and localStorage persistence intact, allowing the user-change effect to perform the profile fetch exactly once.
🤖 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 `@REMEDIATION_PLAN.md`:
- Around line 35-67: The remediation plan’s P1.1–P1.3 status and implementation
details are outdated relative to this PR. Update the relevant Phase 1 sections
to mark completed work appropriately, describe migration of three AI routes to
z-ai-web-dev-sdk instead of four routes with OpenAI/Anthropic, update the
historical PR reference, and explicitly state whether BrowserLLMIntegration
remains for the retained coach flow.
In `@src/app/api/ai/assessment-score/route.ts`:
- Around line 55-72: Runtime-validate parsed AI output before exposing or
persisting it: in src/app/api/ai/assessment-score/route.ts lines 55-72, validate
every required AIAssessmentScore field and ensure score is numeric and
constrained to 0–100; in src/app/api/ai/resume-review/route.ts lines 58-75,
validate every required AIResumeReview field, including improvedVersion. Reject
invalid or incomplete responses through the existing failure path instead of
returning or persisting them.
In `@src/app/api/ai/cover-letter/route.ts`:
- Around line 55-75: Validate the parsed result in the cover-letter route before
returning it, requiring the fields consumed downstream, including draftLetter
and claimsToVerify, with the expected types. Treat missing or malformed data
like JSON parsing failure and return a non-2xx NextResponse error instead of the
current HTTP 200 fallback; preserve the successful return path only for
schema-valid results.
In `@src/app/api/ai/resume-review/route.ts`:
- Around line 8-17: Update the response schema in the resume-review route to be
valid JSON by removing the trailing comma, and add the required improvedVersion
field matching the AIResumeReview contract consumed by the API test and
ResumeLab. Preserve the existing fields and their expected types.
In `@src/app/api/auth/session/route.ts`:
- Around line 29-30: Update the catch block in the session route to return an
HTTP 500 response for database or infrastructure failures instead of returning
200 with { user: null }. Preserve the null-user response only for successfully
processed invalid sessions.
In `@src/components/interview-lab/AuthScreen.tsx`:
- Line 3: Use one consistent clock source for the registration timing flow in
AuthScreen: initialize the timing ref with the current value from the same clock
used when tab switching records timestamps, and calculate the submission elapsed
duration with that clock as well. Update the referenced timing logic near the
ref declaration, tab-switch handler, and submission guard without changing the
three-second rejection behavior.
In `@src/lib/auth-context.tsx`:
- Around line 58-70: The server-validation failure path in the auth
initialization flow must not call setUser with cached localStorage data. Remove
the localStorage fallback inside the catch block while preserving the existing
unauthenticated state until server validation succeeds.
- Around line 133-141: Update the logout function to capture the response from
the POST request and validate res.ok before clearing local state. Treat non-2xx
responses as logout failures and surface the server-side error instead of
reporting success or removing the local user/profile data; preserve the existing
best-effort handling for network failures as appropriate.
- Around line 116-118: Update the form initialization near the honeypot and
_formStart fields so bot detection no longer relies on a client-controlled
timestamp. Use a server-issued challenge or retain the form start time
server-side, and ensure isBot() validates the submission against that trusted
server value while preserving the existing timing check.
In `@src/lib/subscription-guard.ts`:
- Around line 10-45: Update the question response sanitization in the questions
API route so free users retain premium answer fields, matching the
always-allowed contract of checkQuestionBankAccess. Remove the hard-coded
starter/pro tier condition, or replace it with the result of
checkQuestionBankAccess, while preserving all other response behavior.
---
Nitpick comments:
In `@src/lib/auth-context.tsx`:
- Around line 48-51: Remove the direct fetchUserProfile call from the
restoration branch that handles data.user in the auth initialization flow. Keep
setUser(data.user) and localStorage persistence intact, allowing the user-change
effect to perform the profile fetch exactly once.
🪄 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: 1c379f79-66fe-4356-a1db-c71361cad1f9
📒 Files selected for processing (28)
README.mdREMEDIATION_PLAN.md__tests__/api/subscription-manage.test.ts__tests__/api/subscription-webhook.test.ts__tests__/api/subscription.test.tseslint.config.mjssrc/app/api/ai/assessment-score/route.tssrc/app/api/ai/cover-letter/route.tssrc/app/api/ai/resume-review/route.tssrc/app/api/auth/login/route.tssrc/app/api/auth/register/route.tssrc/app/api/auth/session/route.tssrc/app/api/guides/route.tssrc/app/api/questions/route.tssrc/app/api/subscription/checkout/route.tssrc/app/api/subscription/manage/route.tssrc/app/api/subscription/status/route.tssrc/app/api/subscription/usage/route.tssrc/app/api/subscription/webhook/route.tssrc/app/register/page.tsxsrc/components/interview-lab/AuthScreen.tsxsrc/components/interview-lab/LandingPage.tsxsrc/components/interview-lab/PricingPage.tsxsrc/components/interview-lab/SubscriptionBanner.tsxsrc/components/interview-lab/UpgradeModal.tsxsrc/lib/auth-context.tsxsrc/lib/subscription-guard.tssrc/lib/use-subscription.ts
💤 Files with no reviewable changes (8)
- src/app/api/subscription/webhook/route.ts
- tests/api/subscription-webhook.test.ts
- src/app/api/subscription/usage/route.ts
- tests/api/subscription.test.ts
- tests/api/subscription-manage.test.ts
- src/app/api/subscription/status/route.ts
- src/app/api/subscription/checkout/route.ts
- src/app/api/subscription/manage/route.ts
Comment on lines
+35
to
+67
| ## 🔴 Phase 1 — Must fix before public launch | ||
|
|
||
| ### P1.1 — Server AI adapter | ||
| **Files:** `src/lib/browser-llm-integration.ts`, `src/app/api/ai/*/route.ts` (4 routes) | ||
| **Problem:** The `BrowserLLMIntegration` module is marked `"use client"` and depends on `window.ai`. Server routes import it and silently fall back to rule-based templates that fabricate experience claims. | ||
| **Fix:** | ||
| - Create `src/lib/server-ai.ts` with: | ||
| - Schema-validated structured output (zod) | ||
| - Explicit provider configuration (OpenAI/Anthropic) | ||
| - Timeouts and abort handling | ||
| - Input length limits | ||
| - Per-user quota enforcement | ||
| - Truthfulness checks | ||
| - Replace all `BrowserLLMIntegration` imports in API routes | ||
| - Add privacy/provider disclosure to UI | ||
|
|
||
| ### P1.2 — Client auth from server session | ||
| **Files:** `src/lib/auth-context.tsx` | ||
| **Problem:** Auth state restored from `localStorage` (modifiable); no server validation on startup. | ||
| **Fix:** | ||
| - Add `GET /api/auth/session` endpoint returning authenticated user from cookie | ||
| - On app mount, validate session via server endpoint instead of reading localStorage | ||
| - Keep localStorage as a cache layer with server re-validation | ||
| - Ensure logout clears both cookie and localStorage atomically | ||
|
|
||
| ### P1.3 — ESLint fixes & re-enablement | ||
| **Files:** `eslint.config.mjs`, `src/app/page.tsx`, `src/components/interview-lab/AdminPanel.tsx`, `PricingPage.tsx`, `QuestionBank.tsx` | ||
| **Problem:** 35 rules disabled; 9 pre-existing ESLint errors block CI. | ||
| **Fix:** | ||
| - Fix the 9 ESLint errors across 4 files (setState in effects, hoisting, const reassignment) | ||
| - Re-enable important rules incrementally: `no-unused-vars`, `no-console`, `react-hooks/exhaustive-deps`, `no-fallthrough` | ||
| - Remove blanket `off` overrides | ||
| - Add `lint-staged` pre-commit hook |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Synchronize the remediation plan with this PR.
This PR implements Phase 1, but the document still presents P1.1–P1.3 as launch blockers. It also describes four AI routes and OpenAI/Anthropic integration, whereas this PR migrates three routes to z-ai-web-dev-sdk. Update the status, route/provider scope, historical PR reference, and whether BrowserLLMIntegration remains for the retained coach flow.
Also applies to: 167-180
🤖 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 `@REMEDIATION_PLAN.md` around lines 35 - 67, The remediation plan’s P1.1–P1.3
status and implementation details are outdated relative to this PR. Update the
relevant Phase 1 sections to mark completed work appropriately, describe
migration of three AI routes to z-ai-web-dev-sdk instead of four routes with
OpenAI/Anthropic, update the historical PR reference, and explicitly state
whether BrowserLLMIntegration remains for the retained coach flow.
Comment on lines
+55
to
72
| let result; | ||
| try { | ||
| const jsonMatch = responseText.match(/\{[\s\S]*\}/); | ||
| if (jsonMatch) { | ||
| result = JSON.parse(jsonMatch[0]); | ||
| } | ||
| } catch { | ||
| result = null; | ||
| } | ||
|
|
||
| if (!result) { | ||
| return NextResponse.json( | ||
| { error: 'Failed to score assessment' }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
|
|
||
| return NextResponse.json(result); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Runtime-validate AI responses before returning them to consumers.
Both routes treat any parsed object as a valid domain response, allowing incomplete or incorrectly typed model output across the API boundary.
src/app/api/ai/assessment-score/route.ts#L55-L72: validate every requiredAIAssessmentScorefield and constrainscoreto0–100.src/app/api/ai/resume-review/route.ts#L58-L75: validate every requiredAIResumeReviewfield, includingimprovedVersion, before persistence consumes it.
📍 Affects 2 files
src/app/api/ai/assessment-score/route.ts#L55-L72(this comment)src/app/api/ai/resume-review/route.ts#L58-L75
🤖 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/ai/assessment-score/route.ts` around lines 55 - 72,
Runtime-validate parsed AI output before exposing or persisting it: in
src/app/api/ai/assessment-score/route.ts lines 55-72, validate every required
AIAssessmentScore field and ensure score is numeric and constrained to 0–100; in
src/app/api/ai/resume-review/route.ts lines 58-75, validate every required
AIResumeReview field, including improvedVersion. Reject invalid or incomplete
responses through the existing failure path instead of returning or persisting
them.
Comment on lines
+55
to
75
| let result; | ||
| try { | ||
| const jsonMatch = responseText.match(/\{[\s\S]*\}/); | ||
| if (jsonMatch) { | ||
| result = JSON.parse(jsonMatch[0]); | ||
| } | ||
| } catch { | ||
| result = null; | ||
| } | ||
|
|
||
| if (!result) { | ||
| return NextResponse.json({ | ||
| draftLetter: 'Unable to generate cover letter. Please try again.', | ||
| shorterVersion: '', | ||
| subjectLine: '', | ||
| customizationTips: [], | ||
| claimsToVerify: [], | ||
| }); | ||
| } | ||
|
|
||
| return NextResponse.json(result); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate the parsed AI response before returning it.
JSON.parse accepts objects missing draftLetter or claimsToVerify, while the downstream consumer immediately persists both. The fallback also returns HTTP 200, causing the failure message to be saved as a generated letter. Validate the schema and return a non-2xx error for invalid output.
Proposed validation
- if (!result) {
- return NextResponse.json({
- draftLetter: 'Unable to generate cover letter. Please try again.',
- shorterVersion: '',
- subjectLine: '',
- customizationTips: [],
- claimsToVerify: [],
- });
+ if (
+ !result ||
+ typeof result !== 'object' ||
+ typeof result.draftLetter !== 'string' ||
+ !Array.isArray(result.claimsToVerify) ||
+ !result.claimsToVerify.every((claim: unknown) => typeof claim === 'string')
+ ) {
+ return NextResponse.json(
+ { error: 'AI returned an invalid cover letter response' },
+ { status: 502 }
+ );
}📝 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
| let result; | |
| try { | |
| const jsonMatch = responseText.match(/\{[\s\S]*\}/); | |
| if (jsonMatch) { | |
| result = JSON.parse(jsonMatch[0]); | |
| } | |
| } catch { | |
| result = null; | |
| } | |
| if (!result) { | |
| return NextResponse.json({ | |
| draftLetter: 'Unable to generate cover letter. Please try again.', | |
| shorterVersion: '', | |
| subjectLine: '', | |
| customizationTips: [], | |
| claimsToVerify: [], | |
| }); | |
| } | |
| return NextResponse.json(result); | |
| let result; | |
| try { | |
| const jsonMatch = responseText.match(/\{[\s\S]*\}/); | |
| if (jsonMatch) { | |
| result = JSON.parse(jsonMatch[0]); | |
| } | |
| } catch { | |
| result = null; | |
| } | |
| if ( | |
| !result || | |
| typeof result !== 'object' || | |
| typeof result.draftLetter !== 'string' || | |
| !Array.isArray(result.claimsToVerify) || | |
| !result.claimsToVerify.every((claim: unknown) => typeof claim === 'string') | |
| ) { | |
| return NextResponse.json( | |
| { error: 'AI returned an invalid cover letter response' }, | |
| { status: 502 } | |
| ); | |
| } | |
| return NextResponse.json(result); |
🤖 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/ai/cover-letter/route.ts` around lines 55 - 75, Validate the
parsed result in the cover-letter route before returning it, requiring the
fields consumed downstream, including draftLetter and claimsToVerify, with the
expected types. Treat missing or malformed data like JSON parsing failure and
return a non-2xx NextResponse error instead of the current HTTP 200 fallback;
preserve the successful return path only for schema-valid results.
Comment on lines
+8
to
+17
| Respond in the following JSON format only: | ||
| { | ||
| "score": <number 0-100>, | ||
| "missingKeywords": ["<keyword 1>", "<keyword 2>"], | ||
| "weakSections": ["<section 1>", "<section 2>"], | ||
| "improvedSummary": "<rewritten professional summary>", | ||
| "improvedBullets": ["<bullet 1>", "<bullet 2>"], | ||
| "skillsRecommendations": ["<skill 1>", "<skill 2>"], | ||
| "truthWarnings": ["<warning if user fabricated experience>"], | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fix the incomplete and invalid response schema.
The trailing comma makes the example invalid JSON, and improvedVersion is omitted even though AIResumeReview, the API test, and ResumeLab require it.
Proposed fix
"skillsRecommendations": ["<skill 1>", "<skill 2>"],
- "truthWarnings": ["<warning if user fabricated experience>"],
+ "truthWarnings": ["<warning if user fabricated experience>"],
+ "improvedVersion": "<complete revised resume>"
}📝 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
| Respond in the following JSON format only: | |
| { | |
| "score": <number 0-100>, | |
| "missingKeywords": ["<keyword 1>", "<keyword 2>"], | |
| "weakSections": ["<section 1>", "<section 2>"], | |
| "improvedSummary": "<rewritten professional summary>", | |
| "improvedBullets": ["<bullet 1>", "<bullet 2>"], | |
| "skillsRecommendations": ["<skill 1>", "<skill 2>"], | |
| "truthWarnings": ["<warning if user fabricated experience>"], | |
| } | |
| Respond in the following JSON format only: | |
| { | |
| "score": <number 0-100>, | |
| "missingKeywords": ["<keyword 1>", "<keyword 2>"], | |
| "weakSections": ["<section 1>", "<section 2>"], | |
| "improvedSummary": "<rewritten professional summary>", | |
| "improvedBullets": ["<bullet 1>", "<bullet 2>"], | |
| "skillsRecommendations": ["<skill 1>", "<skill 2>"], | |
| "truthWarnings": ["<warning if user fabricated experience>"], | |
| "improvedVersion": "<complete revised resume>" | |
| } |
🤖 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/ai/resume-review/route.ts` around lines 8 - 17, Update the
response schema in the resume-review route to be valid JSON by removing the
trailing comma, and add the required improvedVersion field matching the
AIResumeReview contract consumed by the API test and ResumeLab. Preserve the
existing fields and their expected types.
Comment on lines
+29
to
+30
| } catch { | ||
| return NextResponse.json({ user: null }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not report infrastructure failures as logged-out sessions.
A database failure currently returns 200 { user: null }, so session restoration treats a transient outage as invalid authentication. Reserve { user: null } for invalid sessions and return 500 here.
- } catch {
- return NextResponse.json({ user: null });
+ } catch (error) {
+ console.error('Session retrieval error:', error);
+ return NextResponse.json(
+ { error: 'Failed to retrieve session' },
+ { status: 500 }
+ );📝 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
| } catch { | |
| return NextResponse.json({ user: null }); | |
| } catch (error) { | |
| console.error('Session retrieval error:', error); | |
| return NextResponse.json( | |
| { error: 'Failed to retrieve session' }, | |
| { status: 500 } | |
| ); |
🤖 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/session/route.ts` around lines 29 - 30, Update the catch
block in the session route to return an HTTP 500 response for database or
infrastructure failures instead of returning 200 with { user: null }. Preserve
the null-user response only for successfully processed invalid sessions.
| "use client"; | ||
|
|
||
| import React, { useState, useRef } from "react"; | ||
| import React, { useState, useRef, useEffect } from "react"; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Use the same initialized clock for registration timing.
The ref starts at 0, and tab switching stores performance.now(), but submission subtracts it from Date.now(). The resulting elapsed value is always far above three seconds, so the timing guard never rejects rapid submissions.
Proposed fix
const formStartRef = useRef<number>(0);
- // formStartRef remains at mount timestamp for bot protection
+useEffect(() => {
+ formStartRef.current = Date.now();
+}, []);
const switchTab = (tab: "login" | "register") => {
setActiveTab(tab);
setError("");
setLoading(false);
- formStartRef.current = performance.now();
+ formStartRef.current = Date.now();
};Also applies to: 30-31, 70-70
🤖 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/components/interview-lab/AuthScreen.tsx` at line 3, Use one consistent
clock source for the registration timing flow in AuthScreen: initialize the
timing ref with the current value from the same clock used when tab switching
records timestamps, and calculate the submission elapsed duration with that
clock as well. Update the referenced timing logic near the ref declaration,
tab-switch handler, and submission guard without changing the three-second
rejection behavior.
Comment on lines
+58
to
+70
| } catch { | ||
| // Server unreachable — fall back to cached localStorage data as best effort | ||
| if (!cancelled) { | ||
| try { | ||
| const stored = localStorage.getItem(LS_KEY); | ||
| if (stored) { | ||
| const parsed = JSON.parse(stored); | ||
| setUser(parsed); | ||
| } | ||
| } catch { | ||
| // ignore corrupt storage | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not restore authenticated state from localStorage after server validation fails.
An outage or modified cache currently sets user without a validated session, rendering the client as authenticated while protected APIs reject it. Keep user unauthenticated until the server confirms the session.
🤖 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/auth-context.tsx` around lines 58 - 70, The server-validation failure
path in the auth initialization flow must not call setUser with cached
localStorage data. Remove the localStorage fallback inside the catch block while
preserving the existing unauthenticated state until server validation succeeds.
Comment on lines
116
to
118
| honeypot: '', | ||
| _formStart: Date.now() - 10000, // Fake old timestamp (real form fills in current time) | ||
| _formStart: Date.now() - 10000, | ||
| }), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline src/app/api/auth/register/route.ts --items all --view expanded
rg -n -C4 '\b(_formStart|honeypot)\b' src/app/api/auth/register/route.tsRepository: projectamazonph/Interview-lab
Length of output: 1262
Do not trust _formStart from the client
isBot() treats the submitted timestamp as proof of form age, so an attacker can send any old value and skip the < 2s check. Use a server-issued challenge or server-side start time instead.
🤖 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/auth-context.tsx` around lines 116 - 118, Update the form
initialization near the honeypot and _formStart fields so bot detection no
longer relies on a client-controlled timestamp. Use a server-issued challenge or
retain the form start time server-side, and ensure isBot() validates the
submission against that trusted server value while preserving the existing
timing check.
Comment on lines
+133
to
+141
| const logout = async () => { | ||
| try { | ||
| await fetch('/api/auth/logout', { method: 'POST' }); | ||
| } catch { | ||
| // best effort — cookie will expire naturally | ||
| } | ||
| setUser(null); | ||
| setProfile(null); | ||
| localStorage.removeItem('interviewlab_user'); | ||
| localStorage.removeItem(LS_KEY); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Handle non-2xx logout responses before reporting success.
fetch resolves normally for HTTP 500, after which local state is cleared while the session cookie remains valid. A reload silently authenticates the user again, which is especially unsafe on shared devices. Check res.ok and surface server-side logout 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/auth-context.tsx` around lines 133 - 141, Update the logout function
to capture the response from the POST request and validate res.ok before
clearing local state. Treat non-2xx responses as logout failures and surface the
server-side error instead of reporting success or removing the local
user/profile data; preserve the existing best-effort handling for network
failures as appropriate.
Comment on lines
+10
to
+45
| // Interview Lab is a free companion to Project Amazon PH Academy. | ||
| // All features are available to all users — no tier gating. | ||
|
|
||
| export function checkInterviewAccess(userTier: string, interviewsThisWeek: number): SubscriptionCheck { | ||
| const tier = PRICING_TIERS[userTier as TierKey] ?? PRICING_TIERS.free; | ||
| const limit = tier.limits.interviewsPerWeek; | ||
| if (limit === -1) return { allowed: true }; | ||
| if (interviewsThisWeek < limit) return { allowed: true, remaining: limit - interviewsThisWeek }; | ||
| return { | ||
| allowed: false, | ||
| reason: `You've used all ${limit} interviews this week on the ${tier.name} plan.`, | ||
| remaining: 0, | ||
| upgradeTo: userTier === 'free' ? 'starter' : 'pro', | ||
| }; | ||
| void userTier; void interviewsThisWeek; | ||
| return { allowed: true }; | ||
| } | ||
|
|
||
| export function checkResumeAccess(userTier: string, reviewsThisMonth: number): SubscriptionCheck { | ||
| const tier = PRICING_TIERS[userTier as TierKey] ?? PRICING_TIERS.free; | ||
| const limit = tier.limits.resumeReviewsPerMonth; | ||
| if (limit === -1) return { allowed: true }; | ||
| if (reviewsThisMonth < limit) return { allowed: true, remaining: limit - reviewsThisMonth }; | ||
| return { | ||
| allowed: false, | ||
| reason: `You've used all ${limit} resume reviews this month on the ${tier.name} plan.`, | ||
| remaining: 0, | ||
| upgradeTo: 'starter', | ||
| }; | ||
| void userTier; void reviewsThisMonth; | ||
| return { allowed: true }; | ||
| } | ||
|
|
||
| export function checkCoverLetterAccess(userTier: string, lettersThisMonth: number): SubscriptionCheck { | ||
| const tier = PRICING_TIERS[userTier as TierKey] ?? PRICING_TIERS.free; | ||
| const limit = tier.limits.coverLettersPerMonth; | ||
| if (limit === -1) return { allowed: true }; | ||
| if (lettersThisMonth < limit) return { allowed: true, remaining: limit - lettersThisMonth }; | ||
| return { | ||
| allowed: false, | ||
| reason: `You've used all ${limit} cover letter generations this month on the ${tier.name} plan.`, | ||
| remaining: 0, | ||
| upgradeTo: 'starter', | ||
| }; | ||
| void userTier; void lettersThisMonth; | ||
| return { allowed: true }; | ||
| } | ||
|
|
||
| export function checkPracticeTestAccess(userTier: string, testsThisMonth: number): SubscriptionCheck { | ||
| const tier = PRICING_TIERS[userTier as TierKey] ?? PRICING_TIERS.free; | ||
| const limit = tier.limits.practiceTestsPerMonth; | ||
| if (limit === -1) return { allowed: true }; | ||
| if (testsThisMonth < limit) return { allowed: true, remaining: limit - testsThisMonth }; | ||
| return { | ||
| allowed: false, | ||
| reason: `You've used all ${limit} practice tests this month on the ${tier.name} plan.`, | ||
| remaining: 0, | ||
| upgradeTo: userTier === 'free' ? 'starter' : 'pro', | ||
| }; | ||
| void userTier; void testsThisMonth; | ||
| return { allowed: true }; | ||
| } | ||
|
|
||
| export function checkQuestionBankAccess(userTier: string, questionDifficulty: string): SubscriptionCheck { | ||
| const tier = PRICING_TIERS[userTier as TierKey] ?? PRICING_TIERS.free; | ||
| const accessLevel = tier.limits.questionBankAccess; | ||
| const hierarchy: Record<string, number> = { beginner: 0, intermediate: 1, advanced: 2 }; | ||
| const userLevel = hierarchy[accessLevel] ?? 0; | ||
| const required = hierarchy[questionDifficulty] ?? 0; | ||
| if (userLevel >= required) return { allowed: true }; | ||
| const upgradeTo: TierKey = accessLevel === 'beginner' ? 'starter' : 'pro'; | ||
| return { | ||
| allowed: false, | ||
| reason: `${questionDifficulty} questions require the ${upgradeTo === 'starter' ? 'Starter' : 'Pro'} plan.`, | ||
| upgradeTo, | ||
| }; | ||
| void userTier; void questionDifficulty; | ||
| return { allowed: true }; | ||
| } | ||
|
|
||
| export function checkDownloadAccess(userTier: string, requiredTier: string): SubscriptionCheck { | ||
| if (canAccessTier(userTier, requiredTier)) return { allowed: true }; | ||
| const upgradeTo: TierKey = requiredTier === 'starter' ? 'starter' : 'pro'; | ||
| return { | ||
| allowed: false, | ||
| reason: `This download requires the ${requiredTier === 'starter' ? 'Starter' : 'Pro'} plan.`, | ||
| upgradeTo, | ||
| }; | ||
| void userTier; void requiredTier; | ||
| return { allowed: true }; | ||
| } | ||
|
|
||
| export function checkGuideAccess(userTier: string, guideLevel: string): SubscriptionCheck { | ||
| const tier = PRICING_TIERS[userTier as TierKey] ?? PRICING_TIERS.free; | ||
| const accessLevel = tier.limits.guideAccess; | ||
| const hierarchy: Record<string, number> = { beginner: 0, intermediate: 1, advanced: 2 }; | ||
| const userLevel = hierarchy[accessLevel] ?? 0; | ||
| const required = hierarchy[guideLevel] ?? 0; | ||
| if (userLevel >= required) return { allowed: true }; | ||
| const upgradeTo: TierKey = accessLevel === 'beginner' ? 'starter' : 'pro'; | ||
| return { | ||
| allowed: false, | ||
| reason: `${guideLevel} guides require the ${upgradeTo === 'starter' ? 'Starter' : 'Pro'} plan.`, | ||
| upgradeTo, | ||
| }; | ||
| void userTier; void guideLevel; | ||
| return { allowed: true }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Complete the free-access transition in downstream sanitizers.
These guards allow free-tier access, but src/app/api/questions/route.ts Lines 76-87 still removes premium answer fields unless the user tier is starter or pro. Free users therefore remain partially gated despite this contract. Remove the tier-based sanitizer or derive it from checkQuestionBankAccess.
🤖 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/subscription-guard.ts` around lines 10 - 45, Update the question
response sanitization in the questions API route so free users retain premium
answer fields, matching the always-allowed contract of checkQuestionBankAccess.
Remove the hard-coded starter/pro tier condition, or replace it with the result
of checkQuestionBankAccess, while preserving all other response behavior.
- Make JWT secret validation lazy (not at module import time) - Update rate-limit test to mock $transaction and expect fail-closed - Set JWT_SECRET in vitest setup for all test environments - Extend CI JWT_SECRET to 44 chars (was 28, below 32-char minimum)
This was referenced Jul 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 1 — Completed items
P1.1 — Server AI adapter
BrowserLLMIntegrationwithz-ai-web-dev-sdkin 3 AI routes (cover-letter, resume-review, assessment-score)P1.2 — Client auth from server session
GET /api/auth/sessionendpoint validating the JWT cookie against the databaseAuthProvidernow validates session via server on mount instead of blindly trusting localStorageP1.3 — ESLint fixes & rule re-enablement
Date.now(), unescaped entities)no-unused-vars(warn),prefer-const(warn),no-debugger(warn),no-empty(warn),no-fallthrough(warn),no-unreachable(warn)Free companion cleanup
subscription-guard.ts— all guard functions returnallowed: truePricingPage,UpgradeModal,SubscriptionBannerstubbed as no-opsuse-subscriptionhook returns unlimited limitsSummary
Summary by CodeRabbit