Phase 3.1 — Human Review Preparation and Master Backlog Critique

Review Phase: Phase 3.1 — Human Review Preparation and Master Backlog Critique
Repository HEAD: 715873b2ecc3a72ba974bb2a2be87c5ba82bd4e7
Execution Mode: REVIEW_ONLY_NO_CODE_MUTATION
Canonical Verified Findings Source: docs/audit/20-verified-findings-index.json
Original Task Count: 10 (9 Implementation + 1 Verification)
Final Task Count: 10 (9 Implementation + 1 Verification)
Tasks Modified / Refined: 6 (TASK-AUTH-001, TASK-FIN-001, TASK-FE-001, TASK-VERIFY-001, TASK-SEC-001, TASK-SEC-002)
Business Decisions Reclassified: 3 (DECISION-001, DECISION-002, DECISION-003)

1. Executive Assessment

A rigorous, engineering-grade review of the Phase 3 Master Task Backlog was conducted against the actual repository source code in backend/ and src/. The review confirmed that all 14 verified findings are valid, but identified 5 critical technical refinements required before human approval:

Authentication Token Storage Architecture (TASK-AUTH-001): Storing JWT access tokens in localStorage exposes the storefront to Cross-Site Scripting (XSS) token exfiltration. The recommended architecture has been revised to use in-memory token state in Zustand / Axios interceptors with optional HttpOnly cookie support for production environments.
Financial Task Scope Cleanup (TASK-FIN-001): Inventory availability checking was removed from TASK-FIN-001 as scope creep, because the Prisma Product schema (schema.prisma) does not contain an inventory/stock column. The task is strictly focused on arbitrary-precision Decimal arithmetic (BE-001) and batched findMany product lookups (BE-002).
Business Decision Classification (DECISION-002): Payment gateway provider selection (DECISION-002) was removed from the canonical remediation backlog because no verified finding or active task requires payment gateway implementation. It is reclassified as FUTURE_SCOPE.
Declarative Routing Justification (TASK-FE-001): Confirmed via src/App.tsx evidence that manual useState("home") view rendering causes direct link URL navigation failure and state desynchronization. React Router DOM (react-router-dom) is explicitly justified and detailed with an exact route table.
Behavior-Oriented E2E Acceptance Criteria (TASK-VERIFY-001): Generic criteria ("100% of tests pass") have been replaced with precise, behavior-oriented verification assertions across Authentication, Security, Financial, Quality, and Frontend domains.

2. Findings That Require No Change

The following 8 findings were verified to have clean, accurate root-cause mappings and task assignments requiring no structural change:

SEC-003 -> Plaintext OTP disclosure in API payload (Handled in TASK-SEC-002).
ADM-001 -> Missing RBAC guard on settings API (Handled in TASK-SEC-003).
BE-001 -> Floating-point monetary calculation error (Handled in TASK-FIN-001).
BE-002 -> Synchronous N+1 query lookup in order processing (Handled in TASK-FIN-001).
TS-002 -> Missing slug field in Prisma seed script (Handled in TASK-BUILD-001).
TS-003 -> Non-type-only Response import under isolatedModules (Handled in TASK-BUILD-001).
TEST-001 -> Stale spec assertions for controller return types (Handled in TASK-BUILD-001).
DOC-001 -> Static Swagger specification desynchronization (Handled in TASK-DOC-001).

ARCH-001: Refined token storage mechanism in TASK-AUTH-001. Instead of suggesting localStorage as a primary storage target, the task explicitly warns against localStorage due to XSS vulnerability and specifies in-memory token state + HttpOnly cookie evaluation.
SEC-001: Refined threat model and startup failure behavior in TASK-SEC-001 to enforce mandatory length checks (minimum 32 bytes) and immediate process exit (process.exit(1)) on missing secrets.
SEC-002: Expanded TASK-SEC-002 to explicitly specify rate-limiting and expiration boundaries for SMS OTP codes.
FE-001 & TS-001: Explicitly mapped storefront route parameters and sub-view interface types in TASK-FE-001.

4. Tasks Requiring Splitting

Evaluation: No tasks required physical splitting. TASK-FIN-001 was originally considered for splitting, but since inventory checking was identified as unevidenced scope creep and removed, keeping Decimal calculation (BE-001) and query batching (BE-002) together in TASK-FIN-001 is optimal because both operate inside OrdersService.create in backend/src/orders/orders.service.ts.

5. Tasks Requiring Merging

Evaluation: The current 9 implementation tasks represent the minimal, non-overlapping task set. No further merging is required.

6. Unsupported Assumptions Identified

