Skip to content

confirmDelivery lets a carrier self-confirm their own delivery, enabling fabricated DELIVERED status and rating manipulation #1

Description

@boluwacodes

What / where

ShipmentsService.confirmDelivery in src/shipments/shipments.service.ts (lines 125-133) authorizes the caller with getOwned(shipmentId, userId), which only checks that the caller is either the sender or the carrier of the shipment:

async confirmDelivery(shipmentId: string, userId: string) {
  const shipment = await this.getOwned(shipmentId, userId);
  if (shipment.status !== ShipmentStatus.IN_TRANSIT) {
    throw new BadRequestException('This shipment is not in transit');
  }
  return this.prisma.shipment.update({
    where: { id: shipmentId },
    data: { status: ShipmentStatus.DELIVERED, releasedAmount: shipment.totalAmount },
  });
}

Unlike confirmPickup (lines 106-121), which explicitly re-checks shipment.carrierId !== carrierId and throws ForbiddenException if the caller isn't the assigned carrier, confirmDelivery has no such restriction — it accepts a call from the carrier just as happily as from the sender.

This is also inconsistent with the on-chain contract: StellarService.confirmDeliveryArgs (src/stellar/stellar.service.ts, line ~145) takes a receiver address as the authorizing party, not the carrier, implying delivery confirmation is meant to come from someone other than the carrier who is being paid.

Why it's a problem

A carrier can call the confirmDelivery GraphQL mutation on their own shipment (mutation { confirmDelivery(shipmentId: "shp_1") { status releasedAmount } }) and unilaterally mark it DELIVERED, setting releasedAmount = totalAmount in the off-chain mirror — without the sender/receiver ever confirming receipt.

This off-chain DELIVERED status then unlocks ReviewsService.create (src/reviews/reviews.service.ts), which only gates on shipment.status === 'DELIVERED'. A carrier could therefore self-confirm a delivery that never actually happened and immediately leave/receive a review, inflating their own CarrierProfile.averageRating and completedDeliveries (used to attract future business) with zero validation that the delivery occurred.

src/shipments/shipments.service.spec.ts's confirmDelivery test only exercises the sender confirming delivery — there is no test (and no guard) preventing the carrier from doing the same, so this gap is currently invisible to the test suite.

Suggested fix

Restrict confirmDelivery the same way confirmPickup restricts pickup — e.g. require the caller to be the sender (mirroring "sender confirms receipt on behalf of the named receiver") and throw ForbiddenException if userId !== shipment.senderId, with a regression test added alongside the existing confirmPickup "rejects a carrier who is not assigned" test.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions