popertots/Crussell
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
827 Commits 1 Branch 0 Tags
348a2e5bfc45ae52a291de134bfc645959df8b91
Commit Graph
11 Commits
Author SHA1 Message Date
popertots 4146f8e09a fix: pre-launch review — security, money safety, privacy, legal, code quality 
Security (P0):
- IsJTIRevoked fails closed on DB error (previously accepted revoked tokens)
- Remove dead consume parameter from SCA gate (prevented token replay)
- Rate limiter map TTL-based eviction (prevented memory exhaustion)
- 2FA attempt map already had LRU eviction (verified)

Money Safety (P1):
- Gift card transfer refuses expired destination cards
- Gift card balance deduction has WHERE balance >= amount guard
- Webhook clawback acquires till-sale advisory lock
- Sweep/retry lock keys aligned

Privacy/Cookies (P2):
- Self-host Google Fonts (Playfair Display woff2)
- Replace CARTO map tiles with OpenStreetMap raster tiles
- Replace Wikimedia/icon-icons external images with local SVGs
- Remove external image URLs from CSP

Legal (P3):
- Privacy policy: add 6 missing data categories (gift cards, 2FA, GDPR, notifications, technical, cookies)
- Terms: add Tips section (optionality, non-refundable, same processing as bookings)

Code Quality (P4):
- twofa.Check accepts db.Querier for testability
- depositPromotionMinPct uses literal 0.20 (not misleading alias)
- HolidayHours.svelte uses proper type (not as any[])
- Remove stale TODO comments from main.go

Testing (P5):
- 94 new float64 money validity tests across 3 test files
- Cover VAT, splits, refunds, gift cards, rounding, precision boundaries
- All 27 backend test packages pass
 2026-08-22 00:34:51 +01:00
popertots f9e8385d5a fix: auth/2FA security — stdout-log code delivery is dev/test-only, production fails closed until email/SMS; verification-code hashing, lockout recovery, sabredav fail-closed 
- TWO_FACTOR_ALLOW_LOG_DELIVERY production opt-in REMOVED: plaintext codes are written to the stdout log ([2FA]/[VERIFY]) only in dev/test builds as a local DEV ONLY feature while email/SMS delivery (P6) is implemented. Production builds have no delivery channel and code issuance fails closed (503) under any configuration — no silent log-based code leak
- verification/2FA codes hashed at rest (HMAC-SHA256 via TWO_FACTOR_PEPPER, CHAR(64)); [VERIFY] dev log relay; per-user brute-force budget; password_reset purpose clears lockout for self-service recovery; dummy-bcrypt on login no-user path kills timing oracle
- sabredav weak-password list + entropy gate; .env.example ships fail-closed DAV_ADMIN_PASSWORD
- delete-account re-auth (current_password + fresh 2FA code when enforced)
- prod-tag suite (run-prod-tag-tests.sh) compiles and runs the production 2FA issuance gate: production ALWAYS reports no delivery channel and refuses issuance after the pepper check
- startup_checks_test SNAPSHOT_ENC_KEY values built at runtime so gitleaks sees no secret-shaped literals
- env-docs parity updated (flag removed, 38 vars)
 2026-08-22 00:34:50 +01:00
popertots 1179293777 fix: 2FA single-use consume is DB-atomic; login lockout escalates 15/30/60m with uniform 401 
F5.5 brute-force hardening:
- internal/twofa.Check consume is now a conditional UPDATE (WHERE id AND
  two_factor_pending_code_hash) reporting rows affected: two concurrent
  verifications of the same code on different instances both match the digest,
  but only the first conditional UPDATE can affect a row — the loser sees 0
  rows and fails MissingOrExpired, so one code authorizes exactly ONE operation
  across instances (the per-user mutex only serialized within one process).
- /login lockout is now indistinguishable from a wrong password: a locked
  account returns the same uniform 401 'invalid credentials' and burns the same
  constant-time bcrypt compare (via the shared semaphore), removing the
  account-existence oracle and lockout-probing signal of the old 429.
- Lockout tiers escalate 15m (5+) / 30m (7+) / 60m (10+): an attacker who keeps
  guessing past each unlock makes the lock LONGER, raising the repeat-DoS
  effort while the response stays uniform.
 2026-08-22 00:34:50 +01:00
popertots 4e398a7a2b fix: round-2 loop-B adversarial (503c326 baseline) — B1 webhook race, APPROVED refund semantics, notification cap single-source, 2FA cooldown/StateFor hardening, register bcrypt semaphore 
Round 2 Loop B red-team (money/security/dup-mod adversarial) findings on the full payments overhaul:

