diff --git a/backend/auth/jwt.go b/backend/auth/jwt.go index 13f56c3..eb95f20 100644 --- a/backend/auth/jwt.go +++ b/backend/auth/jwt.go @@ -145,6 +145,18 @@ func IsJTIRevoked(ctx context.Context, jti string) bool { // bound access tokens die immediately when theft is detected (HIGH 1). const familyAliveCacheTTL = 30 * time.Second +// familyAliveRecheckGrace is how close a cached family-alive verdict must be to +// its TTL before verifyFamilyAlive re-validates it against the DB instead of +// trusting the cache (Loop B finding 3). The daily refresh-token cleanup +// captures the affected families, DELETEs them, commits, and then invalidates +// the in-memory cache — a crash between the DELETE commit and the +// invalidation leaves the cache warm for up to familyAliveCacheTTL, accepting a +// bound access token after its family was killed. Re-validating an ALIVE +// verdict within this grace of its expiry bounds that residual window to the +// grace itself; a small grace (5s of a 30s TTL) preserves the MEDIUM-1 +// query-amplification win — only near-expiry lookups re-query. +const familyAliveRecheckGrace = 5 * time.Second + // familyAliveCacheMaxEntries bounds the in-memory map so a flood of distinct // family ids cannot grow it without bound. const familyAliveCacheMaxEntries = 10_000 @@ -172,17 +184,32 @@ func init() { // familyAliveLookup returns a cached verdict for a family key and whether it is // still fresh, evicting expired entries opportunistically. func familyAliveLookup(key string) (alive bool, ok bool) { + e, ok := familyAliveLookupEntry(key) + if !ok { + return false, false + } + return e.alive, true +} + +// familyAliveLookupEntry returns the cached verdict entry (with its expiry) for +// a family key and whether it is still fresh, evicting expired entries +// opportunistically. Unlike familyAliveLookup it hands the caller the entry so +// verifyFamilyAlive can re-validate a near-expiry ALIVE verdict against the DB +// (Loop B finding 3 — the residual crash window of the refresh-token cleanup's +// post-commit cache invalidation); familyAliveLookup stays as the simple +// (alive, ok) accessor used by the tests. +func familyAliveLookupEntry(key string) (familyAliveCacheEntry, bool) { familyAliveCache.mu.Lock() defer familyAliveCache.mu.Unlock() e, ok := familyAliveCache.m[key] if !ok { - return false, false + return familyAliveCacheEntry{}, false } if clock.Now().After(e.expires) { delete(familyAliveCache.m, key) - return false, false + return familyAliveCacheEntry{}, false } - return e.alive, true + return e, true } // familyAliveStore records a DB-confirmed verdict, evicting expired entries @@ -398,6 +425,15 @@ func VerifyToken(tokenString string, ctx context.Context) (userID string, role s // IsJTIRevoked — because genuine theft is already handled by the family kill in // VerifyRefreshToken's reuse branch, and a transient DB error must not turn // into a total 401 outage for every authenticated request. +// +// Loop B finding 3 (crash-safety residual window): an ALIVE verdict within +// familyAliveRecheckGrace of its TTL is re-validated against the DB. The daily +// refresh-token cleanup (handlers/scheduling/scheduled-cleanup.go +// CleanupExpiredRefreshTokens) invalidates the family-alive cache AFTER its +// DELETE commits, so a crash between the commit and the invalidation leaves the +// cache warm for the rest of the TTL — a family killed by that missed +// invalidation would otherwise keep admitting its bound access tokens. The +// near-expiry re-check closes the gap to the grace window. func verifyFamilyAlive(ctx context.Context, token jwtClaimGetter, userID string) error { var familyVal any if err := token.Get(accessTokenFamilyClaim, &familyVal); err != nil { @@ -411,11 +447,15 @@ func verifyFamilyAlive(ctx context.Context, token jwtClaimGetter, userID string) return nil } key := familyID + "|" + userID - if alive, cached := familyAliveLookup(key); cached { - if !alive { + if e, cached := familyAliveLookupEntry(key); cached { + if !e.alive { return fmt.Errorf("token revoked") } - return nil + // A fresh verdict is trusted (the query-amplification win); only a + // near-expiry ALIVE verdict falls through to the DB re-check below. + if clock.Now().Before(e.expires.Add(-familyAliveRecheckGrace)) { + return nil + } } var exists bool err := db.Conn.QueryRow(ctx, diff --git a/backend/handlers/payments/charge_helpers.go b/backend/handlers/payments/charge_helpers.go index 75eb00a..65c9b76 100644 --- a/backend/handlers/payments/charge_helpers.go +++ b/backend/handlers/payments/charge_helpers.go @@ -203,6 +203,39 @@ func writeChargeSnapshot(ctx context.Context, q db.Querier, table, rowID string, } } +// writeChargeSnapshotUnconditional stores the verbatim request JSON on a +// payments/till_sales row WITHOUT the first-attempt immutability guard that +// writeChargeSnapshot applies. It is used ONLY by the gift-card (BuyGiftCard) +// and till-sale (CreateTillSale) charge sites — the two sites that were +// UNCONDITIONAL before the writeChargeSnapshot consolidation (Loop A, finding 1 +// regression check 4a). Their pending-reuse branches deliberately refresh +// square_request_snapshot in the SAME transaction as the square_source_id +// refresh (the Go-reencrypt refresh in giftcards.go / refreshTillSnapshotSource +// in till.go, B6), and this post-commit write stores the FRESH full body for +// the CURRENT attempt. The guard would wrongly skip this write on the reuse +// path when the in-transaction refresh failed best-effort — the row would then +// keep the stale first-attempt body while the live square_source_id column +// already points at the new source (the pre-consolidation code explicitly +// warned against "fixing" these sites into the guarded form). The booking/tip/ +// terminal saved-card paths keep the guarded writeChargeSnapshot: their reuse +// paths do NOT refresh the snapshot, so the guard correctly records the +// immutable first-attempt body. +func writeChargeSnapshotUnconditional(ctx context.Context, q db.Querier, table, rowID string, body any, label string) { + snap, mErr := json.Marshal(body) + if mErr != nil { + log.Printf("Failed to marshal square_request_snapshot for %s %s: %v", label, rowID, mErr) + return + } + stored, eErr := encryptSnapshot(snap) + if eErr != nil { + log.Printf("Failed to encrypt square_request_snapshot for %s %s: %v", label, rowID, eErr) + return + } + if _, sErr := q.Exec(ctx, `UPDATE `+table+` SET square_request_snapshot = $1 WHERE id = $2`, string(stored), rowID); sErr != nil { + log.Printf("Failed to store square_request_snapshot for %s %s: %v", label, rowID, sErr) + } +} + // recheckBookingPayable re-reads the booking status after a Square charge // succeeded (R9): a concurrent cancellation/eviction can move the booking out // of a payable state between the pre-charge status check and the charge diff --git a/backend/handlers/payments/errors.go b/backend/handlers/payments/errors.go index e47c1dc..2e1f9eb 100644 --- a/backend/handlers/payments/errors.go +++ b/backend/handlers/payments/errors.go @@ -74,18 +74,33 @@ func writeVerificationRequiredResponse(w http.ResponseWriter) { // HTTP status a payment handler should return: // // - 503 (Service Unavailable) for AMBIGUOUS failures: transport/network -// errors, Square 5xx responses, context cancellation/deadline, and the +// errors, Square 5xx responses, context cancellation/deadline, the // retryable 4xx statuses 429 (rate limited), 408 (request timeout), and -// 425 (too early) — the money state at Square is unknown, so the frontend -// should treat it as a retry (the pending record is resumed on a same-key -// retry). Square's own docs treat 429 as "retry later"; mapping it (or a -// timeout/early request) to 402 would mislabel a retryable condition as a -// permanent decline. +// 425 (too early), and the structured error code IDEMPOTENCY_KEY_REUSED — +// the money state at Square is unknown, so the frontend should treat it as +// a retry (the pending record is resumed on a same-key retry). Square's own +// docs treat 429 as "retry later"; mapping it (or a timeout/early request) +// to 402 would mislabel a retryable condition as a permanent decline. // - 402 (Payment Required) for DEFINITIVE declines: a structured Square // error (squareAPIError) carrying any OTHER 4xx status (400/402/422 etc.) // means Square positively rejected the charge (card declined/expired, // AVS/CVV failure) — retrying with the same inputs cannot succeed. // +// IDEMPOTENCY_KEY_REUSED (Loop B CRITICAL-ish, finding 1) is AMBIGUOUS, never +// a definitive decline: Square retains the key against the ORIGINAL request +// body, so the error means a PREVIOUS attempt under this key used a different +// body — the original charge may have LANDED at Square. Classifying it 402 +// would make the frontend regenerate the idempotency key (the 402 branch +// clears the cached key) and issue a NEW charge under a fresh key — a double +// charge when the original landed. Classifying it 503 keeps the key: a +// same-key retry with the ORIGINAL body makes Square dedup to the original +// payment (no new charge), and a retry with a different body keeps getting +// IDEMPOTENCY_KEY_REUSED while the pending row stays rescuable by the sweep — +// which already treats IDEMPOTENCY_KEY_REUSED as ambiguous (sweep.go:1088, +// replayErrorProvesNoCharge in square_http_client.go:681). The check keys on +// the structured ErrorCode, not the HTTP status, because Square may surface it +// as 400 or 409 depending on the request shape. +// // A nil error is never expected (callers only invoke this on the error path); // it maps to 402 defensively. The dev mock returns plain errors for simulated // failures, which classify as 503 (ambiguous) — correct for a mock standing in @@ -97,6 +112,9 @@ func chargeFailureStatus(err error) int { if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) { return http.StatusServiceUnavailable } + if square.ErrorCode(err) == "IDEMPOTENCY_KEY_REUSED" { + return http.StatusServiceUnavailable + } status := square.ErrorStatusCode(err) if status == 0 || status >= 500 { return http.StatusServiceUnavailable diff --git a/backend/handlers/payments/errors_test.go b/backend/handlers/payments/errors_test.go index db1eafa..ebaef81 100644 --- a/backend/handlers/payments/errors_test.go +++ b/backend/handlers/payments/errors_test.go @@ -130,9 +130,12 @@ func TestChargeFailureStatus_RetryableCarveOuts(t *testing.T) { // classification, including the ambiguous DEFAULT branch (1xx/2xx/3xx): the // default MUST be 503 (ambiguous → retryable) — never 402, which labels a // definitive decline and suppresses the same-key retry that resumes the pending -// record. 409 (Square IDEMPOTENCY_KEY_REUSED — a key reused with a different -// request body) is a definitive client error and must stay 402, not fall into -// the ambiguous bucket. +// record. A generic 409 (a structured conflict that is NOT the +// IDEMPOTENCY_KEY_REUSED code) is a definitive client error and must stay 402; +// the IDEMPOTENCY_KEY_REUSED code is classified as 503 in the dedicated test +// below (Loop B finding 1 — the original charge may have landed under the +// retained key, so 402 would make the frontend regenerate the key and +// double-charge). func TestChargeFailureStatus_DefaultAndEdgeStatuses(t *testing.T) { tests := []struct { name string @@ -146,7 +149,7 @@ func TestChargeFailureStatus_DefaultAndEdgeStatuses(t *testing.T) { {"401 → 402 (definitive)", http.StatusUnauthorized, http.StatusPaymentRequired}, {"403 → 402 (definitive)", http.StatusForbidden, http.StatusPaymentRequired}, {"408 → 503 (retryable)", http.StatusRequestTimeout, http.StatusServiceUnavailable}, - {"409 → 402 (idempotency-key conflict, definitive)", http.StatusConflict, http.StatusPaymentRequired}, + {"409 → 402 (generic conflict, definitive)", http.StatusConflict, http.StatusPaymentRequired}, {"425 → 503 (retryable)", http.StatusTooEarly, http.StatusServiceUnavailable}, {"429 → 503 (retryable)", http.StatusTooManyRequests, http.StatusServiceUnavailable}, {"500 → 503", http.StatusInternalServerError, http.StatusServiceUnavailable}, @@ -167,6 +170,33 @@ func TestChargeFailureStatus_DefaultAndEdgeStatuses(t *testing.T) { } } +// TestChargeFailureStatus_IdempotencyKeyReused_Ambiguous pins the Loop B +// CRITICAL-ish finding 1 classification: a structured IDEMPOTENCY_KEY_REUSED +// error (Square retained the key against a DIFFERENT request body — the +// original charge may have landed) is AMBIGUOUS and must classify as 503, +// never 402. A 402 would make the frontend regenerate the idempotency key and +// issue a NEW charge under a fresh key — double-charging the customer when the +// original landed. The check keys on the structured ErrorCode, so BOTH the 400 +// (dev mock / real Square) and 409 (real Square) surfaces classify as 503, +// while a generic 409 without the code stays a definitive 402 (covered above). +func TestChargeFailureStatus_IdempotencyKeyReused_Ambiguous(t *testing.T) { + tests := []struct { + name string + err error + want int + }{ + {"IDEMPOTENCY_KEY_REUSED 400 → 503", structuredSquareErrorFull(t, http.StatusBadRequest, "IDEMPOTENCY_KEY_REUSED", "INVALID_REQUEST_ERROR"), http.StatusServiceUnavailable}, + {"IDEMPOTENCY_KEY_REUSED 409 → 503", structuredSquareErrorFull(t, http.StatusConflict, "IDEMPOTENCY_KEY_REUSED", "INVALID_REQUEST_ERROR"), http.StatusServiceUnavailable}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := chargeFailureStatus(tt.err); got != tt.want { + t.Errorf("chargeFailureStatus(%v) = %d, want %d", tt.err, got, tt.want) + } + }) + } +} + // TestCreateBookingPayment_AmbiguousSquareFailure_Returns503 verifies the // charge-failure classification end to end: the dev mock's simulated failure // is a PLAIN error (no structured Square status), so the handler now returns diff --git a/backend/handlers/payments/giftcards.go b/backend/handlers/payments/giftcards.go index cbb4dc1..265c1d7 100644 --- a/backend/handlers/payments/giftcards.go +++ b/backend/handlers/payments/giftcards.go @@ -1699,12 +1699,13 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { // with an IDENTICAL body under the same key — Square compares the whole // request on key reuse, and a reconstructed body returns // IDEMPOTENCY_KEY_REUSED, leaving the row pending forever. The write is - // immutability-guarded (same rule as the booking/tip/terminal flows): it - // records the FIRST attempt's body only — the reuse branch above (B6) - // refreshes square_request_snapshot in the SAME transaction as the - // square_source_id refresh via the Go-reencrypt path, so a reuse never - // needs this post-commit write to overwrite the guard. - writeChargeSnapshot(ctx, db.Conn, "payments", buyPaymentID, paymentReq, "gift-card payment") + // INTENTIONALLY unconditional (writeChargeSnapshotUnconditional — Loop A + // regression check 4a restored it): the reuse branch above (B6) already + // refreshed square_request_snapshot in the SAME transaction as the + // square_source_id refresh, and this post-commit write stores the FRESH + // full body for THIS attempt. The immutability guard would wrongly skip + // this write on the reuse path when the in-tx refresh failed best-effort. + writeChargeSnapshotUnconditional(ctx, db.Conn, "payments", buyPaymentID, paymentReq, "gift-card payment") paymentResult, err := SquareClient.CreatePayment(ctx, paymentReq) if err != nil { diff --git a/backend/handlers/payments/till.go b/backend/handlers/payments/till.go index e0e6359..b9d18f6 100644 --- a/backend/handlers/payments/till.go +++ b/backend/handlers/payments/till.go @@ -1228,13 +1228,15 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { // The snapshot holds PII (buyer email + ccof token), so it is // encrypted at rest via encryptSnapshot (plaintext in dev/mock). // - // The write is INTENTIONALLY unconditional (WHERE id = $2, no - // snapshot-is-null guard like the booking/tip/terminal flows): the - // pending-reuse branch above already refreshed square_request_snapshot - // in the SAME transaction as the square_source_id refresh - // (refreshTillSnapshotSource, B6), so a reuse never needs this - // post-commit write to overwrite the guard. - writeChargeSnapshot(ctx, db.Conn, "till_sales", tillSaleID, paymentReq, "till sale") + // The write is INTENTIONALLY unconditional + // (writeChargeSnapshotUnconditional — Loop A regression check 4a + // restored it): the pending-reuse branch above already refreshed + // square_request_snapshot in the SAME transaction as the + // square_source_id refresh (refreshTillSnapshotSource, B6), and this + // post-commit write stores the fresh full body for THIS attempt. + // The immutability guard would wrongly skip this write on the reuse + // path when the in-tx refresh failed best-effort. + writeChargeSnapshotUnconditional(ctx, db.Conn, "till_sales", tillSaleID, paymentReq, "till sale") paymentResult, squareErr = SquareClient.CreatePayment(ctx, paymentReq) } else if req.PaymentMethod == "online_square" { @@ -1265,13 +1267,15 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { // charge with an IDENTICAL body under the same key — Square // compares the whole request on key reuse, and a reconstructed body // returns IDEMPOTENCY_KEY_REUSED, leaving the row pending forever. - // The write is immutability-guarded (same rule as the - // booking/tip/terminal flows): it records the FIRST attempt's body - // only — the pending-reuse branch above refreshes + // The write is INTENTIONALLY unconditional + // (writeChargeSnapshotUnconditional — Loop A regression check 4a + // restored it): the pending-reuse branch above refreshes // square_request_snapshot in the SAME transaction as the - // square_source_id refresh (refreshTillSnapshotSource, B6), so a - // reuse never needs this post-commit write to overwrite the guard. - writeChargeSnapshot(ctx, db.Conn, "till_sales", tillSaleID, paymentReq, "till sale") + // square_source_id refresh (refreshTillSnapshotSource, B6), and this + // post-commit write stores the FRESH full body for THIS attempt. The + // immutability guard would wrongly skip this write on the reuse path + // when the in-tx refresh failed best-effort. + writeChargeSnapshotUnconditional(ctx, db.Conn, "till_sales", tillSaleID, paymentReq, "till sale") paymentResult, squareErr = SquareClient.CreatePayment(ctx, paymentReq) } diff --git a/backend/handlers/payments/twofa.go b/backend/handlers/payments/twofa.go index 8bd0d50..31ce3c0 100644 --- a/backend/handlers/payments/twofa.go +++ b/backend/handlers/payments/twofa.go @@ -276,14 +276,39 @@ func requireTwoFactorForCardAccess(w http.ResponseWriter, r *http.Request, servi // configured. It also respects the same per-user mint cooldown // (twoFAMintCooldown via the shared twofa.AttemptState.LastMintAt), so a // charge-failure loop cannot mint codes faster than the mint endpoints allow. -// Best-effort: a failure logs and the customer requests a fresh code through -// the normal 2FA flow. +// Best-effort: a failure logs a CRITICAL line + raises a critical-payment +// admin notification (the operator must mint a code manually or fix the +// config) and the customer requests a fresh code through the normal 2FA flow. +// +// PEPPER-CHANGE NOTE (Loop B finding 2): TWO_FACTOR_PEPPER is the ONLY hard +// gate on issuance here (twoFAReissueIssueAllowed) AND on the interactive mint +// paths (twoFAEnsureIssueAllowed in handlers/user). The pepper keys the +// HMAC-SHA256 of every stored pending-code hash, so CHANGING it invalidates +// ALL pending codes — every stored hash was computed with the old pepper and +// can never match a code minted under the new one. A fresh saved-card charge +// that consumed a pre-change code then fails will re-issue a code hashed with +// the NEW pepper, which still cannot match anything the customer holds (their +// code was minted under the old pepper, or was consumed). Operators MUST NOT +// change the pepper without re-minting every user's code (or having the +// customer re-run 2FA setup); main.go's startup check should treat a changed +// pepper as a config incident. func reissueTwoFACodeAfterFailedCharge(ctx context.Context, q db.Querier, userID string, usedSavedCard, fallbackUsed bool, r *http.Request) { if userID == "" || !twoFactorEnforced() || !usedSavedCard || !fallbackUsed { return } if err := twoFAReissueIssueAllowed(); err != nil { - log.Printf("2FA: refused to re-issue a code for user %s after a failed charge: %v", userID, err) + // Loop B HIGH (finding 2): a REFUSED re-issue strands the customer. The + // gate consumed their code for the fresh charge (single-use) and the + // charge failed — with no re-issued code the same-key retry fails + // forever with 400 ErrMissingOrExpired and the customer is locked out. + // This is an OPERATOR-FACING incident, not a silent best-effort miss: + // log a CRITICAL line AND raise the deduped critical-payment admin + // notification (sweep.go's insertCriticalPaymentNotification — the + // DB-backed stand-in for the un-watched CRITICAL logs) so the operator + // knows the customer is blocked and can mint a code manually or fix the + // config (TWO_FACTOR_PEPPER / delivery channel). + log.Printf("CRITICAL: failed to re-issue a 2FA code for user %s after a failed saved-card charge (%v) — the customer's code was consumed by the fresh charge and NO live code remains, so the same-key retry cannot succeed; the operator must mint a code manually or configure TWO_FACTOR_PEPPER and a 2FA delivery channel", userID, err) + insertCriticalPaymentNotification(ctx, nil, &userID) return } // Mint cooldown (B11a): the shared per-user mutex serializes the stamp diff --git a/backend/handlers/payments/twofa_delivery_prod.go b/backend/handlers/payments/twofa_delivery_prod.go index 5548a07..f9a2c09 100644 --- a/backend/handlers/payments/twofa_delivery_prod.go +++ b/backend/handlers/payments/twofa_delivery_prod.go @@ -33,6 +33,15 @@ var errTwoFAPepperRequired = errors.New("TWO_FACTOR_PEPPER is not set; refusing // pepper every stored code would be an offline-brute-forceable unsalted digest // — either way the re-issue refuses (fail-closed), exactly like the interactive // mint paths. Dev/test builds always allow issuance (twofa_delivery_dev.go). +// +// The pepper check is the ONLY hard gate on the re-issue (plus the delivery +// channel). PEPPER-CHANGE HAZARD (Loop B finding 2): the pepper keys the +// HMAC-SHA256 of every stored pending-code hash, so CHANGING TWO_FACTOR_PEPPER +// invalidates ALL pending codes — a re-issued code under the new pepper can +// never match a customer's code minted under the old one. An operator who +// changes the pepper must re-mint every user's code (or the customer must +// re-run 2FA setup), or a fresh saved-card charge whose code was consumed at +// the gate will strand the customer with 400 ErrMissingOrExpired on retry. func twoFAReissueIssueAllowed() error { if os.Getenv("TWO_FACTOR_PEPPER") == "" { return errTwoFAPepperRequired diff --git a/backend/handlers/scheduling/scheduled-cleanup.go b/backend/handlers/scheduling/scheduled-cleanup.go index e034ad2..274cb94 100644 --- a/backend/handlers/scheduling/scheduled-cleanup.go +++ b/backend/handlers/scheduling/scheduled-cleanup.go @@ -250,6 +250,17 @@ func CleanupExpiredRefreshTokens(ctx context.Context) (int, error) { // ids are captured before the delete and invalidated after the commit: a // pre-commit invalidation could race a concurrent VerifyToken that // re-caches the still-present row as alive. + // + // RESIDUAL WINDOW (documented, Loop B finding 3): the invalidation is an + // in-memory cache operation that CANNOT be in the same transaction as the + // DELETE. A crash between the DELETE commit and the + // auth.InvalidateFamilyAliveBatch call leaves the cache warm for up to + // familyAliveCacheTTL, admitting a bound access token after its family was + // killed. The verify path closes this gap itself: verifyFamilyAlive + // (auth/jwt.go) re-validates an ALIVE cache verdict against the DB when it + // is within familyAliveRecheckGrace of its TTL, bounding the residual + // window to the grace (5s of 30s). The remaining window is a process crash + // exactly between the two statements — accepted and documented here. rows, err := tx.Query(ctx, ` SELECT DISTINCT family_id FROM refresh_tokens WHERE expires_at < NOW() diff --git a/backend/handlers/user/twofa_prod.go b/backend/handlers/user/twofa_prod.go index 94484b0..7da1788 100644 --- a/backend/handlers/user/twofa_prod.go +++ b/backend/handlers/user/twofa_prod.go @@ -75,6 +75,16 @@ func twoFADeliveryAvailable() bool { // stored code would be an offline-brute-forceable unsalted digest. Either way // issuance is refused (fail-closed). Dev/test builds always allow issuance // (twofa_dev.go). +// +// The pepper check is the ONLY hard gate here (plus the delivery channel), and +// it is also the ONLY hard gate on the payments re-issue path +// (payments.twoFAReissueIssueAllowed). PEPPER-CHANGE HAZARD (Loop B finding 2): +// the pepper keys the HMAC-SHA256 of every stored pending-code hash, so +// CHANGING TWO_FACTOR_PEPPER invalidates ALL pending codes — every stored hash +// was computed with the old pepper and can never match a code minted under the +// new one. An operator who changes the pepper must re-mint every user's code +// (or have each user re-run 2FA setup), or enforced saved-card charges will +// strand customers with 400 ErrMissingOrExpired forever. func twoFAEnsureIssueAllowed() error { if os.Getenv(twoFAPepperEnv) == "" { return errTwoFAPepperRequired