mirror of https://github.com/Tria-plc/edr-platform.git synced 2026-08-26 18:42:49 +00:00

Files

Muluhabt 7e6778d025 Adding all the tests and fixes to the passengers app

2026-07-21 15:26:29 +03:00

34 KiB

Raw Permalink Blame History

EDR Passenger Platform — Issues Report

Findings from the pricing/backoffice E2E bug-hunt. No product code was changed — this is a report. The harness that reproduces the ✅ findings lives in e2e/ + apps/edr-passenger-api/test/ (docs/e2e-test-matrix.md is the full test matrix; e2e/README.md explains how to run it).

Verification legend

✅ Verified by test — a passing e2e test reproduces the defect (test name references the ID).
🔎 Confirmed by code inspection — unambiguous from the source; not yet wrapped in a test (usually because it lives behind the IAM/RabbitMQ boot wall or needs the running web apps).
⚠️ Suspected — plausible from the source; needs runtime confirmation.

Severity: how much money / trust is at risk, and how easily.

Two structural facts frame everything:

There are two fare systems: fare-engine (live) and configurable-fare (fully built but never called by the live path — fare-engine.calculate never reads fare_configurations). All findings below concern the live fare-engine unless noted.
The domain seed (prisma/seed.ts) is entirely disabled (every step commented out).

CRITICAL — money can be created, stolen, or set by the client

C-1 ✅ Booking total is client-controlled (server fare computed, then discarded)

Where: bookings.service.ts:863-899 (one-way), :1065-1095 (round-trip), guest-booking.service.ts:206-245,494-540. Per-seat: :840 fareMinor = p.seatFareMinor ?? ….
Repro: POST /bookings with reviewedTotalMinor: 1 (or every passenger seatFareMinor: 0).
Expected: server recomputes the authoritative fare and rejects/overrides a mismatched client amount. Actual: the client value is stored as displayTotalMinor; a mismatch is only logger.warn-ed (:873-874), never rejected. A trip can be booked for 1 cent.
Status: ✅ verified — critical-repro.e2e-spec.ts (C-1): a one-way booking submitted with reviewedTotalMinor: 1 is stored with totalMinor === 1 while fareBreakdown.totalMinor is ≥ 30000. Matrix A1–A4.
Fix: recompute the fare server-side at booking creation and reject if the client-supplied total differs beyond a rounding epsilon; never persist a client amount as the charge basis.
Resolution (authenticated paths) ✅ — bookings.service.ts now guards both createOneWayBooking and createRoundTripBooking with assertTotalNotUnderAuthoritative(resolvedTotalMinor, fareCalculation.totalMinor): a booking whose ETB charge basis falls below the server-recomputed authoritative fare (net of promo/loyalty/free-child) by more than a 1% FX-rounding tolerance is rejected with BadRequestException and nothing is persisted. It's a floor (not equality) so legitimate berth surcharges — which only raise the total — still pass. Proven by e2e-ui/specs/portal/ua13-forged-total.spec.ts (now asserts a 4xx + no 1-minor booking; red before the guard, green after).
Resolution (guest paths) ✅ — guest-booking.service.ts now applies the identical assertTotalNotUnderAuthoritative floor guard to both the one-way and round-trip guest booking creation paths (authoritative ETB fare captured before the client-driven per-seat/reviewed branches overwrite the total). Proven by e2e-ui/specs/guest/ua14-forged-seat-fare.spec.ts (forged seatFareMinor=0 + reviewedTotalMinor=0 now rejected with a 4xx and no 0-minor booking persisted; red before the guard, green after). C-1 is now closed on all four booking-creation paths (authenticated one-way/round-trip + guest one-way/round-trip).

C-2 ✅ Loyalty redemption is unbounded and never deducted (free discount)