MONEY:
- HIGH: webhook COMPLETED promotion now resolves the B1 parent row (mirrors the re-poll resolveB1ParentFailed + till-sale clawback) — the sweep no longer re-replays an expired key into stacked unauthorized charges
- HIGH: A6 deposit-with-discount clamp — chargeAmount capped to max(0, remaining-discount) for ALL discount cases; overflow guard compares against the discounted remaining
- MED-HIGH: APPROVED refunds treated as NON-terminal at the webhook (event-driven, may still fail); payments call sites aligned; FAILED can now demote an APPROVED-then-failed row
- MED: B1 refund transport-error fails the row + CRITICAL immediately (no 3-charge stacking)
- MED: till_sales capped-fail surfaces the outstanding funding (gift_card_transactions trace) for manual reversal
- MED: guest-bookings cash/gift-card terminal charges now audited (NULL target); audit reordered post-commit; cancellation refunds audited
- MED: A6 no-discount skip-path returns campaign_fully_redeemed 400 (no success-shaped no-op); skip-path writes a marker row for idempotency

SECURITY:
- HIGH: notification cap centralized in adminnotify (MaxUnacknowledgedCriticalLogs) + applied at ALL insert sites (webhooks x2, jwt refresh_token_reuse, account erasure, sweep, twofa) with suppressed-insert logging; per-issue bucket for reissue alerts
- MED-HIGH: twofa.StateFor saturated state made IMMUTABLE (LastMintAt writes are no-ops; no cross-user throttling); eviction never drops in-window count>0 records
- MED: /register now uses the shared bcrypt semaphore (authBcryptSlots, 20) — botnet CPU burn bounded
- MED: NAT collateral reduced (429-reject only at top progressive tier; lower tiers sleep)
- MED: ClearMintCooldownForUser exposed for fresh-charge success; reissue cooldown-skip raises a capped alert
- LOW: audit coverage gaps (reschedule fee forgiveness, gift-card transfer, clawback) closed

DUP/MOD:
- Frontend deposit-percent literals -> POLICY constants (10 sites); LOYALTY_DISCOUNT_RATE single-sourced; generateUUID adopted; admin PaymentModal overflow-tip confirm path added; £500 gift-card cap named

Verified: 26/26 dev + 24/24 prod (CI condition), both vet tags, frontend tests+build, env-docs 42/42.
 2026-08-22 00:34:50 +01:00
popertots 3866cc5963 fix: round-2 loop-A fresh review (503c326 baseline) — B1 replay cap, A6 discount record, 2FA reissue+cooldown, notification flood, lockout saturation, VAT/refund-status consolidation 
Round 2 Loop A fresh money/security/dup-mod review. 23 findings fixed:

MONEY:
- CRITICAL: B1 duplicate auto-refund gains an attempt cap (b1_attempts col, cap 3) —
  a rejected auto-refund no longer re-replays the expired key every sweep run
  (which minted a stacking unauthorized charge each time); FAILED-webhook
  demotion respects the cap; never re-replay a key whose B1 refund failed
- HIGH: A6 deposit_covered_by_discount skip path now APPLIES the eligible
  campaign discount rows immediately (capped) instead of skipping with no
  discount recorded — no more promised-discount-not-recorded overcharge
- MEDIUM: 2FA code burned by the SAVE gate is re-issued on failed
  new-card+save_card charges (re-issue guard now covers req.SaveCard)
- LOW: GetBookingPaymentSummary excludes tip rows from paidAmount (remaining
  now matches the authoritative tip-excluded balance)

SECURITY:
- MEDIUM: unacknowledged CRITICAL admin-notification flood capped (global cap
  on critical_payment_log + refresh_token_reuse rows)
- MEDIUM: 2FA reissue no longer bypasses the mint cooldown (Check no longer
  clears LastMintAt on gate-verify; cleared on terminal charge success)
- MEDIUM: twofa.StateFor map-saturation returns a shared permanently-locked
  state instead of a fresh 5-guess budget per request
- MEDIUM: ProgressiveRateLimit rejects 429 past maxProgressiveSleepDelayMs
  instead of sleeping unboundedly; login bcrypt concurrency semaphore added
- LOW: loginInProgress 409->429; webhook key-set/URL-unset startup check;
  email-verification per-user attempt counter

