Review a Payment Webhook Handler

1. Signature is not verified

Critical, line 11

Anyone who finds the URL can POST a fake payment.succeeded event and credit any account. Providers sign each webhook (for example Stripe's Stripe-Signature header) precisely so you can reject forged ones.

Fix: Verify the signature over the raw request body with the provider's library and webhook secret before parsing; reject with 400 if it fails. Also reject events whose timestamp is too old (replay protection).

2. Not idempotent: retries credit twice

Critical, line 13-19

Providers deliver at least once. A timeout or a 500 (for example, from the email call) makes the provider retry, and the same payment is credited again.

Fix: Record the event ID in a table with a unique constraint inside the same transaction as the credit, and return 200 immediately if it was already processed.

3. Lost update on the balance

Major, line 15-18

Read-modify-write in application code: two concurrent events for the same user both read the old balance and one credit is lost.

Fix: Update atomically in SQL (UPDATE users SET balance = balance + :amount WHERE id = :id) inside a transaction, or lock the row with SELECT ... FOR UPDATE.

4. Slow side effects inside the webhook

Major, line 19

Sending email synchronously makes the response slow and ties the credit to the email provider's uptime. If the email fails after the credit, the provider retries and the user is credited again.

Fix: Commit the credit, return 200 quickly, and send the receipt from a background job (outbox pattern) that can retry on its own.

5. Unvalidated payload and amount types

Minor, line 11, 14, 16

event["data"]["amount"] is trusted as-is: missing keys raise a 500 (which triggers retries), and a float amount brings rounding errors into money.

Fix: Parse the event into a typed model (Pydantic), keep money as integer minor units (cents), and ignore unknown event types explicitly.

Every webhook review should check three things: authenticity (signature), idempotency (retries), and speed (acknowledge fast, do slow work asynchronously). Candidates who mention at-least-once delivery without being prompted stand out.

from fastapi import APIRouter, HTTPException, Request

from app import payments
from app.db import db
from app.jobs import enqueue

router = APIRouter()


@router.post("/webhooks/payments")
async def payment_webhook(request: Request):
    raw = await request.body()
    try:
        event = payments.verify_and_parse(raw, request.headers.get("Payment-Signature"))
    except payments.InvalidSignature:
        raise HTTPException(400, "invalid signature")

    if event.type != "payment.succeeded":
        return {"ok": True}

    async with db.transaction():
        inserted = await db.execute(
            "INSERT INTO processed_events (event_id) VALUES (:id) ON CONFLICT DO NOTHING", {"id": event.id})
        if not inserted:
            return {"ok": True}  # already processed: a retry
        await db.execute("UPDATE users SET balance_cents = balance_cents + :amount WHERE id = :id",
                         {"amount": event.data.amount_cents, "id": event.data.user_id})
        await enqueue("send_receipt", user_id=event.data.user_id, amount_cents=event.data.amount_cents)

    return {"ok": True}