Where: bookings.service.ts:1705,1707 (and :1028,:1249,:1451); DTO bookings.dto.ts:155. loyaltyMinor = (loyaltyRedemptionPoints ?? 0) * 10 subtracted from the total.
Repro: POST /bookings with loyaltyRedemptionPoints: 999999 on an account with 0 points.
Expected: validate against the account's real balance, cap it, and DEBIT the points. Actual: no balance check, no ledger debit, no cap — the discount applies and the total can hit 0 (or negative). Points are only ever awarded (payments.service.ts:1062), never spent here.
Status: 🔎 (arithmetic path is explicit; the redemption-not-deducted contract is confirmed in the Tier-2 reference). Matrix A5/F6.
Fix: load LoyaltyAccount, reject if points > balance, clamp to a max, and write a LoyaltyLedgerEntry DEBIT inside the booking transaction.

C-3 ✅ Wallet top-up: no ownership check, no payment backing (free money)

Where: wallet.service.ts:50-56; controller wallet.controller.ts:34-39. Also GET /wallet/accounts is @IsPublic() (wallet.controller.ts:23-24) → leaks all balances.
Repro (verified): money-integrity.e2e-spec.ts → topUp(victimId, 1_000_000) credits the victim's wallet with a bare CREDIT ledger entry and no linked payment.
Expected: top-up requires the caller to own the wallet AND a settled payment. Actual: topUp(passengerId, amount) takes the id positionally, checks nothing, and credits unconditionally.
Fix: gate the controller on caller == passengerId (or admin), and only credit after a confirmed PaymentIntent; make GET /wallet/accounts non-public.

C-4 ✅ Payment amount is never validated against the booking

