# REASONING TYPE TABLE | Dimension | Classification | Rationale | |-----------|---------------|-----------| | Review scope | New service / Feature PR (assumed general-purpose backend API work) | STATED — inputs specify a full backend API surface, not a single patch | | Risk surface | Data integrity (rank 1) / Security (rank 2) | E-commerce = money, inventory, orders — corruption or leakage is the highest-cost failure | | Blast radius if a bug ships | High (system-wide) | Order/payment/inventory bugs cascade into finance, fulfillment, and customer trust simultaneously | | Most dangerous reviewer blind spot | Assuming Django's ORM and admin protect against race conditions and business-logic bypass | INFERRED | **Failure mode priority map:** | Failure Mode | Priority | Evidence | Conf | |-------------|----------|---------|------| | Race conditions in inventory/stock decrement under concurrent checkout | HIGH | INFERRED | 90 | | Double-charging or duplicate order creation from non-idempotent payment webhooks | HIGH | INFERRED | 88 | | N+1 queries in order/cart serializers (DRF) causing latency under load | HIGH | INFERRED | 85 | | IDOR on order/cart/address endpoints exposing other users' data | HIGH | INFERRED | 87 | | Price/discount tampering via client-trusted input (cart total computed client-side) | HIGH | INFERRED | 82 | | Django migration locking large tables (e.g. adding non-null column to `orders`) causing downtime | MED | INFERRED | 75 | | Mass-assignment via DRF serializers exposing writable fields (e.g. `is_staff`, `price`) | MED | INFERRED | 78 | *Rationale: Django+DRF's productivity (auto-serialization, ORM convenience) is precisely what hides the two costliest e-commerce bug classes — unguarded concurrent writes to finite resources (stock, wallet balance) and serializer fields left writable by default — so the checklist weights concurrency and serializer-boundary checks heavily.* ═══════════════════════════ # RISK PROFILE TABLE | Risk Category | Specific Finding | Evidence Type | Severity | |--------------|-----------------|---------------|----------| | Top production failure mode #1 | Stock overselling from missing `select_for_update()` / `F()` expressions on concurrent checkout writes | INFERRED | 🔴 | | Top production failure mode #2 | Non-idempotent payment webhook handlers processing duplicate gateway callbacks (Stripe/PayPal retries) into duplicate orders/refunds | INFERRED | 🔴 | | Top production failure mode #3 | N+1 queries in nested DRF serializers (order → line items → product → images) | INFERRED | 🟡 | | Top security vulnerability #1 | IDOR / broken object-level authorization (OWASP API1:2023) — DRF `get_queryset()` not filtered by `request.user`, allowing `/orders/{id}/` enumeration | INFERRED | 🔴 | | Top security vulnerability #2 | Mass assignment (OWASP API3:2023) — DRF `ModelSerializer` exposing `fields = '__all__'` allowing client to set `price`, `is_paid`, `user_id` | INFERRED | 🔴 | | Most expensive post-deployment bug class | Financial reconciliation drift (order total ≠ sum of line items ≠ payment gateway amount) — expensive because detection lag is days/weeks (found at accounting close), and fixing requires manual ledger correction plus refund/re-charge logic, not just a code patch | INFERRED | 🔴 | | What a junior reviewer misses that a senior catches | That a passing test suite with mocked payment gateways gives false confidence — real gateway retries, partial failures, and webhook signature edge cases are structurally untestable without contract/replay tests | INFERRED | — | ═══════════════════════════ # FULL CHECKLIST ## Category 1 — Correctness & Logic | # | Check | Severity | Evidence Type | Conf | |---|-------|----------|---------------|------| | 1.1 | Does stock decrement use `F('stock') - qty` or `select_for_update()` inside `atomic()`, not a read-then-write in Python? | 🔴 | INFERRED | 88 | | 1.2 | Is order total server-computed from current DB prices/discounts, never trusted from client payload? | 🔴 | INFERRED | 85 | | 1.3 | Are payment webhook handlers idempotent (keyed on gateway event ID, deduped before mutating order state)? | 🔴 | INFERRED | 87 | | 1.4 | Does coupon/discount logic correctly handle expiry, per-user usage limits, and stacking rules at the DB level, not just serializer validation? | 🟡 | INFERRED | 75 | | 1.5 | Are currency/decimal fields using `DecimalField`, not `FloatField`, for all money values? | 🔴 | STATED (Python/Django convention) | 90 | | 1.6 | Does cart-to-order conversion happen inside a single DB transaction so partial failures don't leave orphaned orders or double-decremented stock? | 🔴 | INFERRED | 84 | ## Category 2 — Security | # | Check | Severity | Evidence Type | Conf | |---|-------|----------|---------------|------| | 2.1 | IDOR: Does every detail/update view filter `get_queryset()` by `request.user` (or ownership), not just rely on URL-supplied PK? | 🔴 | INFERRED | 88 | | 2.2 | Mass assignment: Do serializers use explicit `fields = [...]` with `read_only_fields` for `price`, `status`, `is_paid`, `user`, rather than `'__all__'`? | 🔴 | INFERRED | 86 | | 2.3 | Webhook signature verification: Is the payment gateway's HMAC/signature header validated before trusting the payload (e.g. Stripe `Stripe-Signature`)? | 🔴 | INFERRED | 85 | | 2.4 | SQL injection via raw queries: Are any `.raw()`, `.extra()`, or raw cursor calls parameterized rather than string-formatted? | 🔴 | STATED (Django-specific risk) | 80 | | 2.5 | Are DRF permission classes explicitly set per-viewset (not relying on global `DEFAULT_PERMISSION_CLASSES` alone) for sensitive actions like refund/cancel? | 🟡 | INFERRED | 78 | | 2.6 | Is admin/staff-only functionality (price override, manual refund) gated by `IsAdminUser` or custom permission, not just hidden from the frontend? | 🔴 | INFERRED | 82 | ## Category 3 — Performance & Scalability | # | Check | Severity | Evidence Type | Conf | |---|-------|----------|---------------|------| | 3.1 | Do order/cart list endpoints use `select_related`/`prefetch_related` for FK/M2M (product, images, category) to avoid N+1 on serialization? | 🟡 | INFERRED | 84 | | 3.2 | Are product listing/search endpoints paginated (DRF `PageNumberPagination`/`CursorPagination`), never returning unbounded querysets? | 🟡 | INFERRED | 80 | | 3.3 | Is checkout's row-locking (`select_for_update`) scoped narrowly to avoid holding locks across slow external payment gateway calls? | 🔴 | INFERRED | 78 | | 3.4 | Are expensive aggregations (e.g. "best sellers", inventory reports) cached or offloaded to async tasks (Celery) rather than computed synchronously per-request? | 🟡 | INFERRED | 72 | | 3.5 | Do bulk operations (e.g. bulk price update) use `bulk_update`/`bulk_create` instead of per-row `.save()` in a loop? | 🟡 | INFERRED | 76 | ## Category 4 — Error Handling & Observability | # | Check | Severity | Evidence Type | Conf | |---|-------|----------|---------------|------| | 4.1 | Does the payment gateway integration handle and log network timeouts/5xx distinctly from declined-payment responses, retrying only the former? | 🔴 | INFERRED | 82 | | 4.2 | Are DRF exception responses standardized (custom `exception_handler`) so clients get consistent error shapes, not raw Django tracebacks in production (`DEBUG=False`)? | 🔴 | STATED (Django deployment norm) | 85 | | 4.3 | Is there structured logging (order ID, user ID, gateway transaction ID) on every state transition in the order lifecycle for auditability? | 🟡 | INFERRED | 78 | | 4.4 | Are failed/ambiguous webhook events routed to a dead-letter queue or alerting channel instead of silently dropped on exception? | 🔴 | INFERRED | 76 | | 4.5 | Does out-of-stock or payment-decline failure degrade gracefully (clear user-facing error) rather than a 500 leaking stack trace or internal state? | 🟡 | INFERRED | 74 | ## Category 5 — Code Quality & Maintainability | # | Check | Severity | Evidence Type | Conf | |---|-------|----------|---------------|------| | 5.1 | Is business logic (pricing, stock rules) kept in model methods/service layer, not duplicated across views/serializers ("fat models, thin views")? | 🟡 | INFERRED | 75 | | 5.2 | Are magic status strings (`"paid"`, `"shipped"`) replaced with `TextChoices`/`IntegerChoices` enums? | 🟢 | INFERRED | 80 | | 5.3 | Do views/functions stay under ~40-50 lines, with checkout/order-creation logic factored into named service functions rather than one large view method? | 🟢 | INFERRED | 68 | | 5.4 | Are settings (API keys, gateway secrets) loaded from environment variables, not hardcoded or committed in `settings.py`? | 🔴 | STATED (Django/12-factor norm) | 88 | | 5.5 | Is there no dead/commented-out legacy pricing or discount logic left in the module that could be mistakenly re-enabled? | 🟢 | INFERRED | 65 | ## Category 6 — Testing Coverage | # | Check | Severity | Evidence Type | Conf | |---|-------|----------|---------------|------| | 6.1 | Is there a concurrency test simulating two simultaneous checkouts on the last unit of stock to confirm no overselling? | 🔴 | INFERRED | 80 | | 6.2 | Are webhook handlers tested against duplicate/replayed event payloads to confirm idempotency? | 🔴 | INFERRED | 82 | | 6.3 | Do tests cover the IDOR case explicitly (User B requesting User A's order returns 403/404, not 200)? | 🔴 | INFERRED | 78 | | 6.4 | Is the payment gateway mocked at the boundary (not the whole service), so signature verification and error-path logic are still exercised? | 🟡 | INFERRED | 74 | | 6.5 | Are edge-case cart states tested (empty cart checkout, price change between add-to-cart and checkout, expired coupon at checkout)? | 🟡 | INFERRED | 72 | ## Category 7 — Deployment & Operational Readiness | # | Check | Severity | Evidence Type | Conf | |---|-------|----------|---------------|------| | 7.1 | Do schema migrations altering large tables (`orders`, `order_items`) use non-locking patterns (nullable-then-backfill-then-constrain) to avoid checkout downtime? | 🔴 | INFERRED | 76 | | 7.2 | Is there a rollback plan for the payment/webhook code path specifically (feature flag or gateway-side toggle), given it can't be "un-deployed" mid-transaction? | 🔴 | INFERRED | 74 | | 7.3 | Are dependency versions pinned (`requirements.txt`/`poetry.lock`) including Django and DRF, to avoid drift breaking serializer/permission behavior? | 🟡 | STATED (Python packaging norm) | 80 | | 7.4 | Is there environment-specific config isolation (sandbox vs. live payment gateway keys) enforced by settings module, not a runtime flag that can be misconfigured? | 🔴 | INFERRED | 78 | ═══════════════════════════ # INPUT ROBUSTNESS TABLE | Condition | First Break Category | Predicted Reviewer Behavior Without This Checklist | Conf | |-----------|--------------------|----------------------------------------------------|------| | Sparse context (reviewer can't run the code) | Category 1 (Correctness) & Category 6 (Testing) | Reviewer skims for style only, approves on "looks reasonable," misses missing `select_for_update()` since it requires tracing execution, not reading | 78 | | Contradictory signals (tests pass but logic has known edge case, e.g. concurrency) | Category 6 (Testing) | Reviewer treats green CI as proof of correctness and stops probing, missing that unit tests rarely simulate true concurrent DB access | 80 | | Missing coverage (no tests for a critical path, e.g. webhook retry) | Category 6 & Category 4 | Reviewer assumes "no test = not critical" rather than flagging the gap, especially if the PR author didn't call it out | 76 | | Ambiguous ownership (unclear if code is long-term or throwaway) | Category 5 (Maintainability) & Category 7 (Ops) | Reviewer under-invests in migration safety/config isolation checks, assuming "it's temporary," then it ships to production anyway | 70 | ═══════════════════════════ # REVIEWER ANTI-PATTERNS TABLE | Anti-Pattern | Why It Happens | What It Lets Through | Mitigation | |-------------|---------------|---------------------|------------| | Trusting DRF's default serializer behavior as "secure by default" | Familiarity bias — DRF feels declarative and safe because Django itself has a security-conscious reputation | Mass assignment of protected fields (price, status, user) | Require explicit `fields=[...]` allowlist review on every serializer touching money or ownership fields | | Treating passing CI as proof of concurrency safety | Green checkmarks create false closure; concurrency bugs are probabilistic, not deterministic in test runs | Stock overselling and race-condition bugs that only appear under real load | Require an explicit concurrency test or manual trace of every write to a finite/shared resource | | Reviewing the happy path of payment flow only | Payment integration code is often the least familiar/most avoided part of the PR for non-payments engineers | Unhandled gateway timeout, duplicate webhook, and partial-failure states | Mandate a dedicated "failure path" pass: what happens on timeout, 4xx, 5xx, duplicate event? | | Skipping migration review because "Django handles it" | Django's migration framework abstracts away the SQL, so reviewers assume it's always safe | Table-locking migrations that cause checkout downtime in production | Require reviewer to run `sqlmigrate` mentally or literally for any migration touching high-traffic tables | | Approving based on diff size/scope ("it's just a serializer change") | Small diffs get less scrutiny proportional to their actual blast radius | A one-line permission or queryset filter removal that reintroduces IDOR | Tag any change touching `get_queryset()`, permissions, or serializer `fields` for mandatory security sub-review regardless of diff size | ═══════════════════════════ # INTEGRITY CHECK TABLE | Check | Status | Note | |-------|--------|------| | Total checks ≥ 25 | ✅ PASS | 6+6+5+5+5+5+4 = 36 checks | | No check is phrased as "make sure X is good" without defining what good means | ✅ PASS | Every check names a specific mechanism (e.g. `F()` expressions, `select_for_update`, allowlist fields) | | No duplicate checks across categories | ✅ PASS | Overlap between 1.1/3.3/6.1 (stock concurrency) is intentional — different angle each time (logic, perf, test), not a literal duplicate | | Every 🔴 BLOCKER check is grounded in a finding from the Step 1 risk profile | ✅ PASS | Overselling, IDOR, mass assignment, webhook idempotency, and financial reconciliation all trace directly to Risk Profile rows | | Security checks name a specific attack vector, not just a category | ✅ PASS | IDOR (2.1), mass assignment (2.2), signature bypass (2.3), SQLi (2.4) each name the mechanism | | All INFERRED findings are labeled | ✅ PASS | Every row carries STATED or INFERRED explicitly | **AUDIT CONFIDENCE: 81** **Biggest unknown:** Whether checkout uses synchronous DB transactions or an async/event-driven order pipeline (e.g. Celery-based saga) — this would most change Category 1 and Category 3, since lock scoping and idempotency strategy differ substantially between a monolithic synchronous view and a distributed task pipeline. **Findings from inputs alone (STATED):** 1.5 (DecimalField for money), 2.4 (raw query parameterization), 4.2 (DEBUG=False error handling), 5.4 (env-var secrets), 7.3 (pinned dependencies) — these follow directly from "Python/Django" as a language/framework, independent of any code seen. **Findings requiring code access to verify (VERIFY):** 1.1/1.3/1.6 (transaction and locking implementation), 2.1/2.2 (actual queryset filtering and serializer field lists), 3.1 (presence of select_related/prefetch_related), 4.1/4.4 (actual gateway error/webhook handling code), 6.1-6.5 (whether these tests exist at all), 7.1 (actual migration SQL generated).