OMS Web

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)