Gate 5 — Quality & Security Audit (ticket #3273, forgot-password OTP flow)
===========================================================================
Run date: 2026-09-08
Feature files audited:
  app/(auth)/forgot-password/page.tsx
  app/(auth)/forgot-password/verify/page.tsx
  app/(auth)/forgot-password/reset/page.tsx
  components/auth/ForgotPasswordForm.tsx
  components/auth/VerifyOtpForm.tsx
  components/auth/ResetPasswordForm.tsx
  components/auth/SignInForm.tsx

Supporting files read (for context/cross-reference):
  lib/actions/forgot-password.ts
  lib/actions/auth.ts
  lib/actions/shared.ts
  lib/auth/flowToken.ts
  lib/validation/schemas.ts (forgot-password + passwordPolicy sections)
  middleware.ts
  next.config.mjs
  app/(auth)/layout.tsx
  app/(auth)/login/page.tsx
  components/auth/BrandPanel.tsx
  components/forms/ResetPasswordForm.tsx  (existing profile-section variant)
  components/ui/fields/PasswordField.tsx
  components/ui/SubmitButton.tsx

Skill resolution (per gate spec):
  1. nextjs-tailwind-code-quality-audit — read via file
     /opt/drcarrot-deploy/cross-repo-orchestration/skills/nextjs-tailwind-code-quality-audit/SKILL.md
  2. nextjs-tailwind-dry-architecture-audit — read via file
     /opt/drcarrot-deploy/cross-repo-orchestration/skills/nextjs-tailwind-dry-architecture-audit/SKILL.md
     (NOTE: file content partially garbled/corrupted after line ~20; audit conducted
     using the readable header description + standard DRY analysis principles)
  3. nextjs-tailwind-css-dry-architecture-audit — read via file
     /opt/drcarrot-deploy/cross-repo-orchestration/skills/nextjs-tailwind-css-dry-architecture-audit/SKILL.md
  4. nextjs-tailwind-security-audit — read via file
     /opt/drcarrot-deploy/cross-repo-orchestration/skills/nextjs-tailwind-security-audit/SKILL.md

═══════════════════════════════════════════════════════════════════════════
FINDING 1 — [security] Missing failed-attempt counter on OTP verification
═══════════════════════════════════════════════════════════════════════════
File: lib/actions/forgot-password.ts
Location: verifyResetOtp (lines 231-287)
Category: auth / input-validation
Priority: P2 (Medium)
Fixed: false

Problem: verifyResetOtp does not track or limit the number of incorrect OTP
submissions against a given OTP record. The resendOtp action correctly caps
resend attempts at OTP_MAX_ATTEMPTS (5), but the verification path itself has
no failed-attempt counter. Within the 10-minute OTP window, a caller holding
the fp_otp_token cookie can submit unlimited guesses.

Attack scenario:
  1. Attacker initiates the flow for a target mobile (they just need the number).
  2. requestPasswordResetOtp returns the same shape for both registered and
     unregistered numbers (anti-enumeration is correct), but if the number IS
     registered, an OTP is created and the attacker receives the fp_otp_token
     cookie.
  3. Attacker scripts rapid POST requests to the verifyResetOtp Server Action
     with sequential 6-digit codes. At ~100ms/request, a single client makes
     ~6,000 attempts in 10 minutes → ~0.6% success probability. A distributed
     attacker (botnet) at 1,000 req/s reaches ~600,000 attempts → ~45% success.
  4. On a correct guess, the attacker receives fp_reset_token and can set a new
     password, taking over the account.

Evidence: verifyResetOtp (lines 245-259) — on hash mismatch, returns a field
error but does NOT increment any counter or lock the OTP record:
  ```
  const submittedHash = await hashOtp(otp);
  if (submittedHash !== otpRecord.otpHash) {
    return { ok: false, fieldErrors: { otp: "Invalid code. ..." } };
  }
  ```

Impact: Account takeover for any user whose mobile number the attacker knows.

Recommended remediation: Add a `verifyAttempts` field to the passwordResetOtp
model (or reuse the existing `attempts` field with a separate cap). Increment
on each failed verification, and reject with an expiry-style message after N
failures (e.g., 5). This mirrors the existing resendOtp rate limit.

Risk of change: Low — adds a DB column and an increment+check. Does not change
the happy path. Requires a Prisma migration.

═══════════════════════════════════════════════════════════════════════════
FINDING 2 — [dry-architecture] RULES + STRENGTH_LABELS arrays duplicated
═══════════════════════════════════════════════════════════════════════════
File: components/auth/ResetPasswordForm.tsx (lines 31-39)
       + components/forms/ResetPasswordForm.tsx (lines 23-31)
Category: dup-logic
Priority: P2
Fixed: false

Problem: The RULES array (5 password policy rules with label+test) and
STRENGTH_LABELS array are byte-for-byte identical between the new auth-flow
ResetPasswordForm and the existing profile-section ResetPasswordForm. The
code itself acknowledges the copy (line 29: "Strength computation and RULES
array copied from the existing form"). Both represent the same concept: the
client-side password policy checklist that mirrors lib/validation/schemas.ts's
passwordPolicy Zod schema.

Evidence:
  auth/ResetPasswordForm.tsx lines 31-39:
    const RULES: { label: string; test: (v: string) => boolean }[] = [
      { label: "Minimum 8 characters", test: (v) => v.length >= 8 },
      { label: "1 uppercase letter", test: (v) => /[A-Z]/.test(v) },
      ...
    ];
    const STRENGTH_LABELS = ["Weak", "Fair", "Good", "Strong"];

  forms/ResetPasswordForm.tsx lines 23-31:
    const RULES: { label: string; test: (v: string) => boolean }[] = [
      { label: "Minimum 8 characters", test: (v) => v.length >= 8 },
      { label: "1 uppercase letter", test: (v) => /[A-Z]/.test(v) },
      ...
    ];
    const STRENGTH_LABELS = ["Weak", "Fair", "Good", "Strong"];

Why these are the same concept: Both enforce and display the same password
policy. A policy change (e.g. adding a "no common passwords" rule) must update
both or they drift, silently showing different rules on two screens.

Recommended fix: Extract to a shared module, e.g.
  lib/validation/password-rules.ts
exporting RULES and STRENGTH_LABELS. Both components import from it.

Risk of change: None — pure extraction, no behavioral change.

═══════════════════════════════════════════════════════════════════════════
FINDING 3 — [dry-architecture] Strength meter computation logic duplicated
═══════════════════════════════════════════════════════════════════════════
File: components/auth/ResetPasswordForm.tsx (lines 49-57)
       + components/forms/ResetPasswordForm.tsx (lines 47-54)
Category: dup-logic
Priority: P2
Fixed: false

Problem: The strength bar computation (rulesMet, metCount, strengthBars,
strengthLabel) is identical between the two ResetPasswordForm variants:
  const rulesMet = useMemo(() => RULES.map((r) => r.test(newPassword)), [newPassword]);
  const policyOk = rulesMet.every(Boolean);
  const matchOk = newPassword.length > 0 && newPassword === confirmPassword;
  const canSubmit = policyOk && matchOk;
  const metCount = rulesMet.filter(Boolean).length;
  const strengthBars = newPassword.length === 0 ? 0 : Math.max(1, Math.ceil(...));
  const strengthLabel = STRENGTH_LABELS[Math.max(0, strengthBars - 1)];

Why these are the same concept: This is the same password-strength UI
computation. Any UX change (different strength tiers, different labels) must
update both.

Recommended fix: Extract a custom hook, e.g. usePasswordStrength(password),
returning { rulesMet, policyOk, strengthBars, strengthLabel }.

Risk of change: None — pure extraction, no behavioral change.

═══════════════════════════════════════════════════════════════════════════
FINDING 4 — [css-dry] Auth shell wrapper duplicated 4 times
═══════════════════════════════════════════════════════════════════════════
Files:
  app/(auth)/login/page.tsx (line 6)
  components/auth/ForgotPasswordForm.tsx (line 24)
  components/auth/VerifyOtpForm.tsx (line 100)
  components/auth/ResetPasswordForm.tsx (line 60)
Concept: Auth page shell (full-screen two-panel layout)
Priority: P2
Fixed: false

Evidence — the same class combination in all 4 locations:
  <div className="fixed inset-0 z-[100] flex overflow-hidden bg-surface">
    <BrandPanel />
    <div className="flex flex-grow items-center justify-center">
      {/* form content */}
    </div>
  </div>

Minor drift: ResetPasswordForm's inner div adds `overflow-auto py-10` to
handle its taller form content.

Why these are the same concept: Every auth-flow page (login, forgot-password
×3) renders an identical shell: fixed fullscreen overlay, BrandPanel on the
left (45%), centred form column on the right. A design change to this shell
(e.g. background, z-index, BrandPanel width) must touch all 4 files.

Recommended primitive: An AuthShell component:
  <AuthShell scrollable={boolean}>
    {children}
  </AuthShell>
The `scrollable` prop covers the ResetPasswordForm's overflow-auto variant.

Risk of change: Low — each call site becomes <AuthShell>{form}</AuthShell>.
Visual output unchanged. The login page.tsx version (server component) would
need AuthShell to be a server component too (it has no client-side state), or
the page can accept being wrapped by a client AuthShell since its content
(SignInForm) is already a client component.

═══════════════════════════════════════════════════════════════════════════
FINDING 5 — [css-dry] Back-navigation link pattern duplicated
═══════════════════════════════════════════════════════════════════════════
Files:
  components/auth/ForgotPasswordForm.tsx (lines 28-34)
  components/auth/VerifyOtpForm.tsx (lines 107-113)
Concept: Back-navigation link in auth flow
Priority: P3
Fixed: false

Evidence:
  ForgotPasswordForm:
    <Link href="/login"
      className="mb-[14px] flex cursor-pointer items-center gap-1.5">
      <ChevronLeft className="h-4 w-4 text-accent" aria-hidden="true" />
      <span className="text-[13px] font-medium text-accent">Back to Sign In</span>
    </Link>

  VerifyOtpForm:
    <Link href="/forgot-password"
      className="mb-[14px] flex cursor-pointer items-center gap-1.5">
      <ChevronLeft className="h-4 w-4 text-accent" aria-hidden="true" />
      <span className="text-[13px] font-medium text-accent">Back</span>
    </Link>

Identical class combination, same ChevronLeft icon, only href and label differ.
Only 2 instances — borderline for extraction, but the pattern is likely to
recur if more auth-flow steps are added.

Recommended primitive: A small <BackLink href label /> component, or fold into
AuthShell as an optional `backHref`/`backLabel` prop.

Risk of change: None — visual output unchanged.

═══════════════════════════════════════════════════════════════════════════
FINDING 6 — [dry-architecture] Password policy checklist card UI duplicated
═══════════════════════════════════════════════════════════════════════════
Files:
  components/auth/ResetPasswordForm.tsx (lines 127-175)
  components/forms/ResetPasswordForm.tsx (lines 126-142)
Concept: Password policy checklist with live check/uncheck indicators
Priority: P3
Fixed: false

Problem: Both components render the same UI pattern: a rounded card with
"PASSWORD POLICY" header, a list of rules with SVG checkmark icons that
toggle green/grey based on rulesMet[i], and a static "Not last 3 passwords"
item. The SVG markup is identical (same dimensions, viewBox, path, stroke
colors). Layout context differs (auth version is single-column below the
form; profile version is a 280px side card), so the outer container classes
differ — but the card internals are the same concept.

Recommended fix: Extract a <PasswordPolicyChecklist rulesMet={boolean[]} />
component for the card's inner content. Each consumer wraps it in its own
layout-appropriate container.

Risk of change: Low — structural extraction, same visual output. Needs
careful CSS comparison to ensure both render identically after extraction.

═══════════════════════════════════════════════════════════════════════════
FINDING 7 — [code-quality] Forward-compat: sync cookies() in non-async pages
═══════════════════════════════════════════════════════════════════════════
Files:
  app/(auth)/forgot-password/verify/page.tsx (line 14)
  app/(auth)/forgot-password/reset/page.tsx (line 14)
Category: structure
Priority: P3
Fixed: false

Problem: Both page components are non-async functions that call cookies()
synchronously. This works in Next.js 14 where cookies() is synchronous, but
Next.js 15 makes cookies() an async function. When upgrading, these functions
will need `async` and `await`.

Evidence:
  export default function VerifyOtpPage() {    // not async
    const token = cookies().get("fp_otp_token"); // sync call
    ...
  }

Note: The auth layout (app/(auth)/layout.tsx) IS already async and awaits
getSession(). This finding is low-priority — it's a future upgrade concern,
not a current bug.

Recommended fix: Mark both functions async, prefix cookies() with await:
  export default async function VerifyOtpPage() {
    const token = (await cookies()).get("fp_otp_token");
    ...
  }

Risk of change: None in Next.js 14 — async Server Components are supported.
Proactively future-proofs for Next.js 15.

═══════════════════════════════════════════════════════════════════════════
FINDING 8 — [security] No security headers in next.config.mjs
═══════════════════════════════════════════════════════════════════════════
File: next.config.mjs
Category: headers
Priority: P3 (Low)
Fixed: false

Problem: next.config.mjs contains no `headers()` configuration for CSP,
X-Frame-Options, X-Content-Type-Options, or HSTS. The deployed architecture
includes a shared reverse proxy (192.168.1.116 per CLAUDE.md) which may set
these headers upstream, but defense-in-depth recommends configuring them at
the application level too — especially CSP with nonces for inline scripts.

Evidence:
  const nextConfig = {
    experimental: {
      serverActions: { bodySizeLimit: "5mb", allowedOrigins: [...] },
    },
  };
  // No headers() export

Recommended remediation: Add a headers() function to next.config.mjs with at
minimum:
  - X-Content-Type-Options: nosniff
  - X-Frame-Options: DENY (unless embedding is intended)
  - Strict-Transport-Security (if not set by proxy)
  - Referrer-Policy: strict-origin-when-cross-origin
  - Content-Security-Policy (nonce-based for scripts)

Risk of change: Low for X-Content-Type-Options/X-Frame-Options/Referrer-Policy.
CSP requires testing to avoid breaking legitimate inline styles/scripts.

═══════════════════════════════════════════════════════════════════════════
FINDING 9 — [code-quality] Fragile cross-file line-number reference
═══════════════════════════════════════════════════════════════════════════
File: components/auth/ResetPasswordForm.tsx (line 29)
Category: structure
Priority: P3
Fixed: false

Problem: Comment references specific line numbers in another file:
  "Strength computation and RULES array copied from the existing form
   (components/forms/ResetPasswordForm.tsx lines 23–54) to stay in sync."

Line-number references in comments become stale as the target file changes.
This comment is already partially addressed by Finding 2 (extract RULES to a
shared module), which would make this comment unnecessary.

═══════════════════════════════════════════════════════════════════════════
AREAS CHECKED AND FOUND CLEAN
═══════════════════════════════════════════════════════════════════════════

[code-quality]
  ✓ No unused imports across all 7 feature files — every import is referenced.
  ✓ No dead exports — all exported components are imported by their page shells
    (verified via grep across the repo).
  ✓ No `any`, `as any`, `@ts-ignore`, `@ts-expect-error` in any feature file.
  ✓ Component sizes are reasonable: ForgotPasswordForm (90 lines),
    VerifyOtpForm (195 lines — larger due to OTP input UX, but well-structured),
    auth/ResetPasswordForm (191 lines), SignInForm (67 lines), BrandPanel (49 lines),
    page shells (12-19 lines each).
  ✓ Error handling: all forms handle field errors, global errors, and success
    messages. VerifyOtpForm handles all three error sources (OTP error, session
    error, resend error) distinctly.
  ✓ No expensive render-path computations — VerifyOtpForm's `digits.join("")`
    is trivially cheap (6 chars), ResetPasswordForm's rulesMet is correctly
    wrapped in useMemo.
  ✓ No deep nesting or complex conditionals.

[dry-architecture]
  ✓ ActionState type is properly shared via lib/actions/shared.ts — not
    duplicated per action file.
  ✓ BrandPanel is used via shared component import in all 4 auth pages — no
    duplicated brand markup.
  ✓ PasswordField and SubmitButton are reused as shared UI primitives — no
    duplicated password input or submit button markup.
  ✓ Zod schemas centralized in lib/validation/schemas.ts — the passwordPolicy
    schema is shared between changePasswordSchema and resetPasswordViaOtpSchema.

[css-dry]
  ✓ "field-error" and "field-input" are @apply-based CSS classes (defined in
    globals.css) used consistently — not duplicated utility clusters.
  ✓ PasswordField encapsulates the password input + toggle button pattern —
    no duplicated show/hide password markup across forms.

[security]
  ✓ Anti-enumeration: requestPasswordResetOtp returns identical response shape
    for registered vs unregistered mobile numbers.
  ✓ OTP generation uses crypto.getRandomValues (CSPRNG), not Math.random.
  ✓ OTP stored as SHA-256 hash, never as plaintext (AUD-03 compliance).
  ✓ Single-use OTP: usedAt sentinel prevents replay after successful verification.
  ✓ Rate-limited resend: OTP_MAX_ATTEMPTS (5) caps resend requests per record.
  ✓ Flow tokens are HMAC-SHA256 signed via AUTH_SECRET with expiry check —
    fail-closed (verifyFlowToken returns null on any error).
  ✓ Cookies: httpOnly, secure (production), sameSite: lax — appropriate for
    same-site server-action flow. Not using "strict" because SMS-link-click
    would not send strict cookies in some browsers.
  ✓ All Server Actions validate inputs via Zod before any DB access.
  ✓ No raw SQL — Prisma ORM exclusively. No injection surface.
  ✓ No dangerouslySetInnerHTML, eval(), new Function() in any feature file.
  ✓ No hardcoded secrets or NEXT_PUBLIC_* misuse in feature files.
  ✓ Password reset atomically: password update + history record + all-session
    revocation in a single $transaction.
  ✓ Session revocation: all sessions deleted on password reset (no "keep current
    session" carve-out — appropriate since there IS no current session).
  ✓ Password history: last-3 check using the same logic as changeMyPassword —
    no divergent implementation.
  ✓ History trim: keeps only last 3 entries, preventing unbounded growth.
  ✓ Middleware correctly excludes forgot-password/* from session requirement
    (line 49 matcher).
  ✓ Auth layout redirects authenticated users away from auth pages (layout.tsx
    line 19).
  ✓ login action: generic error message for all failure modes (unknown user,
    wrong password, inactive account) — no information leakage.
  ✓ No .env files in git (checked .gitignore patterns).

═══════════════════════════════════════════════════════════════════════════
SUMMARY
═══════════════════════════════════════════════════════════════════════════
P1: 0
P2: 5 (1 security, 2 dry-architecture, 2 css-dry)
P3: 4 (2 code-quality, 1 dry-architecture, 1 css-dry, 1 security — note:
       Finding 9 is subsumed by Finding 2's fix)

Gate status: PASS (zero unresolved P1 findings)