DUP/MOD:
- formatCurrency single source (frontend format.ts, 7 files consolidated);
  SquareRefundStatusToLocal single source (errors.go, all sites); admin
  audit-log helper dedup; SCA retry model unified (proactive on all 6
  surfaces); buyDailyTotal/daily-cap mirror via backend; lock TTL from
  backend; generateUUID at all card-form sites; magic numbers named
  (defaultPostgresHost, epsilon, fee constants); admin CASH + gift-card
  terminal charges now audited; DAV_SKIP_INIT documented in manuals

Verified: 26/26 dev + 24/24 prod (GO_TESTING=1, the CI condition), both vet
tags, frontend tests+build, env-docs 42/42.
 2026-08-22 00:34:50 +01:00
popertots 36887167c6 fix: loop-A fresh review (503c326 baseline) — overflow-guard bypass, discounted-deposit retry, GDPR audit scrub, till cap, sweep rescue, 2FA reissue + SCA retry, consolidation round 
Loop A fresh money/security/dup-mod review of the whole payments overhaul. 28 consolidated findings fixed:

MONEY:
- HIGH-1: B12 overflow guard now uses the discounted obligation — a pre-start deposit can never mint an unintended tip; the discount is never truncated to £0 when the customer pays the discounted deposit
- HIGH-2: discounted-deposit pending-reuse retry compares pendingStoredAmountPence vs chargeAmount (the actual Square amount), not req.Amount — no more permanent amount_mismatch 400 on lost-response retries
- MEDIUM-3: sweep rescue now carves overflow as a tip record + runs completion side-effects (was booking overflow as service revenue, skipping completion)
- MEDIUM-4 (shared w/ security): admin_audit_log.admin_id made nullable + anonymize_user/delete_guest_user NULL it + scrub details.card_last4 — 2fa_fallback_charge PII no longer survives account deletion
- MEDIUM-5: till gift-card payment now passes the £5,000/day admin cap (giftcard_limits)
- LOW-6: expired gift-card balance surfaced as expired/zero in GetUserGiftCardBalance

