# EDR Passenger β€” Solutions Concrete fixes for the confirmed findings in `docs/ISSUES.md`. Ordered by priority. Each references the exact site and the intended change. Code sketches are illustrative, not drop-in patches. **Test coverage backing these:** 28 automated tests (25 API `jest` + 3 UI Playwright) reproduce the βœ… findings. Fix a finding β†’ its πŸ”΄ test flips from "bug present" to failing; update the test to assert the corrected behavior. --- ## P0 β€” deploy blockers (money creation/theft, broken booking) ### C-10 β€” Authenticated `POST /bookings` is broken `bookings.service.ts:773` resolve unconditionally; delete the UUID-format gate: ```ts // BEFORE: resolves only when passengerId is NOT a UUID (never fires for real IAM ids) if (dto.passengerId && !dto.passengerId.match(/^[0-9a-f-]{36}$/i)) { … } // AFTER: always translate the authenticated identity β†’ the Passenger.id if (dto.passengerId) { const passenger = await this.prisma.passenger.findUnique({ where: { iamUserId: dto.passengerId }, select: { id: true }, }); if (passenger) dto = { ...dto, passengerId: passenger.id }; // else: leave as-is only if it already IS a Passenger.id (guest/admin paths) } ``` Keep the controller's "don't trust the body" intent (`bookings.controller.ts:528`) β€” it's correct to take identity from the JWT; the service just has to map iamUserId β†’ Passenger.id. Regression test: `test/authed-booking-passengerid.e2e-spec.ts`. ### C-1 β€” Booking total is client-controlled `bookings.service.ts:863-899`: stop trusting `reviewedTotalMinor`/`seatFareMinor`. Recompute the fare server-side and reject a mismatch: ```ts const server = fareCalculation.totalMinor; if (dto.reviewedTotalMinor != null && Math.abs(dto.reviewedTotalMinor - server) > 1) { throw new BadRequestException('Price changed β€” please review the updated fare'); } resolvedTotalMinor = server; // never persist a client amount as the charge basis ``` Apply the same to `guest-booking.service.ts:206-245`. ### C-2 β€” Loyalty redemption unbounded + never deducted `bookings.service.ts:1705` (and `:1028/:1249/:1451`): validate + debit inside the booking transaction: ```ts const acct = await tx.loyaltyAccount.findUnique({ where: { passengerId } }); const pts = Math.min(dto.loyaltyRedemptionPoints ?? 0, acct?.pointsBalance ?? 0, MAX_REDEEM); const loyaltyMinor = pts * POINTS_TO_MINOR; await tx.loyaltyLedgerEntry.create({ data: { accountId: acct.id, delta: -pts, reason: 'REDEEMED', balanceAfter: acct.pointsBalance - pts } }); await tx.loyaltyAccount.update({ where: { id: acct.id }, data: { pointsBalance: { decrement: pts } } }); ``` ### C-3 β€” Wallet top-up: no ownership, no backing - `wallet.controller.ts:34`: enforce `req.user` owns `:passengerId` (or is admin) before top-up. - `wallet.service.ts:50`: only credit after a confirmed `PaymentIntent` (a top-up is a purchase). - `wallet.controller.ts:23-24`: remove `@IsPublic()` from `GET /wallet/accounts`. ### C-4 β€” Payment amount never validated - `intents.service.ts:541-548`: on `confirmedAmountMinor !== intent.amountMinor`, do NOT mark SUCCEEDED β€” set a `AMOUNT_MISMATCH` state and alert. Populate `confirmedAmountMinor` in each webhook handler (e.g. `waafi-webhook.service.ts:63`). - `payments.service.ts:809` `finalizePaymentSuccess`: assert the settled amount equals `booking.totalMinor` before confirming. ### C-5 β€” Late webhook resurrects an expired/cancelled booking `payments.service.ts:809` `finalizePaymentSuccess`: refuse to confirm unless the booking is still `PENDING_PAYMENT`; route a late success to the refund/again-available flow: ```ts if (booking.status !== 'PENDING_PAYMENT') { await this.refundLatePayment(intent); return { alreadyFinalized: true }; } ``` ### C-6 β€” Wallet double-spend (no row lock) `payments.service.ts:461-484`: lock the row or use an atomic conditional update: ```ts const res = await tx.$executeRaw`UPDATE passenger."WalletAccount" SET "balanceMinor" = "balanceMinor" - ${total} WHERE "passengerId" = ${booking.passengerId} AND "balanceMinor" >= ${total}`; if (res === 0) return { success: false }; // insufficient / lost the race ``` Regression test: `critical-repro.e2e-spec.ts` (C-6, barrier-forced interleave). ### C-7 β€” Refund computed but never disbursed `bookings.service.ts:2017-2027`: on `booking.cancelled`, actually disburse β€” credit the wallet or call the provider refund β€” and drive `refundStatus PENDING β†’ PROCESSING β†’ COMPLETED`. Add a reconciliation sweep for stuck `PENDING` rows. ### C-8 β€” Exchange-rate writes missing the ADMIN check (any passenger can write FX) `fare-engine/currency.controller.ts:25,32`: add `@PassengerAdmin()` (+ `@ApiBearerAuth`) to the `PUT` and `PATCH` handlers, matching the already-guarded `DELETE`. (Not unauthenticated β€” the global JwtGuard requires a token; the gap is the missing *authorization*. Verified live: passenger β†’ 200.) ### C-9 β€” `@Roles('ADMIN')` is dead Register the guard globally so `@Roles` is enforced: ```ts // app.module.ts providers { provide: APP_GUARD, useClass: RolesGuard } ``` …or convert `configurable-fare` / `segment-fare` / `system-config` controllers to the working `@PassengerAdmin()`/`@PassengerStaff()` guards. --- ## P1 β€” pricing correctness (HIGH) - **H-1 promo β†’ negative total** (`fare-engine.service.ts:192`): `totalEtbMinor = Math.max(0, subtotal - discount)`; DTO `@Min(0) @Max(100)` on `percentOff`, `@Min(0)` on `amountOffMinor` (`promos.dto.ts:20,26`). - **H-2 missing FX β†’ 1.0** (`currency.service.ts:142`): remove the silent `return 1.0` β€” throw / block the quote so it fails closed. - **H-3/H-4 FX divergence & unit confusion** (`currency.service.ts`): collapse the 4 routines into one `convert(fromMinor, from, to): {minor|major}` with one rounding + one fallback policy; give it a branded `Minor`/`Major` return type and audit every `amountMinor` assignment across the payment boundary. - **H-5 `percentOff:0` treated as FIXED** (`fare-engine.service.ts:185`): use `promo.percentOff != null ? … : promo.amountOffMinor`. - **H-6 `insuranceFeeMinor` dual meaning** (`fare-engine.service.ts:130,154,167`): split into `insuranceMultiplierBps` and `insuranceFeeMinor`; use one consistently. - **H-7 domestic ETB fare Γ— USDβ†’ETB rate** (`fare-engine.service.ts:157`): don't apply a currency conversion to a domestic base fare β€” separate the minor-unit scaling from FX. - **H-8 baggage ignores seat class** (`excess-baggage.service.ts:53`): `findFirst({ where: { seatClassId } })`. - **H-9 baggage/supp skip conversion + DJF rounding** (`excess-baggage.service.ts:166`, `supplementary-charges.service.ts:132`): route through `convertMinorToChargeMajor`. - **H-10 future-dated FX applied now** (`currency.service.ts:88,137`): add `effectiveDate: { lte: new Date() }` to the rate lookups. - **H-11 non-deterministic fare resolution** (`fare-engine.service.ts:84,298`): add `orderBy: { validFrom: 'desc' }`; include `validFrom` in `SegmentFareRule`'s unique key (`schema.prisma:1113`) to allow dated versions. - **H-12 divergent free-child rules** (`fare-engine.service.ts:172` vs `bookings.service.ts:1690` vs `:1622`): extract ONE `computeFare()` used by quote, booking, and payment. - **H-13 promo silently dropped β†’ overcharge** (`review/page.tsx:587`, `search.service.ts:940-970`): book the breakdown's discounted `totalMinor`, not the client-summed undiscounted per-pax fares β€” or return discounted per-pax fares. Fold into the C-1 fix (server recomputes the authoritative total incl. promo). --- ## P2 β€” config validation & safety (MEDIUM) / UI (LOW) - **M-1 negative fares**: add `@Min(0)` to `baseFareMinor` (`schedules.dto.ts:85,95`) and `basePrice` (`seat-classes.dto.ts:29`). - **M-2 promo bounds/date**: `@Min(0) @Max(100)` on `percentOff`, `@IsDateString()` on `validUntil` (`promos.dto.ts`); add min-spend / usage-limit / max-cap columns. - **M-3 `PATCH /config` arbitrary**: replace the raw body with a whitelisted, typed DTO with per-key range checks (`system-config.controller.ts:34`). - **M-4 past-date / double-booked schedules** (`schedules.service.ts:105,124`): reject past `departureAt`; widen the overlap check to the train across all routes. - **M-5 deletes ignore references** (`stations`/`seat-classes`/`currencies`/`schedules` services): add referential guards before delete/disable; wrap the schedule cascade (`schedules.service.ts:438-485`) in a transaction. - **M-6 non-atomic booking write** (`bookings.service.ts:883-926`): wrap create + confirmSeats + tier increment in one `$transaction`. - **L-1 DJF shown with 2 decimals** (`portal/src/utils/format.ts:22`): format per `CHARGE_CURRENCY_DECIMALS` (DJF = 0). - **L-2 client-side fare math** (`portal/src/utils/fare-utils.ts`, `review/page.tsx`): render only server-computed amounts; never submit a client-derived total (ties to C-1). - **L-3 loyalty accrual currency** (`payments.service.ts:1062`): accrue from the actual charged amount/currency, not ETB minor. --- ## Suggested sequencing 1. **C-10, C-3, C-8, C-9** β€” quickest high-impact (a few lines each): unblock authenticated booking, stop free wallet credit, guard FX writes, enforce roles. 2. **C-1, C-2, C-4, C-5, C-6, C-7** β€” the money-integrity core (needs transactions + validation). 3. **H-2, H-3, H-4, H-7** β€” the FX/units foundation others compound on. 4. **H-1, H-5, H-8..H-12, M-1, M-2** β€” pricing correctness + validation. 5. **M-3..M-6, L-1..L-3** β€” config safety + UI consistency.