Files
Crussell/backend/handlers/payments/adversarial_attack_test.go
T
popertots 5e3dc9b428 fix: comprehensive payment system hardening (4 review passes)
CRITICAL fixes:
- C1: JWT exp claim now validated via jwtauth.VerifyToken (was Decode)
- C2: OverrideAmount validated post-substitution (prevents negative money minting)
- C3: Terminal gift-card payments store gift_card_id; refund credits user balance
- C4: Refund dedup returns stored amount, not req.Amount (prevents admin mislead)
- C5: Booking recheck uses FOR UPDATE (prevents TOCTOU with cancellation)
- C6: processChargeGroup idempotency key stable (charge-only, prevents double-refund)

MAJOR fixes:
- M2: Gift-card refund UPDATE checks RowsAffected; 0 rows -> failed
- M3: ProcessCancellationRefund returns commit error (was swallowed)
- M5: Dispute webhook handling (created + state.updated + disputes table)

MEDIUM fixes:
- ME1: CORS restricted to FRONTEND_ORIGIN env var (was reflect-any)
- ME2: anonymize_user() scrubs users.notes, bookings.notes, name_history, refresh_tokens
- ME3: Webhook handlers now mutate state (payment.updated, refund.updated)

Frontend fixes:
- Same-key retry on 503 (ambiguous failure) wired to all 8 payment flows
- CHARGE_AND_STORE intent for save-card flows (SCA compliance)
- Nonce staleness check verified across all flows

Additional fixes from adversarial re-review:
- F1: Till-sale completed dedup echoes stored amount (C4-class)
- F2: Cash/giftcard terminal path uses FOR UPDATE (C5-class)
- F3: Square-success UPDATE checks RowsAffected (till sales)
- F4: Dispute reason truncated to 192 chars (prevents INSERT failure)
- F5: Booking-user lookup failure marks refund failed (prevents silent money loss)
- F6: Saved-card/tip rechecks wrapped in transaction (C5 residual)

Tests:
- 15 adversarial attack tests (negative override, zero override, terminal gift card,
  refund dedup, TOCTOU, deleted gift card, advisory lock, overcharge, zero/negative/huge
  amount, raw PAN, missing auth, gift card balance, concurrent refunds)
- 14 webhook state tests (dispute created/state, payment/refund updated)
- 3 CORS tests, 3 GDPR tests, 1 HTTP timeout test
- Full suite passes with -race (25 packages, 0 failures)

25 files changed, +1532/-275 lines
2026-08-22 00:34:49 +01:00

640 lines
28 KiB
Go

