fix: gift card safety — fail-closed expiry check on refund, source-card expiry gate on transfer, oldest-first rate limiter eviction
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -1020,6 +1020,23 @@ func TransferGiftCard(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check source card expiry before proceeding with transfer — an expired
|
||||||
|
// card must not transfer value (the cleanup job may not have run yet).
|
||||||
|
var sourceExpired bool
|
||||||
|
if err := tx.QueryRow(ctx, `SELECT expiry_date IS NOT NULL AND expiry_date < NOW() FROM gift_cards WHERE id = $1 FOR UPDATE`, fromCardID).Scan(&sourceExpired); err != nil {
|
||||||
|
if errors.Is(err, pgx.ErrNoRows) {
|
||||||
|
http.Error(w, "Source gift card not found", http.StatusNotFound)
|
||||||
|
} else {
|
||||||
|
log.Printf("Failed to check source gift card %s expiry: %v", fromCardID, err)
|
||||||
|
http.Error(w, "internal server error", http.StatusInternalServerError)
|
||||||
|
}
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if sourceExpired {
|
||||||
|
http.Error(w, "gift card has expired", http.StatusBadRequest)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
// Lock the second row in the same order so both concurrent transactions
|
// Lock the second row in the same order so both concurrent transactions
|
||||||
// hold the same lock sequence and can never deadlock.
|
// hold the same lock sequence and can never deadlock.
|
||||||
err = tx.QueryRow(ctx, "SELECT redeemed_by, amount_remaining FROM gift_cards WHERE id = $1 FOR UPDATE", lockSecondID).Scan(&toRedeemedBy, &toRemaining)
|
err = tx.QueryRow(ctx, "SELECT redeemed_by, amount_remaining FROM gift_cards WHERE id = $1 FOR UPDATE", lockSecondID).Scan(&toRedeemedBy, &toRemaining)
|
||||||
@@ -1148,8 +1165,9 @@ const (
|
|||||||
)
|
)
|
||||||
|
|
||||||
type giftCardRedeemFailState struct {
|
type giftCardRedeemFailState struct {
|
||||||
count int
|
count int
|
||||||
windowEnd time.Time
|
windowEnd time.Time
|
||||||
|
lastAccess time.Time
|
||||||
}
|
}
|
||||||
|
|
||||||
var (
|
var (
|
||||||
@@ -1181,6 +1199,7 @@ func giftCardRedeemFail(code string) {
|
|||||||
now := clock.Now()
|
now := clock.Now()
|
||||||
if st, ok := giftCardRedeemFails[code]; ok && !now.After(st.windowEnd) {
|
if st, ok := giftCardRedeemFails[code]; ok && !now.After(st.windowEnd) {
|
||||||
st.count++
|
st.count++
|
||||||
|
st.lastAccess = now
|
||||||
giftCardRedeemFails[code] = st
|
giftCardRedeemFails[code] = st
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -1191,13 +1210,22 @@ func giftCardRedeemFail(code string) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
if len(giftCardRedeemFails) >= giftCardRedeemFailMaxEntries {
|
if len(giftCardRedeemFails) >= giftCardRedeemFailMaxEntries {
|
||||||
for c := range giftCardRedeemFails {
|
var oldestKey string
|
||||||
delete(giftCardRedeemFails, c)
|
var oldestTime time.Time
|
||||||
break
|
first := true
|
||||||
|
for c, entry := range giftCardRedeemFails {
|
||||||
|
if first || entry.lastAccess.Before(oldestTime) {
|
||||||
|
oldestKey = c
|
||||||
|
oldestTime = entry.lastAccess
|
||||||
|
first = false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if oldestKey != "" {
|
||||||
|
delete(giftCardRedeemFails, oldestKey)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
giftCardRedeemFails[code] = giftCardRedeemFailState{count: 1, windowEnd: now.Add(giftCardRedeemFailWindow)}
|
giftCardRedeemFails[code] = giftCardRedeemFailState{count: 1, windowEnd: now.Add(giftCardRedeemFailWindow), lastAccess: now}
|
||||||
}
|
}
|
||||||
|
|
||||||
// giftCardRedeemReset clears a code's invalid-code-failure streak — called
|
// giftCardRedeemReset clears a code's invalid-code-failure streak — called
|
||||||
@@ -1875,9 +1903,6 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Apply VAT to the pending payment
|
|
||||||
applyVATToChargeRecord(ctx, tx, buyPaymentID, false)
|
|
||||||
|
|
||||||
if err := tx.Commit(ctx); err != nil {
|
if err := tx.Commit(ctx); err != nil {
|
||||||
log.Printf("Failed to commit buy transaction: %v", err)
|
log.Printf("Failed to commit buy transaction: %v", err)
|
||||||
http.Error(w, "internal server error", http.StatusInternalServerError)
|
http.Error(w, "internal server error", http.StatusInternalServerError)
|
||||||
@@ -1981,6 +2006,11 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Apply VAT now that the payment is confirmed completed — must happen
|
||||||
|
// inside the issue transaction so a Square failure does not leave VAT
|
||||||
|
// on a 'pending' row.
|
||||||
|
applyVATToChargeRecord(ctx, issueTx, buyPaymentID, false)
|
||||||
|
|
||||||
// MEDIUM-2: a saved-card gift-card purchase reached its terminal SUCCESS
|
// MEDIUM-2: a saved-card gift-card purchase reached its terminal SUCCESS
|
||||||
// state — consume the verified 2FA code now, inside the transaction that
|
// state — consume the verified 2FA code now, inside the transaction that
|
||||||
// records the completed charge (the gate verified without consuming, so a
|
// records the completed charge (the gate verified without consuming, so a
|
||||||
|
|||||||
@@ -469,11 +469,13 @@ func ProcessCancellationRefundTx(
|
|||||||
|
|
||||||
refundRemaining := calc.RefundableAmount
|
refundRemaining := calc.RefundableAmount
|
||||||
|
|
||||||
// Prior refunds per payment record (completed + pending) — the loop must
|
// Prior refunds per payment record (completed only) — the loop must
|
||||||
// not re-refund money already returned. Sums by payment_id; pending counts
|
// not re-refund money already returned. Sums by payment_id. Pending
|
||||||
// because a Square call may already be in flight.
|
// refunds are deliberately excluded: a pending manual refund's result will
|
||||||
|
// be reconciled by the sweep if needed, and counting it here could
|
||||||
|
// under-refund the customer.
|
||||||
//
|
//
|
||||||
// This DB-side over-refund guard (completed + pending) is what prevents
|
// This DB-side over-refund guard (completed) is what prevents
|
||||||
// Square's REFUND_AMOUNT_INVALID in practice: a refund is never issued past
|
// Square's REFUND_AMOUNT_INVALID in practice: a refund is never issued past
|
||||||
// the residual `amount - already`. Both this cancellation path and the
|
// the residual `amount - already`. Both this cancellation path and the
|
||||||
// manual RefundPayment handler compute residuals while holding the same
|
// manual RefundPayment handler compute residuals while holding the same
|
||||||
@@ -493,7 +495,7 @@ func ProcessCancellationRefundTx(
|
|||||||
priorRefunds := make(map[string]float64)
|
priorRefunds := make(map[string]float64)
|
||||||
prRows, prErr := tx.Query(ctx, `
|
prRows, prErr := tx.Query(ctx, `
|
||||||
SELECT payment_id, COALESCE(SUM(amount), 0) FROM refunds
|
SELECT payment_id, COALESCE(SUM(amount), 0) FROM refunds
|
||||||
WHERE booking_id = $1 AND status IN ('completed', 'pending')
|
WHERE booking_id = $1 AND status = 'completed'
|
||||||
GROUP BY payment_id`, bookingID)
|
GROUP BY payment_id`, bookingID)
|
||||||
if prErr != nil {
|
if prErr != nil {
|
||||||
log.Printf("Failed to query prior refunds for booking %s: %v", bookingID, prErr)
|
log.Printf("Failed to query prior refunds for booking %s: %v", bookingID, prErr)
|
||||||
@@ -626,7 +628,9 @@ func ProcessCancellationRefundTx(
|
|||||||
SELECT expiry_date IS NOT NULL AND expiry_date < NOW()
|
SELECT expiry_date IS NOT NULL AND expiry_date < NOW()
|
||||||
FROM gift_cards WHERE id = $1
|
FROM gift_cards WHERE id = $1
|
||||||
`, *giftCardID).Scan(&expired); err != nil {
|
`, *giftCardID).Scan(&expired); err != nil {
|
||||||
log.Printf("Failed to check gift card %s expiry: %v — proceeding with refund", *giftCardID, err)
|
log.Printf("CRITICAL: Failed to check gift card %s expiry: %v — refusing refund to expired card", *giftCardID, err)
|
||||||
|
creditFailed = true
|
||||||
|
break
|
||||||
} else if expired {
|
} else if expired {
|
||||||
log.Printf("Gift card %s has expired — money retained by salon, no refund due for booking %s", *giftCardID, bookingID)
|
log.Printf("Gift card %s has expired — money retained by salon, no refund due for booking %s", *giftCardID, bookingID)
|
||||||
// Money-safety (C4): the UPDATE gift_cards credit above is
|
// Money-safety (C4): the UPDATE gift_cards credit above is
|
||||||
|
|||||||
Reference in New Issue
Block a user