Gate 5 — Quality & Security Audit (ticket #3269, forgot-password flow)
======================================================================
Generated: 2026-09-04
Feature files audited:
  - middleware.ts
  - app/(auth)/forgot-password/page.tsx
  - app/(auth)/verify-otp/page.tsx
  - app/(auth)/reset-password/page.tsx
  - components/auth/ForgotPasswordForm.tsx
  - components/auth/VerifyOtpForm.tsx
  - components/auth/CompleteResetForm.tsx

Supporting files inspected (referenced by feature files):
  - lib/actions/forgot-password.ts
  - lib/queries/forgot-password.ts
  - lib/forgot-password/index.ts
  - lib/actions/shared.ts
  - lib/validation/schemas.ts
  - lib/actions/auth.ts (login/logout for comparison)
  - components/auth/SignInForm.tsx (DRY baseline)
  - components/auth/BrandPanel.tsx (DRY baseline)
  - components/forms/ResetPasswordForm.tsx (DRY baseline — authenticated reset)
  - app/(auth)/layout.tsx (session guard)
  - app/(auth)/login/page.tsx (DRY page-shell baseline)
  - next.config.mjs (security headers, CSRF origin)

Skill resolution (per gate instructions):
  1. nextjs-tailwind-code-quality-audit — resolved via read_file (path 3)
     cross-repo-orchestration/skills/nextjs-tailwind-code-quality-audit/SKILL.md
  2. nextjs-tailwind-dry-architecture-audit — resolved via read_file (path 3)
     cross-repo-orchestration/skills/nextjs-tailwind-dry-architecture-audit/SKILL.md
     NOTE: skill file content partially corrupted/garbled (lines 21-70).
     Audit applied based on the readable header description and standard DRY
     analysis principles.
  3. nextjs-tailwind-css-dry-architecture-audit — resolved via read_file (path 3)
     cross-repo-orchestration/skills/nextjs-tailwind-css-dry-architecture-audit/SKILL.md
  4. nextjs-tailwind-security-audit — resolved via read_file (path 3)
     cross-repo-orchestration/skills/nextjs-tailwind-security-audit/SKILL.md


=======================================================================
AUDIT 1: [code-quality] — Code Quality & Maintainability
=======================================================================

Finding CQ-1 [code-quality]
  File: components/auth/VerifyOtpForm.tsx
  Location: lines 50-55, useEffect for verifyState
  Category: dead-code
  Priority: P3
  Problem: The useEffect watching verifyState is a no-op. Its body contains
    only a comment explaining why message-level errors are shown below the
    field rather than in a toast — but it performs no side-effect at all.
    This is a comment masquerading as an effect.
  Evidence:
    useEffect(() => {
      if (verifyState !== initialVerifyState && !verifyState.ok && verifyState.message) {
        // message-level errors ... shown below the field, not in the toast
      }
    }, [verifyState]);
  Why it matters: A reader seeing a useEffect must trace its body to
    determine it does nothing. This costs comprehension time on every read.
  Recommended fix: Delete the entire useEffect block. Move the comment to
    the JSX location where message-level errors are rendered (line 110-112)
    as a standalone code comment.
  Risk of change: None — the effect has no side-effects. Removing it cannot
    change observable behavior.
  Fixed: false (would change the file)

Finding CQ-2 [code-quality]
  File: lib/actions/forgot-password.ts + lib/queries/forgot-password.ts
  Location: maskMobile function — line 113 (actions) and line 34 (queries)
  Category: dead-code / duplication
  Priority: P3
  Problem: The maskMobile function is independently defined in two files with
    identical logic (mask all but last 4 chars with '*'). This is technically
    dead code in one or the other depending on perspective — each file uses
    only its own local copy.
  Evidence:
    actions/forgot-password.ts:113 — function maskMobile(mobile: string): string {
    queries/forgot-password.ts:34  — function maskMobile(mobile: string): string {
  Why it matters: A bug fix or policy change to masking logic must be applied
    in two places. Easily missed.
  Recommended fix: Extract to a single shared utility (e.g.
    lib/forgot-password/utils.ts) and import in both files.
  Risk of change: Low — behavior-identical consolidation.
  Fixed: false

Finding CQ-3 [code-quality]
  File: components/auth/CompleteResetForm.tsx + components/forms/ResetPasswordForm.tsx
  Location: RULES array and STRENGTH_LABELS constant (both files, top-level)
  Category: duplication
  Priority: P3
  Problem: The password policy rules array (5 rules, each with label + test
    function) and the STRENGTH_LABELS array are defined identically in both
    CompleteResetForm (unauthenticated reset) and ResetPasswordForm
    (authenticated reset). Both files note they are "the same set" — this is
    an intentional same-concept duplication.
  Evidence:
    CompleteResetForm.tsx:18-24 — const RULES: { label: string; test: ... }[]
    ResetPasswordForm.tsx:23-29 — const RULES: { label: string; test: ... }[]
    (identical content, identical labels, identical test functions)
  Why it matters: A policy change (e.g. minimum length goes from 8 to 10)
    needs updating in two places. The comment in CompleteResetForm explicitly
    notes they should be "the same set" — so a shared import is the intent.
  Recommended fix: Extract RULES and STRENGTH_LABELS to a shared module
    (e.g. lib/validation/password-rules.ts) and import in both components.
  Risk of change: None — pure data, same values.
  Fixed: false

Areas checked and found clean:
  - No unused imports in any feature file.
  - No unreachable code in any feature file.
  - Component sizes are appropriate — none of the 7 files exceeds ~180
    lines, and each handles a single responsibility.
  - TypeScript usage is clean — no `any`, no unsafe assertions, no
    @ts-ignore/@ts-expect-error in the feature files.
  - Error handling in Server Actions is thorough: every Prisma call is
    inside a well-structured flow with explicit error returns.
  - No deep nesting — early returns and guard clauses are used consistently.
  - No inefficient algorithms — the password-history check iterates at most
    3 rows (current + 2 history), which is constant-bounded.


=======================================================================
AUDIT 2: [dry-architecture] — Cross-file Logic Duplication
=======================================================================

NOTE: The nextjs-tailwind-dry-architecture-audit skill file is partially
corrupted (garbled text from line 21 onward). Audit applied based on the
readable header/description and standard DRY analysis methodology.

Finding DRY-1 [dry-architecture]
  File: lib/actions/forgot-password.ts + lib/queries/forgot-password.ts
  Location: RESEND_COOLDOWN_MS constant — line 58 (actions) and line 29 (queries)
  Category: duplicate-constants
  Priority: P2
  Problem: The RESEND_COOLDOWN_MS constant (60_000) is defined independently
    in two files. The queries file includes a "must match" comment pointing
    at the actions file, but there is no compile-time enforcement — a future
    edit to one file that forgets the other introduces a subtle behavioral
    discrepancy (the countdown shown to the user would disagree with the
    server-enforced cooldown).
  Evidence:
    lib/actions/forgot-password.ts:58  — const RESEND_COOLDOWN_MS = 60_000;
    lib/queries/forgot-password.ts:29  — const RESEND_COOLDOWN_MS = 60_000;
      // 60 seconds, must match lib/actions/forgot-password.ts
  Why these are the same concept: Both represent the identical "minimum
    interval between OTP sends" constraint — the action enforces it, and the
    query computes the countdown from it.
  Recommended fix: Export RESEND_COOLDOWN_MS from a single shared location
    (e.g. lib/forgot-password/constants.ts or export from the actions file
    and import in queries). Removes the "must match" comment because the
    source of truth becomes singular.
  Risk of change: None — same value, just one canonical definition.
  Fixed: false

Finding DRY-2 [dry-architecture]
  File: lib/actions/forgot-password.ts + lib/queries/forgot-password.ts
  Location: maskMobile function (same as CQ-2)
  Category: duplicate-utility
  Priority: P3
  Problem: Same as CQ-2 above — maskMobile is duplicated across files.
    Cross-referencing with code-quality finding. The overlap is real: both
    audits independently flag this because it's simultaneously dead-code-adjacent
    (each file has a copy only it uses) and a DRY violation (same concept).
  Recommended fix: Same as CQ-2 — extract to shared utility.
  Fixed: false

Finding DRY-3 [dry-architecture]
  File: components/auth/CompleteResetForm.tsx + components/forms/ResetPasswordForm.tsx
  Location: RULES, STRENGTH_LABELS, strength-bar computation, policy-card JSX
  Category: duplicate-business-logic + duplicate-UI
  Priority: P2
  Problem: The entire password-strength UI block — RULES array, STRENGTH_LABELS,
    strengthBars/metCount computation, the policy-card JSX with its SVG
    checkmarks — is duplicated between these two components. The only
    functional difference is:
      - CompleteResetForm has no currentPassword field (unauthenticated flow)
      - CompleteResetForm has no Cancel button
      - ResetPasswordForm uses bg-[rgb(230,230,230)] where CompleteResetForm
        uses bg-border-subtle for the inactive strength bar (token drift)
      - ResetPasswordForm uses text-[rgb(163,163,163)] where CompleteResetForm
        uses text-text-faint (same concept, drifted tokens)
    The form shells are different enough to not consolidate, but the
    strength meter + policy card are a perfect extraction target.
  Evidence (strength computation — identical):
    CompleteResetForm:48-49 — const strengthBars = ... Math.ceil((metCount / RULES.length) * 4)
    ResetPasswordForm:53    — const strengthBars = ... Math.ceil((metCount / RULES.length) * 4)
  Evidence (policy card — structurally identical, token drift):
    CompleteResetForm:116-163 — bg-accent-subtle, text-text-faint
    ResetPasswordForm:126-141 — bg-accent-subtle, text-[rgb(163,163,163)]
  Recommended fix: Extract a <PasswordStrengthMeter> and <PasswordPolicyCard>
    component pair (or a combined <PasswordPolicyPanel>) that accepts RULES
    as a prop or imports from the shared module (DRY-3 from code-quality).
    Both forms import and compose it. The token drift in the strength bar
    inactive state (bg-[rgb(230,230,230)] vs bg-border-subtle) gets resolved
    to one canonical token during extraction.
  Risk of change: Low — pure extraction. Visual output identical once the
    drifted token is unified.
  Fixed: false

Areas checked and found clean:
  - The three page files (forgot-password, verify-otp, reset-password)
    share a page shell pattern (BrandPanel + centered form). This was
    evaluated for extraction — but the shell is a single div with a
    BrandPanel and a flex container, totaling 4-5 lines. It already IS the
    shared abstraction (BrandPanel is a shared component). The outer div's
    slight variations (gap-0 vs overflow-y-auto vs plain center) reflect
    real layout differences per step. A wrapper component would gain almost
    nothing and add indirection. NOT flagged.
  - Server Action patterns (validate → load flow → guard → act) are
    consistent but not copy-paste-identical — each action has different
    business logic in the "act" phase. Consolidation would over-abstract.
  - The barrel file (lib/forgot-password/index.ts) correctly re-exports
    without duplicating logic.


=======================================================================
AUDIT 3: [css-dry] — Tailwind CSS Component Architecture
=======================================================================

Finding CSSDRY-1 [css-dry]
  File: app/(auth)/forgot-password/page.tsx, verify-otp/page.tsx,
        reset-password/page.tsx, login/page.tsx
  Location: Page-level wrapper div
  Category: repeated-class-combination
  Priority: P3
  Problem: The auth page shell class string
    "fixed inset-0 z-[100] flex overflow-hidden bg-surface" is repeated
    verbatim in all four auth page files.
  Evidence:
    forgot-password/page.tsx:14 — className="fixed inset-0 z-[100] flex overflow-hidden bg-surface"
    verify-otp/page.tsx:35      — className="fixed inset-0 z-[100] flex overflow-hidden bg-surface"
    reset-password/page.tsx:35  — className="fixed inset-0 z-[100] flex overflow-hidden bg-surface"
    login/page.tsx:6            — className="fixed inset-0 z-[100] flex overflow-hidden bg-surface"
  Why these are the same concept: All are the full-screen auth-page
    container. A visual change to the auth page background would need
    updating in 4 places.
  Recommended fix: This is borderline — the wrapper is 1 line in each file,
    and BrandPanel already extracts the main shared UI. A lightweight
    <AuthPageShell> component wrapping children in this div + BrandPanel
    would reduce each page to just the form. Alternatively, move this
    pattern into the (auth) layout itself (currently a bare fragment
    returning children). The layout approach is the more idiomatic Next.js
    solution — the layout already exists and currently does nothing
    presentational.
  Risk of change: Low — pure markup extraction. However moving BrandPanel
    into the layout changes where the component renders in the tree; verify
    each page's form centering still works.
  Fixed: false

Finding CSSDRY-2 [css-dry]
  File: components/auth/CompleteResetForm.tsx + components/forms/ResetPasswordForm.tsx
  Location: Strength meter bar markup + policy card markup
  Category: copy-pasted-UI
  Priority: P2
  Problem: The strength meter (4 bars with clsx conditional coloring) and
    the password policy card (SVG checkmarks, rule list) are copy-pasted
    between CompleteResetForm and ResetPasswordForm with minor token drift.
    This is the CSS/markup dimension of DRY-3.
  Evidence (strength bars — identical structure):
    CompleteResetForm:97-112 — flex max-w-[180px] flex-grow gap-[3px], h-[5px]
    ResetPasswordForm:113-119 — identical structure
  Evidence (policy card drift):
    CompleteResetForm:103 — bg-border-subtle (inactive bar)
    ResetPasswordForm:117 — bg-[rgb(230,230,230)] (hardcoded, same color intent)
    CompleteResetForm:139 — text-text-faint (inactive rule text)
    ResetPasswordForm:133 — text-[rgb(163,163,163)] (hardcoded, same intent)
  Recommended fix: Extract shared <PasswordStrengthMeter> and
    <PasswordPolicyCard> components (same recommendation as DRY-3). During
    extraction, unify the drifted tokens to the design-system version
    (bg-border-subtle, text-text-faint).
  Risk of change: Low. The token drift fix means ResetPasswordForm's
    hardcoded rgb values change to the semantic token — verify the computed
    color matches (likely identical; bg-border-subtle resolves to the same
    rgb in the theme).
  Fixed: false

Areas checked and found clean:
  - Form class patterns across auth components use existing shared
    components (PasswordField, SubmitButton, PhoneInput) — good reuse.
  - The inline SVG checkmark in the policy card is a valid candidate for
    a tiny component, but it's a single <svg> element used only in the
    policy card context. Extracting it gains little; flagged only as part
    of the broader policy-card extraction (CSSDRY-2).
  - Button variants: ForgotPasswordForm uses <SubmitButton>, VerifyOtpForm
    uses <SubmitButton> + a plain <button> for resend. The resend button
    is intentionally different (text-link style, not a primary button) —
    not a duplication issue.


=======================================================================
AUDIT 4: [security] — Security Vulnerabilities
=======================================================================

Finding SEC-1 [security]
  Severity: Low
  File: middleware.ts
  Location: line 46 (matcher), line 58 (loginUrl with ?next=)
  Category: input-validation (open redirect — mitigated)
  Priority: P3
  Problem: The middleware sets a `next` query parameter on the login redirect
    URL using the current pathname. This is the standard "return to the page
    you were trying to reach" pattern. However, the `next` parameter is
    currently never consumed — the login action (lib/actions/auth.ts)
    hard-redirects to /orders unconditionally (line 100). This means:
    (a) The return-to-original-page feature is incomplete/unused.
    (b) If a future change wires up the `next` param for post-login
        redirect without an allow-list, it becomes an open-redirect vector.
    The current state is safe — the param is set but never read.
  Attack scenario: None currently exploitable. Potential future risk only.
  Impact: None today.
  Recommended remediation: Either remove the `next` param from the redirect
    (dead code cleanup), or — when implementing the return-to feature —
    validate that `next` starts with "/" and contains no protocol/host
    prefix (i.e. `if (!next.startsWith('/') || next.startsWith('//'))
    next = '/orders'`).
  Fixed: false

Finding SEC-2 [security]
  Severity: Low
  File: components/auth/VerifyOtpForm.tsx
  Location: line 93-104, input element for OTP code
  Category: input-validation (client-side only)
  Priority: P3
  Problem: The OTP input field uses `maxLength={6}` and
    `inputMode="numeric"` as client-side hints, but has no `pattern`
    attribute and no `required` attribute. A direct form submission (e.g.
    via JavaScript or modified HTML) could send a non-numeric or empty
    string. This is fully mitigated by the server-side verifyOtpSchema
    (Zod: /^\d{6}$/) which rejects any non-6-digit input, so this is a
    defense-in-depth observation, not an exploitable finding.
  Attack scenario: Not exploitable — server validation catches all invalid
    input.
  Impact: None.
  Recommended remediation: Add `required` and `pattern="[0-9]{6}"` to the
    input for consistent client-side validation UX (shows native browser
    validation before the round-trip). Optional improvement, not a security
    gap.
  Fixed: false

Areas checked and confirmed secure:
  - Authentication bypass: The middleware matcher correctly excludes
    forgot-password, verify-otp, and reset-password from the session gate.
    The (auth)/layout.tsx bounces authenticated users to /orders. Both
    directions are covered.
  - Server Action authorization: All four Server Actions (requestResetCode,
    resendResetCode, verifyResetCode, completeReset) are intentionally
    unauthenticated (they're the forgot-password flow for users who CAN'T
    log in). Each re-validates the flow state from the DB on every call —
    no action trusts client state or the UI to gate access.
  - Input validation: All four actions use Zod schemas
    (forgotPasswordSchema, verifyOtpSchema, newPasswordSchema) before
    touching any data. No raw user input reaches Prisma queries — all
    queries use parameterized ORM methods, never string interpolation.
  - Injection: No dangerouslySetInnerHTML, eval, new Function, or raw SQL
    anywhere in the feature files or their action layer.
  - Secrets exposure: No NEXT_PUBLIC_* variables used. No hardcoded secrets.
    OTP is stored as SHA-256 hash only; plaintext never persisted or logged.
  - Cookie security: oms_reset_flow cookie is HttpOnly, Secure in
    production, SameSite=lax, path="/", with a TTL matching OTP_TTL_MS.
    Same flags on the main session cookie. Correct.
  - CSRF: Next.js Server Actions have built-in CSRF protection (Origin
    header check); next.config.mjs allowedOrigins is configured. Covered.
  - Timing attacks: OTP comparison uses timingSafeHexEqual with
    constant-time bitwise XOR over the full hash length. Correct.
  - Brute force: MAX_ATTEMPTS=5 with full flow deletion on exhaustion (not
    just locking). 10-minute TTL. Adequate for a 6-digit OTP.
  - Account enumeration: requestResetCode returns identical success
    message/behavior whether or not the mobile exists. Correct.
  - Session revocation on reset: completeReset deletes ALL sessions for the
    user in the same transaction as the password update. Correct and
    important — prevents an attacker holding a session from surviving a
    password reset.
  - Password history: Checks current hash + 2 most recent history entries
    (3 total), matching the authenticated changeMyPassword flow. Correct.
  - Error leakage: No stack traces or internal details in any error
    response. All errors are user-friendly messages.
  - Open redirect: middleware sets ?next= but it's never consumed (SEC-1).
    completeReset redirects to hard-coded /login. verifyResetCode redirects
    to hard-coded /reset-password. No user-controlled redirect targets.
  - No dependencies with known security issues introduced by these files.


=======================================================================
MERGED FINDING SUMMARY
=======================================================================

Total findings: 7 (0 P1, 2 P2, 5 P3)

P2 findings (reported, not blocking — no P1):
  DRY-1  [dry-architecture]  RESEND_COOLDOWN_MS duplicated constant
  DRY-3  [dry-architecture]  Password strength UI logic fully duplicated
         (also flagged as CSSDRY-2 from the CSS lens — overlap noted,
         both kept as they represent different audit perspectives on the
         same underlying issue)

P3 findings:
  CQ-1   [code-quality]  No-op useEffect in VerifyOtpForm
  CQ-2   [code-quality]  Duplicated maskMobile utility
         (also flagged as DRY-2 — overlap noted)
  CQ-3   [code-quality]  Duplicated RULES/STRENGTH_LABELS
         (subset of DRY-3/CSSDRY-2 — overlap noted)
  CSSDRY-1 [css-dry]     Auth page shell class string repeated 4x
  SEC-1  [security]      Unused ?next= param (dormant open-redirect seed)
  SEC-2  [security]      OTP input missing required/pattern (server-mitigated)

P1 findings: 0
Auto-fixes applied: 0
  (No findings met the "clearly safe, no behavior change" auto-fix bar.
   CQ-1 is the closest candidate but involves removing code from a file
   this gate has read-only access to.)

GATE STATUS: PASS (zero unresolved P1 findings)