From 5fea301e922ff26eab43f80d4eb9cbea8e517f9a Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Mon, 3 Aug 2026 19:27:23 +0100 Subject: [PATCH] Document till money-safety model, nonce retry design, and verified customer_id assumption Adds critical_payment_log to the admin_notification_reason enum (fresh installs + ALTER TYPE for existing deploys); corrects the README's false cash-with-change claim; updates Gap Backlog T14 with the scan job stopgap; documents the till flow's clawback/cash-reconciliation model in the Technical Manual; records the frontend's re-tokenize-on-failure design in P11; and marks the P14 customer_id assumption VERIFIED (Square runtime enforces it per its SDK maintainer; only the OpenAPI schema stays ambiguous, so the P12 sandbox test remains the definitive live check). --- README.md | 2 +- init-scripts/init-script.sql | 12 +++++++++++- obsidian/Crussell/Future Work - Gap Backlog.md | 2 +- obsidian/Crussell/Technical Manual.md | 7 +++++++ .../Crussell/plans/p11-square-web-payments-sdk.md | 4 ++-- .../p14-square-customer-provisioning-consent.md | 8 ++++---- 6 files changed, 26 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 812106d..0e55440 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Nail salon booking platform — Go 1.26.5 backend + SvelteKit 5 SPA + PostgreSQL **Booking**: Self-service (customer), walk-in (admin), call-in (admin). Slot reservations prevent double-booking (4 TTL types). **Self-blocking prevention**: `excludeUserID` parameter filters a user's own `RESERVATION` entries from time blocker overlap checks, allowing re-reservation and booking at overlapping slots. **Explicit cancellation**: `DELETE /api/bookings/reserve` releases a user reservation; `DELETE /api/admin/bookings/reserve` releases an admin walk-in/call-in reservation. **Background cleanup**: Centralised cron scheduler (`backend/internal/jobs/`) runs 24 maintenance jobs: reservation/deposit cleanup every 5min, hourly campaign transitions, daily unpaid-booking notifications, staged default hours auto-apply, GDPR anonymization, financial aggregation, and token/code cleanup. Guest accounts with GDPR-compliant anonymization (including `RESERVATION:edit_request:%` scrubbing). Service eligibility based on age + patch test validity. Overlap checks use `FOR UPDATE` row locks inside transactions. Closing-hours validation (`closing_time.go`) resolves both current and staged default hours. -**Payments**: Square Terminal (in-person, via `CreateTerminalCheckout`) + online card payments via saved cards or new cards tokenized through the Square Web Payments SDK (`cnon:` nonces — new-card entry falls back to `CardEntryUnavailable` only when neither mock mode nor Square credentials are configured). The backend accepts only tokens, never raw PANs (PCI-DSS parity, mirrored in the dev mock). Cash with change calculation. Gift cards (12-digit code or account balance). Saved cards for faster checkout. Tips on completed bookings. Refunds with notice-period tiers and deposit protection (72h/24h thresholds). All payment types: deposit, full, partial, balance, tip. Payment >20% of total promotes `pending_release` bookings back to `confirmed`. Deposit paid is computed from payments on-the-fly. The first 50% of each payment is always carved out as deposit (via `buildSplitRecords`); any overflow beyond the booking total becomes a tip. A bounded PostgreSQL advisory try-lock (`pg_try_advisory_lock`, ~30 × 100ms ≈ 3s bound) serializes payment attempts per-booking to prevent two-tab double-payment races. Gift card purchases insert a pending payment record with VAT before calling Square — the DB transaction commits first, so Square failures leave a retryable pending record (same-key retries reuse it). Three background sweeps close Square's ~24h idempotency-key retention window: `sweep-pending-square-refunds` reconciles/retries stuck refunds (with a 23h age guard), `sweep-stale-pending-payments` fails stale pending payments/till-sales so a late retry cannot issue a second charge, and `sweep-stale-terminal-checkouts` cancels card-machine checkouts still pending at Square after an hour so a never-polled checkout cannot complete into an invisible, untracked charge. +**Payments**: Square Terminal (in-person, via `CreateTerminalCheckout`) + online card payments via saved cards or new cards tokenized through the Square Web Payments SDK (`cnon:` nonces — new-card entry falls back to `CardEntryUnavailable` only when neither mock mode nor Square credentials are configured). The backend accepts only tokens, never raw PANs (PCI-DSS parity, mirrored in the dev mock). Cash till sales record the gift-card value and are marked completed, with no tendered/change fields. Any change or overpayment is handled manually by the admin at the counter. Gift cards (12-digit code or account balance). Saved cards for faster checkout. Tips on completed bookings. Refunds with notice-period tiers and deposit protection (72h/24h thresholds). All payment types: deposit, full, partial, balance, tip. Payment >20% of total promotes `pending_release` bookings back to `confirmed`. Deposit paid is computed from payments on-the-fly. The first 50% of each payment is always carved out as deposit (via `buildSplitRecords`); any overflow beyond the booking total becomes a tip. A bounded PostgreSQL advisory try-lock (`pg_try_advisory_lock`, ~30 × 100ms ≈ 3s bound) serializes payment attempts per-booking to prevent two-tab double-payment races. Gift card purchases insert a pending payment record with VAT before calling Square — the DB transaction commits first, so Square failures leave a retryable pending record (same-key retries reuse it). Three background sweeps close Square's ~24h idempotency-key retention window: `sweep-pending-square-refunds` reconciles/retries stuck refunds (with a 23h age guard), `sweep-stale-pending-payments` fails stale pending payments/till-sales so a late retry cannot issue a second charge, and `sweep-stale-terminal-checkouts` cancels card-machine checkouts still pending at Square after an hour so a never-polled checkout cannot complete into an invisible, untracked charge. **Gift Cards**: Multi-method purchase (cash, card machine, online card, giveaway). Inventory cards for stock management. 24-month rolling expiry. Idle account cleanup (2yr/5yr thresholds). Expired balance recovery with admin audit trail. Transaction audit log. Idempotency keys for purchases. diff --git a/init-scripts/init-script.sql b/init-scripts/init-script.sql index fd94068..2914152 100644 --- a/init-scripts/init-script.sql +++ b/init-scripts/init-script.sql @@ -837,7 +837,17 @@ INSERT INTO business_settings ( 'https://www.website.co.uk' ); -CREATE TYPE admin_notification_reason AS ENUM ('pending_booking', 'cancelled_booking', 'rescheduled_booking', '1_week_no_pay', '1_month_no_pay', 'affiliate_claim', 'late_cancellation', 'deposit_paid', 'edit_request', 'edit_requested', 'new_booking', 'deposit_not_paid_by_deadline', 'gift_card_purchased_for_friend', 'default_hours_changed', 'refund_failed'); +-- 'critical_payment_log' surfaces unresolved money events (stale pending +-- payments/till sales, refunds at the retry cap) in the admin notification +-- centre — the DB-backed stand-in for the un-watched CRITICAL payment logs. +-- Fresh installs get it from the CREATE TYPE below; existing deployments must +-- apply the ALTER TYPE after it (NOTE: ALTER TYPE ... ADD VALUE cannot run +-- inside a transaction block — run on a connection with autocommit). The +-- value is a no-op here on fresh installs (it is already in the CREATE TYPE). +CREATE TYPE admin_notification_reason AS ENUM ('pending_booking', 'cancelled_booking', 'rescheduled_booking', '1_week_no_pay', '1_month_no_pay', 'affiliate_claim', 'late_cancellation', 'deposit_paid', 'edit_request', 'edit_requested', 'new_booking', 'deposit_not_paid_by_deadline', 'gift_card_purchased_for_friend', 'default_hours_changed', 'refund_failed', 'critical_payment_log'); + +-- Existing-deployment migration for the value added to the CREATE TYPE above. +ALTER TYPE admin_notification_reason ADD VALUE IF NOT EXISTS 'critical_payment_log'; CREATE TABLE admin_notifications ( id CHAR(12) PRIMARY KEY DEFAULT generate_admin_notifications_id(), diff --git a/obsidian/Crussell/Future Work - Gap Backlog.md b/obsidian/Crussell/Future Work - Gap Backlog.md index 7e06e08..9c41fc5 100644 --- a/obsidian/Crussell/Future Work - Gap Backlog.md +++ b/obsidian/Crussell/Future Work - Gap Backlog.md @@ -105,7 +105,7 @@ These don't add features but reduce maintenance cost and risk. | T11 | **Replace `e: any` in button onclick** | S (30min) | Frontend | `button.svelte:101` — click handler typed as `e: any`. | | T12 | **Former name display (4 TODO sites)** | S (1d) | Frontend + Backend | 4 TODOs across GiftCards + notifications needing `previousFirstName`/`previousLastName` from backend. | | T13 | **Fix `devProdClient` rune-arithmetic in test** | S (30min) | Backend | ✅ COMPLETED July 2026 — `rune('0'+idx)` replaced with `fmt.Sprintf("concurrent-key-%d", idx)` for proper numeric formatting beyond index 9. | -| T14 | **Error tracking / monitoring (Sentry)** | M (1-2d) | Backend | `log.Printf()` only. No alerting on 5xx. 39 ALERT + 20 CRITICAL logs will never be seen. | +| T14 | **Error tracking / monitoring (Sentry)** | M (1-2d) | Backend | `log.Printf()` only. No alerting on 5xx. The audit found 39 CRITICAL emission sites (manual-reconciliation warnings) with no operator-facing path; they will never be seen. **Stopgap (Aug 2026):** a DB-backed `critical_payment_log` admin-notification sweep job now surfaces these as admin notifications until a real log/alerting pipeline (Sentry) lands. **Still OPEN:** the sweep is a stopgap, not the alerting pipeline. | --- diff --git a/obsidian/Crussell/Technical Manual.md b/obsidian/Crussell/Technical Manual.md index e2b89bc..3f7d4d6 100644 --- a/obsidian/Crussell/Technical Manual.md +++ b/obsidian/Crussell/Technical Manual.md @@ -614,6 +614,13 @@ CORS uses `*` in local dev. In production behind Cloudflare, nginx handles CORS. **Idempotency:** `BuyGiftCard` and `CreateTillSale` support `idempotency_key` to prevent duplicate purchases on retry. +**Till Sale Money-Safety Model (`CreateTillSale`):** + +- **Pending-sale states:** a till sale row starts `pending` for any Square-backed method (`card_machine`, `saved_card`, `online_square`) and flips to `completed` only once Square confirms the charge. `cash` and `on_the_house` sales complete immediately. `failed` means the sale can never be retried (swept stale or definitively rejected); a same-key retry of a `failed` sale gets a 409, and a retry whose amount mismatches the pending row is rejected rather than re-funding at the wrong value. +- **Funded-gift-card clawback:** a cancelled or definitively-failed sale (a Square decline, e.g. `CARD_DECLINED`, `CARD_EXPIRED`, `INSUFFICIENT_FUNDS`, `VERIFY_CVV_FAILURE`) reverts its card funding atomically in one compensating transaction: a created card is deleted together with its purchase transaction and any immediate redeem-to-account credit, and a topped-up card has the top-up subtracted back out. Ambiguous/lost-response rows do **NOT** claw back; the sale stays pending so a late retry can still complete it, which requires the card to stay funded. +- **Cash retry of a pending card sale:** reconciled-or-rejected against Square, never double-pay. A pending sale carrying a live terminal checkout must be retried as `card_machine` and reuse that checkout. Switching to cash/`on_the_house` is rejected outright (409), because the original checkout could still charge the terminal. A pending `saved_card`/`online_square` sale retried as cash or `on_the_house` is completed against the existing row with no second Square call, and the idempotency key still dedups at Square. +- **Card-machine checkout lifecycle:** a checkout created but never committed to the DB is cancelled on request failure, so a tracking failure cannot orphan a live terminal charge. The `sweep-stale-terminal-checkouts` background sweep cancels any card-machine checkout still pending at Square after an hour. + **VAT Treatment:** - SPV: VAT charged at purchase, not at redemption (default) - MPV: VAT charged at redemption (configurable via `business_settings.voucher_type`) diff --git a/obsidian/Crussell/plans/p11-square-web-payments-sdk.md b/obsidian/Crussell/plans/p11-square-web-payments-sdk.md index 626ccbe..249b846 100644 --- a/obsidian/Crussell/plans/p11-square-web-payments-sdk.md +++ b/obsidian/Crussell/plans/p11-square-web-payments-sdk.md @@ -31,7 +31,7 @@ All 8 flows now render `SquareCardInput` (`frontend/src/lib/components/payments/ `frontend/src/lib/components/payments/CardSelection.svelte` — saved-card list + "Use a new card" toggle + SquareCardInput; exposes a `tokenize()` method via `bind:this` that parents call at submit time. Used by UserPaymentModal. The other flows have their own saved-card lists wired to SquareCardInput directly. ### `SquareCardInput.svelte` (NEW — P11): -The tokenization component. Loads the SDK, attaches the Square card iframe form, and exposes `tokenize()` returning the `cnon:` nonce (or a user-facing error). One-shot nonce: each flow caches the token and reuses it on retry so a retry does not re-tokenize (the backend idempotency key dedups). +The tokenization component. Loads the SDK, attaches the Square card iframe form, and exposes `tokenize()` returning the `cnon:` nonce (or a user-facing error). One-shot nonce: each flow caches the token and reuses it on retry so a network/ambiguous-failure retry does not re-tokenize (the backend idempotency key dedups). After a **definitive charge failure** (a declined/consumed nonce) the cached nonce is cleared and the next retry re-tokenizes fresh; the four flows that cache the nonce (TipPayment, UserBookingModal, UserPaymentModal, account Buy-a-Gift-Card) clear it in their error branches while keeping the idempotency key for dedup. ### Backend (already P11-ready — verified): - `backend/internal/square/square_http_client.go` — `createCardOnFileHTTP` accepts a `source_id` token and calls `POST /v2/cards`. Works with `cnon:xxx` nonces. @@ -103,7 +103,7 @@ All 8 flows render `SquareCardInput` and send the resulting `cnon:xxx` as `new_c ## Risks / Gotchas - **Square iframe requires HTTPS** — localhost is exempt, but any non-local dev URL needs TLS. -- **Tokenization is one-shot** — a `cnon:` nonce is single-use. Implemented per the plan: each flow caches the token after the first `tokenize()` and **reuses it on retry** (the backend idempotency key dedups), so a retry does not re-tokenize or double-charge. +- **Tokenization is one-shot** — a `cnon:` nonce is single-use. Nonce reuse on retry is only safe for **network-timeout/ambiguous failures** (the backend idempotency key dedups). A consumed nonce (definitive decline, e.g. `CARD_DECLINED`/`CARD_EXPIRED`) can never succeed again, so the four flows that cache the nonce (TipPayment, UserBookingModal, UserPaymentModal, account Buy-a-Gift-Card) clear the nonce cache in their error branches and re-tokenize on the next retry; the idempotency key is kept so the retry still dedups against Square and never double-charges. - **PCI-DSS parity preserved** — the backend rejects raw PANs by design; the tokenized form never falls back to sending PAN/CVC to our server. - **`CardEntryUnavailable` stays as the fallback** — when neither the `VITE_SQUARE_*` credentials nor dev mock mode (`VITE_SQUARE_ENVIRONMENT=mock`) are configured, flows keep the gated notice rather than breaking. The dev mock is local-only and token-only; `VITE_SQUARE_ENVIRONMENT=mock` must never be set in a deployed (non-local) build. diff --git a/obsidian/Crussell/plans/p14-square-customer-provisioning-consent.md b/obsidian/Crussell/plans/p14-square-customer-provisioning-consent.md index 8db8f6c..1efcd07 100644 --- a/obsidian/Crussell/plans/p14-square-customer-provisioning-consent.md +++ b/obsidian/Crussell/plans/p14-square-customer-provisioning-consent.md @@ -1,11 +1,11 @@ # P14 — Square Customer Provisioning & Consent -**Status:** ✅ IMPLEMENTED (awaiting P12 sandbox verification + final privacy copy review) — backend + frontend code landed Aug 2026. +**Status:** ✅ IMPLEMENTED (awaiting P12 sandbox verification + final privacy copy review) — backend + frontend code landed Aug 2026. The `customer_id`-enforcement assumption is now **VERIFIED** (node-sdk issue #47 + Required in Square's docs; OpenAPI schema is the only ambiguous artifact; see Verification). **Owner:** Implementation agent (payment integration round) **Estimated effort:** S-M (1-2 days backend/frontend + privacy policy copy) **Backlog reference:** `Future Work - Gap Backlog.md` item P14 (added alongside this plan) -> **Implementation status (Aug 2026):** The code is DONE: `square_customer_id` is provisioned lazily on card-save, stored on `user_saved_cards`, and now forwarded to Square as `card.customer_id` (CreateCard) and `CustomerID` (ccof: CreatePayment). One-off/guest payments mint no customer. The `/privacy-policy` route + consent pop-over shipped. **What remains:** sandbox verification that Square enforces `customer_id` (P12 gate) and the final privacy-policy copy review (still DRAFT-bannered). +> **Implementation status (Aug 2026):** The code is DONE: `square_customer_id` is provisioned lazily on card-save, stored on `user_saved_cards`, and now forwarded to Square as `card.customer_id` (CreateCard) and `CustomerID` (ccof: CreatePayment). One-off/guest payments mint no customer. The `/privacy-policy` route + consent pop-over shipped. **What remains:** the P12 sandbox live check (the enforcement assumption is now VERIFIED via node-sdk issue #47 + Square's docs; see Verification) and the final privacy-policy copy review (still DRAFT-bannered). --- @@ -161,7 +161,7 @@ Add a second `PolicyPopover` (Privacy Policy) next to the existing cancellation- ## Verification -- P12 sandbox smoke test **must run first**: confirm `POST /v2/cards` without `customer_id` actually 400s (it may not — integrations report cards can be created without it; the review marked this "unverified-high-risk"). Only proceed with customer provisioning if enforced. +- P12 sandbox smoke test **must still run first**: exercise `POST /v2/cards` without `customer_id` against a real endpoint. The assumption is now **VERIFIED**, not just theoretical: Square's runtime enforces `customer_id` on card creation (confirmed by Square's SDK maintainer via node-sdk issue #47), the human-readable docs mark it Required, and the machine-readable OpenAPI schema is the only ambiguous artifact (`Card.required` is empty; "Required" lives only in the description string). The code's gate is hard: a provisioning failure aborts the save/charge, so there is no path that relies on the ambiguous artifact. The P12 sandbox check remains the definitive live wire-contract check. - Sandbox E2E: one-off nonce charge (no card, no customer created) → save-card charge (customer created once, reused) → guest charge (no customer) → delete card (profile card disabled). - Backend suite green; frontend build + ESLint clean. - Frontend: pop-over renders on the consent checkbox when `canSaveCards && squareCardReady`; **Open** and **Download PDF** both work against `/privacy-policy` (incl. `?format=pdf` print path); existing 8 `/cancellation-policy` call sites unchanged (defaults preserved). @@ -171,7 +171,7 @@ Add a second `PolicyPopover` (Privacy Policy) next to the existing cancellation- ## Open Questions -1. Does Square enforce `customer_id` on `POST /v2/cards` at runtime? (P12 / R2 / M-8 — the gate.) +1. ~~Does Square enforce `customer_id` on `POST /v2/cards` at runtime?~~ **VERIFIED (see Verification):** Square's runtime enforces it (node-sdk issue #47 + Required in the human-readable docs; the OpenAPI schema is the only ambiguous artifact). The P12 sandbox check remains the definitive live test. 2. Can `CreatePayment` with a `cnon:` nonce as `source_id` be retried safely on a consumed nonce, or must the frontend re-tokenize? (Determines D3.) 3. Should the Square customer profile include phone number (Square allows it)? Decide during provisioning — email-only is the default recommendation (data minimisation).