//go:build test && dev
package payments
import (
"context"
"encoding/json"
"log/slog"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"
"crussell/db"
"crussell/internal/square"
"crussell/testutils"
"crussell/testutils/fixtures"
"crussell/testutils/jwt"
"github.com/stretchr/testify/require"
)
// =============================================================================
// ADVERSARIAL QA ATTACK TESTS — break-the-app scenarios
// =============================================================================
// TestAttack_NegativeOverrideAmount_MintsGiftCardBalance attempts to mint
// money: a terminal giftcard payment with a NEGATIVE override_amount. The
// handler validates only req.Amount, then replaces amount with the override —
// a negative override on the giftcard path flips the balance deduction into a
// credit (balance - (-50) = balance + 50).
func TestAttack_NegativeOverrideAmount_MintsGiftCardBalance(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataAtTime(t, ctx, tx, time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC))
// Give the booking user a gift-card account balance.
_, err := tx.Exec(ctx, "INSERT INTO user_giftcard_balances (user_id, balance) VALUES ($1, 100.00)", userID)
require.NoError(t, err)
adminID, err := fixtures.CreateTestAdminUser(tx)
require.NoError(t, err)
adminToken := jwt.GenerateTestToken(adminID, "admin")
// amount=5000 (£50) passes ValidateAmount; override_amount=-5000 is the attack.
neg := int64(-5000)
pm := "giftcard"
req := CreateTerminalPaymentRequest{
Amount: 5000,
PaymentType: "full",
OverrideAmount: &neg,
PaymentMethod: &pm,
}
w := makePaymentRequest(CreateTerminalPayment, "POST", "/api/admin/bookings/"+bookingID+"/payment", req, adminToken, ctx)
var balance float64
err = tx.QueryRow(ctx, "SELECT balance FROM user_giftcard_balances WHERE user_id = $1", userID).Scan(&balance)
require.NoError(t, err)
// INVARIANT: a payment request must never increase a gift-card balance.
// Expected: 400 reject (invalid override) and balance unchanged at 100.00.
// Actual (bug): 200 COMPLETED, balance mints to 150.00.
if w.Code != http.StatusBadRequest && w.Code != http.StatusOK {
t.Fatalf("unexpected status %d: %s", w.Code, w.Body.String())
}
if balance != 100.00 {
t.Errorf("BUG: negative override_amount changed balance to %.2f (was 100.00) — money minted/lost", balance)
}
var payCount int
_ = tx.QueryRow(ctx, "SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND amount < 0", bookingID).Scan(&payCount)
if payCount > 0 {
t.Errorf("BUG: %d negative-amount payment record(s) created", payCount)
}
}
// TestAttack_ZeroOverrideAmount_CreatesFreePayment attempts a £0 gift-card
// payment via override_amount=0. The giftcard balance is untouched but a
// completed £0 payment is recorded.
func TestAttack_ZeroOverrideAmount_CreatesFreePayment(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataAtTime(t, ctx, tx, time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC))
_, err := tx.Exec(ctx, "INSERT INTO user_giftcard_balances (user_id, balance) VALUES ($1, 100.00)", userID)
require.NoError(t, err)
adminID, err := fixtures.CreateTestAdminUser(tx)
require.NoError(t, err)
adminToken := jwt.GenerateTestToken(adminID, "admin")
zero := int64(0)
pm := "giftcard"
req := CreateTerminalPaymentRequest{
Amount: 5000,
PaymentType: "full",
OverrideAmount: &zero,
PaymentMethod: &pm,
}
w := makePaymentRequest(CreateTerminalPayment, "POST", "/api/admin/bookings/"+bookingID+"/payment", req, adminToken, ctx)
_ = w
var zeroPayCount int
_ = tx.QueryRow(ctx, "SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND amount = 0", bookingID).Scan(&zeroPayCount)
if zeroPayCount > 0 {
t.Errorf("BUG: %d zero-amount completed payment record(s) created", zeroPayCount)
}
}
// TestAttack_TerminalGiftCard_CancellationRefund_LosesBalance: a user pays a
// booking from their gift-card ACCOUNT balance via the terminal. The terminal
// giftcard branch never stores gift_card_id on the payment row, so the
// cancellation refund loop cannot credit the balance back ("cannot refund to
// card. Skipping") yet still inserts a COMPLETED refund row — money is lost
// while the books claim it was refunded.
func TestAttack_TerminalGiftCard_CancellationRefund_LosesBalance(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataAtTime(t, ctx, tx, time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC))
_, err := tx.Exec(ctx, "INSERT INTO user_giftcard_balances (user_id, balance) VALUES ($1, 100.00)", userID)
require.NoError(t, err)
adminID, err := fixtures.CreateTestAdminUser(tx)
require.NoError(t, err)
adminToken := jwt.GenerateTestToken(adminID, "admin")
// Pay £30 from the gift-card account balance via terminal.
pm := "giftcard"
req := CreateTerminalPaymentRequest{Amount: 3000, PaymentType: "full", PaymentMethod: &pm}
w := makePaymentRequest(CreateTerminalPayment, "POST", "/api/admin/bookings/"+bookingID+"/payment", req, adminToken, ctx)
require.Equal(t, http.StatusOK, w.Code, "terminal giftcard payment failed: %s", w.Body.String())
var balanceAfterPay float64
require.NoError(t, tx.QueryRow(ctx, "SELECT balance FROM user_giftcard_balances WHERE user_id = $1", userID).Scan(&balanceAfterPay))
require.Equal(t, 70.00, balanceAfterPay)
// Cancel the booking >72h before start → full £30 refund.
innerTx := db.TxFromContext(ctx)
require.NotNil(t, innerTx)
now := time.Date(2099, 12, 28, 8, 0, 0, 0, time.UTC)
start := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)
result, err := ProcessCancellationRefundTx(ctx, innerTx, bookingID, 50, 30, start, now, "client_cancelled", &userID, false)
require.NoError(t, err)
require.NotNil(t, result)
if result.RefundableAmount != 30 {
t.Fatalf("expected refundable 30, got %.2f", result.RefundableAmount)
}
var balanceAfterCancel float64
require.NoError(t, tx.QueryRow(ctx, "SELECT balance FROM user_giftcard_balances WHERE user_id = $1", userID).Scan(&balanceAfterCancel))
// INVARIANT: a completed £30 refund must put the £30 back on the account.
// Expected: 100.00. Actual (bug): 70.00 — refund row 'completed' but no money returned.
if balanceAfterCancel != 100.00 {
t.Errorf("BUG: after full refund, balance is %.2f (was 70.00 post-payment) — refund recorded but money not credited", balanceAfterCancel)
}
// The refund row must be pending (Square) or completed-with-money-moved.
var refundStatus string
var refundAmount float64
if err := tx.QueryRow(ctx, "SELECT status, amount FROM refunds WHERE booking_id = $1 ORDER BY created_at DESC LIMIT 1", bookingID).Scan(&refundStatus, &refundAmount); err != nil {
t.Fatalf("no refund row recorded: %v", err)
}
if refundStatus != "completed" || refundAmount != 30 {
t.Errorf("BUG: refund row is status=%s amount=%.2f — expected completed £30 (it is completed but balance was NOT credited back)", refundStatus, refundAmount)
}
}
// TestAttack_Refund_SameKeyDifferentAmount_ReportsWrongAmount: the client
// reuses an idempotency key for a refund with a DIFFERENT amount. The dedup
// returns the existing completed refund but echoes the NEW requested amount in
// the response — the admin believes the new amount was refunded.
func TestAttack_Refund_SameKeyDifferentAmount_ReportsWrongAmount(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
_, bookingID, _ := setupTestData(t, ctx, tx)
adminID, err := fixtures.CreateTestAdminUser(tx)
require.NoError(t, err)
adminToken := jwt.GenerateTestToken(adminID, "admin")
paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 50, "in_person_card", "full", "completed")
require.NoError(t, err)
_, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_attack_samekey' WHERE id = $1", paymentID)
require.NoError(t, err)
handler := RefundPayment
key := "attack-same-key-diff-amount"
w1 := makePaymentRequest(handler, "POST", "/api/admin/payments/"+paymentID+"/refund", RefundRequest{Amount: 2000, Reason: "first", IdempotencyKey: key}, adminToken, ctx)
require.Equal(t, http.StatusOK, w1.Code, "first refund failed: %s", w1.Body.String())
// Same key, DIFFERENT amount (£30).
w2 := makePaymentRequest(handler, "POST", "/api/admin/payments/"+paymentID+"/refund", RefundRequest{Amount: 3000, Reason: "second", IdempotencyKey: key}, adminToken, ctx)
require.Equal(t, http.StatusOK, w2.Code, "second refund failed: %s", w2.Body.String())
var resp RefundResponse
require.NoError(t, json.Unmarshal(w2.Body.Bytes(), &resp))
// The stored refund row for this key must hold the actually-refunded amount.
var storedAmount float64
require.NoError(t, tx.QueryRow(ctx, "SELECT amount FROM refunds WHERE idempotency_key IS NOT NULL AND payment_id = $1 ORDER BY created_at DESC LIMIT 1", paymentID).Scan(&storedAmount))
// INVARIANT: the response must report the amount actually refunded (£20),
// never the newly-requested £30 for a dedup'd key.
if resp.Amount != int64(storedAmount*100) {
t.Errorf("BUG: same-key different-amount retry reports refunded %d pence, but stored refund is %.2f — admin misled", resp.Amount, storedAmount)
}
if resp.Status != "completed" {
t.Errorf("expected completed, got %s", resp.Status)
}
}
// TestAttack_AdvisoryLockTimeout_Returns409 holds the booking payment advisory
// lock for longer than the ~3s bound and verifies the handler returns 409
// (not a hang, not a double charge).
func TestAttack_AdvisoryLockTimeout_Returns409(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
token := jwt.GenerateUserToken(userID)
// Hold the session advisory lock on a dedicated pool connection.
holder, err := db.Conn.Acquire(context.Background())
require.NoError(t, err)
defer holder.Release()
_, err = holder.Exec(context.Background(), "SELECT pg_advisory_lock(hashtext($1))", "crussell:payment:"+bookingID)
require.NoError(t, err)
defer func() {
_, _ = holder.Exec(context.Background(), "SELECT pg_advisory_unlock(hashtext($1))", "crussell:payment:"+bookingID)
}()
cardToken := "cnon:attack-lock-card"
req := CreateBookingPaymentRequest{
Amount: 5000,
PaymentType: "full",
NewCardToken: &cardToken,
IdempotencyKey: "attack-lock-timeout-key",
}
start := time.Now()
w := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", req, token, ctx)
elapsed := time.Since(start)
// Expected: 409 Conflict after ~3s (bounded try-lock), no charge.
if w.Code != http.StatusConflict {
t.Errorf("BUG: expected 409 when advisory lock is held, got %d: %s", w.Code, w.Body.String())
}
if elapsed > 15*time.Second {
t.Errorf("BUG: handler hung for %v waiting on the lock", elapsed)
}
// No payment record and no Square charge.
var payCount int
_ = tx.QueryRow(ctx, "SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND idempotency_key = 'attack-lock-timeout-key'", bookingID).Scan(&payCount)
if payCount != 0 {
t.Errorf("BUG: %d payment record(s) created while lock held", payCount)
}
}
// TestAttack_RefundOvercharge verifies the over-refund guard: refunding more
// than the payment amount must be rejected.
func TestAttack_RefundOvercharge(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
_, bookingID, _ := setupTestData(t, ctx, tx)
adminID, err := fixtures.CreateTestAdminUser(tx)
require.NoError(t, err)
adminToken := jwt.GenerateTestToken(adminID, "admin")
paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 30, "in_person_card", "full", "completed")
require.NoError(t, err)
_, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_attack_overcharge' WHERE id = $1", paymentID)
require.NoError(t, err)
w := makePaymentRequest(RefundPayment, "POST", "/api/admin/payments/"+paymentID+"/refund", RefundRequest{Amount: 5000, Reason: "overcharge"}, adminToken, ctx)
if w.Code != http.StatusBadRequest {
t.Errorf("BUG: over-refund expected 400, got %d: %s", w.Code, w.Body.String())
}
var refundCount int
_ = tx.QueryRow(ctx, "SELECT COUNT(*) FROM refunds WHERE payment_id = $1", paymentID).Scan(&refundCount)
if refundCount != 0 {
t.Errorf("BUG: over-refund created %d refund row(s)", refundCount)
}
}
// TestAttack_ZeroAmount_Rejected: £0 booking payment must be rejected.
func TestAttack_ZeroAmount_Rejected(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
token := jwt.GenerateUserToken(userID)
cardToken := "cnon:attack-zero-card"
w := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment",
CreateBookingPaymentRequest{Amount: 0, PaymentType: "full", NewCardToken: &cardToken, IdempotencyKey: "attack-zero"}, token, ctx)
if w.Code != http.StatusBadRequest {
t.Errorf("BUG: zero amount expected 400, got %d: %s", w.Code, w.Body.String())
}
}
// TestAttack_NegativeAmount_Rejected: negative booking payment must be rejected.
func TestAttack_NegativeAmount_Rejected(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
token := jwt.GenerateUserToken(userID)
cardToken := "cnon:attack-negative-card"
w := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment",
CreateBookingPaymentRequest{Amount: -1000, PaymentType: "full", NewCardToken: &cardToken, IdempotencyKey: "attack-negative"}, token, ctx)
if w.Code != http.StatusBadRequest {
t.Errorf("BUG: negative amount expected 400, got %d: %s", w.Code, w.Body.String())
}
}
// TestAttack_HugeAmount_Rejected: a £1,000,000+ booking payment must be rejected.
func TestAttack_HugeAmount_Rejected(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
token := jwt.GenerateUserToken(userID)
cardToken := "cnon:attack-huge-card"
w := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment",
CreateBookingPaymentRequest{Amount: 100_000_000, PaymentType: "full", NewCardToken: &cardToken, IdempotencyKey: "attack-huge"}, token, ctx)
if w.Code != http.StatusBadRequest {
t.Errorf("BUG: huge amount expected 400, got %d: %s", w.Code, w.Body.String())
}
}
// TestAttack_RawPAN_NoCharge: a raw PAN as new_card_token must not produce a
// completed payment (PCI-DSS: only cnon:/ccof: tokens are accepted). The mock
// mirrors production and rejects non-token source IDs.
func TestAttack_RawPAN_NoCharge(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
token := jwt.GenerateUserToken(userID)
pan := "4111111111111111"
w := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment",
CreateBookingPaymentRequest{Amount: 5000, PaymentType: "full", NewCardToken: &pan, IdempotencyKey: "attack-rawpan"}, token, ctx)
// The request must not be treated as success.
if w.Code == http.StatusOK {
t.Errorf("BUG: raw PAN accepted as a charge: %s", w.Body.String())
}
var completed int
_ = tx.QueryRow(ctx, "SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND status = 'completed' AND idempotency_key = 'attack-rawpan'", bookingID).Scan(&completed)
if completed != 0 {
t.Errorf("BUG: raw PAN produced a completed payment")
}
}
// TestAttack_MissingAuth_Rejected: payment route without a user context must be rejected.
func TestAttack_MissingAuth_Rejected(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
_, bookingID, _ := setupTestDataPast(t, ctx, tx)
cardToken := "cnon:attack-noauth-card"
w := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment",
CreateBookingPaymentRequest{Amount: 5000, PaymentType: "full", NewCardToken: &cardToken, IdempotencyKey: "attack-noauth"}, "", ctx)
if w.Code == http.StatusOK {
t.Errorf("BUG: unauthenticated payment accepted: %s", w.Body.String())
}
}
// TestAttack_GiftCardSpendBeyondBalance: spending more than the remaining gift
// card balance must be rejected — a negative balance must never occur.
func TestAttack_GiftCardSpendBeyondBalance(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataAtTime(t, ctx, tx, time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC))
_, err := tx.Exec(ctx, "INSERT INTO user_giftcard_balances (user_id, balance) VALUES ($1, 10.00)", userID)
require.NoError(t, err)
adminID, err := fixtures.CreateTestAdminUser(tx)
require.NoError(t, err)
adminToken := jwt.GenerateTestToken(adminID, "admin")
pm := "giftcard"
req := CreateTerminalPaymentRequest{Amount: 5000, PaymentType: "full", PaymentMethod: &pm} // £50 > £10 balance
w := makePaymentRequest(CreateTerminalPayment, "POST", "/api/admin/bookings/"+bookingID+"/payment", req, adminToken, ctx)
if w.Code != http.StatusBadRequest {
t.Errorf("BUG: spend beyond gift-card balance expected 400, got %d: %s", w.Code, w.Body.String())
}
var balance float64
_ = tx.QueryRow(ctx, "SELECT balance FROM user_giftcard_balances WHERE user_id = $1", userID).Scan(&balance)
if balance < 0 {
t.Errorf("BUG: gift card balance went negative: %.2f", balance)
}
if balance != 10.00 {
t.Errorf("balance changed to %.2f after rejected payment (want 10.00)", balance)
}
}
// TestAttack_ConcurrentRefunds_NoOverRefund: two concurrent full-amount refunds
// on the same payment must result in exactly ONE Square refund call and one
// refund row.
func TestAttack_ConcurrentRefunds_NoOverRefund(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestData(t, ctx, tx)
adminID, err := fixtures.CreateTestAdminUser(tx)
require.NoError(t, err)
adminToken := jwt.GenerateTestToken(adminID, "admin")
paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 50, "in_person_card", "full", "completed")
require.NoError(t, err)
_, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = 'sqp_attack_concurrent_refund' WHERE id = $1", paymentID)
require.NoError(t, err)
cleanupConcurrentTestRows(t, context.Background(), userID, bookingID)
origClient := SquareClient
slow := &slowRefundClient{SquareClient: square.NewDevClient(), delay: 300 * time.Millisecond}
SquareClient = slow
defer func() { SquareClient = origClient }()
innerTx := db.TxFromContext(ctx)
require.NotNil(t, innerTx)
require.NoError(t, innerTx.Commit(ctx))
pool := context.Background()
startBoth := make(chan struct{})
codes := make([]int, 2)
done := make(chan struct{}, 2)
for i := 0; i < 2; i++ {
go func(idx int) {
<-startBoth
req := RefundRequest{Amount: 5000, Reason: "concurrent", IdempotencyKey: "attack-concurrent-refund-" + string(rune('a'+idx))}
w := makePaymentRequest(RefundPayment, "POST", "/api/admin/payments/"+paymentID+"/refund", req, adminToken, pool)
codes[idx] = w.Code
done <- struct{}{}
}(i)
}
close(startBoth)
<-done
<-done
okCount := 0
for _, c := range codes {
if c == http.StatusOK {
okCount++
}
}
if okCount != 1 {
t.Errorf("BUG: expected exactly 1 successful refund, got %d (codes %v)", okCount, codes)
}
var refundCount int
_ = db.Conn.QueryRow(pool, "SELECT COUNT(*) FROM refunds WHERE payment_id = $1", paymentID).Scan(&refundCount)
if refundCount != 1 {
t.Errorf("BUG: expected exactly 1 refund row, got %d", refundCount)
}
}
// TestAttack_BookingPaymentRacingCancellation_NoCompletedPaymentWithoutRefund
// exercises the C5 fix: the post-charge booking-status recheck
// (recheckBookingPayable in charge_helpers.go) now reads the booking row with
// `FOR UPDATE`, so a concurrent cancellation — which also locks the row with
// FOR UPDATE before writing its cancelled status — serializes with it. Without
// the row lock, the recheck's plain SELECT could read the pre-cancellation
// status and commit a COMPLETED payment on a CANCELLED booking; the
// cancellation refund loop had already run while the charge was still pending,
// so NO refund would ever be generated for the money taken at Square.
//
// INVARIANT: never a completed payment on a cancelled booking without a
// refund. Either the payment is rejected (no completed payment row) or it
// succeeds AND a refund is generated.
func TestAttack_BookingPaymentRacingCancellation_NoCompletedPaymentWithoutRefund(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestData(t, ctx, tx) // status in_progress, start 2099-12-31
token := jwt.GenerateUserToken(userID)
cleanupConcurrentTestRows(t, context.Background(), userID, bookingID)
// Commit the setup so the payment and the cancellation run at pool level on
// INDEPENDENT connections — the advisory lock and the FOR UPDATE row lock
// only serialize across separate sessions.
innerTx := db.TxFromContext(ctx)
require.NotNil(t, innerTx)
require.NoError(t, innerTx.Commit(ctx))
// Delay the Square charge so the cancellation can interleave between the
// pending-record commit and the post-charge status recheck.
origClient := SquareClient
slow := &slowCreatePaymentClient{SquareClient: square.NewDevClient(), delay: 300 * time.Millisecond}
SquareClient = slow
defer func() { SquareClient = origClient }()
pool := context.Background()
key := "attack-c5-" + bookingID
cardToken := "cnon:attack-c5-card"
req := CreateBookingPaymentRequest{
Amount: 5000,
PaymentType: "full",
NewCardToken: &cardToken,
IdempotencyKey: key,
}
var payRec *httptest.ResponseRecorder
payDone := make(chan struct{})
go func() {
defer close(payDone)
payRec = makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", req, token, pool)
}()
// Wait until the pending payment record is committed — the payment has now
// passed the pre-charge status check and is inside the (slow) Square call,
// holding the advisory lock. Starting the cancellation from here makes the
// interleaving deterministic.
deadline := time.Now().Add(10 * time.Second)
for {
var n int
if err := db.Conn.QueryRow(pool, `SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND idempotency_key = $2 AND status = 'pending'`, bookingID, key).Scan(&n); err == nil && n > 0 {
break
}
if time.Now().After(deadline) {
t.Fatal("timed out waiting for the payment to reach the pending-record commit")
}
time.Sleep(5 * time.Millisecond)
}
start := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)
cancelAt := time.Date(2099, 12, 28, 8, 0, 0, 0, time.UTC) // >72h notice → full-refund tier
cancelDone := make(chan struct{})
go func() {
defer close(cancelDone)
cancelTx, err := db.Conn.Begin(pool)
if err != nil {
t.Errorf("cancellation: begin tx: %v", err)
return
}
defer cancelTx.Rollback(pool)
// FOR UPDATE on the SAME booking row the payment's post-charge recheck
// locks (C5). Held until commit, so the recheck MUST block on it and
// then observe the committed cancelled status.
var st string
if err := cancelTx.QueryRow(pool, `SELECT status FROM bookings WHERE id = $1 FOR UPDATE`, bookingID).Scan(&st); err != nil {
t.Errorf("cancellation: lock booking row: %v", err)
return
}
if _, err := cancelTx.Exec(pool, `UPDATE bookings SET status = 'client_cancelled', updated_at = NOW() WHERE id = $1`, bookingID); err != nil {
t.Errorf("cancellation: set status: %v", err)
return
}
// Run the cancellation refund loop while the charge is still pending —
// it must find no COMPLETED payment to refund.
if _, err := ProcessCancellationRefundTx(pool, cancelTx, bookingID, 50, 50, start, cancelAt, "client_cancelled", &userID, false); err != nil {
t.Errorf("cancellation: refund: %v", err)
return
}
// Hold the row lock past the payment's Square round-trip so the
// payment's recheck is forced to block on it, then commit.
time.Sleep(600 * time.Millisecond)
if err := cancelTx.Commit(pool); err != nil {
t.Errorf("cancellation: commit: %v", err)
return
}
}()
<-payDone
<-cancelDone
// Ground truth: the booking is cancelled.
var bookingStatus string
require.NoError(t, db.Conn.QueryRow(pool, `SELECT status FROM bookings WHERE id = $1`, bookingID).Scan(&bookingStatus))
require.Equal(t, "client_cancelled", bookingStatus)
// C5 INVARIANT: never a completed payment on a cancelled booking without a
// refund.
var completedCount int
require.NoError(t, db.Conn.QueryRow(pool, `SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND status = 'completed'`, bookingID).Scan(&completedCount))
var refundCount int
require.NoError(t, db.Conn.QueryRow(pool, `SELECT COUNT(*) FROM refunds WHERE booking_id = $1`, bookingID).Scan(&refundCount))
if completedCount > 0 && refundCount == 0 {
t.Errorf("BUG (C5): %d completed payment(s) on cancelled booking %s with NO refund — money taken at Square but never refunded", completedCount, bookingID)
}
// Case-by-case: a 200 payment MUST be accompanied by a refund; any other
// result MUST NOT leave a completed payment behind.
if payRec.Code == http.StatusOK {
if refundCount == 0 {
t.Errorf("BUG (C5): payment succeeded (HTTP 200) on cancelled booking %s but no refund was generated", bookingID)
}
} else {
var status string
if err := db.Conn.QueryRow(pool, `SELECT status FROM payments WHERE booking_id = $1 AND idempotency_key = $2`, bookingID, key).Scan(&status); err == nil && status == "completed" {
t.Errorf("BUG (C5): payment failed (HTTP %d) but the payment row is status %q", payRec.Code, status)
}
}
}
// TestAttack_DeletedGiftCard_RefundMarkedFailed exercises the M2 fix: the
// cancellation refund loop checks `RowsAffected()` on the gift-card balance
// UPDATE in ProcessCancellationRefundTx. When the gift card has been deleted,
// the UPDATE matches 0 rows — no money can move back onto the card — so the
// refund row must be recorded 'failed' (with a CRITICAL log), NEVER 'completed'.
// A 'completed' row on a deleted card would claim the customer's money was
// returned when it never moved.
func TestAttack_DeletedGiftCard_RefundMarkedFailed(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestData(t, ctx, tx)
// Direct card redemption: a completed giftcard payment bound to a specific
// gift card (gift_card_id set — NOT the user's pooled account balance).
var gcID string
require.NoError(t, tx.QueryRow(ctx, `
INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by)
VALUES (50.00, 50.00, $1) RETURNING id`, userID).Scan(&gcID))
paymentID, err := fixtures.CreateTestPayment(tx, bookingID, 50, "giftcard", "full", "completed")
require.NoError(t, err)
_, err = tx.Exec(ctx, "UPDATE payments SET gift_card_id = $1 WHERE id = $2", gcID, paymentID)
require.NoError(t, err)
// Delete the gift card (audit rows first so the FK allows the delete).
_, err = tx.Exec(ctx, "DELETE FROM gift_card_transactions WHERE gift_card_id = $1", gcID)
require.NoError(t, err)
_, err = tx.Exec(ctx, "DELETE FROM gift_cards WHERE id = $1", gcID)
require.NoError(t, err)
var gone int
require.NoError(t, tx.QueryRow(ctx, "SELECT COUNT(*) FROM gift_cards WHERE id = $1", gcID).Scan(&gone))
require.Zero(t, gone, "precondition: the gift card must be deleted")
// Capture the CRITICAL slog output the M2 fix emits when the gift-card
// refund UPDATE matches 0 rows.
var sb syncBuffer
origLogger := slog.Default()
slog.SetDefault(slog.New(slog.NewTextHandler(&sb, nil)))
defer slog.SetDefault(origLogger)
innerTx := db.TxFromContext(ctx)
require.NotNil(t, innerTx)
start := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)
cancelAt := time.Date(2099, 12, 28, 8, 0, 0, 0, time.UTC) // >72h → full £50 refund
result, err := ProcessCancellationRefundTx(ctx, innerTx, bookingID, 50, 50, start, cancelAt, "client_cancelled", &userID, false)
require.NoError(t, err)
require.NotNil(t, result)
if result.RefundableAmount != 50 {
t.Fatalf("expected refundable 50, got %.2f", result.RefundableAmount)
}
// INVARIANT: the refund row must be 'failed' — the money could not be
// credited to a deleted card, so 'completed' would falsely claim a refund.
var refundStatus string
var refundAmount float64
require.NoError(t, tx.QueryRow(ctx, "SELECT status, amount FROM refunds WHERE booking_id = $1 ORDER BY created_at DESC LIMIT 1", bookingID).Scan(&refundStatus, &refundAmount))
if refundStatus != "failed" {
t.Errorf("BUG (M2): refund on deleted gift card is status=%q — must be 'failed' (money never moved)", refundStatus)
}
if refundAmount != 50 {
t.Errorf("expected refund amount 50, got %.2f", refundAmount)
}
// The M2 fix must have emitted the CRITICAL log for the 0-row UPDATE.
logs := sb.String()
if !strings.Contains(logs, "CRITICAL") || !strings.Contains(logs, "affected 0 rows") {
t.Errorf("BUG (M2): expected a CRITICAL 'gift card refund UPDATE affected 0 rows' log, got: %q", logs)
}
}