diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 0bc6919..f592ad0 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -153,12 +153,15 @@ Per owner approval on 2026-11-03, each of the 19 fully merged remote branches wa | M3 Deployment & DB Migration | `feature/m3-deployment-migration` / #26 | **Merged** @ `46b85c1` into `launch-mvp` (Deployment architecture, DB migration policy, forensic production verification, environment inventory, migration test suite `test/migrations.test.js`) | | M4 Payment & Booking Readiness | `feature/m4-payment-booking-audit` / #27 | **Merged** @ `69cafae` into `launch-mvp` (Booking & payment architecture, payment readiness & financial safety report, booking integrity test report, 10/10 automated tests in `test/booking-integrity.test.js`) | | M4.1 UPI-First Checkout & Dynamic QR | `feature/m4-1-upi-checkout` / #28 | **Merged** @ `c3b4e83` into `launch-mvp` (UPI-First checkout, pure SVG dynamic QR, provider-neutral adapter with PhonePe PG primary, mobile intent links, SHA256 constant-time webhook verification, migration 015, 13/13 automated tests in `test/upi-checkout.test.js`) | +| M4.2 UPI Security & Readiness Audit | `feature/m4-2-upi-security-audit` | **Completed** (Independent audit of M4.1 UPI checkout: identified 2 Critical and 3 High severity defects including webhook checksum formula discrepancy, missing amount validation, guest checkout crash, and concurrency overbooking risk; verdict: NOT READY FOR LIVE PAYMENTS; `ENABLE_LIVE_PAYMENTS=false` preserved) | | M5–M10 | — | not started | ## 11. Where the detail lives | Topic | Doc | |---|---| +| UPI security audit | [UPI_SECURITY_AUDIT.md](UPI_SECURITY_AUDIT.md) | +| UPI provider readiness assessment | [UPI_PROVIDER_READINESS.md](UPI_PROVIDER_READINESS.md) | | UPI-first checkout architecture | [arch/UPI_PAYMENT_INTEGRATION.md](arch/UPI_PAYMENT_INTEGRATION.md) | | UPI operational readiness report | [launch/UPI_READINESS_REPORT.md](launch/UPI_READINESS_REPORT.md) | | Booking and payment architecture | [arch/BOOKING_AND_PAYMENT_ARCHITECTURE.md](arch/BOOKING_AND_PAYMENT_ARCHITECTURE.md) | diff --git a/docs/UPI_PROVIDER_READINESS.md b/docs/UPI_PROVIDER_READINESS.md new file mode 100644 index 0000000..f19d3ff --- /dev/null +++ b/docs/UPI_PROVIDER_READINESS.md @@ -0,0 +1,124 @@ +# NIGHTLIFEZ Payment Provider Readiness Assessment: PhonePe PG & Fallback Rails + +**Milestone:** M4.2 — Independent UPI Payment Security & Production Readiness Audit +**Date:** November 2026 +**Auditor:** Independent Payments Architect +**Safety Gate:** `ENABLE_LIVE_PAYMENTS=false` Enforced across all environments +**Readiness Verdict:** **NOT READY FOR LIVE PRODUCTION** · **MOCK / SIMULATED READY ONLY** + +--- + +## 1. Executive Summary + +Milestone M4.1 positioned **PhonePe Payment Gateway** as the primary UPI checkout provider and **PayU** as the designated secondary/fallback provider. + +This independent audit evaluated the concrete source code in `lib/upi/adapter.js`, `lib/services/upi-payments.js`, and `lib/handlers/upi-checkout.js` against official developer documentation, API specifications, and operational requirements. + +### Key Audit Conclusions: +1. **PhonePe PG**: The integration is **partially implemented and predominantly mocked/offline**. The codebase does not initiate payment orders with PhonePe’s server-side API (`/pg/v1/pay`). Instead, it synthesizes a direct NPCI UPI URI offline. While status checking and refunds are coded, they cannot interact with PhonePe’s gateway without prior server-side order initiation. Furthermore, webhook signature verification uses an incorrect hashing formula that fails against authentic PhonePe payloads. +2. **PayU Fallback**: **NOT IMPLEMENTED IN CODE**. PayU was described in architectural documentation, but zero lines of executable PayU adapter code, hash generators, or webhook receivers exist in the repository. It is purely architecturally proposed. +3. **Mock Sandbox**: **IMPLEMENTED AND FULLY TESTED**. The `MockUpiAdapter` allows deterministic unit testing and local development, but does not provide real payment processing. + +--- + +## 2. PhonePe Payment Gateway Capability Breakdown + +| Capability / Flow | Implementation Status | Test Status | Classification | Audit Notes & Findings | +| :--- | :--- | :--- | :--- | :--- | +| **Payment Intent Creation (`/pg/v1/pay`)** | **Not Implemented** | **Simulated / Offline** | **Not Implemented** | Does not call PhonePe's `/pg/v1/pay` or `/checkout/v2/pay`. Generates a direct peer/merchant UPI URI (`upi://pay?pa=nightlifez@ybl...`) locally. PhonePe's servers have no record of the order. | +| **Dynamic QR Generation** | **Implemented** | **Tested** | **Implemented (Local Engine)** | Pure ECMAScript QR generator (`lib/upi/qr.js`) converts local NPCI UPI string to vector SVG. Does not use PhonePe's gateway-hosted dynamic QR. | +| **Mobile UPI App Intents** | **Implemented** | **Tested** | **Implemented (Local Engine)** | Deep links constructed locally for generic `upi://`, Google Pay (`gpay://`), PhonePe (`phonepe://`), Paytm (`paytmmp://`), CRED (`credpay://`), BHIM (`bhim://`). | +| **Order & Merchant Identifiers** | **Implemented** | **Tested** | **Requires Merchant Onboarding** | Uses default UAT test merchant `PGTESTPAYUAT`. Prefix format `ORD-UPI-{timestamp}-{hex}` complies with PhonePe 35-character limit. | +| **Request Checksum (`signRequest`)** | **Implemented** | **Unit Tested** | **Implemented** | Implements `SHA256(body + endpoint + saltKey) + "###" + saltIndex` correctly for outbound GET/POST requests. | +| **Server-to-Server Webhook Verification** | **Defective Implementation** | **Flawed Mock Test** | **Requires Remediation** | **CRITICAL DEFECT (SEC-01)**: Hashed entire raw HTTP JSON body instead of the unescaped Base64 `response` string. Rejects 100% of authentic PhonePe callbacks. | +| **Authoritative Status Query (`/pg/v1/status`)** | **Implemented** | **Mocked in Tests** | **Requires Merchant Onboarding** | Endpoints `/pg/v1/status/{merchantId}/{orderId}` implemented correctly, but will return `TRANSACTION_NOT_FOUND` unless `/pg/v1/pay` is called first. | +| **Automated Refunds (`/pg/v1/refund`)** | **Implemented** | **Mocked in Tests** | **Implemented but Untested on Real PG** | Implements `/pg/v1/refund` with Base64 payload, `X-VERIFY`, and auto-revocation of Telegram access. Unit tested via HTTP stub. | +| **Timeout & Outage Handling** | **Partially Implemented** | **Unit Tested** | **Partially Implemented** | Catch blocks wrap network errors into `PaymentAdapterError` with status 502. However, automated retry queues and circuit breakers are absent. | +| **Preview vs Production Credential Isolation** | **Configured** | **Verified** | **Requires Merchant Onboarding** | Reads from `PHONEPE_MERCHANT_ID`, `PHONEPE_SALT_KEY`, `PHONEPE_HOST_URL`. All environments currently default to PhonePe Sandbox UAT keys. | + +--- + +## 3. PayU Fallback Audit + +User Requirement: +> "Determine whether PayU is: +> - Fully implemented. +> - Partially implemented. +> - Only architecturally prepared. +> - Not implemented. +> Do not describe a provider as fallback-ready merely because the adapter interface supports it." + +### Findings: +- **Search Query**: A search across the entire `lib/` directory for "PayU", "payu", or PayU hash algorithms (`sha512(key|txnid|amount|productinfo|firstname|email|udf1...|salt)`) yielded **0 results**. +- **Code State**: No PayU adapter class exists. No PayU webhook route (`/api/payments/webhook/payu`) exists. No PayU configuration variables exist in `.env.example` or `config.js`. +- **Verdict**: **ONLY ARCHITECTURALLY PREPARED / NOT IMPLEMENTED IN CODE**. +- While the `PaymentAdapter` interface (`createPaymentIntent`, `getPaymentStatus`, `verifyWebhook`, `initiateRefund`) was designed to accommodate PayU, zero executable PayU code has been written. + +--- + +## 4. Architectural Disconnect: Online PG vs Direct UPI URI + +The most critical architectural discovery of this audit is the **fundamental divergence between PhonePe Payment Gateway and the implemented UPI checkout mechanism**: + +```mermaid +flowchart TD + subgraph Current Implementation ["Current Implementation (Offline NPCI UPI)"] + A1[User clicks Checkout] --> B1[Server generates ORD-UPI-XXX] + B1 --> C1[Local buildUpiUri: nightlifez@ybl] + C1 --> D1[User scans QR or taps GPay/PhonePe] + D1 --> E1[Money sent directly to VPA nightlifez@ybl] + E1 -.->|DISCONNECT| F1[PhonePe PG has NO RECORD of this transaction] + F1 -.->|PhonePe Webhook NEVER FIRES| G1[Booking cannot confirm automatically] + end + + subgraph Standard PhonePe PG Flow ["Standard PhonePe PG Flow (Required for Gateway Automation)"] + A2[User clicks Checkout] --> B2[Server calls POST /pg/v1/pay] + B2 --> C2[PhonePe returns Gateway Intent URL & Dynamic QR] + C2 --> D2[User pays via PhonePe hosted rail] + D2 --> E2[PhonePe captures funds in Merchant Nodal Account] + E2 --> F2[PhonePe S2S Webhook POST /api/payments/webhook/phonepe] + F2 --> G2[Server cryptographically confirms booking] + end +``` + +### Consequences: +1. In the current implementation, scanning the QR code sends funds directly to the VPA `nightlifez@ybl` across standard bank-to-bank UPI. +2. Because PhonePe's `/pg/v1/pay` was never called, PhonePe's servers have no transaction order registered. +3. PhonePe will **never** trigger the webhook at `/api/payments/webhook/phonepe`. +4. Calling PhonePe's status API will fail with `TRANSACTION_NOT_FOUND`. +5. Therefore, the dynamic QR and mobile intent checkout currently functions as a **manual VPA transfer with zero automated server confirmation**. + +--- + +## 5. Payment Readiness Decision + +### Final Verdict: **NOT READY FOR PRODUCTION** +### Current State: **MOCK / SIMULATED READY ONLY** + +### Missing Prerequisites Before Production Activation: + +#### A. Code & Architectural Prerequisites (Engineering) +- [ ] **Fix Webhook Signature Calculation**: Update `verifyWebhookSignature` in `lib/upi/adapter.js` to compute `SHA256(response + saltKey) + "###" + saltIndex` on the decoded payload object. +- [ ] **Call PhonePe Gateway Initiation API**: Update `createPaymentIntent` to execute `POST /pg/v1/pay` (or PhonePe's QR API) so PhonePe's banking servers register the order before presenting the QR/intent to the customer. +- [ ] **Strict Webhook Amount Verification**: Validate `verified.amountPaise === payment.amount` before marking any payment `succeeded` or creating a booking pass. +- [ ] **Enforce Authentication on `/checkout/:slug/upi`**: Prevent unauthenticated guests from generating payment intents that crash with `NOT NULL bookings.user_id` on confirmation. +- [ ] **Active Intent Inventory Reservation**: Include pending unexpired payments in `seatsTaken` to prevent event overbooking under high traffic. +- [ ] **Fix QR Generator Bit Placement**: Correct `lib/upi/qr.js` to prevent Dark Module overwriting and handle multi-block capacities in Version 8. +- [ ] **CSS Viewport Isolation**: Toggle display between `.checkout-mobile` and `.checkout-desktop` via responsive media queries. + +#### B. Commercial & Regulatory Prerequisites (Business & Operations) +- [ ] **PhonePe KYB Onboarding**: Complete business KYC, bank account verification, and merchant contract execution. +- [ ] **Production API Credentials**: Acquire Production Merchant ID, Salt Key, and Salt Index from the PhonePe Merchant Portal. +- [ ] **VPA Whitelisting**: Ensure the merchant VPA is officially linked to the PhonePe merchant nodal account. +- [ ] **S2S Webhook URL Configuration**: Register `https://strangersmeetup.com/api/payments/webhook/phonepe` in PhonePe dashboard. +- [ ] **End-to-End ₹1 Pilot Transaction**: Execute and verify a live ₹1 payment and subsequent refund in a staging/preview environment before releasing to customers. + +--- + +## 6. Safety Gate Enforcement + +The safety switch remains locked: +```bash +ENABLE_LIVE_PAYMENTS=false +``` +Under no circumstances should this flag be set to `true` until all engineering defects in Section 5A are resolved, verified by automated tests, and formal business signoff in Section 5B is granted. diff --git a/docs/UPI_SECURITY_AUDIT.md b/docs/UPI_SECURITY_AUDIT.md new file mode 100644 index 0000000..67998af --- /dev/null +++ b/docs/UPI_SECURITY_AUDIT.md @@ -0,0 +1,275 @@ +# NIGHTLIFEZ Independent Security Audit: M4.1 UPI Checkout & Payment Architecture + +**Milestone:** M4.2 — Independent UPI Payment Security & Production Readiness Audit +**Target Commit:** `c3b4e83` (Merged PR #28 on `launch-mvp`) +**Auditor:** Independent Security Reviewer & Senior Payments Architect +**Date:** November 2026 +**Safety Status:** Enforced (`ENABLE_LIVE_PAYMENTS=false`) +**Overall Readiness Rating:** **NOT READY FOR PRODUCTION** (Critical Architectural & Security Defects Identified) + +--- + +## 1. Executive Summary + +An independent, zero-trust security audit of the Milestone M4.1 UPI-first checkout implementation was conducted across the database schema, cryptographic verification routines, provider adapter contracts, concurrency controls, QR encoding math, and user journey handlers. + +While M4.1 established clean architectural decoupling (provider-neutral adapter, additive migration 015, and immutable audit ledger structure), **critical discrepancies exist between the reported completion state and the actual codebase**. In particular: + +1. **Fatal Webhook Verification Incompatibility**: The PhonePe webhook signature verification algorithm hashes the full JSON body rather than the Base64 response string, guaranteeing that 100% of authentic callbacks from PhonePe will be rejected as forged. +2. **Missing Amount Verification (Underpayment Vulnerability)**: Webhook processing confirms bookings without validating that the amount paid matches the event/tier price. +3. **Guest Checkout PostgreSQL Crash**: Payments initiated by unsigned guests fail during webhook execution with a database `NOT NULL` constraint violation on `bookings.user_id`. +4. **Capacity Overbooking Under High Concurrency**: Unconfirmed checkout intents do not reserve capacity, allowing simultaneous buyers to purchase beyond available ticket quotas. +5. **Authorization Bypass on Status Polling**: Unauthenticated users can query order details and booking passes due to a flawed truthy conditional in `getUpiPaymentStatus`. +6. **QR Generator Spec Non-Compliance**: The zero-dependency QR generator overwrites the mandatory ISO/IEC 18004 Dark Module and drops data codewords in Version 8 QR matrices. + +--- + +## 2. Severity-Ranked Vulnerability & Defect Register + +### Summary Table + +| ID | Title | Severity | Impact | Affected Files | +| :--- | :--- | :--- | :--- | :--- | +| **SEC-01** | Real PhonePe Webhook Signature Mismatch | **CRITICAL** | All real PhonePe webhooks rejected (100% failure rate in production) | [`lib/upi/adapter.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/upi/adapter.js#L82) | +| **FIN-01** | Missing Amount Verification on Webhook Ingestion | **CRITICAL** | Underpayment attack; user pays ₹1 to confirm ₹5,000 tickets | [`lib/services/upi-payments.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/services/upi-payments.js#L243), [`lib/services/upi-payments.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/services/upi-payments.js#L333) | +| **DATA-01** | Guest Checkout Webhook Crash (`NOT NULL bookings.user_id`) | **HIGH** | Money collected, but booking fails to create; 500 error on webhook | [`lib/handlers/upi-checkout.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/handlers/upi-checkout.js#L22), [`lib/services/upi-payments.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/services/upi-payments.js#L277) | +| **CONCUR-01** | Simultaneous Checkout Intent Capacity Bypass | **HIGH** | Event overbooking; unconfirmed intents ignore capacity locks | [`lib/services/upi-payments.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/services/upi-payments.js#L96) | +| **AUTH-01** | Unauthenticated Information Disclosure & Polling IDOR | **HIGH** | Unauthenticated users can read order details and booking IDs | [`lib/services/upi-payments.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/services/upi-payments.js#L190) | +| **QR-01** | ISO 18004 Dark Module Overwrite & Codeword Truncation | **MEDIUM** | Scanners may fail to decode dynamic QR codes in Region 2 | [`lib/upi/qr.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/upi/qr.js#L141), [`lib/upi/qr.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/upi/qr.js#L258) | +| **UI-01** | Missing Viewport Isolation in Checkout Template | **MEDIUM** | Both mobile buttons and desktop QR render simultaneously | [`lib/views/checkout-upi.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/views/checkout-upi.js#L48) | +| **LOG-01** | Shared Rate Limit Keys with SuperProfile Webhook | **LOW** | IP rate-limiting collides between payment gateways | [`lib/handlers/upi-checkout.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/handlers/upi-checkout.js#L134) | +| **DB-01** | `payment_audits` Table Lacks Database-Level Append-Only Guard | **LOW** | Internal DB update could theoretically modify audit rows | [`migrations/015_upi_payment_integration.sql`](file:///Users/pradeepjakuraa/strangersmeetup/migrations/015_upi_payment_integration.sql#L38) | + +--- + +### Detailed Findings + +#### SEC-01: Real PhonePe Webhook Signature Mismatch (Fatal Incompatibility) +- **Severity:** **CRITICAL** +- **Location:** [`lib/upi/adapter.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/upi/adapter.js#L82-L91) +- **Description:** + PhonePe sends Server-to-Server (S2S) webhook notifications in the following JSON format: + ```json + { "response": "" } + ``` + According to official PhonePe Payment Gateway documentation, the checksum header `X-VERIFY` is computed as: + $$\text{X-VERIFY} = \text{SHA256}(\text{response} + \text{saltKey}) + \text{"###"} + \text{saltIndex}$$ + where `response` is the exact Base64 string value extracted from the JSON object. + + In `lib/upi/adapter.js`: + ```javascript + verifyWebhookSignature(signatureHeader, rawResponseBody) { + ... + const expectedHash = crypto.createHash('sha256').update(`${rawResponseBody}${this.saltKey}`).digest('hex'); + return safeEqual(gotHash, expectedHash); + } + ``` + The code passes `rawResponseBody` (the full raw HTTP text `{"response":"..."}`) directly into the SHA256 digest instead of extracting `parsed.response`. +- **Proof of Vulnerability:** + An empirical test script was executed feeding an official PhonePe S2S webhook payload and signature: + ```text + Does PhonePeAdapter accept real PhonePe signature? + REJECTED with error: Invalid PhonePe webhook signature + ``` +- **Remediation:** + Parse or extract the Base64 `response` property before calculating the SHA256 hash. Support both formats if testing with raw strings. + +--- + +#### FIN-01: Missing Amount Verification on Webhook Ingestion (Underpayment Attack) +- **Severity:** **CRITICAL** +- **Location:** [`lib/services/upi-payments.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/services/upi-payments.js#L243-L348) +- **Description:** + When a webhook arrives from PhonePe, `verified.amountPaise` contains the actual amount debited from the customer's bank account. + However, in `processUpiWebhook`: + ```javascript + if (status === 'succeeded') { + const res = await applySuccessfulUpiPayment({ + orderId, + providerTxnId: verified.providerTxnId, + providerPaymentId: verified.providerPaymentId, + actor: `webhook:${verified.provider}` + }); + ``` + The extracted `verified.amountPaise` is discarded and **never passed** to `applySuccessfulUpiPayment`. + In `applySuccessfulUpiPayment`: + ```javascript + const booking = (await c.query( + `INSERT INTO bookings (..., amount_paise, ...) VALUES (..., $5, ...)`, + [..., payment.amount, ...] + )).rows[0]; + ``` + The function blindly assumes the customer paid `payment.amount`. If a malicious actor alters the UPI payment amount in a tampered client or if a payment gateway reports partial settlement (e.g. ₹1 instead of ₹4,999), the booking is confirmed as fully paid, issuing valid tickets and private venue access. +- **Remediation:** + Assert `if (verified.amountPaise !== Number(payment.amount))` immediately in `processUpiWebhook` and `applySuccessfulUpiPayment`. If mismatched, mark the payment as `disputed` or `flagged_amount_mismatch`, alert finance, and refuse to confirm the booking. + +--- + +#### DATA-01: Guest Checkout Webhook Crash (`NOT NULL bookings.user_id`) +- **Severity:** **HIGH** +- **Location:** [`lib/handlers/upi-checkout.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/handlers/upi-checkout.js#L27), [`lib/services/upi-payments.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/services/upi-payments.js#L106), [`lib/services/upi-payments.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/services/upi-payments.js#L282) +- **Description:** + In [`migrations/003_identity_events_bookings.sql`](file:///Users/pradeepjakuraa/strangersmeetup/migrations/003_identity_events_bookings.sql#L119), the `bookings` table defines: + `user_id UUID NOT NULL REFERENCES users(id)` + + In [`lib/handlers/upi-checkout.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/handlers/upi-checkout.js#L27): + ```javascript + const user = await getSessionUser(req); + ``` + `upiCheckoutPage` does not enforce authentication (`requireUser`). If an unauthenticated guest visits `/checkout/:slug/upi`, `user` is `null`. + In `createUpiPaymentIntent`: + `resolvedUserId = null`, inserting a payment row with `user_id = NULL`. + When payment completes and the webhook fires: + ```javascript + await c.query(`INSERT INTO bookings (..., user_id, ...) VALUES ($1, $2, ...)`, + [event.id, payment.user_id, ...]); + ``` + PostgreSQL throws: + `null value in column "user_id" of relation "bookings" violates not-null constraint` (SQLSTATE 23502). + The webhook handler catches this, logs an error, and returns HTTP 500. The customer’s money has been debited, but no booking is created, no ticket is generated, and no invite link is granted. +- **Remediation:** + Either enforce authentication on `/checkout/:slug/upi` via `requireUser` / redirect to `/signin?next=...`, or establish an explicit guest claiming mechanism with cryptographic claim tokens. + +--- + +#### CONCUR-01: Simultaneous Checkout Intent Capacity Bypass (Overbooking Vulnerability) +- **Severity:** **HIGH** +- **Location:** [`lib/services/upi-payments.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/services/upi-payments.js#L96), [`lib/services/bookings.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/services/bookings.js#L7) +- **Description:** + In `createUpiPaymentIntent`: + ```javascript + const takenEvent = await seatsTaken(c, event.id); + const takenTier = await tierSeatsTaken(c, tier.id); + ``` + `seatsTaken` and `tierSeatsTaken` query only the `bookings` table. Active unexpired intents in `payments` (`WHERE status IN ('pending', 'processing') AND expires_at > now()`) are completely ignored. + Furthermore, `applySuccessfulUpiPayment` contains zero capacity checks when confirming a booking. + + **Scenario**: An event has 1 remaining seat. 20 users load the checkout page simultaneously. All 20 receive valid dynamic QR codes. If 5 users pay, all 5 webhooks will insert confirmed rows into `bookings`, overbooking the venue by 500%. + + *Audit Note on Test 9*: `test/upi-checkout.test.js` contained a false-positive test for concurrency because it called `applySuccessfulUpiPayment` on the first user before creating the second user's intent, testing sequential confirmation rather than simultaneous in-flight intents. +- **Remediation:** + Incorporate active unexpired payment intents into `seatsTaken` (`payments.status IN ('pending', 'processing') AND expires_at > now()`), or lock capacity and verify remaining seats inside `applySuccessfulUpiPayment` transaction, transitioning oversubscribed payments to `refund_pending` / `overbooked`. + +--- + +#### AUTH-01: Unauthenticated Information Disclosure & Polling IDOR +- **Severity:** **HIGH** +- **Location:** [`lib/services/upi-payments.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/services/upi-payments.js#L190-L192) +- **Description:** + In `getUpiPaymentStatus`: + ```javascript + if (payment.user_id && user && payment.user_id !== user.id && user.role !== 'super_admin' && user.role !== 'finance') { + throw new HttpError(403, 'forbidden', 'Access denied to this payment order'); + } + ``` + Because the condition requires `user` to be truthy, an **unauthenticated anonymous caller** (`user === null`) completely bypasses the block! + An attacker scanning or guessing order IDs (or intercepting network links) can query `GET /api/checkout/upi/status/:orderId` without logging in. The endpoint returns sensitive order details, total amount, event title, payment status, and the `bookingId`. +- **Remediation:** + Ensure that if `payment.user_id` is set, unauthenticated requests are strictly refused: + ```javascript + if (payment.user_id && (!user || (payment.user_id !== user.id && user.role !== 'super_admin' && user.role !== 'finance'))) { + throw new HttpError(403, 'forbidden', 'Access denied to this payment order'); + } + ``` + +--- + +#### QR-01: ISO/IEC 18004 Dark Module Overwrite & Codeword Truncation +- **Severity:** **MEDIUM** +- **Location:** [`lib/upi/qr.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/upi/qr.js#L141-L159), [`lib/upi/qr.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/upi/qr.js#L251-L260) +- **Description:** + 1. **Dark Module Overwrite**: Under ISO/IEC 18004 Section 7.9, the Dark Module at coordinate `(size - 8, 8)` must always be dark (binary 1). In `lib/upi/qr.js`, the loop writing format information bits to Region 2 used `if (i < 8) matrix[size - 1 - i][8] = bit`. For `i = 7`, this wrote bit 7 directly onto `matrix[size - 8][8]`. When format bit 7 was 0, it turned the Dark Module white (false), corrupting format information in Region 2. + 2. **Uneven Block Truncation**: In Version 8-M (used for ~130 character UPI strings), the total data capacity is 154 codewords split across 4 blocks (two blocks of 38, two blocks of 39). `lib/upi/qr.js` uses `dataLenPerBlock = Math.floor(154 / 4) = 38` for all blocks, silently dropping the final 2 data codewords and producing incorrect Reed-Solomon parity blocks. +- **Proof of Defect:** + ```text + Matrix size: 33 + Module at (size - 8, 8) [Dark Module position]: false + Is Dark Module 1 (black) as required by ISO 18004? false + ``` +- **Remediation:** + Update the format bit placement loop to `if (i < 7)`, place bits 7–14 along row 8 (`size - 15 + i`), and properly allocate unequal block capacities according to ISO/IEC 18004 Table 9. + +--- + +#### UI-01: Missing Viewport Isolation in Checkout Template +- **Severity:** **MEDIUM** +- **Location:** [`lib/views/checkout-upi.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/views/checkout-upi.js#L48-L105) +- **Description:** + The checkout template defines both `#mobile-checkout` (UPI deep links and app buttons) and `#desktop-checkout` (scannable SVG QR code). + However, in the embedded CSS, there are no media queries toggling display between desktop and mobile viewports (`display: none`). Both containers render simultaneously on all screen sizes, confusing mobile users with a giant scannable QR code and desktop users with non-functional mobile deep link chips. +- **Remediation:** + Add CSS media queries: + ```css + @media (min-width: 769px) { .checkout-mobile { display: none; } } + @media (max-width: 768px) { .checkout-desktop { display: none; } } + ``` + +--- + +#### LOG-01: Shared Rate Limit Keys with SuperProfile Webhook +- **Severity:** **LOW** +- **Location:** [`lib/handlers/upi-checkout.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/handlers/upi-checkout.js#L134), [`lib/handlers/upi-checkout.js`](file:///Users/pradeepjakuraa/strangersmeetup/lib/handlers/upi-checkout.js#L147) +- **Description:** + `phonepeWebhook` uses rate limit prefixes `sp_hook:` and `sp_bad_sig:`, which were originally created for SuperProfile webhooks (`handlers/superprofile.js`). +- **Remediation:** + Namespace rate-limit keys cleanly to `phonepe_hook:` and `phonepe_bad_sig:`. + +--- + +#### DB-01: `payment_audits` Table Lacks Database-Level Append-Only Guard +- **Severity:** **LOW** +- **Location:** [`migrations/015_upi_payment_integration.sql`](file:///Users/pradeepjakuraa/strangersmeetup/migrations/015_upi_payment_integration.sql#L38-L48) +- **Description:** + The `payment_audits` table is documented as an immutable audit ledger, but contains no PostgreSQL trigger or rule preventing `UPDATE` or `DELETE` statements. +- **Remediation:** + In a future migration, add an engine-level trigger: + ```sql + CREATE OR REPLACE FUNCTION prevent_audit_mutation() RETURNS trigger AS $$ + BEGIN RAISE EXCEPTION 'payment_audits is append-only'; END; + $$ LANGUAGE plpgsql; + CREATE TRIGGER trg_payment_audits_immutable BEFORE UPDATE OR DELETE ON payment_audits + FOR EACH ROW EXECUTE FUNCTION prevent_audit_mutation(); + ``` + +--- + +## 3. Test Verification & Empirical Evidence + +All verification was conducted using standard, sanctioned tooling against a real local PostgreSQL database (`sm_test`): + +```bash +# 1. Full unit, migration, integration test suite +TEST_DATABASE_URL=postgres://localhost:5432/sm_test npm test +``` +**Output**: 446 tests total, 444 passed, 2 skipped (unconfigured Stars sandbox), 0 failed (duration: 54.3s). + +```bash +# 2. Migration integrity verification +TEST_DATABASE_URL=postgres://localhost:5432/sm_test node --test test/migrations.test.js +``` +**Output**: 6/6 passed (13 sequential migrations verified: `003_` through `015_`). + +```bash +# 3. Prohibited legacy provider names scan +TEST_DATABASE_URL=postgres://localhost:5432/sm_test node --test --test-name-pattern="are gone from code" test/superprofile-checkout.test.js +``` +**Output**: 1/1 passed (0 hits across code, configs, UI, and docs). + +```bash +# 4. Code quality & linting check +npm run lint +``` +**Output**: `140/140 files OK`. + +--- + +## 4. Remediation Plan & Owner Action Items + +Before activating UPI payments in Production: +1. **Fix PhonePe Checksum Calculation**: Update `verifyWebhookSignature` to extract and hash the Base64 `response` payload. +2. **Implement Strict Amount Matching**: Reject any webhook or status update where `amountPaise !== payment.amount`. +3. **Enforce Authentication on UPI Checkout**: Protect `/checkout/:slug/upi` with `requireUser(req)` or redirect to sign-in. +4. **Fix Concurrency Inventory Allocation**: Count active unexpired payment intents in `seatsTaken` and verify capacity at confirmation time. +5. **Close IDOR Vulnerability**: Ensure unauthenticated requests to `/api/checkout/upi/status/:id` return HTTP 401/403. +6. **Correct QR Matrix Math**: Fix format information bit placement and multi-block capacity calculation in `lib/upi/qr.js`. +7. **Isolate Viewports**: Add responsive CSS media queries in `checkout-upi.js`.