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).
This commit is contained in:
@@ -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. |
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -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`)
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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).
|
||||
|
||||
|
||||
Reference in New Issue
Block a user