9.3 KiB
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:
// 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:
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:
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: enforcereq.userowns:passengerId(or is admin) before top-up.wallet.service.ts:50: only credit after a confirmedPaymentIntent(a top-up is a purchase).wallet.controller.ts:23-24: remove@IsPublic()fromGET /wallet/accounts.
C-4 — Payment amount never validated
intents.service.ts:541-548: onconfirmedAmountMinor !== intent.amountMinor, do NOT mark SUCCEEDED — set aAMOUNT_MISMATCHstate and alert. PopulateconfirmedAmountMinorin each webhook handler (e.g.waafi-webhook.service.ts:63).payments.service.ts:809finalizePaymentSuccess: assert the settled amount equalsbooking.totalMinorbefore 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:
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:
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:
// 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)onpercentOff,@Min(0)onamountOffMinor(promos.dto.ts:20,26). - H-2 missing FX → 1.0 (
currency.service.ts:142): remove the silentreturn 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 oneconvert(fromMinor, from, to): {minor|major}with one rounding + one fallback policy; give it a brandedMinor/Majorreturn type and audit everyamountMinorassignment across the payment boundary. - H-5
percentOff:0treated as FIXED (fare-engine.service.ts:185): usepromo.percentOff != null ? … : promo.amountOffMinor. - H-6
insuranceFeeMinordual meaning (fare-engine.service.ts:130,154,167): split intoinsuranceMultiplierBpsandinsuranceFeeMinor; 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 throughconvertMinorToChargeMajor. - H-10 future-dated FX applied now (
currency.service.ts:88,137): addeffectiveDate: { lte: new Date() }to the rate lookups. - H-11 non-deterministic fare resolution (
fare-engine.service.ts:84,298): addorderBy: { validFrom: 'desc' }; includevalidFrominSegmentFareRule's unique key (schema.prisma:1113) to allow dated versions. - H-12 divergent free-child rules (
fare-engine.service.ts:172vsbookings.service.ts:1690vs:1622): extract ONEcomputeFare()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 discountedtotalMinor, 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)tobaseFareMinor(schedules.dto.ts:85,95) andbasePrice(seat-classes.dto.ts:29). - M-2 promo bounds/date:
@Min(0) @Max(100)onpercentOff,@IsDateString()onvalidUntil(promos.dto.ts); add min-spend / usage-limit / max-cap columns. - M-3
PATCH /configarbitrary: 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 pastdepartureAt; widen the overlap check to the train across all routes. - M-5 deletes ignore references (
stations/seat-classes/currencies/schedulesservices): 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 perCHARGE_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
- 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.
- C-1, C-2, C-4, C-5, C-6, C-7 — the money-integrity core (needs transactions + validation).
- H-2, H-3, H-4, H-7 — the FX/units foundation others compound on.
- H-1, H-5, H-8..H-12, M-1, M-2 — pricing correctness + validation.
- M-3..M-6, L-1..L-3 — config safety + UI consistency.