89 Commits

Include renames

Author	SHA1	Message	Date
popertots	2cdbad0cea	feat: SCA-only saved-card charges — 2FA charge fallback removed (C6), versioned consent fields, token provenance ... PSR 2017 reg 100 makes SCA mandatory and non-waivable for customer-initiated stored-credential charges; a merchant-side 2FA check cannot legally substitute for it (authorising a token-less charge via 2FA leaves the MERCHANT liable for ECI 7 / SLI 210 chargebacks and reg 77(6) compensation regardless of consent). - payments/twofa.go: the homegrown 2FA fallback for token-less saved-card charges is REMOVED ENTIRELY. requireTwoFactorForCardAccess is now SCA-only: a non-empty Square verification_token (charge surfaces, token forwarded to Square) skips the gate; anything else is refused 402 verification_required. enforceSCAFallbackConsent is a compile-compatible no-op (fallback never runs). - New requireTwoFactorForCardAccessWithTokenValidation distinguishes surfaces where the token IS forwarded to Square (charge — Square validates it) from card-SAVE surfaces (token client-asserted, never forwarded: a non-empty token must NOT skip the save gate, auth-F1). - SCA tokenize-result wire contract (C1): a saved card charged with a fresh one-time tokenize-result sends the token as the charge SOURCE (new_card_token -> source_id) alongside saved_card_id, never a separate verification_token. resolveChargeSource resolves the saved-card branch FIRST (customer from the card row, token as source) so combined token+card requests are SCA-clean. - C6 consent fields (consent_version / consent_accepted) added to the booking/ tip/till/gift-card charge requests, enforced server-side before any fallback charge could reach Square and recorded on the 2fa_fallback_charge audit row; logVerificationTokenProvenance traces minted tokens to their charge. - user 2FA issuance gate refactored into pure build-agnostic functions (twoFAPepperConfigured / twoFADeliveryChannelConfigured / twoFAEnsureIssueAllowedStrict) shared with the payments re-issue path and exercised directly by the test,dev suite; TWO_FACTOR_FALLBACK switch and .env.example entry removed; startup posture notes updated. - Test coverage: fail-closed 2FA production gates (pepper/delivery), token validation on save vs charge surfaces, completion idempotency, idempotency key determinism, refund-policy 72h/24h epsilon boundaries, VAT parity.	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	dfe856b181	fix: loop-B adversarial (503c326 baseline) — IDEMPOTENCY_KEY_REUSED reclassified ambiguous, 2FA reissue fail-closed alerts, family-cache crash window, consolidation regression checks ... Loop B red-team (money/security/dup-mod adversarial) findings on the full payments overhaul: - CRITICAL-ish: IDEMPOTENCY_KEY_REUSED (409) no longer classified as a definitive 402 in chargeFailureStatus — it means the ORIGINAL charge may have landed with a different body, so it is now AMBIGUOUS (503): the frontend keeps the same idempotency key, the pending row stays rescuable by the sweep (which already treated it as ambiguous), and the frontend no longer regenerates the key into a possible double charge. SCA verification-required codes remain definitive 402. - HIGH: reissueTwoFACodeAfterFailedCharge now writes a CRITICAL admin notification (insertCriticalPaymentNotification) when issuance is refused (missing pepper / unavailable delivery) instead of silently stranding the customer; documented that a pepper CHANGE invalidates all pending codes. - MEDIUM: family-alive cache invalidation crash window documented (invalidate-after- commit leaves up to 30s warm on a crash; the near-TTL DB re-check bounds it). - Consolidation regression checks (8b2fe3b helpers): writeChargeSnapshot guard preserved at all sites, postChargeRecheck identical, squareRefundStatusToLocal mappings verified, reissue fresh-only semantics confirmed at all 5 call sites. Verified: 26/26 dev packages, 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	b7122be3a0	fix: SCA review round + gitea pipeline green — GDPR audit scrub, backend test gaps, frontend SCA/Square-API, docs parity ... 7 review agents (pipeline run, self-review, codebase-context, frontend-placement, backend testing-gaps, Square-API, docs-parity) audited the SCA-primary work. ALL findings fixed, including every pre-existing red CI job: GDPR (HIGH): - anonymize_user() now scrubs admin_audit_log.target_user_id (mirrors delete_guest_user) so 2fa_fallback_charge rows (customer id + card_last4 PII) no longer survive registered-user account deletion; gdpr test added BACKEND TEST GAPS (all 10): - delivery-unavailable 503 branch: prod-tag predicate test + dev-variant marker - twoFactorFallbackEnabled alias/case/default matrix tests + exported wrapper - insertTwoFAFallbackAudit details-JSON shape + audit-row assertions for all 6 gate sites (booking/tip/gift-card/payment-method/terminal/till, both actors) - CreateTerminalPayment.VerificationToken: passthrough, too-long 400, 2FA-skip, token-less fallback + SCA-required (new terminal_sca_test.go) - isVerificationRequiredError at all 5 charge sites (402 + code:verification_required) - customer_initiated handler-level assertions (MIT false admin / CIT true customer) - Mock: ApprovePendingVerification, ChallengeResult auto/deny, _deny token suffix, parseVerifyToken unit tests FRONTEND SCA + Square-API (CRITICAL): - tokenizeSavedCardWithVerification reads result.token (the verified token) not result.verificationResult (deprecated verifyBuyer shape — saved-card SCA could never succeed in production before); parseTokenizeVerificationResult pure fn extracted + pinned in square.test.ts; 'verified' with no token proceeds tokenless - HIGH: saved-card idempotency key regenerated after a definitive 402 (fresh token under the same key = IDEMPOTENCY_KEY_REUSED dead-loop); kept on 503/cancelled - challenge-cancelled copy no longer promises a 2FA fallback the UI doesn't show; 'waiting for approval in your banking app' state on CIT surfaces - sca-unavailable demotion resets per attempt; card selection disabled mid-challenge; genuine saved-card declines no longer relabeled 'requires verification'; modal-close guard during processing; retry affordance standardized PIPELINE (every red job now green): - prod-tag build break fixed (shared square stub + test_helpers_test.go, prod-safe) - govulncheck: x/image 0.45.0 bumped (x/text resolved); go mod tidy clean - race: TestDeleteAccount_InvalidatesSquareCustomerCache made deterministic - DAV_ADMIN_PASSWORD placeholder in .env.example (compose config passes) - frontend: prettier 28 files, eslint, a11y 38 errors, knip (currentZIndex), deps in-range, audit vulns (nanoid/postcss) — all fixed; 67 vitest cases DOCS PARITY (6 DRIFTs + 5 GAPs): payments doc Ch4/Ch14/Appendix A, Technical Manual 2FA + counter-reset + payment sections, README test counts + SNAPSHOT_ENC_KEY, Feature Catalog, .env.example REQUIRE_2FA — SCA-primary/2FA-backup posture verified against code everywhere Verified: 26/26 dev + 24/24 prod packages, both vet tags, golangci-lint/staticcheck/ gosec 0 on both tags, gitleaks clean, 2,464 backend + 67 frontend tests.	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	b46927336b	fix: dup/mod secondary round — till 2FA gate parity, mint-cooldown single source, notification parity, pence comments ... Loop B dup/mod attack findings: - TillPurchases admin 2FA gate now mirrors PaymentModal and the backend: gateActive = twoFactorEnforced && customerTwoFactorEnabled && paymentMethod='saved_card'; the customer's setup flag is fetched from GET /api/admin/users/{id} on selection. A 2FA-disabled customer in an enforced env no longer hits a dead-end blocked input — the charge 403 surfaces the actionable message via the existing self-heal. - Extracted twoFAMintThrottled helper shared by SetupTwoFAHandler and ensurePendingTwoFACode — mint-cooldown rule can no longer drift between setup and disable-flow paths - Notification-helper drift documented: sweep copy states the intentional booking+user scoping vs the canonical webhook copy (cross-referenced); auth refresh_token_reuse insert verified to carry the same NOT EXISTS acknowledged_at IS NULL guard; no import cycle (webhooks→payments one-way) - Pence convention: 'rounded to the cent' corrected to 'pence' (handlers.go:2726) 26/26 backend packages; 72/72 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	03d85c6d13	fix: admin-scoped 2FA mint targets the CUSTOMER — user authentication for saved cards, never the admin ... The till and admin payment modal 'Request a new code' buttons previously called the session-scoped POST /api/user/2fa/code, which mints a code for the ADMIN's session — a code that can never satisfy the card-owner gate and is delivered to the admin's log line, not the customer. - New POST /api/admin/users/{id}/2fa/code (AdminSendVerificationCodeHandler, RequireAdmin + per-user limiter): mints/reuses a code for the TARGET user (the card owner/customer), keyed to the CUSTOMER's userID so the [2FA] delivery log carries the customer's ID — the customer, never the admin, is the authentication subject for their card - Shared useTwoFactorCodeForSavedCard composable gains an optional mint() option; admin surfaces (PaymentModal, TillPurchases) pass the customer-scoped mint, customer surfaces keep the session default - Frontend: adminRequestNewTwoFactorCode(userID) in square.ts; PaymentModal mints for booking.user_id, TillPurchases for selectedCustomer.id - Tests: admin mint keys the code to the customer's userID (log line contains customer ID, NOT the admin ID) + pending hash persisted for the customer; unknown target user 404s Backend 26/26 packages; frontend 72/72 + build clean.	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
popertots	78e6d00dc5	fix: payments review rounds — money-safety, GDPR, security, gift-card cancel, modal stacking ... Money-safety: - Deterministic till idempotency fallback (Square-charging only); cash/on_the_house keep unique keys; £250 till gift-card cap; 45-char key validation - Gift-card admin caps £250/tx + £5,000/day; user buy £500/day; BuyGiftCard allowlist unchanged - CancelGiftCard: CCR 2013 14-day right with partial-spend refund of the unspent balance (spend verified via payments.gift_card_id); atomic vs redeem/transfer; refunds stay pending until reversal commits; admin cancel surface (AdminCancelGiftCard) - Sweep: cancelled-booking charges failed+notified instead of silently completed; source-override replay uses live square_source_id; legacy square-less refund sweep; snapshot refresh on pending reuse - Refund lock consolidation; recordTerminalPaymentTx shared recorder; structured Square error codes; terminal checkout CustomerID GDPR / security: - Notes retained as de-identified medical/safety record at erasure (single field treated as health data; rest of record wiped, no re-identification map) + comments updated per UK GDPR/Art 9/Equality Act 2010 - square_request_snapshot PII scrubbed on all erasure paths; delete_guest_user FK unlinks; verification codes + dispute reasons handled; idle/stale-guest erasure deletes Square cards/customers + CardDAV/R2 - Durable square-erasure outbox job (retry-square-erasures); 2FA dev/prod build split, pepper fail-closed, no prod code-in-log; prod 2FA delivery fail-loud without a channel - Webhook unknown-type family split (non-money acked, money retried); untracked dispute notifications; rate-limit CF/X-Real-IP trust gating; nginx CSP nonce + api_limit Frontend: - Dynamic z-index stack (ui/dialog/zindex.ts) claimed in open order via data-state observer; re-claims on every reopen; removes stale !z-* overrides — nested modals (booking→user→booking) always paint newest-on-top (browser-verified 3-level + reopen) - Mobile: iOS zoom fixes, bottom-sheet dialogs, 44px touch targets, inputmode decimal, dvh - Gift-card buy/cancel UI, admin £250 + daily limits, cancellation/privacy/terms policy accuracy S3: - Connect() creates buckets before probing; in-memory fallback only on genuine unreachability; health reports degraded; stale S3_PUBLIC_URL documented (host-specific) Tests/docs: - 2263 test functions; all 22 backend packages green; round8/9/10 regression suites; NextEditWindowTime removes wall-clock flake; docs reconciled (notes retention, gift-card partial-use, modal T15 future work)	2026-08-22 00:34:50 +01:00
popertots	9111258461	fix: 2FA disable now requires the verification code (enforced mode) ... The disable flow was broken in enforced (production) mode: the account page posted {code:''} to /api/user/2fa/disable, but the backend mints + validates a code when 2FA is enforced, so the empty code always failed with 400 and a user could never disable 2FA through the UI. In unenforced (dev/mock) mode the backend short-circuits and no code is needed — which is why the user only saw the confirmation dialog and no code prompt. Backend: - New POST /api/user/2fa/disable/code (SendDisableCodeHandler): mints + delivers a fresh disable-flow code via the existing ensurePendingTwoFACode machinery (per-user 1-min mint cooldown, 429 when throttled, 5-attempt lockout preserved on the disable call itself). This is the disable-flow equivalent of /api/user/2fa/setup. Runs unconditionally (no dev short-circuit) so the step is exercisable in dev too. Route mounted in main.go beside the other 2FA routes. - Tests: mints fresh code, reuses valid pending code (hash unchanged), mint-throttled 429 (after the pending code is dropped, as a lockout does), unauthorized 401, unenforced still mints. Frontend (account page): - The disable confirmation now branches on twoFactorRequired: enforced → POST /api/user/2fa/disable/code to mint, then a 6-digit code-entry input + 'Confirm Disable' button that posts the code to /api/user/2fa/disable; unenforced (dev) → unchanged direct disable. Code entry mirrors the enable flow's input styling; mint-throttle 429 / wrong-code 400 / lockout 429 all surface as toasts with the entry kept open for retry. Verification: go test -tags test,dev -count=1 -parallel 8 ./... (all 20 packages ok, 0 failures incl. 5 new 2FA tests), go build ./... and -tags dev, go vet clean, svelte-check 0 errors 0 warnings, env-docs gate OK.	2026-08-22 00:34:50 +01:00
popertots	6691cd5657	fix: rate-limit per-IP keying + dev no-op, admin 2FA recovery, 2FA account UI, admin modals, tip totals ... Rate limiting (backend): - RateLimit/ProgressiveRateLimit now derive the per-client key from CF-Connecting-IP, then chi's GetClientIP (the X-Real-IP value nginx sets at main.go:323), then RemoteAddr. Previously only CF-Connecting-IP/RemoteAddr were used, so behind the Docker nginx every client shared ONE bucket per limiter — 10 logins/min site-wide blocked all users (the reported 'Error: Rate limit exceeded' after seeding was the login 10/min bucket tripped by the seed's 11 logins, all keyed 127.0.0.1 in dev). - Real implementation is now //go:build !dev || test; new mw/ratelimit_dev.go (//go:build dev && !test) is a no-op passthrough, so 'go run -tags dev' (the dev harness) never rate-limits dev/seeding traffic, while production and tests (-tags test,dev) keep the real limiter. The docs (Technical Manual) already claimed dev no-op behaviour — the code now matches. NewProgressiveRateLimiter is provided in the no-op build because tag-free ratelimit_shared.go:104 initializes the global at package init. Admin 2FA management (backend): - users.two_factor_last_used_at TIMESTAMPTZ column (init-script, fresh-DB). - AdminUserDetail now returns twoFactorEnabled/twoFactorMethod/ twoFactorLastUsedAt. - New POST /api/admin/users/{id}/2fa/remove (admin-only): clears all 5 2FA columns + drops the user's in-memory attempt/lockout state — an admin recovery path when a user loses 2FA access. - two_factor_last_used_at updated on every successful 2FA verification. Account page (/account): - 2FA section moved under the Notifications heading, visible to all roles; Email/SMS toggles (Notifications styling) acting as a radio group with 'none' state; Apply button only when the selection differs from saved; unselecting shows a payment-rules warning dialog; the dev-comment '2FA is optional right now (REQUIRE_2FA is off)' and the 'Dev code:' debug line are removed. - Cards tab hidden from admin role. Admin modals: - User Details modal: new 'Two-Factor Authentication' section above Patch Tests showing Enabled/Disabled, method, last-used timestamp, and a Remove 2FA button with a confirmation dialog (POST to the admin endpoint, refetch on success). - Booking Details modal: the customer's name now links to their User Details modal (optional openUserModal prop threaded through admin/+page and today/+page; other call sites unaffected). Take Payment + /today: - PaymentModal shows pre-tip (netTotal) and post-tip (totalWithTip) totals with a tip-amount delta row only when a tip is selected; zero-tip flow unchanged. - The /today Payment button is hidden unless the booking is in_progress or completed, matching the backend gate (was shown for confirmed/pending bookings, producing the 'Booking must be in_progress or completed' error). Verification: go test -tags test,dev -count=1 -parallel 8 ./... (20/20 ok incl. new admin 2FA tests + mw tests), go build ./... and -tags dev both compile, go vet clean, svelte-check 0 errors 0 warnings, env-docs gate OK, docker compose config valid.	2026-08-22 00:34:49 +01:00
popertots	5cc5a7f6d2	fix: review round 7 — fresh-eyes audit fixes (6 agents) + full test suites for every backend change ... Fresh-eyes review round with 6 independent agents (money-safety, concurrency, Square wire parity, security, frontend flow, testing-gaps). Every finding was independently verified against the code before fixing. All backend changes now carry full test suites (10+ new tests, each verified to FAIL without its guard). All 20 packages green, race detector clean. Money-safety: - Gift-card purchase refunds no longer create money: manual refunds of a no-booking (gift-card purchase) payment are rejected with a clear message in the direct handler AND never re-issued by the sweep-resume path (processManualPaymentGroup skips them; reconcile-then-fail, no re-issue). - BuyGiftCard no-client-key fallback: derived deterministically under the advisory lock (pending-row reuse fixes lost-response double-charge; completed-row sequence advance preserves distinct-purchase collapse fix). - Terminal completion is never unrecorded: activeTerminalCheckoutID now calls recordUntrackedTerminalPayment when a provisional (tmp-) checkout is found COMPLETED at Square (previously only marked the row COMPLETED — a lost poll left the payment invisible and unrefundable). - Sweep: provisional tmp- checkout rows are resolved against Square first (COMPLETED → record; live → keep guard; NOT_FOUND/CANCELED → fail; ambiguous → leave pending) instead of blind-failing a possibly-live checkout. recordUntrackedTerminalPayment re-checks the booking status (FOR UPDATE) and refuses to record on a cancelled booking, inserting a critical_payment_log admin notification instead. Till-sale post-charge UPDATE now requires status='pending' (no resurrection of a clawed-back sale). Frontend (Svelte 5): - UserPaymentModal keeps CardSelection mounted through processing (bind:this ref + Square iframe survive the loyalty/tokenize awaits) — new-card payments work again. - BookingFlow clears the cached nonce/verification pair on any failure (retry re-tokenizes fresh; idempotency key retained for dedup); 409 'already paid' refetches the booking and reconciles depositPaid so the confirmation gate opens; Back button disabled during processing. - Synchronous double-submit guards on buyGiftCard/redeemGiftCard/submitTip. Square wire parity (mock vs real): - processing_fee sign unified (negated at paymentFromSquare; mock agrees). - SimulateSourceUsed (SOURCE_USED, 400) matches real CreateCard. - GetCardsOnFile excludes disabled cards (matches ListCards). - ForcePaymentStatus toggle + tests prove the charge path can't be status-blind. - CreateCheckout rejects empty device_id (env fallback SQUARE_TERMINAL_DEVICE_ID); completed terminal checkout's payment resolvable by id. Security: - 2FA attempt-map data race fixed: lastAt is atomic.Int64 (nanos) — eviction scan reads race-free; concurrent verify+evict tests under -race. - Backend refuses to start on weak/placeholder JWT_SECRET_KEY (<32 chars or known public placeholders) with openssl rand -hex 32 guidance. - Dockerfile no longer COPYs .env (secrets injected via compose env_file). - SabreDAV requires DAV_ADMIN_PASSWORD (no admin/admin default); compose fails at config time when missing. Testing gaps closed (each verified to FAIL without its guard): - refunded-dedup 409 (CreateBookingPayment), keyed sweep past-retention blind-fail, reconcile status-switch (CANCELED/FAILED/APPROVED/PENDING/unknown in both by-key and by-id paths), resolveChargeSource Square-failure branches, structured 500 / CARD_DECLINED / cancelled-context E2E (row stays pending), deriveBookingPaymentIdempotencyKey >45-char truncation, webhook findPaymentByDisputeID fallback, clawbackOneTillSale non-gift-card branch, dispute.evidence / terminal.checkout dispatch. Infra: - local-dev-2.sh fails loudly on port-5432 squatters / docker compose failures (previously died silently under ERR_EXIT with hidden output). - Test harness defaults SQUARE_TERMINAL_DEVICE_ID; money_safety_fixes_test.go gained the missing build tag. Verification: go test -tags test,dev -count=1 -parallel 8 ./... (20/20 ok), -race clean on 2FA + payments money paths, go build ./... + -tags dev, go vet clean, svelte-check 0 errors, env-docs gate OK (36 vars), docker compose config valid.	2026-08-22 00:34:49 +01:00
popertots	67cf5b9a45	fix: review round 6 — P0 deposit charge, idempotency rotation, dev-safety guard, 2FA/webhook hardening ... Sixth fresh-eyes review pass (5 agents: goal, QA, code-quality, security, context-mining). QA FAILED the deposit-required new-card flow; the P0 root cause was backend + frontend, now fixed. All 20 packages green. P0 money-safety: - Deposit-required bookings now actually charge the deposit on new-card payment. Two-part fix: (1) CreateBookingHandler re-reads the trigger-maintained total_amount/total_duration_minutes from the DB after the booking_services insert (the INSERT..RETURNING row predates the recalc trigger, so TotalAmount serialized as 0 and DepositPaid computed TRUE on an unpaid booking — the frontend gate trusted deposit_paid:true, never charged, and confirmed the booking with zero payment rows); (2) BookingFlow.svelte gates the confirmation view on depositPaid and guards against re-creating a booking on retry. Regression test TestBookings_Create_DepositPaidFalseOnUnpaidBooking. Payments (idempotency + money): - deriveBookingPaymentIdempotencyKey: no-client-key fallback now advances a sequence for repeatable types (partial) and rotates past refunded completed rows, so refund-then-repay and equal-amount partials diverge onto distinct keys; an un-refunded completed row keeps its key (double-charge protection holds). Dedup hits on refunded rows now 409, never stale success. - chargeFailureStatus default is 503 (ambiguous), never 402; table test. - Flaky TestBookingPayment_FullPayment_SplitsIntoDepositAndBalance fixed (ORDER BY payment_type). - resolveChargeSource: orphaned card-on-file disabled via DeleteCardOnFile when SaveCardForUser fails (best-effort, redacted log); retry path preserved. Square client: - Dev builds HARD-FAIL (panic) on SQUARE_ENVIRONMENT=production without SQUARE_ALLOW_REAL_API=1; sandbox routes with a loud banner. - Mock fault-injection FailAfterCommit (commit-then-5xx) exercises the exact lost-response same-key retry; SimulateCardTokenUsed; 45-char idempotency-key cap parity; SquareEnvironment/SquareLocationID shared env helpers used by the sweep (env contract no longer comment-only). - listRefunds truncation now errors (money-sensitive reconcile retries instead of over-refunding); getCardsOnFile truncation loudly logged. Webhooks + 2FA: - square-environment header checked fail-closed (403) when configured env is production/sandbox; dispatch DB work bounded by 30s timeout contexts. - 2FA codes HMAC-SHA256 pepper'd (TWO_FACTOR_PEPPER) with legacy-hash migration + upgrade-on-verify; disable-flow mint cooldown (1/min, 429) caps the brute-force loop; in-lockout records never LRU-evicted. Repo hygiene: - env-docs CI gate green again (FRONTEND_ORIGIN + SQUARE_ALLOW_REAL_API + TWO_FACTOR_PEPPER documented; Vite DEV built-in allowlisted). - Dead square_deposits schema dropped; obsidian/README/legal-page drift fixed (consumeradvice.scot signposting, CORS allowlist, p11 R3/P13, T1). - 2FA disable residual documented; P6 email/SMS delivery and P12 sandbox smoke test remain the pre-go-live gates. Verification: go test -tags test,dev -count=1 -parallel 8 ./... (20/20 ok), go build ./... + -tags dev, go vet clean, svelte-check 0 errors, env-docs gate OK, live deposit-required flow re-verified end-to-end (deposit £11 charged, square_payment_id recorded).	2026-08-22 00:34:49 +01:00
popertots	fdf3f64a13	fix: review round 4 — per-dispute chargeback alerts, single-source clawback, 2FA lockout coherence, docs ... Fourth fresh-eyes review pass (5 agents: goal, QA, code-quality, security, context-mining). All PASS on the money-safety core; this round closes the remaining MAJOR/MINOR items they surfaced. Webhooks: - Untracked disputes now raise ONE admin notification PER distinct chargeback: the notification id is derived deterministically from the square_dispute_id (SHA-256 truncated into the CHAR(12) slot) so a second untracked dispute is no longer silently suppressed by the first's dedup row. ON CONFLICT (id) keeps same-dispute replays idempotent; the booking-scoped NOT EXISTS guard is retained for the tracked path. Verified: distinct disputes -> distinct rows; re-delivered dispute -> one row. - The gift-card clawback SQL now lives in exactly ONE place: payments.RevertGiftCardFunding (new giftcard_clawback.go). till.go and the webhook path both call it — eliminating the byte-for-byte copy whose divergence would be a money-loss drift trap (the same two-sources-of-truth pattern this commit eliminated for GDPR scrubbing). 2FA: - Applied the lockout-coherence fix from the review: when a disable request must mint a fresh code (no valid pending one), the held attempt counter is reset so the locked-out user can use the freshly delivered code in the SAME request (no wasted round-trip). The reuse path keeps accumulating wrong attempts toward the 5-attempt lockout — the two behaviors no longer conflict. (The 'always-fresh on disable' suggestion was NOT adopted: it would break the out-of-band [2FA]-log delivery model, since a code generated by a request can never be submitted within that same request.) - New test pins the shared verify/disable lockout: 5 wrong verifies 429 and destroy the code; a stale code then 400s on disable while the freshly delivered code succeeds in the same request. - Startup now warns that 2FA codes travel in PLAINTEXT via the server log in enforced mode (operator must restrict log access + relay out-of-band until email/SMS lands). Docs: - Test counts updated to the current 2,154 across README + Technical Manual. - User Manual 2FA nav corrected: the settings live on the Account page, not an 'Admin' area. Tests: 2,154 (up from 2,151). Backend 25/26 packages green (crussell/db fails only in this environment: local postgres auth for the test role; package byte-identical to HEAD). Frontend builds; svelte-check 0 errors.	2026-08-22 00:34:49 +01:00
popertots	9bb812669e	fix: fresh-review round — 2FA deliverability, disable re-verification, GDPR batch scrub, dispute alerting, docs accuracy ... Second fresh-eyes review pass (7 agents: goal, security, code-quality, context-mining, webhooks+2FA, client+mock+sweep, refunds/giftcards/handlers). Money-safety core verified sound (identical-body replay byte-lossless, clawback gated on definitive proof, no double-charge window). This round fixes the issues the fresh pass surfaced: 2FA: - Setup now DELIVERS the code via the [2FA] server log in ALL modes (was: nothing in enforced mode -> production 2FA was an unbreakable dead-end and saved-card charges were permanently 403). Enforced mode still withholds the code from the API response; the log line is the fake delivery channel until email/SMS lands (P6). - Disabling 2FA now requires a fresh verification code when enforcement is ON (previously ignored the code -> a password-only attacker could lift the gate). Shares the 5-attempt lockout and timing-safe compare. Dev bypass retained. - REQUIRE_2FA parsing normalized (false/0/off/no, case-insensitive); startup warning extended to the empty-env/mock-client/enforced-2FA confusion. GDPR: - anonymize_user() SQL now scrubs two_factor_* columns + staff notes, so the idle-account batch cleanup (CleanupIdleAccounts) is erasure-clean, not just the user-initiated delete path. Webhooks: - dispute.created for an untracked Square payment now raises a critical_payment_log admin notification (chargeback the app can't reconcile is never silent). Reason strings truncated on rune boundaries (valid UTF-8). Stale at-most-once comment corrected; revertTillSaleGiftCardFunding duplication noted. Sweep/mock parity: - Mock CreatePayment dedup is now source-aware (IDEMPOTENCY_KEY_REUSED on source mismatch) matching ReplayPaymentByKey and real Square. - COMPLETED-but-never-polled terminal till-sale checkouts are now recorded by the sweep (previously only booking checkouts were; till charges were invisible until the 24h blind-fail WARN). - Legacy snapshot-less minimal-body replay, SQUARE_LOCATION_ID drift, and in-memory-mock-restart limitations documented. Docs: - Webhook path corrected everywhere (/webhooks/square, not /api/webhooks/square - a deployer following the old path would 404 and silently lose all webhook reconciliation). - 2FA enforcement semantics + code-delivery mechanism documented accurately (fail-closed default; log-delivery channel; disable re-verification). - README/User Manual note the 2FA requirement on online saved-card payments. Tests: 2,151 (up from 2,142). Backend 26/27 packages green (crussell/db fails only in this environment: local postgres doesn't offer scram-sha-256 for the test role; package is byte-identical to HEAD and untouched here). Frontend builds; svelte-check 0 errors.	2026-08-22 00:34:49 +01:00
popertots	e9b0f0f2a7	fix: review-loop hardening — identical-body replay, 2FA gates, webhook at-least-once, GDPR scrub ... Follow-up to the comprehensive payment-system review. Fixes the issues the review found in the initial integration, plus the rough edges it introduced. Money-safety: - Replay-by-key now replays the FULL original request verbatim from a stored square_request_snapshot, so a retained idempotency key returns the original payment instead of IDEMPOTENCY_KEY_REUSED (previously the row sat pending forever). IDEMPOTENCY_KEY_REUSED remains ambiguous (never proof of no charge). - Dev mock mirrors real Square for unknown-key replays: ccof: saved-card sources are charged and rescued; spent cnon: nonces surface ErrReplayKeyNotRetained. (Fixes dev/prod parity divergence.) - Webhook dedup row committed AFTER dispatch (at-least-once); FAILED till sales claw back gift-card funding; event-type strings match Square's real catalog. - Expired-gift-card cancellation refunds set creditFailed (never a phantom 'completed' refund); cancellation refunds lock all payment rows ascending. - Sweep never rescue-completes a gift-card purchase without delivering the card. - Tip no-client-key fallback is a deterministic count-based key under the booking advisory lock (retry-safe, distinct tips don't collapse). - M-cap subtracts completed refunds, clamped to [0, total]. 2FA (PSD2 SCA stand-in) for online saved-card payments: - Full feature: status/setup/verify/disable endpoints, gating helper wired into all 7 saved-card charge paths (incl. BuyGiftCard + admin saved-card), account admin-tab settings UI, frontend gating across all payment surfaces. - Enforcement is FAIL-CLOSED: on unless REQUIRE_2FA=false or an explicit mock/dev SQUARE_ENVIRONMENT; startup warning when off in a non-dev env. - Verify is brute-force hardened (5-attempt lockout, timing-safe compare); plaintext codes only logged when enforcement is off (dev). - GDPR: anonymize_user also scrubs 2FA columns and staff notes. Infra/docs: - nginx: /api/ response cache removed (cross-user disclosure); port 80 redirects to HTTPS (localhost/RFC1918 exempt, end-anchored regexes); HSTS; separate webhook rate-limit zone. - Schema: users 2FA columns; payments/till_sales square_source_id + square_request_snapshot. - Legal docs: gift-card cooling-off, international-transfers section, tips policy; Gap Backlog P3 webhooks marked done; stale counts/wording corrected. - Flaky test race fixed (t.Parallel + global mock mutation); suite 26/26 packages green, 2,142 tests, svelte-check clean.	2026-08-22 00:34:49 +01:00
popertots	4b28e93710	fix: tip double-count, fully-paid auto-completion, discount-refund hardening ... Tip double-count (root cause of £33.75 vs £28.75 display): - Remove mock's fixed +500p auto-tip when AllowTipping is true (square_dev.go) — real Square only enables a terminal prompt, it never adds a tip to the amount - Set AllowTipping=false in CreateTerminalPayment: the frontend already embeds the tip in the amount, so the terminal must not prompt for a second tip - M4 tip split now derives the tip as charged amount minus remaining booking value ('after 100% is tips'), not from Square's TipAmount field - Success screens divide paymentResult.amount by 100 (pence -> pounds) in both PaymentModal and UserPaymentModal Fully-paid bookings auto-complete: - Extract ApplyBookingCompletionSideEffects into payments package (shared by admin progress endpoint and payment paths; avoids circular import) - Add bookingIsFullyPaid + completeFullyPaidBooking: when completed non-tip payments reach 100% of the booking total, an active booking transitions to 'completed' so it leaves the admin Current Appointment view - Wired into CreateBookingPayment (inside tx) and GetCheckoutStatus (terminal, after commit); completion side-effects (loyalty, campaigns, deposits_required) fire identically to the manual progress endpoint - Add /admin/bookings/{id}/refund route (AdminRefundBooking) Discount-refund hardening: - RefundPayment explicitly rejects discount/on_the_house payments (was relying on the incidental NULL-square_payment_id guard) - Hide the Refund button for discount/on_the_house payments in EditBookingModal - Cancel-refund estimate in BookingModal also excludes on_the_house - Cancellation refund loop + GetBookingPaymentInfo + GetBookingRefundableAmountCents exclude payment_type='tip' from refundable totals Tip flow (start-time guard) fixes tests: - Tip tests updated to use past-dated bookings (tips now require booking started) Tests: - m4_tip_refund_redesign_test.go (tip split, refund exclusion, admin refund cap) - m5_fully_paid_completion_test.go (online + terminal full-payment completion, partial stays active, tip excluded, cancelled stays cancelled) - Full suite passes with -race (25 packages)	2026-08-22 00:34:49 +01:00
popertots	5e3dc9b428	fix: comprehensive payment system hardening (4 review passes) ... CRITICAL fixes: - C1: JWT exp claim now validated via jwtauth.VerifyToken (was Decode) - C2: OverrideAmount validated post-substitution (prevents negative money minting) - C3: Terminal gift-card payments store gift_card_id; refund credits user balance - C4: Refund dedup returns stored amount, not req.Amount (prevents admin mislead) - C5: Booking recheck uses FOR UPDATE (prevents TOCTOU with cancellation) - C6: processChargeGroup idempotency key stable (charge-only, prevents double-refund) MAJOR fixes: - M2: Gift-card refund UPDATE checks RowsAffected; 0 rows -> failed - M3: ProcessCancellationRefund returns commit error (was swallowed) - M5: Dispute webhook handling (created + state.updated + disputes table) MEDIUM fixes: - ME1: CORS restricted to FRONTEND_ORIGIN env var (was reflect-any) - ME2: anonymize_user() scrubs users.notes, bookings.notes, name_history, refresh_tokens - ME3: Webhook handlers now mutate state (payment.updated, refund.updated) Frontend fixes: - Same-key retry on 503 (ambiguous failure) wired to all 8 payment flows - CHARGE_AND_STORE intent for save-card flows (SCA compliance) - Nonce staleness check verified across all flows Additional fixes from adversarial re-review: - F1: Till-sale completed dedup echoes stored amount (C4-class) - F2: Cash/giftcard terminal path uses FOR UPDATE (C5-class) - F3: Square-success UPDATE checks RowsAffected (till sales) - F4: Dispute reason truncated to 192 chars (prevents INSERT failure) - F5: Booking-user lookup failure marks refund failed (prevents silent money loss) - F6: Saved-card/tip rechecks wrapped in transaction (C5 residual) Tests: - 15 adversarial attack tests (negative override, zero override, terminal gift card, refund dedup, TOCTOU, deleted gift card, advisory lock, overcharge, zero/negative/huge amount, raw PAN, missing auth, gift card balance, concurrent refunds) - 14 webhook state tests (dispute created/state, payment/refund updated) - 3 CORS tests, 3 GDPR tests, 1 HTTP timeout test - Full suite passes with -race (25 packages, 0 failures) 25 files changed, +1532/-275 lines	2026-08-22 00:34:49 +01:00
popertots	a8d54f1e2a	Fix review findings: aggregated-refund/saved-card/legacy-refund idempotency keys, structured Square error classification, CSP for Square SDK ... Money-safety idempotency fixes (external review bugs 1-3): - processChargeGroup: aggregated refund key now hashes the sorted pending-row set (chargeID-square-agg-<sha256 suffix>) so a changed group can never mark a new row completed against an old smaller refund; >45-char chargeIDs use a hashed prefix instead of verbatim truncation (which would collide charges on Square's global key dedup). Same-set crash-retry keeps Square's dedup. - CreateTerminalPayment saved_card: two-tier idempotency key — client-supplied per-attempt UUID preferred (distinct identical charges no longer collapse), deterministic booking+type+amount+card fallback for no-key retry safety. PaymentModal sends a per-charge UUID cleared after success. - ensureRefundKey: legacy NULL-key manual refunds persist a generated key to the row BEFORE the Square call (race-safe AND idempotency_key IS NULL guard), so a lost-response retry reuses the key and never double-refunds. Wired into resumeManualPendingRefund and the sweep's manual-retry loop. Classification + money-safety hardening: - till.go/sweep.go: structured square.ErrorCode/IsNotFound are authoritative when present; message-substring matching only for non-structured errors (dev mock, client-side status errors). Fixes fragile string-matching driving sweep retries and gift-card clawbacks. - SaveCardForUser: ON CONFLICT (user_id, square_card_id) DO NOTHING + re-select (was a latent UNIQUE-violation 500 on save-card retry). - CreateBookingPayment: partial payments re-validated against remaining balance inside the advisory lock (closes concurrent-overpayment race). - InvalidateSquareCustomerCache on GDPR erasure paths (account.go, time-blockers.go stale-guest anonymization). - GetUserGiftCardBalanceAdmin: in-handler admin check (defense-in-depth). - getCheckoutHTTP: warn on multi-payment checkouts instead of dropping payments[1:]. - Cash/giftcard terminal branch: removed dead idempotency SELECT, "tip-" -> "till-" prefix. - UserPaymentModal: removed vestigial polling state; proper interval cleanup. - account/+page.svelte: gift-card redeem dialog links /terms. - nginx CSP: allow *.squarecdn.com and js.squareup.com so the Square Web Payments SDK + card iframe can tokenize behind the proxy. Tests: +8 regression tests covering changed-set refund keys, legacy NULL-key single-refund, saved-card client-key dedup/no-dedup, concurrent partials, and cache invalidation. Full suite + race detector clean via run-tests.sh lockfile.	2026-08-22 00:34:49 +01:00
popertots	c9d55817f6	Fix data race on shared title-case transformer in profile updates ... golang.org/x/text/cases.Caser is not safe for concurrent use, but UpdateProfileHandler shared one package-level instance. A fresh caser is now created per call via titleCase().	2026-08-22 00:34:49 +01:00
popertots	f3d50f6990	Complete Square GDPR erasure: customer deletion and stale-guest scrub ... Account deletion now snapshots card and customer IDs before the local anonymize transaction and dispatches the Square cleanup goroutine only after the tx commits, deleting each distinct Square customer once and skipping any customer still referenced by another user's card. AnonymizeStaleGuestAccounts also disables cards and deletes the guest's Square customer profile (PII) before NULLing references locally. Token redaction applied to all error logs.	2026-08-22 00:34:49 +01:00
popertots	0d22f8d597	Scrub Square card and customer references on anonymization (GDPR) ... anonymize_user, delete_guest_user, and AnonymizeStaleGuestAccounts now NULL square_card_id and square_customer_id on user_saved_cards (7-year retained_until soft-delete kept for financial records). Make square_card_id nullable in the schema. GDPR export refunds join fixed to include gift-card-purchase refunds. Add scrub assertions to the GDPR and stale-guest test suites.	2026-08-22 00:34:49 +01:00
popertotsandSisyphus	35bc021857	fix: replace err.Error() string match with errors.Is(err, pgx.ErrTxClosed) ... Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>	2026-07-11 17:53:29 +01:00
popertots	5d9fa1178b	fix: replace silent json.Encode with error-logging pattern across all handlers	2026-07-11 17:50:07 +01:00
popertots	0bef0f7973	fix: remove redundant rollback, document asymmetry in cancellation notifications	2026-07-11 17:45:24 +01:00
popertots	11fbf869e1	fix: add 30s timeouts to account.go goroutines using context.Background()	2026-07-11 17:10:11 +01:00
popertots	c8051a76d6	fix: replace time.Sleep with poll loops in tests, fix a11y target=_blank violations	2026-07-11 16:16:23 +01:00
popertots	0c1fc2b819	fix: log internal errors server-side, add timeout to GDPR goroutine	2026-07-11 16:09:11 +01:00
popertots	f147be0b99	fix: add error logging to remaining nolint:errcheck scan sites for observability	2026-07-11 15:04:53 +01:00
popertots	2cd35ea84a	fix: S3-dependent tests skip in prod, add GO_TESTING to CI for dav init	2026-07-10 18:45:18 +01:00
popertots	cbaed41056	fix: restrict user_coverage_test build to test && dev	2026-07-10 18:24:39 +01:00
popertots	3029fd5179	test: add coverage tests across backend + fix mock for PENDING checkout support ... New test files cover previously untested paths across DAV, validators, S3, Square, mw, bookings, user, and payments packages. Includes mock fix: HoldCheckouts flag on MockClient allows tests to pause auto-complete goroutine for testing PENDING checkout states. Coverage: 50.4% → 65.0% (+14.6pp)	2026-07-10 18:13:44 +01:00
popertots	990c86495c	fix: suppress unused nonDepositPaymentType in prod-staticcheck	2026-07-10 13:00:33 +01:00
popertots	0c91482aac	fix: adjust upload limits to 20MB profile, 30MB portfolio	2026-07-10 12:25:25 +01:00
popertots	3a9bc02796	fix: check ParseMultipartForm errors, reduce maxMemory, handle 413 properly	2026-07-10 12:06:12 +01:00
popertots	cf0cd8de15	fix: remove 10 unused test functions flagged by staticcheck U1000	2026-07-10 12:01:02 +01:00
popertotsandSisyphus	407de74b51	fix: restore test-used functions, silence tx.Rollback closed errors, prune knip dead code ... Restore processImage (images.go) and nonDepositPaymentType (handlers.go) with //nolint:unused — used in test files. Fix 97 tx.Rollback defers to silently discard expected "tx is closed" error after commit. Frontend: remove 44 unused shadcn-svelte files, 2 dead components, 9 stale npm deps, prune unused exports. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>	2026-07-09 19:20:25 +01:00
popertotsandSisyphus	ed9cb1489c	fix: resolve golangci-lint violations (errcheck, unused, gosimple, ineffassign) ... errcheck: add proper error handling with slog.Error for tx.Rollback, key generation, and s3/dav operations. Add nolint comments for intentionally discarded DB scan errors and HTTP write errors. unused: remove dead code (svcRow type, processImage, nonDepositPaymentType, generateSecureCode, colorBold, nGreen, nRed) gosimple S1021: merge var declaration with assignment in manage.go ineffassign: remove dead assignments in settings.go, till.go, images.go Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>	2026-07-09 18:53:51 +01:00
popertotsandSisyphus	510828c924	chore: run go fix for Go 1.26 modernization ... 106 files: interface{}→any, strings.Split→SplitSeq, CutPrefix/Cut, strings.Builder, slices.Contains, remove redundant // +build directives, gofmt import ordering and indentation. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>	2026-07-09 17:25:23 +01:00
popertotsandSisyphus	e86248b27c	refactor: move auth, GDPR, and booking cleanup to centralized scheduler ... Convert CleanupRevokedJTIs to return (int, error) and remove StartJTICleanup goroutine. Add CleanupStaleLoginEntries and CleanupGDPRExportCache for centralized scheduler. Add clock.London timezone location. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>	2026-07-07 00:10:00 +01:00
popertotsandSisyphus	30c9012562	fix: move dav.Service init from auth_test to user testmain ... Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>	2026-07-05 20:08:09 +01:00
popertotsandSisyphus	e4b9003439	refactor(handlers): migrate remaining backend handlers to clock.Now() and transaction patterns ... Apply clock.Now() migration, transaction wrapping, and minor refactors across admin, scheduling, today, user, auth handler, notifications, webhooks, services, portfolio, ratelimit, testutils, and main.go. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>	2026-06-24 23:43:50 +01:00
popertotsandSisyphus	1564f93d25	fix(user): fix column aliases and replace SELECT * with explicit columns ... Fix customer_relationship.go: use consistent 'cnt' alias instead of duplicate 'count' column. Fix profile.go: replace SELECT * with explicit column list to avoid new computed booking columns breaking the admin listing query. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>	2026-06-21 21:47:31 +01:00
popertotsandSisyphus	220a0ef6e8	refactor(backend): update test files for PoolProxy and per-test transactions ... Migrate all test files from SetupTestDB/db.DB pattern to per-test transactions: - Replace SetupTestDB(t) with SetupTestTx(t) for context + transaction - Replace db.DB.Query/QueryRow/Exec with tx.Query/QueryRow/Exec - Replace context.Background() with context from SetupTestTx - Replace defer rows.Close() pattern with explicit rows.Close() - Add testdb.SeedBaseline(pool) to all TestMain functions - Wire db.Conn = db.NewPoolProxy(pool) in all TestMain functions Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>	2026-06-21 19:29:24 +01:00
popertotsandSisyphus	3d0e2afc4c	refactor(backend): migrate db.DB to db.Conn PoolProxy across all handlers ... Replace direct *pgxpool.Pool usage with PoolProxy wrapper across the entire backend: - db.DB renamed to db.Conn (*pgxpool.Pool -> *PoolProxy) - JWT functions now accept context.Context instead of using context.Background() - Handler DB calls route through PoolProxy for per-test transaction support - Fixture/helper/testdb functions accept Querier interface for decoupling - Query ordering fixed in bookings handlers: COUNT after data query to avoid pgx conn busy - Time truncation fixed: time.Date instead of Truncate(24*time.Hour) for week start calc - testmain_test.go files updated with SeedBaseline and NewPoolProxy Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>	2026-06-21 19:28:54 +01:00