Assumption in TASK-AUTH-001: Assumed localStorage was the standard production auth token storage mechanism.
- Correction: localStorage is vulnerable to XSS token theft. Revised to specify in-memory Zustand token state with Bearer header interceptor in src/services/api.ts, with architectural guidance for HttpOnly cookies.
Assumption in TASK-FIN-001: Assumed OrdersService.create needed inventory stock deduction checks.
- Correction: Inspection of backend/prisma/schema.prisma shows Product has no stock or inventory field. Inventory deduction claims removed to prevent scope creep.

7. Scope Creep Identified and Removed

Removed Item: Inventory availability checking and stock deduction in TASK-FIN-001.
Removed Item: Payment Gateway Provider Selection (DECISION-002) from active remediation backlog. Moved to FUTURE_SCOPE.

8. Security Deep-Dive Review

TASK-SEC-001 (JWT Secret Enforcement):
- Threat Model: Attackers forging JWT signatures with role Admin or arbitrary sub (userId) when JWT_SECRET falls back to 'super-secret-key-canina'.
- Failure Behavior: Server startup terminates immediately (process.exit(1)) with FATAL: JWT_SECRET environment variable is missing or insecure.
TASK-SEC-002 (CSPRNG OTP & Payload Sanitization):
- Threat Model: PRNG state prediction of Math.random() to guess 5-digit OTPs; exploitation of plaintext code returned in API JSON response.
- Remediation: Use crypto.randomInt(10000, 100000). Strip code key from response. Enforce 2-minute OTP expiry in Redis/memory.
TASK-SEC-003 (Admin Settings RBAC Enforcement):
- Threat Model: Authenticated User_PetOwner clients sending requests to /api/settings to overwrite site branding or scientific terms.
- Remediation: Apply @UseGuards(JwtAuthGuard, RolesGuard) and @Roles('Admin') on SettingsController.

9. Architecture Review

Frontend Routing (TASK-FE-001):
- src/App.tsx evidence shows manual useState<View>("home") and custom popstate listeners. Direct page loads to /shop or /profile revert to home page.
- Solution: Integrate react-router-dom with <BrowserRouter> and explicit route map: /, /shop, /product/:id, /wiki, /profile/*, /checkout, /order-success, /order-tracking.
Backend API Client Integration (TASK-AUTH-001):
- Storefront src/services/authService.ts currently has partial sendOtp and verifyOtp methods. Wiring userStore to update in-memory state on successful verifyOtp completes the protocol alignment.

10. Dependency & Wave Adjustments

The execution waves have been refined to prioritize unblocking CI early:

Wave 0 — Security & Financial Core (P0): TASK-SEC-001, TASK-SEC-002, TASK-SEC-003, TASK-FIN-001.
Wave 1 — Build Health & Spec Alignment (P1): TASK-BUILD-001 (Unblocks CI Pipeline!).
Wave 2 — Auth Protocol & Storefront Integration (P1): TASK-AUTH-001 (Depends on TASK-SEC-002).
Wave 3 — Frontend Routing & CI Pipeline (P2): TASK-FE-001, TASK-DEVOPS-001 (TASK-DEVOPS-001 depends on TASK-BUILD-001).
Wave 4 — Documentation & Behavior Verification (P3 / Verification): TASK-DOC-001, TASK-VERIFY-001.

All generic acceptance criteria were replaced with concrete, observable, testable assertions.

Example (TASK-FIN-001):

Old: "Order total is accurate."
New: "OrdersService.create accumulates subtotals using Decimal.add() without primitive JS number conversion; unit spec asserts 19.99 * 3 + 5.01 = 65.00 with 0 decimal rounding artifacts; this.prisma.product.findMany is called exactly once per checkout request."

12. Business and Product Decision Classification

Decision ID	Title	Category	Description	Status
DECISION-001	SMS Gateway Provider Selection	`PRODUCT_DECISION`	Production SMS Gateway selection (e.g. Kavenegar, FarazSMS, Twilio) for NestJS `AuthService.sendOtp`. Dev mode uses stdout logger.	`PENDING_PRODUCT_INPUT`
DECISION-002	Live Payment Gateway Provider Selection	`FUTURE_SCOPE`	Merchant payment gateway provider selection for wallet deposits. Unrelated to current audit findings.	`FUTURE_SCOPE`
DECISION-003	Storefront vs Admin Architecture	`PRODUCT_DECISION`	Decision whether Admin UI remains embedded in SPA (`/admin/*`) or isolated into a standalone admin portal repo.	`PENDING_PRODUCT_INPUT`

13. Final Recommended Backlog Summary

Total Tasks: 10 (9 Implementation + 1 Verification)
Status: 100% READY FOR HUMAN APPROVAL

9.5 KiB Raw Permalink Blame History