Review: Order Paid Webhook Written by an AI Assistant

Published 10 Nov, 2026

Your team asked an AI coding assistant to mark orders as paid when the payment provider sends a webhook. It opened this pull request. You are the reviewer, and nothing merges without your approval.

The assistant's description:

Adds POST /webhooks/payments. Every event is verified with an HMAC-SHA256 signature before anything is trusted. On charge.succeeded the order is marked as paid and the customer gets a receipt email.

Where it runs: a public Flask app. The provider signs the raw request body it sends (compact JSON) and puts the hex digest in X-Signature. It may deliver the same event more than once, and it retries any delivery that does not get a 2xx response, for up to three days. Each event has a unique id. The secret comes from an environment variable; staging and preview environments run the same code, and not all of them set it.

Your task

Flag every problem you would block the merge for. Click a line number to comment, and Shift-click to cover a range.
For each one, choose the issue type and severity, and explain what fails and how to fix it.
You are scored on the problems you catch, the right issue type and a clear explanation. Missing a critical problem fails the round.

Aim for 10 minutes, as in a real review round.

What happens when PAYMENTS_WEBHOOK_SECRET is not set? Who else can compute that HMAC?

The provider signed the exact bytes it sent. Are those the bytes being checked?

How is the signature compared, and what can an attacker learn from how long the comparison takes?

The provider delivers the same charge.succeeded event twice. What does the customer receive?

What happens when reference does not match any order?

A strong reviewer blocks this change because the signature check fails open: without the secret, anyone can forge a "paid" event, and with it, genuine events from the provider are rejected.

Empty secret by default (line 13, critical). If the variable is missing, the key is "", and anyone can compute a valid HMAC with an empty key and mark any order as paid without paying. Fail at start-up when the secret is missing: os.environ["PAYMENTS_WEBHOOK_SECRET"].
Signature checked against re-serialised JSON (line 26, major). The provider signed its raw body, but json.dumps adds spaces after : and , and may escape characters differently, so genuine events fail with 401 and orders are never marked paid. Verify request.get_data() before parsing it.
Comparison is not constant-time (line 19, minor). == stops at the first wrong character, which leaks how much of a guessed signature is right. Use hmac.compare_digest(expected, signature).
Duplicate deliveries are processed again (lines 30–36, major). The provider can send the same event more than once, so the customer gets two receipts, and a late duplicate can set a refunded order back to paid. Record each event id and ignore ones already handled, and skip orders that are already paid.
Unknown order crashes the handler (lines 32–33, minor). If no order matches, order is None and the handler raises, returning 500. The provider then retries for three days. Log it and return 2xx, or return 404 deliberately.

import hashlib
import hmac
import json
import os

from flask import Blueprint, abort, request

from .mail import send_receipt
from .models import Order, db

bp = Blueprint("webhooks", __name__)

WEBHOOK_SECRET = os.environ.get("PAYMENTS_WEBHOOK_SECRET", "")

def verify_signature(body, signature):
    """The provider signs each event with HMAC-SHA256 of the JSON body."""
    expected = hmac.new(WEBHOOK_SECRET.encode(), body, hashlib.sha256).hexdigest()
    return expected == signature

@bp.post("/webhooks/payments")
def payments_webhook():
    event = request.get_json()
    signature = request.headers.get("X-Signature", "")
    body = json.dumps(event).encode()
    if not verify_signature(body, signature):
        abort(401)

if event["type"] == "charge.succeeded":
        charge = event["data"]
        order = db.session.get(Order, charge["reference"])
        order.status = "paid"
        order.paid_amount_pence = charge["amount"]
        db.session.commit()
        send_receipt(order)

return "", 204

{"issues":[
{"id": "secret", "file": "webhooks.py", "lines": [13, 13], "category": "security", "severity": "critical", "title": "Missing secret falls back to an empty key", "explanation": "With no environment variable the HMAC key is an empty string, so anyone can sign a forged charge.succeeded event and get orders marked paid for free.", "keywords": ["empty", "default", "fail open", "missing", "forge", "environ[", "required", "start-up", "startup"], "accept": ["design"], "fix": "Require the secret and fail at start-up if it is missing, e.g. os.environ[\"PAYMENTS_WEBHOOK_SECRET\"]."},
{"id": "rawbody", "file": "webhooks.py", "lines": [26, 26], "category": "bug", "severity": "major", "title": "Signature verified over re-serialised JSON", "explanation": "The provider signed its raw compact body, but json.dumps produces different bytes (spaces, escaping), so genuine events are rejected.", "keywords": ["raw", "get_data", "bytes", "serialis", "serializ", "json.dumps", "whitespace", "spaces"], "accept": ["security"], "fix": "Verify the HMAC over request.get_data() before parsing the JSON."},
{"id": "timing", "file": "webhooks.py", "lines": [19, 19], "category": "security", "severity": "minor", "title": "Signature compared with ==", "explanation": "String equality stops at the first difference, leaking timing information about the expected signature.", "keywords": ["compare_digest", "timing", "constant", "side channel", "side-channel"], "fix": "Use hmac.compare_digest(expected, signature)."},
{"id": "dupes", "file": "webhooks.py", "lines": [30, 36], "category": "bug", "severity": "major", "title": "Duplicate events are processed again", "explanation": "The provider can deliver an event more than once; each delivery sends another receipt, and a late duplicate can mark a refunded order as paid again.", "keywords": ["duplicate", "idempoten", "event id", "twice", "dedup", "already paid", "replay", "at-least-once"], "accept": ["design"], "fix": "Store processed event ids and skip repeats; do nothing if the order is already paid."},
{"id": "missing", "file": "webhooks.py", "lines": [32, 33], "category": "edge", "severity": "minor", "title": "Unknown order reference raises", "explanation": "db.session.get returns None for an unknown reference, so the handler raises and returns 500, and the provider retries for three days.", "keywords": ["none", "not found", "missing", "500", "attributeerror", "retry", "unknown"], "accept": ["bug"], "fix": "Check for None, log it and return a 2xx (or a deliberate 404)."}
]}

Topics #002 AI Code Review Easy Python Security Webhooks