SECURITY:
- 2FA single-use consume made atomic at verify time for all 5 saved-card gates (fresh charges consume; pending-reuse retries don't); deferred consumption removed
- reissueTwoFACodeAfterFailedCharge routed through the fail-closed issuance gate (pepper check, cooldown) + fresh-only semantics (only when a code was actually consumed)
- family-alive cache invalidated on the stale-family cleanup DELETE (no 30s warm window after expiry)
- frontend 503-retry no longer reuses a consumed 2FA code — aligns with backend re-issue

DUP/MOD:
- reissue helper single-sourced (5 call sites), squareRefundStatusToLocal (10 inline switches), writeChargeSnapshot (7 sites, immutability guard on gift-card/till), postChargeRecheck (3+1 sites), scanIdempotencySlot (2), applyVATToChargeRecord (3 patterns), user_saved_cards upsert (2), BuyGiftCard pending INSERT via service
- till completed-dedup now re-validates paymentHasLiveRefund (aligns with booking/tip/gift-card)
- frontend 402 idempotency-key regeneration added to PaymentModal (aligns with other CIT surfaces)
- PAYMENT_METHOD_SAVED_CARD constant standardised ('saved_card' everywhere)
- admin audit coverage added for AdminRefundBooking + gift-card buy/top-up
- audit-helper cross-package dedup (user/twofa.go now calls payments' exported insert)

Verified: 26/26 dev + 24/24 prod packages, both vet tags, frontend tests + build, gitleaks clean.
 2026-08-22 00:34:50 +01:00
popertots 9a182db932 fix: full-scope review — tip-inclusive amount_due, sweep deposit-strand, A6 clamp cap, B13 clawback, 2FA single-use, mint audit, account-deletion re-auth, refresh dedup 
Full-scope Loop A restart review (18 findings across money/security/dup-mod):

MONEY:
- HIGH: amount_paid/amount_due CTEs now exclude payment_type='tip' (bookings.go x6, today.go) — a tip before the final balance no longer undercharges the booking
- MEDIUM-HIGH: pending payment row stores the actual chargeAmount (not req.Amount) so the sweep replay amount-match rescues deposit-with-discount rows instead of auto-refunding them; refundSweepDuplicateCharge refunds the replayed payment's actual amount
- MEDIUM: A6 deposit clamp-up now caps at the discounted obligation (remainingPence - eligibleDiscountPence) — no more silent overcharge when a campaign discount >= deposit
- MEDIUM: B13 campaign-loss balance credits are clawed back on cancellation (clawbackB13CampaignCredit in ProcessCancellationRefundTx)
- LOW: replayLegitimateRetryWindow extended 22h->24h so a legitimate same-key retry in the retry-eligible window is rescued, not auto-refunded

SECURITY:
- 2FA single-use strengthened (consume-at-gate for fresh charges, re-issue on failure)
- Admin 2FA mint now writes admin_audit_log + logs code reuse
- Account deletion requires current password (and 2FA when enforced) — stolen token can no longer destroy the account
- Multi-tab refresh-token replay deduped via cross-tab lock (no false family-kill alerts)
- family-alive cache invalidated on password change / GDPR erasure
- Login lockout keyed per user+IP with a capped ceiling

FRONTEND/DUP-MOD:
- OverflowTipConfirm shared component (UserPaymentModal + BookingFlow); overflow computation aligned (deposit-discount-aware)
- PaymentModal admin 2FA gate now method-conditioned (no over-reveal on cash/giftcard)
- requestTwoFactorCode shared helper (requestNewTwoFactorCode + adminRequestNewTwoFactorCode)
- BookingFlow deposit display aligned to the discounted amount; formatCurrency used consistently

26/26 backend packages; 80/80 frontend tests + build; env-docs 41/41.
 2026-08-22 00:34:50 +01:00
popertots 7c424b28b8 fix: loop-B adversarial findings — tip-type double-charge, tip-refund capacity, loyalty stamp farming, gate ordering, auth amplification, admin audit log 
Loop B restart (money/security/dup-mod adversarial) fixes:
- CRITICAL: CreateTerminalPayment rejects payment_type='tip' (mirrors CreateBookingPayment) — a tip-typed admin charge no longer records the FULL amount as a tip and double-collects (all is-paid computations exclude tip rows)
- HIGH: tip refunds can no longer re-open booking capacity — refunded_total subqueries filter payment_type <> 'tip' (service.go) and RefundPayment rejects tip rows
- MEDIUM: loyalty-stamp farming closed — stamp award once-per-booking via loyalty_stamp_awarded_at column (init-script.sql) + existing same-day guard
- MEDIUM: CreateTipPayment/CreateBookingPayment 2FA gates moved AFTER the idempotency completed-dedup (code consumed only on new money paths; terminal path already correct) — lost-response retries return the completed payment instead of 400
- MEDIUM: replayRescueLowerBoundSkew widened to 5m (DB-clock-skew stranded originals now rescued)
- MEDIUM-1: verifyFamilyAlive DB amplification reduced via 30s bounded family-alive cache; admin route group rate-limited
- MEDIUM-3: admin saved-card charges now write admin_audit_log (handlers.go helper + till); [2FA] log line decoupled from user identity
- LOW-1: logout scoped to the presented token's family (no cross-session kill)
- LOW-2: refresh-reuse grace widened for same-IP replays
- LOW-4: squareEnvironmentMismatch enforced for empty env
- LOW-5: uuid.ts hard-fails on Math.random fallback (crypto.randomUUID)
- Cash/giftcard tip-enabled overflow mirrors the card-terminal carve

26/26 backend packages; 72/72 frontend tests + build; env-docs 41/41.
 2026-08-22 00:34:50 +01:00
popertots a6a4683b74 fix: review round — B1 clock-skew tolerance + re-poll escalation, refresh-token access-token revocation, shared 2FA composable, per-package-DB test alignment 
Three fresh reviews (money/security/dup-mod) cross-validated findings:
- MEDIUM: B1 'new charge' discrimination adds a lower-bound tolerance (replayRescueLowerBoundSkew) so a retained-key replay of the ORIGINAL charge (DB clock ahead of Square) is never auto-refunded; ambiguous margins leave PENDING + CRITICAL
- MEDIUM: B1 re-poll escalates after stalePendingB1RefundAge (48h) — FAILED/REJECTED refunds go terminal (fail parent, claw back till-sale funding, CRITICAL notification); no more unbounded re-polling / stranded parents without webhooks
- DRIFT-REAL: processManualPaymentGroup now checks PENDING/FAILED/REJECTED on the synchronous refund response (mirrors processChargeGroup/manual handler) — no more premature 'completed'
- HIGH: refresh-token family kill now also invalidates the attacker's freshly-minted ACCESS token — access tokens carry a family_id claim and VerifyToken rejects tokens whose family was deleted (GenerateTokenForFamily + family-alive check); 30s grace window for concurrent two-tab refresh (no false theft alert)
- LOW: 2FA mint endpoint returns remaining_seconds; in-memory 2FA counters documented; 90-day refresh expiry single-sourced (RefreshTokenLifetime + make_interval)
- Dup/mod: NEW shared useTwoFactorCodeForSavedCard Svelte composable replaces 6 surface copies of the 2FA gate logic (Request-a-new-code added to BookingFlow + TillPurchases); account page adopts generateUUID
- Test architecture: removed t.Parallel() from 8 global-SquareClient-swapping tests per Testing Architecture doc line 89 (B1 flaky-test lesson) — fixes within-package race
- SQL alias pence rename (total_cents/paid_cents -> total_pence/paid_pence)

26/26 backend packages; 72/72 frontend tests + build; env-docs 41/41.
 2026-08-22 00:34:50 +01:00
popertots 4d5d2cd381 fix: restart-loop-A findings — pending sweep refunds, tip carve on discounts, TOCTOU redemption, single-use 2FA code + mint endpoint, refresh-token family revocation, admin 2FA code UX 
Restart of Loop A (fresh review -> fix -> verify) findings from commit 5e967fa:
- B1: sweep auto-refund treats Square PENDING refunds as NON-terminal (row stays pending, no gift-card clawback, refunds row inserted for payments AND till_sales, re-polls the deterministic sweepdup- key); Square-less pre-pass exempts square_refund_id IS NOT NULL rows
- M4: terminal tip carve accounts for pending campaign discounts (headroom = total - pending - paid) so explicit tips aren't absorbed as service revenue; no-tip case stays a single record
- max_redemptions TOCTOU closed with atomic conditional UPDATE ... RETURNING; exhausted-at-apply surfaces campaign_fully_redeemed
- 2FA: verification code is single-use on the saved-card gate (VerifyForUser consume=true, interactive flows unaffected); new POST /api/user/2fa/code mints a fresh code for enabled users (RequireAuth + RequireNonGuest + mint cooldown + per-user limiter)
- Refresh tokens: family_id + used_at columns; reuse of an already-rotated token revokes the ENTIRE family and inserts a refresh_token_reuse admin alert; rotation mints descendants in the same family
- Frontend: 2FA code input + Request-a-new-code on all saved-card surfaces; admin modal keys code input to customer 2FA + 403 self-heal; tip-display note for pending discounts; 76 frontend tests
- Verified: all 26 backend packages pass, frontend build+tests green, env-docs 41/41
 2026-08-22 00:34:50 +01:00
popertots fe88f2084d fix: review-loop B — adversarial findings (sweep auto-refund, admin clamp, 2FA real challenge, opaque refresh tokens, gated client IP, GBP pence) 
Loop B aggressive adversarial round (3 attack agents) + fix + secondary + verification:
- CRITICAL: sweep replay auto-refunds provably-created-later duplicate charges (gated on parseable CreatedAt); 22h legitimate-retry window == 22h sweep cutoff (no dead zone)
- HIGH: admin Take Payment clamps to remaining obligation (cash/giftcard/saved-card/terminal); no unintended tip from overflow; campaign credit against remaining
- HIGH: /api/services/eligible-for/{id} requires auth + owner-or-admin (DOB/age + patch-test health-data leak closed)
- HIGH: opaque refresh-token rotation (login/refresh return {token, jti, refreshToken}; refresh REQUIRES opaque token; single-use rotation; logout revokes; access token rejected at refresh)
- HIGH: saved-card charges require a REAL 2FA verification code (B6/B10) — backend gate on all 8 charge paths + shared TwoFactorCodeInput frontend component on all 7 surfaces; 2FA gate is no longer setup-flag-only
- MEDIUM: ungated CF-Connecting-IP in reserve/admin_reserve gated via exported mw.ClientIP; 2FA limiter keyed on userID alone (no header-rotation bypass); ChangePassword actually revokes JTI + refresh tokens; 2FA setup mint cooldown + persistent failed-attempt counter; campaign redemption race surfaces campaign_fully_redeemed
- Terminal saved-card VAT applied (was under-collected); age-guard reconcile failures notify; isWeakJWTSecret entropy gate; gift-card redeem per-card counter + per-user limiter; webhook signature key startup validation
- NEW internal/twofa package (single source of truth breaking the payments<->user import cycle); consolidation of duplicate 2FA hash/verify
- Frontend: refresh-token storage + rotation, TwoFactorCodeInput component, amountPaidPence in admin modal, B5/B6/B10 contract wiring; 70 frontend tests
- Tests: loop_b_fixes_test.go, internal/twofa tests, updated auth/services/profile/twofa/mw tests

All 26 backend packages pass (incl. internal/twofa); frontend 70/70 + build clean; env-docs 41/41.
 2026-08-22 00:34:50 +01:00