Fix booking, tip, and terminal payment paths: cnon-direct charges, ccof customer_id, provisional terminal rows

One-off new-card charges now pass the cnon: nonce directly as source_id (no card-on-file, no customer). Save-card charges forward customer_id; legacy saved cards lazily provision a Square customer before charging (EnsureSquareCustomerForSavedCard). Terminal checkouts insert the terminal_checkouts row FIRST with a provisional tmp- id, then update with the real checkout_id, closing the crash window. GetDiscountPreviewHandler gains the fail-closed ownership check (IDOR). DeletePaymentMethod disables the card at Square before soft-delete. Sweep resolves provisional/tmp- terminal rows without a Square round-trip. GetCheckoutStatus rejects tmp- ids.

backend/handlers/payments/handlers.go

@@ -153,6 +153,35 @@ func GetDiscountPreviewHandler(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "unauthorized", http.StatusUnauthorized)
 		return
 	}
+	userRole, _ := r.Context().Value(mw.UserRoleKey).(string)
+
+	service := NewPaymentService()
+
+	// Fail closed (R5): a non-admin request must own the booking. The previous
+	// handler computed the discount preview for ANY booking id the caller
+	// supplied — leaking another user's booking total and eligible discounts
+	// (IDOR). Mirror GetBookingPaymentSummary's ownership check exactly: a
+	// request with no user context gets 401, and a non-owner gets 403.
+	if userRole != "admin" {
+		if userID == "" {
+			http.Error(w, "Authentication required", http.StatusUnauthorized)
+			return
+		}
+		bookingUserID, err := service.GetBookingUserID(r.Context(), bookingID)
+		if err != nil {
+			if errors.Is(err, pgx.ErrNoRows) {
+				http.Error(w, "Booking not found", http.StatusNotFound)
+				return
+			}
+			log.Printf("Failed to get booking user: %v", err)
+			http.Error(w, "internal server error", http.StatusInternalServerError)
+			return
+		}
+		if bookingUserID != userID {
+			http.Error(w, "Unauthorized", http.StatusForbidden)
+			return
+		}
+	}
 
 	preview := calculateDiscountPreview(r.Context(), bookingID, userID)
 
@@ -495,10 +524,32 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
 			http.Error(w, "internal server error", http.StatusInternalServerError)
 			return
 		}
+		// A ccof: source can NEVER be charged without a CustomerID — Square
+		// rejects the payment. A saved-card row created before P14 has an empty
+		// square_customer_id; lazily provision the booking user's Square
+		// customer and persist it on the row BEFORE charging (R6). A card with
+		// no bookable owner cannot be provisioned — refuse the charge.
+		if card.SquareCustomerID == "" {
+			if !bookingUserID.Valid {
+				http.Error(w, "Saved card has no owner and cannot be charged", http.StatusBadRequest)
+				return
+			}
+			provisioned, provErr := service.EnsureSquareCustomerForSavedCard(r.Context(), *req.UserSavedCardID, bookingUserID.String)
+			if provErr != nil {
+				log.Printf("Failed to provision Square customer for saved card %s (user %s): %v", *req.UserSavedCardID, bookingUserID.String, provErr)
+				http.Error(w, "Failed to process card", http.StatusInternalServerError)
+				return
+			}
+			card.SquareCustomerID = provisioned
+		}
 
 		// Serialize saved-card charges per booking (same lock as online booking
 		// payments) so concurrent double-clicks can't both pass the idempotency
