parsa/canina
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
f4cdca4ed1
canina/docs/audit/phase3.1-backlog-review.md
parsaaghayi f4cdca4ed1
Some checks failed
Deploy Canina / deploy (push) Failing after 29s
Details
docs(audit): finalize Phase 3 Master Task Backlog and audit planning artifacts
2026-08-06 20:54:44 +03:30

9.5 KiB
Raw Blame History

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:

  1. 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.
  2. 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).
  3. 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.
  4. 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.
  5. 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).

3. Findings Requiring Task Refinements

  • 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

  1. 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.
  2. 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.

11. Acceptance Criteria Refinements

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