Where: passenger side payments.service.ts:809-848,910-939; payment side intents.service.ts:541-548 (mismatch only logger.error, intent still SUCCEEDED). Webhook handlers never set confirmedAmountMinor (e.g. waafi-webhook.service.ts:63-69).
Repro: ✅ verified — critical-repro.e2e-spec.ts (C-4): finalizePaymentSuccess on an intent with amountMinor: 1 sets a totalMinor: 30000 booking to CONFIRMED — no amount comparison.
Expected: reject/hold on amount mismatch. Actual: any provider "success" confirms the booking in full; short payments are undetectable. Matrix G1/G7.
Fix: compare provider-confirmed amount to the intent/booking total in applyProviderResult and finalizePaymentSuccess; do not confirm on mismatch.
Resolution (passenger side) ✅ — payments.service.ts handlePaymentEvent (the consumer of the payment service's mark-paid relay — the passenger-side settlement entry point) now compares the provider-settled event.amountMinor against the booking's display-currency total (displayTotalMinor, i.e. the amount the passenger was quoted) before materializing the intent or finalizing. A short payment (below the expected amount beyond a 1% rounding tolerance) is refused with { processed: false, reason: 'amount-mismatch' } and the booking is left unconfirmed — no ticket. Amount-only by design: the display↔charge-currency divergence for USD/DJF (UA-1b/2/3) is tracked separately, so the guard compares against displayTotalMinor to stay correct for both ETB and the currently-diverging currencies. Proven by e2e-ui/specs/portal/ua15-telebirr-shortpay.spec.ts (forged amountMinor:1 now leaves the booking unconfirmed; red before the guard, green after). The payment-side intents.service.ts mismatch (applyProviderResult) lives in edr-payment-api and is out of scope for the passenger-app fix.

C-5 🔎 A late webhook re-confirms an expired/cancelled booking

Where: payments.service.ts:809-848 (finalizePaymentSuccess never reads booking.status); expiry cron bookings.service.ts:2123-2128 (hardcoded 20 min).
Repro: let a PENDING_PAYMENT booking expire (seats released), then deliver the payment webhook.
Expected: reject payment for a cancelled/expired booking (and refund). Actual: the booking is re-set CONFIRMED and tickets are re-issued for already-released seats. Matrix G2.
Fix: in finalizePaymentSuccess, refuse to confirm unless status is PENDING_PAYMENT; route late successes to a refund/again-available flow.

C-6 ✅ Wallet debit has no row lock → concurrent double-spend

Where: payments.service.ts:461-484 — $transaction reads balance, checks, debits, with no SELECT … FOR UPDATE / pessimistic lock.
Repro: ✅ verified — critical-repro.e2e-spec.ts (C-6): two concurrent initiateWalletPayment on a wallet funded for one ticket both succeed (two DEBITs, two confirmations). The test forces the read-before-write interleaving with a barrier (only scheduling is controlled; the service logic runs unmodified) — the missing lock is what makes that interleaving lose money.
Expected: one succeeds, one fails; balance never over-drawn. Actual: both reads see the same balance, both pass the check → the wallet is double-spent. Matrix F4.
Fix: pessimistic lock the wallet row (or an atomic conditional UPDATE … WHERE balance >= x).

C-7 ✅ Refund is computed (80%) but never disbursed

Where: bookings.service.ts:2017-2027 — refundAmount = floor(total*0.8), writes BookingCancellation{ refundStatus:'PENDING' }; the only booking.cancelled listener is a notification (notifications.service.ts:750). No PaymentRefund, no wallet credit, no provider refund anywhere.
Repro (verified): money-integrity.e2e-spec.ts → cancel a CONFIRMED booking; refundAmount returned, refundStatus PENDING, zero PaymentRefund rows, wallet unchanged.
Fix: implement disbursement (wallet credit or provider refund) and move refundStatus through PROCESSING → COMPLETED; reconcile stuck PENDING rows.

C-8 ✅ Exchange-rate writes are missing the ADMIN check (any passenger can rewrite FX) — CORRECTED

⚠️ Corrected by live testing — the original claim (unauthenticated FX writes) was a false positive: @tria-plc/api-common's SharedAuthModule registers a global APP_GUARD = JwtGuard (shared-auth.module APP_GUARD), so anonymous requests get 401. The metadata-only J1 check saw no method-level guard and wrongly concluded "unauthenticated". The real defect is authorization, not authentication.
Where: fare-engine/currency.controller.ts:25 (PUT), :32 (PATCH) — authenticated but no @PassengerAdmin (only :42 DELETE has it).
Repro (verified live): e2e-ui/specs/propagation/pb-config-propagation.spec.ts (BC-11) — anon → 401, but a regular passenger token → 200 rewrites the live USD↔ETB rate.
Expected: FX writes are admin-only. Actual: any logged-in user (incl. a passenger) can rewrite USD↔ETB↔DJF rates, which every international fare multiplies by (fare-engine.service.ts:132,157,195). Needs a valid login (not anonymous), so HIGH, not CRITICAL — but a single passenger can still distort all international pricing. Same class as C-9.
Fix: add @PassengerAdmin() (or @PassengerStaff([currencies.manage])) to PUT/PATCH.
Resolution ✅ — fare-engine/currency.controller.ts now decorates both @Put() and @Patch(':id') with @PassengerAdmin() + @ApiBearerAuth('IAM-auth'), matching the existing @Delete handler. @PassengerAdmin() is the repo's established guard decorator (JwtGuard + PassengerPermissionGuard(admin)) — no new auth code, and no @edr/auth placeholder needed since the permission infra already exists and the seeded staff admin carries the permission. The sibling /currencies write surfaces (currency.controller.ts, currencies.controller.ts) were already guarded, so /fare-engine/exchange-rates was the sole gap. Proven by e2e-ui/specs/propagation/pb-config-propagation.spec.ts (BC-11): anon → 401, regular passenger PUT and PATCH → 403, staff admin → 200; red before the guard, green after.

C-9 ✅ `@Roles('ADMIN')` is dead everywhere (RolesGuard never wired) — verified live

Where: common/roles.guard.ts defines RolesGuard but it is never registered (no APP_GUARD, no @UseGuards(RolesGuard)). The global JwtGuard (SharedAuthModule) does authN but NOT authZ, so @Roles(...) is inert on: configurable-fare.controller.ts:20,111,187 (fare configs + feature toggle), segments/segment-fare.controller.ts:15 (/admin/segment-fares), system-config.controller.ts:23,32 (GET/PATCH /config).
Repro (verified live): a regular passenger token → PATCH /config (@Roles('ADMIN')) → HTTP 200 (wrote admin-only system config); POST /admin/fare-configurations → 400 (reached DTO validation, i.e. it passed the role guard). So any authenticated user bypasses the ADMIN gate.
Expected: these are admin-only. Actual: any authenticated IAM user (incl. a passenger) can CRUD fare configuration and system config. Matrix J2–J4.
Fix: register RolesGuard globally (or via @UseGuards) so @Roles is enforced, OR convert these to the working @PassengerAdmin()/@PassengerStaff() guards used elsewhere.

C-10 ✅ Authenticated `POST /bookings` is BROKEN (passengerId resolution regression)

Where: bookings.controller.ts:528-532 overrides passengerId with the JWT user id (req.user.id, the iamUserId — "never trust the request body", added in commit 25fdf88a). bookings.service.ts:773 resolves an iamUserId → Passenger ONLY when it is non-UUID. IAM user ids are UUIDs, and registration creates Passenger.id ≠ iamUserId (passenger-auth.service.ts:225 — only iamUserId is set; id auto-generates). So the resolver never fires and booking.create (bookings.service.ts:905) uses the iamUserId directly as passengerId.
Repro: ✅ verified two ways — (1) live browser: the full UI booking flow returns HTTP 400 P2003 on Booking_passengerId_fkey for a logged-in passenger whose Passenger.id ≠ iamUserId (the realistic case); (2) deterministic API test test/authed-booking-passengerid.e2e-spec.ts — create() with a UUID iamUserId fails the FK, while create() with the real Passenger.id succeeds (control). The UI suite only goes green because seed-ui.ts deliberately sets Passenger.id == iamUserId.
Expected: every IAM-authenticated passenger can book. Actual: every authenticated POST /bookings fails with a foreign-key error; only the guest path (/bookings/guest, which creates a fresh passenger) works. This is a regression — before 25fdf88a, the controller used the frontend-supplied passengerId (the real Passenger.id), which worked.
Fix: resolve the passenger by iamUserId unconditionally (passenger.findUnique({ where: { iamUserId } })) in the controller or service — drop the UUID-format gate at bookings.service.ts:773 — and pass the resolved Passenger.id to booking.create. (Keep the "don't trust the body" intent; just translate the identity correctly.)
⚠️ Confirm the deployment window: verify whether 25fdf88a is already in production. If so, authenticated bookings are down platform-wide; if it's only on dev, this is a pre-release blocker.

HIGH — pricing is wrong or exploitable

Where: fare-engine.service.ts:185-192 — total = subtotal - discount, no Math.max(0,…). DTO gaps: promos.dto.ts:20 (percentOff no @Max(100)), :26 (amountOffMinor unbounded).
Repro (verified): pricing-fare-engine.e2e-spec.ts → promo percentOff:150 and a fixed amountOffMinor > subtotal both yield a negative totalMinor.
Fix: clamp the total at 0; bound percentOff to [0,100] and amountOffMinor at the DTO.

H-2 ✅ Missing FX rate is silently substituted with 1.0

Where: currency.service.ts:142-147 (getExchangeRate returns 1.0 + a warn).
Repro (verified): pricing-fare-engine.e2e-spec.ts (C1) — deleting the USD→ETB rate collapses the fare ~100×; pricing-currency.e2e-spec.ts (C2b) — silent 1.0 vs getRateOrThrow throwing.
Fix: fail closed (reject the quote/booking) when a required rate is absent; never price at parity by default.
Resolution ✅ — currency.service.ts getExchangeRate no longer substitutes 1.0 on a missing rate; it logs and throws BadRequestException (No exchange rate configured for X->Y), matching getRateOrThrow. Fare pricing therefore fails closed: with the USD→ETB pair deleted the fare engine (fare-engine.service.ts:157) throws, so the search returns no priced class for the affected currency (the per-seat-class fare error is caught in search.service.ts:1041, so the trip is listed without a fare rather than 500ing), and an authoritative calculateFare on the booking path — which does not swallow the error — rejects the booking. No path prices at parity by default. Proven by e2e-ui/specs/propagation/pb-config-propagation.spec.ts (PB-10): with the rate present the USD search returns a priced fare; with it deleted the search returns an empty faresByClass instead of a ~100×-collapsed fare (red before the fix, green after). Note: getExchangeRate still resolves only the direct rate (no inverse/bridge) — unifying it with getRateOrThrow is the separate H-3 cleanup; failing closed here is strictly safer than the old silent 1.0.

H-3 ✅ Display path and charge path diverge on the same FX state (100×)

Where: getExchangeRate (:131, no inverse fallback) vs getRateOrThrow (:81, inverse + bridge). The fare/display uses the former; the charge uses the latter.
Repro (verified): pricing-currency.e2e-spec.ts (C2) — with only the inverse rate present, getExchangeRate(USD,ETB)=1.0 but getRateOrThrow(USD,ETB)=100 → displayed fare and charged amount differ 100×. Matrix C2/C5.
Fix: one shared conversion routine with one rounding rule and one fallback policy.
Resolution (booking-record coherence, UA-1b/UA-2/UA-3w) ✅ — the stored booking record no longer mislabels its amount. Every booking.create path (bookings.service.ts one-way/round-trip/ transit/round-trip-transit + the guest equivalents) now stores currency: Currency.ETB (the actual currency of totalMinor/the ETB charge basis) instead of the display currency. The passenger-facing amount stays in displayCurrency/displayTotalMinor (Birr for Ethiopian, DJF for Djiboutian, USD for Other), and every read endpoint already prefers those. The portal results/page.tsx on-select now carries the passenger-currency fare (displayAmountMinor) forward, aligning with the seats page's already-displayAmountMinor fare logic. Net effect (agreed model A): the passenger sees and is charged in their own currency; the internal charge basis stays ETB (the unit every downstream calc — wallet debit, loyalty, refund, gateway conversion — already assumes), now honestly labeled. Proven by e2e-ui/specs/portal/ua2-usd-booking.spec.ts and ua3-djf.spec.ts (UA-3w): currency is ETB while displayCurrency/displayTotalMinor carry USD/DJF — red before the fix (currency was USD/DJF), green after; UA-1 (ETB) unchanged. The deeper H-3 (unify getExchangeRate/getRateOrThrow) and H-4 (branded Minor/Major units) refactors remain open.

H-4 ✅ Conversion routines return different UNITS for the same money

Where: displayMinorToChargeMajor/convertMinorToChargeMajor return major units; convertEtbMinorToChargeMinor returns minor (currency.service.ts:27,61,35); payments.service.ts:250-281 writes the major result into a field named amountMinor.
Repro (verified): pricing-currency.e2e-spec.ts (C5) — same amount comes out 100× apart.
Fix: make the unit explicit in names/types (a Minor/Major branded type) and audit every amountMinor assignment across the payment boundary.

H-5 ✅ A `percentOff: 0` promo wrongly applies a fixed discount

Where: fare-engine.service.ts:185 — promo.percentOff ? percent : amountOffMinor; 0 is falsy.
Repro (verified): pricing-fare-engine.e2e-spec.ts (D4) — promo {percentOff:0, amountOffMinor:5000} deducts 5000 instead of 0.
Fix: test percentOff != null rather than truthiness.

H-6 🔎 `insuranceFeeMinor` means two different things in the same column

Where: used as a multiplier (/100) in the seat-class/route paths (fare-engine.service.ts:130,154) but as a flat fee in the segment/schedule paths (:167) and in the schema comment (schema.prisma:95).
Effect: the same stored value produces different fares depending on which fare source wins. Matrix B1.
Fix: split into two columns (insuranceMultiplier vs insuranceFeeMinor) or normalise usage.

H-7 🔎 Domestic ETB fares are multiplied by the USD→ETB rate

Where: seat-class/route formula base = round(distanceKm × rate/100 × insurance × usdToEtbRate) (fare-engine.service.ts:157-160). For a LOCAL (ETB) fare this multiplies by USD→ETB.
Effect: fares only look right when USD→ETB happens to equal the major→minor factor (≈100). Set a realistic rate (~132) and every domestic fare is ~30% off. Matrix B2. (The harness pins USD→ETB = 100 precisely because the formula depends on it — itself the smell.)
Fix: don't apply a USD→ETB conversion to a domestic ETB base fare; separate unit scaling from currency conversion.

H-8 🔎 Excess-baggage rate ignores the seat class ✅ (calc verified)

Where: excess-baggage.service.ts:53 — baggageAllowance.findFirst({ orderBy:{createdAt:'asc'}}) (oldest global row, no where).
Repro (verified): money-integrity.e2e-spec.ts (E1/E2) — with a LOCAL (rate 50) and an INTL (rate 200) allowance, the charge uses 50 regardless; fee = feePerKgMinor × excessWeightKg.
Fix: look up the allowance by the booking's seatClassId.

H-9 🔎 Baggage/supplementary charges skip currency conversion & DJF rounding

Where: excess-baggage.service.ts:166 and supplementary-charges.service.ts:132 pass amountMinor / 100 (major units) with the raw currency and no per-currency rounding to paymentClient.initiate.
Effect: wrong amount for DJF (0-decimal) and any non-ETB currency. Matrix E2/E-supp.
Fix: route these through the same convert*ChargeMajor rounding used for booking payments.

H-10 🔎 A future-dated FX rate is applied immediately ✅ (verified)

Where: currency.service.ts:88-99,137-140 — orderBy effectiveDate desc, no effectiveDate <= now filter.
Repro (verified): pricing-currency.e2e-spec.ts (C3) — a rate dated one year out is used now.
Fix: filter effectiveDate <= now() in rate lookups (matching how fare rules already filter).

H-11 🔎 Inconsistent / non-deterministic fare-rule resolution

Where: pickBestFareRule has no effective-date tiebreak (fare-engine.service.ts:298); a global (tripId=null) FareRule is matched then ignored (:139); segment/schedule lookups use findFirst with no orderBy (:84), and SegmentFareRule's unique key excludes validFrom (schema.prisma:1113) so fares can't be versioned by date. Matrix B4/B5.
Fix: add deterministic ordering (effective-date desc) and include validFrom in the segment uniqueness so dated versions are possible.

H-12 🔎 Divergent "free child" rules across quote / booking / package

Where: quote fare-engine.service.ts:172 uses min(child, adult); booking bookings.service.ts:1690 uses child-1; package :1622 uses min(child, adult); package RT child fare round(adult × 0.1) float (payments.service.ts:135,180, bookings.service.ts:34-40).
Effect: the price shown at quote can differ from what the booking charges for multi-adult / multi-child parties. Matrix B6/B7.
Fix: one shared fare function used by quote, booking, and payment.

Where: GET /search/fare-breakdown (search.service.ts:940-970) computes the discount into a SEPARATE discountMinor / discounted totalMinor, but returns per-passenger displayFareMinor undiscounted. The review page (portal/src/app/booking/review/page.tsx:587) reduces the per-passenger fares and sends their sum as reviewedTotalMinor — i.e. the undiscounted subtotal — ignoring discountMinor. Promo only enters via the ?promoCode= URL param (no UI input).
Repro: ✅ verified in-browser — e2e-ui/specs/portal/ua8-promo-drop.spec.ts: with a valid 10% promo, the breakdown shows discountMinor > 0 and totalMinor < subtotalMinor, yet the booking is stored at the full subtotalMinor.
Expected: the discounted total is booked and charged. Actual: the customer is charged full price despite a valid promo — a silent overcharge (and a broken promo feature). Matrix D / UA-8.
Fix: book the breakdown's discounted totalMinor (not the client-summed per-pax undiscounted fares); or return discounted per-pax fares. Best combined with C-1 (server recomputes the authoritative total, promo included, and rejects a client mismatch).
Resolution (authed one-way) ✅ — bookings.service.ts createOneWayBooking now applies the authoritative promo discount server-side. The portal still forwards promoCode in the booking body, so calculateFare already computes discountMinor — the total-resolution branches simply never subtracted it. When the total comes from a client-summed subtotal (per-seat sum or reviewedTotalMinor, both undiscounted), the code now subtracts fareCalculation.discountMinor (converted to display currency for the display total) so the stored/charged totalMinor = subtotal − discount. The engine-fallback branch already booked the discounted totalMinor, so it is excluded (via a usedClientSubtotal flag) to avoid double-subtracting; no-op when no promo applies (discountMinor === 0), so UA-11 (expired promo) and the non-promo specs are unaffected. This composes with the C-1 floor guard: after the discount is applied the resolved total equals the authoritative fare, so the guard passes. Proven by e2e-ui/specs/portal/ua8-promo-drop.spec.ts (booking now stored at subtotal − discount; red before the fix, green after). The round-trip and guest paths share the same latent frontend drop but have no UI spec yet — tracked for a follow-up; the guest service additionally still overrides its discounted total with reviewedTotalMinor (see the PROMO REALITY note in docs/ui-e2e-test-matrix.md).

MEDIUM — backoffice config accepts invalid data / unsafe deletes

M-1 ✅ Negative fares accepted (missing `@Min`)

Where: schedules.dto.ts:85,95 (CreateFareRuleDto/CreateSegmentFareRuleDto.baseFareMinor, @IsInt only); seat-classes.dto.ts:29 (basePrice). Sibling segments/segment-fare.dto.ts:24 does have @Min(0) — inconsistent.
Repro (verified): config-validation.e2e-spec.ts (H1/H2) — negative values pass validation; the guarded sibling rejects them.
Fix: add @Min(0) to every money DTO field.
Resolution (seat-class base price) ✅ — seat-classes.dto.ts CreateSeatClassDto.basePrice and insuranceFeeMinor now carry @Min(0); because UpdateSeatClassDto extends PartialType(...) the constraint applies to PATCH /seat-classes/:id too. A negative basePrice is rejected with 400 at the DTO layer (matching the backoffice form's min=0), so it never reaches the DB. Proven by e2e-ui/specs/propagation/pb-config-propagation.spec.ts (BC-7): basePrice:-500 → 400, a valid write still succeeds (red before the @Min, green after). The other money DTOs named above (schedules.dto.ts CreateFareRuleDto/CreateSegmentFareRuleDto.baseFareMinor) are not exercised by a UI spec and remain a follow-up for full M-1 closure.

Where: promos.dto.ts:20 (percentOff no @Max(100)/@Min(0)), :29 (validUntil @IsString, not @IsDateString).
Repro (verified): config-validation.e2e-spec.ts (H4/H5) — percentOff:200 and validUntil:"not-a-real-date" both pass.
Fix: @Min(0) @Max(100) on percentOff; @IsDateString() on validUntil; add min-spend / usage-limit / max-cap columns (all currently absent — schema.prisma:785).
Resolution (percentOff bounds) ✅ — promos.dto.ts CreatePromotionDto.percentOff now carries @Min(0) @Max(100) and amountOffMinor carries @Min(0), so POST /promos with percentOff:200 is rejected with 400 at the DTO layer while a valid ≤100% promo still saves. Proven by e2e-ui/specs/backoffice/config-validation.spec.ts (BC-8): percentOff:200 → 400, percentOff:50 → 201 (red before the bounds, green after). The validUntil @IsDateString tightening and the missing min-spend/usage-limit/max-cap columns remain a follow-up (not exercised by BC-8).

M-3 🔎 `PATCH /config` accepts arbitrary unvalidated key/values

Where: system-config.controller.ts:34 (no DTO) → system-config.service.ts:56-59 stores a raw Record<string,string>. Setting seat_hold_duration_minutes = -1 or "abc" is persisted. Matrix H7.
Fix: a whitelisted, typed DTO with per-key numeric/range validation.
Resolution ✅ — system-config.dto.ts adds UpdateSystemConfigDto, a whitelisted body listing every known config key, each @Type(() => Number) @IsInt() @Min(...) (seat-hold bounded 1..60, throttle limits/TTLs @Min(1), hour windows @Min(0)). The controller now accepts the DTO (so the global whitelisting ValidationPipe strips unknown keys and enforces the ranges) and persists the validated values back as strings. PATCH /config {seat_hold_duration_minutes:"-1"} (or "abc") is rejected with 400; a sane value still stores. Proven by e2e-ui/specs/backoffice/config-validation.spec.ts (BC-9): -1 → 400, 15 → stored (red before the DTO, green after).

M-4 🔎 Past-dated schedules accepted; train can be double-booked across routes

Where: schedules.service.ts:105 only checks arrivalAt > departureAt (no "future" check); :124-132 blocks only same-train+same-route+same-day, so the same train can run two routes at overlapping times. Matrix H3/H4.
Fix: reject past departureAt; widen the overlap check to the train across all routes.
Resolution (past departure) ✅ — schedules.service.ts createSchedule now rejects a departureAt in the past (dep.getTime() < Date.now() → 400) alongside the existing arrivalAt > departureAt check. Proven by e2e-ui/specs/backoffice/config-validation.spec.ts (BC-10): a 2020 departure → 400 while a future schedule still creates (red before the guard, green after). Scoped to creation (an admin may still need to edit metadata on an already-departed schedule via updateSchedule). The cross-route train double-booking overlap widening remains a follow-up (not exercised by BC-10).

M-5 🔎 Deletes ignore referencing bookings; one cascade is non-transactional

Where: station delete ignores bookings (stations.service.ts:110-137); seat-class delete ignores bookings/bookingSeat (seat-classes.service.ts:53-81); currencies.deleteCurrency wipes all rate rows for a pair with no dependency check (currencies.service.ts:119-134) → future fares for that pair fall to the 1.0 fallback (H-2); schedule cascade delete is a deep multi-step delete with no transaction (schedules.service.ts:438-485) → partial-delete on failure. Matrix I3–I6.
Fix: referential guards before delete/disable; wrap the schedule cascade in a transaction.

M-6 🔎 Not atomic: booking create + seat confirm + tier increment

Where: bookings.service.ts:883-926 — separate awaits, no wrapping transaction; seat-conflict check-then-write race in tickets.service.ts:357-372. Matrix G8.
Fix: wrap the create/confirm/increment in a single transaction.

LOW / UI

L-1 🔎 Portal shows DJF with 2 decimals but charges whole francs

Where: portal/src/utils/format.ts:22-28 (Intl.NumberFormat('en-US', … minimumFractionDigits:2) for every currency) vs charge rounding currency.service.ts:9-13 (DJF = 0 decimals). Matrix C6/K3.
Status: needs the Playwright/UI suite (not yet run — see below).
Fix: format per CHARGE_CURRENCY_DECIMALS.

L-2 🔎 Portal reimplements fare math client-side (can diverge from the engine)

Where: portal/src/utils/fare-utils.ts:50,67,93; portal/src/app/booking/review/page.tsx:160, 180-181,478-480 computes the displayed total / reviewedTotalMinor. Matrix K1/K2 + ties to C-1.
Fix: display only server-computed amounts; never submit a client-derived total.

L-3 🔎 Loyalty points accrued on ETB minor regardless of charge currency

Where: payments.service.ts:1062,1067 — floor(amountMinor/100) on booking.totalMinor (always ETB minor). Matrix F5.
Fix: accrue from the actual charged amount/currency.

Not yet covered (honest gaps)

Suite K (browser / Playwright) — L-1 and L-2 (UI price rendering & client-side fare math) are confirmed by source reading but not yet reproduced in a browser. Running them needs the portal
- backoffice Next.js apps up with a seeded search result. Scaffolding is the remaining step of the "light Playwright" scope.
C-1, C-4, C-6 are now reproduced (critical-repro.e2e-spec.ts). C-5 (late-webhook resurrection) remains inspection-only — reproducing it end-to-end needs a booted payment-api + webhook POSTs; the passenger-side gap (finalizePaymentSuccess ignores booking.status) is directly readable.
configurable-fare module bugs (no rounding, discounts: TODO, no currency, no date/overlap enforcement) are real but the module is dormant; only relevant if you plan to switch to it.

Suggested priority order to fix

C-1, C-2, C-3, C-8, C-9 — anyone can set prices / mint wallet balance / rewrite FX / reach admin config. These are actively exploitable.
C-4, C-5, C-6, C-7 — payment/refund integrity (short-pay confirms, late-webhook resurrection, wallet race, refunds never paid).
H-2, H-3, H-4, H-7 — the FX/units foundation; several other bugs compound on top of it.
H-1, H-5, H-8..H-12, M-1, M-2 — pricing correctness + validation gaps.
M-3..M-6, L-1..L-3 — config safety and UI consistency.

34 KiB Raw Permalink Blame History Unescape Escape