-		// check. Mirrors the CreateBookingPayment lock (R4).
+		// check. Mirrors the CreateBookingPayment lock (R4). The lock is
+		// acquired with a bounded try-lock loop (R6): a blocking pg_advisory_lock
+		// would hold the pinned pool connection for the full Square round-trip of
+		// whichever request holds the lock, and ~4 concurrent same-booking
+		// requests would exhaust the whole pool.
 		pinConn, err := db.Conn.Acquire(r.Context())
 		if err != nil {
 			log.Printf("Failed to acquire connection for saved-card lock: %v", err)
@@ -506,13 +557,17 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		defer pinConn.Release()
-		if _, err := pinConn.Exec(r.Context(), `
+		lockOK, err := acquireAdvisoryLock(r.Context(), pinConn, "crussell:payment:"+bookingID)
-			SELECT pg_advisory_lock(hashtext('crussell:payment:' || $1))
+		if err != nil {
-		`, bookingID); err != nil {
 			log.Printf("Failed to acquire saved-card serialization lock for %s: %v", bookingID, err)
 			http.Error(w, "internal server error", http.StatusInternalServerError)
 			return
 		}
+		if !lockOK {
+			log.Printf("Saved-card serialization lock for %s not acquired within bound — a payment is in progress", bookingID)
+			http.Error(w, "Payment in progress, try again", http.StatusConflict)
+			return
+		}
 		defer func() {
 			if _, err := pinConn.Exec(context.Background(), `
 				SELECT pg_advisory_unlock(hashtext('crussell:payment:' || $1))
@@ -624,6 +679,32 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 
+		// Defensive post-charge recheck (R9): the window is tiny — this branch
+		// only runs on in_progress/completed bookings and the pending record
+		// committed moments ago — but a concurrent cancellation/eviction can
+		// still move the booking between the Square call and this record. A
+		// charge landing on a cancelled/lapsed booking must not be recorded as
+		// completed (the cancellation refund path computes refunds from
+		// completed payments). Mark the row failed and alert ops: money was
+		// taken at Square and MUST be refunded manually.
+		var recheckStatus string
+		if err := db.Conn.QueryRow(r.Context(), `SELECT status FROM bookings WHERE id = $1`, bookingID).Scan(&recheckStatus); err != nil {
+			log.Printf("CRITICAL: Square payment %s was processed for booking %s but re-reading booking status failed: %v — manual reconciliation required",
+				paymentResult.SquarePayID, bookingID, err)
+			http.Error(w, "internal server error", http.StatusInternalServerError)
+			return
+		}
+		if !bookingStatusAllowsCompletedPayment(recheckStatus) {
+			log.Printf("CRITICAL: Square payment %s was processed but booking %s is now %q — marking saved-card payment %s failed; money taken at Square MUST be refunded manually",
+				paymentResult.SquarePayID, bookingID, recheckStatus, paymentID)
+			if _, upErr := db.Conn.Exec(r.Context(), `UPDATE payments SET status = 'failed' WHERE id = $1`, paymentID); upErr != nil {
+				log.Printf("CRITICAL: Square payment %s landed on %q booking %s but marking payment %s failed errored: %v — manual reconciliation required",
+					paymentResult.SquarePayID, recheckStatus, bookingID, paymentID, upErr)
+			}
+			http.Error(w, "This booking is no longer accepting payments", http.StatusConflict)
+			return
+		}
+
 		if _, upErr := db.Conn.Exec(r.Context(),
 			`UPDATE payments SET status = 'completed', square_payment_id = $1 WHERE id = $2`,
 			paymentResult.SquarePayID, paymentID,
@@ -668,7 +749,8 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
 	// Serialize terminal-checkout creation per booking. This is the backend
 	// half of the double-submit fix: a lost-response retry must not create a
 	// second live Square checkout for the same booking while the first is in
-	// flight.
+	// flight. Bounded try-lock (R6) so a contended lock never blocks the pool
+	// across the Square round-trip.
 	pinConn, err := db.Conn.Acquire(r.Context())
 	if err != nil {
 		log.Printf("Failed to acquire connection for terminal checkout lock: %v", err)
@@ -676,13 +758,17 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	defer pinConn.Release()
-	if _, err := pinConn.Exec(r.Context(), `
+	lockOK, err := acquireAdvisoryLock(r.Context(), pinConn, "crussell:payment:"+bookingID)
-		SELECT pg_advisory_lock(hashtext('crussell:payment:' || $1))
+	if err != nil {
-	`, bookingID); err != nil {
 		log.Printf("Failed to acquire terminal checkout serialization lock for %s: %v", bookingID, err)
 		http.Error(w, "internal server error", http.StatusInternalServerError)
 		return
 	}
+	if !lockOK {
+		log.Printf("Terminal checkout serialization lock for %s not acquired within bound — a checkout is in progress", bookingID)
+		http.Error(w, "Payment in progress, try again", http.StatusConflict)
+		return
+	}
 	defer func() {
 		if _, err := pinConn.Exec(context.Background(), `
 			SELECT pg_advisory_unlock(hashtext('crussell:payment:' || $1))
@@ -701,6 +787,25 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Insert the tracked terminal_checkouts row FIRST with a provisional
+	// (pre-Square) checkout_id, THEN create the checkout at Square, THEN update
+	// the row with the real checkout_id (R3). A hard crash between the insert
+	// and the Square call leaves a visible PENDING row the in-flight guard and
+	// sweep can resolve as failed — the old order (CreateCheckout first) left a
+	// live untracked checkout the sweep could not see. The provisional id is
+	// synthetic ("tmp-<idempotency key>") because the column is a NOT NULL
+	// PRIMARY KEY; a row carrying one is provably pre-Square (no checkout was
+	// ever created for it).
+	provisionalID := "tmp-" + idempotencyKey
+	if _, err := db.Conn.Exec(r.Context(), `
+		INSERT INTO terminal_checkouts (checkout_id, booking_id, payment_type, status, amount)
+		VALUES ($1, $2, $3, 'PENDING', $4)
+	`, provisionalID, bookingID, req.PaymentType, float64(amount)/100.0); err != nil {
+		log.Printf("Failed to record provisional terminal checkout for booking %s: %v", bookingID, err)
+		http.Error(w, "internal server error", http.StatusInternalServerError)
+		return
+	}
+
 	checkoutReq := square.CreateCheckoutReq{
 		Amount:         amount,
 		Currency:       "GBP",
@@ -712,21 +817,43 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
 	checkout, err := SquareClient.CreateCheckout(r.Context(), checkoutReq)
 	if err != nil {
 		log.Printf("Failed to create checkout: %v", err)
+		// The provisional row is pre-Square and can never produce a charge —
+		// mark it failed so a retry can proceed (best-effort; log CRITICAL if
+		// the row update itself fails, since the row would then wedge the
+		// booking's in-flight guard).
+		if _, upErr := db.Conn.Exec(r.Context(), `
+			UPDATE terminal_checkouts SET status = 'failed', updated_at = NOW()
+			WHERE checkout_id = $1 AND status = 'PENDING'
+		`, provisionalID); upErr != nil {
+			log.Printf("CRITICAL: failed to mark provisional terminal checkout %s failed after CreateCheckout error (%v): %v — MANUAL RECONCILIATION REQUIRED", provisionalID, err, upErr)
+		}
 		http.Error(w, "Failed to create payment", http.StatusInternalServerError)
 		return
 	}
 
-	// Record the checkout with the payment type the admin charged so
+	// Attach the real Square checkout id to the tracked row (the provisional
-	// GetCheckoutStatus records the payment with that type.
+	// id was never seen by the client, so no poller can race this).
-	if _, err := db.Conn.Exec(r.Context(), `
+	tag, upErr := db.Conn.Exec(r.Context(), `
-		INSERT INTO terminal_checkouts (checkout_id, booking_id, payment_type, status, amount)
+		UPDATE terminal_checkouts SET checkout_id = $1, updated_at = NOW()
-		VALUES ($1, $2, $3, 'PENDING', $4)
+		WHERE checkout_id = $2
-	`, checkout.ID, bookingID, req.PaymentType, float64(amount)/100.0); err != nil {
+	`, checkout.ID, provisionalID)
-		log.Printf("Failed to record terminal checkout %s: %v", checkout.ID, err)
+	if upErr != nil {
+		log.Printf("CRITICAL: terminal checkout %s was created at Square but the tracking UPDATE (from provisional %s) failed: %v — manual reconciliation required", checkout.ID, provisionalID, upErr)
 		// The checkout is live at Square but untracked — best-effort cancel so
 		// a customer cannot complete a charge the backend can't record.
 		if cErr := SquareClient.CancelCheckout(r.Context(), checkout.ID); cErr != nil {
-			log.Printf("CRITICAL: failed to cancel orphaned terminal checkout %s after the DB insert failed: %v — MANUAL RECONCILIATION REQUIRED: the checkout may still be live at Square", checkout.ID, cErr)
+			log.Printf("CRITICAL: failed to cancel orphaned terminal checkout %s after the tracking UPDATE failed: %v — MANUAL RECONCILIATION REQUIRED: the checkout may still be live at Square", checkout.ID, cErr)
+		}
+		http.Error(w, "internal server error", http.StatusInternalServerError)
+		return
+	}
+	if tag.RowsAffected() == 0 {
+		// The provisional row vanished while the Square call was in flight
+		// (the sweep resolved it as stale) — the checkout is now live at
+		// Square but untracked.
+		log.Printf("CRITICAL: terminal checkout %s was created at Square but provisional row %s was already resolved — the checkout is untracked; MANUAL RECONCILIATION REQUIRED", checkout.ID, provisionalID)
+		if cErr := SquareClient.CancelCheckout(r.Context(), checkout.ID); cErr != nil {
+			log.Printf("CRITICAL: failed to cancel untracked terminal checkout %s: %v — MANUAL RECONCILIATION REQUIRED", checkout.ID, cErr)
 		}
 		http.Error(w, "internal server error", http.StatusInternalServerError)
 		return
@@ -768,6 +895,24 @@ func activeTerminalCheckoutID(ctx context.Context, bookingID string) string {
 		return ""
 	}
 
+	// A provisional (pre-Square) row carries a synthetic "tmp-" checkout_id (or
+	// an empty one for legacy rows) — no checkout was ever created at Square
+	// for it, so it is PROVABLY not live (R3). A hard crash between the
+	// terminal_checkouts insert and the Square CreateCheckout call is the only
+	// way one exists. Mark it failed so a fresh checkout can be created instead
+	// of wedging the booking behind a non-existent Square checkout.
+	if checkoutID == "" || strings.HasPrefix(checkoutID, "tmp-") {
+		log.Printf("Provisional (pre-Square) terminal checkout row %q for booking %s resolved as failed — no live checkout at Square", checkoutID, bookingID)
+		if checkoutID != "" {
+			if _, upErr := db.Conn.Exec(ctx, `
+				UPDATE terminal_checkouts SET status = 'failed', updated_at = NOW() WHERE checkout_id = $1
+			`, checkoutID); upErr != nil {
+				log.Printf("Failed to mark provisional terminal checkout %s failed: %v", checkoutID, upErr)
+			}
+		}
+		return ""
+	}
+
 	// Resolve against Square: once the terminal charge finished, the checkout
 	// is COMPLETED and its payment may already be recorded — it must not
 	// block a subsequent charge on the same booking.
@@ -814,6 +959,16 @@ func GetCheckoutStatus(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "not found", http.StatusNotFound)
 		return
 	}
+	// Reject provisional "tmp-" checkout ids: CreateTerminalPayment stores a
+	// synthetic "tmp-<idempotency key>" id in terminal_checkouts until Square
+	// returns the real checkout id (provisional-row design), so such an id was
+	// never a real checkout — resolving it against Square would come back
+	// NOT_FOUND and surface as a 500. Answer 404 instead (mirrors the guard in
+	// CreateTerminalPayment and the sweep).
+	if strings.HasPrefix(checkoutID, "tmp-") {
+		http.Error(w, "Checkout not found", http.StatusNotFound)
+		return
+	}
 
 	bookingID := r.URL.Query().Get("booking_id")
 	if bookingID == "" {
@@ -857,6 +1012,7 @@ func GetCheckoutStatus(w http.ResponseWriter, r *http.Request) {
 		// dedup SELECT and BOTH INSERT, with the second dying on the
 		// idempotency_key UNIQUE constraint after the customer already paid —
 		// the same double-record race every other payment path guards against.
+		// Bounded try-lock (R6) so a contended lock never blocks the pool.
 		terminalLockKey := paymentResult.SquarePayID
 		pinConn, err := db.Conn.Acquire(r.Context())
 		if err != nil {
@@ -865,13 +1021,17 @@ func GetCheckoutStatus(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 		defer pinConn.Release()
-		if _, err := pinConn.Exec(r.Context(), `
+		lockOK, err := acquireAdvisoryLock(r.Context(), pinConn, "crussell:terminal:"+terminalLockKey)
-			SELECT pg_advisory_lock(hashtext('crussell:terminal:' || $1))
+		if err != nil {
-		`, terminalLockKey); err != nil {
 			log.Printf("Failed to acquire terminal-completion serialization lock for %s: %v", terminalLockKey, err)
 			http.Error(w, "internal server error", http.StatusInternalServerError)
 			return
 		}
+		if !lockOK {
+			log.Printf("Terminal-completion serialization lock for %s not acquired within bound — a poll is already recording this checkout", terminalLockKey)
+			http.Error(w, "Payment in progress, try again", http.StatusConflict)
+			return
+		}
 		defer func() {
 			if _, err := pinConn.Exec(context.Background(), `
 				SELECT pg_advisory_unlock(hashtext('crussell:terminal:' || $1))
@@ -1145,6 +1305,11 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
 	// Using db.Conn.Exec() for both would be unsafe — each call may get a
 	// different pool connection, and pg_advisory_unlock on a different session
 	// is a silent no-op, leaking the lock.
+	//
+	// R6: the lock is acquired with a bounded try-lock loop rather than the
+	// blocking pg_advisory_lock. A blocking lock would pin the pool connection
+	// for the whole Square round-trip (~30s), so ~4 concurrent same-booking
+	// payments would exhaust the default pool and hang every request.
 	pinConn, err := db.Conn.Acquire(r.Context())
 	if err != nil {
 		log.Printf("Failed to acquire connection for payment lock: %v", err)
@@ -1153,13 +1318,17 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
 	}
 	defer pinConn.Release()
 
-	if _, err := pinConn.Exec(r.Context(), `
+	lockOK, err := acquireAdvisoryLock(r.Context(), pinConn, "crussell:payment:"+bookingID)
-		SELECT pg_advisory_lock(hashtext('crussell:payment:' || $1))
+	if err != nil {
-	`, bookingID); err != nil {
 		log.Printf("Failed to acquire payment serialization lock for %s: %v", bookingID, err)
 		http.Error(w, "Internal server error", http.StatusInternalServerError)
 		return
 	}
+	if !lockOK {
+		log.Printf("Payment serialization lock for %s not acquired within bound — a payment is already in progress", bookingID)
+		http.Error(w, "Payment in progress, try again", http.StatusConflict)
+		return
+	}
 	defer func() {
 		if _, err := pinConn.Exec(context.Background(), `
 			SELECT pg_advisory_unlock(hashtext('crussell:payment:' || $1))
@@ -1294,42 +1463,51 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
 	var savedCardCustomerID string
 
 	if req.NewCardToken != nil && *req.NewCardToken != "" {
-		// CreateCardOnFile runs before the charge. If the subsequent payment
+		// A cnon: nonce charge needs NO card-on-file and NO customer (R6). The
-		// fails, this card-on-file is intentionally NOT deleted: the pending
+		// old code tokenized every new card via CreateCardOnFile even for
-		// record's retry re-creates it via the deterministic sha256 idempotency
+		// one-off non-save charges, which (a) created an orphan card at Square
-		// key, and Square returns the same card — deleting it would break that
+		// for a payment that only ever uses the nonce once, and (b) would have
-		// retry. The orphan is harmless (Square-side only, never charged).
+		// charged the resulting ccof: source without a CustomerID — Square
-		//
+		// rejects a card-on-file source that carries no customer.
-		// P14: when the card is being SAVED, provision (or reuse) the user's
-		// Square customer profile BEFORE tokenizing so the new card is created
-		// against that customer. One-off non-save charges pass "" — a cnon:
-		// nonce charge needs no customer.
-		squareCustomerID := ""
 		if req.SaveCard {
-			var custErr error
+			// Save path: provision (or reuse) the user's Square customer BEFORE
-			squareCustomerID, custErr = service.EnsureSquareCustomer(r.Context(), userID)
+			// tokenizing so the new card is created against that customer, and
+			// forward the customer id on the charge. A ccof: source MUST carry
+			// its customer (R6) — the charge below sets
+			// CustomerID = savedCardCustomerID = squareCustomerID.
+			squareCustomerID, custErr := service.EnsureSquareCustomer(r.Context(), userID)
 			if custErr != nil {
 				log.Printf("Failed to provision Square customer for user %s: %v", userID, custErr)
 				http.Error(w, "Failed to process card", http.StatusInternalServerError)
 				return
 			}
-		}
+			cardOnFile, err := SquareClient.CreateCardOnFile(r.Context(), userID, *req.NewCardToken, squareCustomerID)
-		cardOnFile, err := SquareClient.CreateCardOnFile(r.Context(), userID, *req.NewCardToken, squareCustomerID)
+			if err != nil {
-		if err != nil {
+				log.Printf("Failed to create card on file: %v", err)
-			log.Printf("Failed to create card on file: %v", err)
+				http.Error(w, "Failed to process card", http.StatusInternalServerError)
-			http.Error(w, "Failed to process card", http.StatusInternalServerError)
+				return
-			return
+			}
-		}
+			sourceID = cardOnFile.CardID
+			savedCardCustomerID = squareCustomerID
 
-		sourceID = cardOnFile.CardID
+			// CreateCardOnFile runs before the charge. If the subsequent payment
-
+			// fails, this card-on-file is intentionally NOT deleted: the pending
-		if req.SaveCard {
+			// record's retry re-creates it via the deterministic sha256
-			cardID, err := service.SaveCardForUser(r.Context(), userID, cardOnFile.CardID, cardOnFile.Brand, cardOnFile.Last4, cardOnFile.ExpMonth, cardOnFile.ExpYear, cardOnFile.Fingerprint)
+			// idempotency key (the SAVE path), and Square returns the same card —
+			// deleting it would break that retry.
+			cardID, err := service.SaveCardForUser(r.Context(), userID, squareCustomerID, cardOnFile.CardID, cardOnFile.Brand, cardOnFile.Last4, cardOnFile.ExpMonth, cardOnFile.ExpYear, cardOnFile.Fingerprint)
 			if err != nil {
 				log.Printf("Failed to save card: %v", err)
 			} else {
 				savedCardID = &cardID
 			}
+		} else {
+			// One-off new-card charge: use the cnon: nonce DIRECTLY as the
+			// source. No card-on-file is created (nothing to orphan, no
+			// customer needed) — this also eliminates the orphan-card
+			// accumulation the old non-save tokenize-then-charge flow left
+			// behind at Square.
+			sourceID = *req.NewCardToken
 		}
 		if savedCardID == nil && req.SaveCard {
 			log.Printf("Card was not saved despite save_card=true for user %s", userID)
@@ -1345,6 +1523,20 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
 			http.Error(w, "internal server error", http.StatusInternalServerError)
 			return
 		}
+		// A ccof: source can NEVER be charged without a CustomerID — Square
+		// rejects the payment. A saved-card row created before P14 has an empty
+		// square_customer_id; lazily provision the user's Square customer and
+		// persist it on the row BEFORE charging (R6). Provisioning failure
+		// aborts the charge with a 500.
+		if card.SquareCustomerID == "" {
+			provisioned, provErr := service.EnsureSquareCustomerForSavedCard(r.Context(), *req.CardID, userID)
+			if provErr != nil {
+				log.Printf("Failed to provision Square customer for saved card %s (user %s): %v", *req.CardID, userID, provErr)
+				http.Error(w, "Failed to process card", http.StatusInternalServerError)
+				return
+			}
+			card.SquareCustomerID = provisioned
+		}
 		sourceID = card.SquareCardID
 		savedCardID = req.CardID
 		savedCardCustomerID = card.SquareCustomerID
@@ -1930,7 +2122,8 @@ func RefundPayment(w http.ResponseWriter, r *http.Request) {
 
 	// Serialize refund attempts per payment to prevent two concurrent refunds
 	// both passing the over-refund guard and both charging Square. Mirrors the
-	// tip/gift-card advisory-lock pattern.
+	// tip/gift-card advisory-lock pattern. Bounded try-lock (R6) so a
+	// contended lock never blocks the pool across the Square round-trip.
 	refundLockKey := paymentID
 	pinConn, err := db.Conn.Acquire(r.Context())
 	if err != nil {
@@ -1939,13 +2132,17 @@ func RefundPayment(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	defer pinConn.Release()
-	if _, err := pinConn.Exec(r.Context(), `
+	lockOK, err := acquireAdvisoryLock(r.Context(), pinConn, "crussell:refund:"+refundLockKey)
-		SELECT pg_advisory_lock(hashtext('crussell:refund:' || $1))
+	if err != nil {
-	`, refundLockKey); err != nil {
 		log.Printf("Failed to acquire refund serialization lock for %s: %v", paymentID, err)
 		http.Error(w, "internal server error", http.StatusInternalServerError)
 		return
 	}
+	if !lockOK {
+		log.Printf("Refund serialization lock for %s not acquired within bound — a refund is already in progress", paymentID)
+		http.Error(w, "Refund in progress, try again", http.StatusConflict)
+		return
+	}
 	defer func() {
 		if _, err := pinConn.Exec(context.Background(), `
 			SELECT pg_advisory_unlock(hashtext('crussell:refund:' || $1))
@@ -2497,42 +2694,48 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
 	var savedCardCustomerID string
 
 	if req.NewCardToken != nil && *req.NewCardToken != "" {
-		// CreateCardOnFile runs before the charge. If the subsequent payment
+		// A cnon: nonce charge needs NO card-on-file and NO customer (R6). The
-		// fails, this card-on-file is intentionally NOT deleted: the pending
+		// old code tokenized every new card via CreateCardOnFile even for
-		// record's retry re-creates it via the deterministic sha256 idempotency
+		// one-off non-save charges, which (a) created an orphan card at Square
-		// key, and Square returns the same card — deleting it would break that
+		// for a payment that only ever uses the nonce once, and (b) would have
-		// retry. The orphan is harmless (Square-side only, never charged).
+		// charged the resulting ccof: source without a CustomerID — Square
-		//
+		// rejects a card-on-file source that carries no customer.
-		// P14: when the card is being SAVED, provision (or reuse) the user's
-		// Square customer profile BEFORE tokenizing so the new card is created
-		// against that customer. One-off non-save charges pass "" — a cnon:
-		// nonce charge needs no customer.
-		squareCustomerID := ""
 		if req.SaveCard {
-			var custErr error
+			// Save path: provision (or reuse) the user's Square customer BEFORE
-			squareCustomerID, custErr = service.EnsureSquareCustomer(r.Context(), userID)
+			// tokenizing so the new card is created against that customer, and
+			// forward the customer id on the charge. A ccof: source MUST carry
+			// its customer (R6) — the charge below sets
+			// CustomerID = savedCardCustomerID = squareCustomerID.
+			squareCustomerID, custErr := service.EnsureSquareCustomer(r.Context(), userID)
 			if custErr != nil {
 				log.Printf("Failed to provision Square customer for user %s: %v", userID, custErr)
 				http.Error(w, "Failed to process card", http.StatusInternalServerError)
 				return
 			}
-		}
+			cardOnFile, err := SquareClient.CreateCardOnFile(r.Context(), userID, *req.NewCardToken, squareCustomerID)
-		cardOnFile, err := SquareClient.CreateCardOnFile(r.Context(), userID, *req.NewCardToken, squareCustomerID)
+			if err != nil {
-		if err != nil {
+				log.Printf("Failed to create card on file: %v", err)
-			log.Printf("Failed to create card on file: %v", err)
+				http.Error(w, "Failed to process card", http.StatusInternalServerError)
-			http.Error(w, "Failed to process card", http.StatusInternalServerError)
+				return
-			return
+			}
-		}
+			sourceID = cardOnFile.CardID
+			savedCardCustomerID = squareCustomerID
 
-		sourceID = cardOnFile.CardID
+			// CreateCardOnFile runs before the charge. If the subsequent payment
-
+			// fails, this card-on-file is intentionally NOT deleted: the pending
-		if req.SaveCard {
+			// record's retry re-creates it via the deterministic sha256
-			cardID, err := service.SaveCardForUser(r.Context(), userID, cardOnFile.CardID, cardOnFile.Brand, cardOnFile.Last4, cardOnFile.ExpMonth, cardOnFile.ExpYear, cardOnFile.Fingerprint)
+			// idempotency key (the SAVE path), and Square returns the same card —
+			// deleting it would break that retry.
+			cardID, err := service.SaveCardForUser(r.Context(), userID, squareCustomerID, cardOnFile.CardID, cardOnFile.Brand, cardOnFile.Last4, cardOnFile.ExpMonth, cardOnFile.ExpYear, cardOnFile.Fingerprint)
 			if err != nil {
 				log.Printf("Failed to save card: %v", err)
 			} else {
 				savedCardID = &cardID
 			}
+		} else {
+			// One-off new-card tip: use the cnon: nonce DIRECTLY as the source.
+			// No card-on-file is created (nothing to orphan, no customer needed).
+			sourceID = *req.NewCardToken
 		}
 		if savedCardID == nil && req.SaveCard {
 			log.Printf("Card was not saved despite save_card=true for user %s", userID)
@@ -2548,6 +2751,19 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
 			http.Error(w, "internal server error", http.StatusInternalServerError)
 			return
 		}
+		// A ccof: source can NEVER be charged without a CustomerID — Square
+		// rejects the payment. A saved-card row created before P14 has an empty
+		// square_customer_id; lazily provision the user's Square customer and
+		// persist it on the row BEFORE charging (R6).
+		if card.SquareCustomerID == "" {
+			provisioned, provErr := service.EnsureSquareCustomerForSavedCard(r.Context(), *req.CardID, userID)
+			if provErr != nil {
+				log.Printf("Failed to provision Square customer for saved card %s (user %s): %v", *req.CardID, userID, provErr)
+				http.Error(w, "Failed to process card", http.StatusInternalServerError)
+				return
+			}
+			card.SquareCustomerID = provisioned
+		}
 		sourceID = card.SquareCardID
 		savedCardID = req.CardID
 		savedCardCustomerID = card.SquareCustomerID
@@ -2557,6 +2773,8 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
 	// tip payments across browser tabs or retries. Uses a PostgreSQL session-level
 	// advisory lock scoped to the booking ID.
 	// See CreateBookingPayment lines 815-846 for the same pattern.
+	// Bounded try-lock (R6) so a contended lock never blocks the pool across
+	// the Square round-trip.
 	pinConn, err := db.Conn.Acquire(r.Context())
 	if err != nil {
 		log.Printf("Failed to acquire connection for tip lock: %v", err)
@@ -2565,13 +2783,17 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
 	}
 	defer pinConn.Release()
 
-	if _, err := pinConn.Exec(r.Context(), `
+	lockOK, err := acquireAdvisoryLock(r.Context(), pinConn, "crussell:tip:"+bookingID)
-		SELECT pg_advisory_lock(hashtext('crussell:tip:' || $1))
+	if err != nil {
-	`, bookingID); err != nil {
 		log.Printf("Failed to acquire tip serialization lock for %s: %v", bookingID, err)
 		http.Error(w, "Internal server error", http.StatusInternalServerError)
 		return
 	}
+	if !lockOK {
+		log.Printf("Tip serialization lock for %s not acquired within bound — a tip payment is already in progress", bookingID)
+		http.Error(w, "Payment in progress, try again", http.StatusConflict)
+		return
+	}
 	defer func() {
 		if _, err := pinConn.Exec(context.Background(), `
 			SELECT pg_advisory_unlock(hashtext('crussell:tip:' || $1))
@@ -2726,6 +2948,32 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Step 3a: post-charge recheck (R9). A concurrent cancellation/eviction
+	// can move the booking out of a payable state between the pre-charge
+	// status check and the Square charge completing. A tip landing on a
+	// cancelled/lapsed booking must NOT be recorded as completed — the
+	// cancellation refund path computes refunds from completed payments and
+	// would silently exclude it. Mark the tip row failed and alert ops: money
+	// was taken at Square and MUST be refunded manually (mirrors
+	// CreateBookingPayment's post-charge recheck).
+	var tipRecheckStatus string
+	if err := db.Conn.QueryRow(r.Context(), `SELECT status FROM bookings WHERE id = $1`, bookingID).Scan(&tipRecheckStatus); err != nil {
+		log.Printf("CRITICAL: Square tip payment %s (ID=%s) was processed but re-reading booking %s status failed: %v — manual reconciliation required",
+			paymentResult.Status, paymentResult.SquarePayID, bookingID, err)
+		http.Error(w, "internal server error", http.StatusInternalServerError)
+		return
+	}
+	if !bookingStatusAllowsCompletedPayment(tipRecheckStatus) {
+		log.Printf("CRITICAL: Square tip payment %s (ID=%s) for booking %s was processed but booking is now %q — marking tip %s failed; money taken at Square MUST be refunded manually",
+			paymentResult.Status, paymentResult.SquarePayID, bookingID, tipRecheckStatus, paymentID)
+		if _, upErr := db.Conn.Exec(r.Context(), `UPDATE payments SET status = 'failed' WHERE id = $1`, paymentID); upErr != nil {
+			log.Printf("CRITICAL: Square tip payment %s (ID=%s) landed on %q booking %s but marking tip %s failed errored: %v — manual reconciliation required",
+				paymentResult.Status, paymentResult.SquarePayID, tipRecheckStatus, bookingID, paymentID, upErr)
+		}
+		http.Error(w, "This booking is no longer accepting tips", http.StatusConflict)
+		return
+	}
+
 	// Step 3: Square succeeded — update the payment record.
 	_, upErr := db.Conn.Exec(r.Context(),
 		`UPDATE payments SET status = 'completed', square_payment_id = $1 WHERE id = $2`,

backend/handlers/payments/service.go

@@ -11,6 +11,8 @@ import (
 	"log"
 	"log/slog"
 	"math"
+	"strings"
+	"sync"
 	"time"
 
 	"github.com/jackc/pgx/v5"
@@ -493,7 +495,7 @@ func (s *PaymentService) GetBookingRemainingBalanceCents(ctx context.Context, bo
 
 func (s *PaymentService) GetUserPaymentMethods(ctx context.Context, userID string) ([]SavedCard, error) {
 	rows, err := db.Conn.Query(ctx, `
-		SELECT id, square_card_id, brand, last_4, exp_month, exp_year, fingerprint, is_default, COALESCE(square_customer_id, '')
+		SELECT id, COALESCE(square_card_id, ''), brand, last_4, exp_month, exp_year, fingerprint, is_default, COALESCE(square_customer_id, '')
 		FROM user_saved_cards
 		WHERE user_id = $1 AND deleted_at IS NULL
 		ORDER BY is_default DESC, created_at DESC
@@ -522,6 +524,35 @@ func (s *PaymentService) GetUserPaymentMethods(ctx context.Context, userID strin
 }
 
 func (s *PaymentService) DeletePaymentMethod(ctx context.Context, cardID, userID string) error {
+	// Load the Square card id and disable the card at Square BEFORE the local
+	// soft-delete (R8). Without this the card stays ENABLED at Square and keeps
+	// accepting ccof: charges even though the user deleted it locally — the
+	// account-deletion path already calls DeleteCardOnFile; this mirrors it for
+	// single-card deletes. The Square call is best-effort: a local delete must
+	// never be blocked by a Square failure. A NOT_FOUND answer means Square no
+	// longer has the card (nothing to disable); any other error is logged and
+	// ignored so the local delete proceeds regardless.
+	var sqCardID sql.NullString
+	err := db.Conn.QueryRow(ctx, `
+		SELECT square_card_id FROM user_saved_cards
+		WHERE id = $1 AND user_id = $2
+	`, cardID, userID).Scan(&sqCardID)
+	if err != nil && !errors.Is(err, pgx.ErrNoRows) {
+		return err
+	}
+	// ErrNoRows: the card is not owned by this user — the soft-delete below is
+	// a silent no-op (matching the pre-R8 behaviour), so skip the Square call.
+	if sqCardID.Valid && sqCardID.String != "" {
+		if err := SquareClient.DeleteCardOnFile(ctx, sqCardID.String); err != nil {
+			msg := strings.ToUpper(err.Error())
+			if square.ErrorCode(err) == "NOT_FOUND" || strings.Contains(msg, "NOT_FOUND") || strings.Contains(msg, "NOT FOUND") {
+				// Square already removed/disabled the card — nothing to do.
+			} else {
+				slog.Warn("failed to disable Square card on local delete — card may remain enabled at Square", "square_card_id", sqCardID.String, "err", err)
+			}
+		}
+	}
+
 	tx, err := db.Conn.Begin(ctx)
 	if err != nil {
 		log.Printf("Failed to begin transaction: %v", err)
@@ -618,7 +649,18 @@ func (s *PaymentService) CreatePaymentMethodFromToken(ctx context.Context, userI
 // subsequent card save by the same user. Square dedups on a deterministic
 // idempotency key derived from the email, so a response-lost retry returns the
 // same customer instead of minting a duplicate.
+//
+// R7: the created id is written back to the user's saved-card rows (when any
+// exist) AND cached in a package-level map, so a second ensureSquareCustomer
+// call in the same request flow (e.g. the handler's save-card branch followed
+// by SaveCardForUser) never re-hits the DB and never re-mints a customer. The
+// cache is process-local and dev-friendly; the row write makes it durable for
+// the next process/request.
 func (s *PaymentService) ensureSquareCustomer(ctx context.Context, userID string) (string, error) {
+	if v, ok := squareCustomerCache.Load(userID); ok {
+		return v.(string), nil
+	}
+
 	var customerID sql.NullString
 	err := db.Conn.QueryRow(ctx, `
 		SELECT square_customer_id FROM user_saved_cards
@@ -627,6 +669,7 @@ func (s *PaymentService) ensureSquareCustomer(ctx context.Context, userID string
 		LIMIT 1
 	`, userID).Scan(&customerID)
 	if err == nil && customerID.Valid {
+		squareCustomerCache.Store(userID, customerID.String)
 		return customerID.String, nil
 	}
 	if err != nil && !errors.Is(err, pgx.ErrNoRows) {
@@ -644,6 +687,18 @@ func (s *PaymentService) ensureSquareCustomer(ctx context.Context, userID string
 	if err != nil {
 		return "", fmt.Errorf("failed to create Square customer for card save: %w", err)
 	}
+
+	// Persist the minted id so the NEXT process/request reuses it instead of
+	// re-running CreateCustomer (the cache above only serves this process).
+	// Best-effort: the cache covers the immediate double-ensure within one
+	// request, and the save-card INSERT below carries the id anyway.
+	if _, upErr := db.Conn.Exec(ctx, `
+		UPDATE user_saved_cards SET square_customer_id = $1
+		WHERE user_id = $2 AND square_customer_id IS NULL
+	`, customer.ID, userID); upErr != nil {
+		log.Printf("Failed to persist Square customer id for user %s (non-fatal): %v", userID, upErr)
+	}
+	squareCustomerCache.Store(userID, customer.ID)
 	return customer.ID, nil
 }
 
@@ -656,17 +711,43 @@ func (s *PaymentService) EnsureSquareCustomer(ctx context.Context, userID string
 	return s.ensureSquareCustomer(ctx, userID)
 }
 
-func (s *PaymentService) SaveCardForUser(ctx context.Context, userID, squareCardID, brand, last4 string, expMonth, expYear int, fingerprint string) (string, error) {
+// EnsureSquareCustomerForSavedCard returns the Square customer id for a
-	// P14: SaveCardForUser is only ever called in save-card flows, so lazily
+// saved-card row, lazily provisioning + persisting one when the row predates
-	// ensure the Square customer exists and persist its id on the saved-card
+// P14 (square_customer_id empty). A ccof: source can NEVER be charged without
-	// row for reuse by subsequent card saves from the same user.
+// a CustomerID — Square rejects the payment — so every saved-card charge path
-	squareCustomerID, err := s.ensureSquareCustomer(ctx, userID)
+// calls this before CreatePayment. Provisioning failure aborts the charge.
+func (s *PaymentService) EnsureSquareCustomerForSavedCard(ctx context.Context, savedCardID, userID string) (string, error) {
+	var customerID sql.NullString
+	if err := db.Conn.QueryRow(ctx, `
+		SELECT square_customer_id FROM user_saved_cards
+		WHERE id = $1 AND user_id = $2
+	`, savedCardID, userID).Scan(&customerID); err != nil {
+		return "", err
+	}
+	if customerID.Valid && customerID.String != "" {
+		return customerID.String, nil
+	}
+	provisioned, err := s.EnsureSquareCustomer(ctx, userID)
 	if err != nil {
 		return "", err
 	}
+	if _, upErr := db.Conn.Exec(ctx, `
+		UPDATE user_saved_cards SET square_customer_id = $1
+		WHERE id = $2 AND user_id = $3
+	`, provisioned, savedCardID, userID); upErr != nil {
+		return "", upErr
+	}
+	return provisioned, nil
+}
 
+// SaveCardForUser persists a tokenized card as a saved card for the user.
+// squareCustomerID is the user's provisioned Square customer profile id
+// (P14) — the caller has already ensured it via EnsureSquareCustomer, so this
+// method NEVER re-provisions (R7: a second ensureSquareCustomer would re-query
+// the DB and, on a first-save flow, re-run CreateCustomer).
+func (s *PaymentService) SaveCardForUser(ctx context.Context, userID, squareCustomerID, squareCardID, brand, last4 string, expMonth, expYear int, fingerprint string) (string, error) {
 	var id string
-	err = db.Conn.QueryRow(ctx, `
+	err := db.Conn.QueryRow(ctx, `
 		INSERT INTO user_saved_cards (
 			user_id, square_card_id, square_customer_id, brand, last_4, exp_month, exp_year, fingerprint, is_default, created_at
 		) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, false, NOW())
@@ -689,7 +770,7 @@ func (s *PaymentService) GetCardByID(ctx context.Context, cardID, userID string)
 func (s *PaymentService) GetCardByIDQuerier(ctx context.Context, q db.Querier, cardID, userID string) (*SavedCard, error) {
 	var c SavedCard
 	err := q.QueryRow(ctx, `
-		SELECT id, square_card_id, brand, last_4, exp_month, exp_year, fingerprint, is_default, COALESCE(square_customer_id, '')
+		SELECT id, COALESCE(square_card_id, ''), brand, last_4, exp_month, exp_year, fingerprint, is_default, COALESCE(square_customer_id, '')
 		FROM user_saved_cards
 		WHERE id = $1 AND user_id = $2 AND deleted_at IS NULL
 	`, cardID, userID).Scan(&c.ID, &c.SquareCardID, &c.Brand, &c.Last4, &c.ExpMonth, &c.ExpYear, &c.Fingerprint, &c.IsDefault, &c.SquareCustomerID)
@@ -700,4 +781,11 @@ func (s *PaymentService) GetCardByIDQuerier(ctx context.Context, q db.Querier, c
 	return &c, nil
 }
 
+// squareCustomerCache is a package-level process-local cache of
+// userID → Square customer id, populated on the first successful provisioning
+// (R7). It prevents a second ensureSquareCustomer call in the same request
+// flow (or a rapid retry) from re-running the DB query and re-minting a
+// customer. The durable record remains the user_saved_cards row.
+var squareCustomerCache sync.Map
+
 var SquareClient square.SquareClient

backend/handlers/payments/sweep.go

@@ -296,6 +296,20 @@ func SweepStaleTerminalCheckouts(ctx context.Context) (int, error) {
 
 	resolved := 0
 	for _, r := range pending {
+		// A provisional (pre-Square) terminal_checkouts row carries a synthetic
+		// "tmp-" checkout_id (or an empty one) — no checkout was ever created
+		// at Square for it, so it is PROVABLY not live (R3). Resolve it to
+		// failed directly without a Square round-trip; a hard crash between the
+		// row insert and the Square CreateCheckout call is the only way one
+		// exists.
+		if r.CheckoutID == "" || strings.HasPrefix(r.CheckoutID, "tmp-") {
+			if markTerminalCheckoutRowFailed(ctx, r) {
+				resolved++
+			}
+			log.Printf("Provisional (pre-Square) terminal checkout row %s (%s) resolved as failed — no live checkout at Square", r.RowID, r.Kind)
+			continue
+		}
+
 		// Conservative status check: only cancel a checkout that is provably
 		// still waiting at Square. A COMPLETED checkout must never be
 		// cancelled, and an ambiguous status (network error) is left alone for