Harden Square HTTP client and dev mock: status codes, token validation, deadline wire format

Add StatusCode/Category/Field to squareAPIError and an IsNotFound helper so 400/401/404/429/5xx are distinguishable structurally instead of by substring. Validate cnon:/ccof: token prefixes in createPayment/createCardOnFile (PCI parity with the mock). Reject ccof charges without customer_id in the mock so dev parity catches the production bug. Emit Deadline as the RFC 3339 duration (PT5M) and correct the deprecated-comment.
2026-08-22 00:34:49 +01:00
parent 54a5b1024e
commit 12af3af3b3
5 changed files with 399 additions and 39 deletions
@@ -96,7 +96,14 @@ func (c *httpClient) doJSON(ctx context.Context, method, path string, body, targ
 		if json.Unmarshal(respBody, &errResp) == nil && len(errResp.Errors) > 0 {
 			se := errResp.Errors[0]
 			msg := fmt.Sprintf("square: %s %s: [%s/%s] %s (field: %s)", method, path, se.Category, se.Code, se.Detail, se.Field)
-			return &squareAPIError{Code: se.Code, Detail: se.Detail, err: errors.New(msg)}
+			return &squareAPIError{
+				Code:       se.Code,
+				Detail:     se.Detail,
+				Category:   se.Category,
+				Field:      se.Field,
+				StatusCode: resp.StatusCode,
+				err:        errors.New(msg),
+			}
 		}
 		return fmt.Errorf("square: %s %s: HTTP %d: %s", method, path, resp.StatusCode, string(respBody))
 	}
@@ -228,8 +235,11 @@ type sqTerminalCheckout struct {
 	ReferenceID string   `json:"reference_id,omitempty"`
 	Note        string   `json:"note,omitempty"`
 	PaymentIDs  []string `json:"payment_ids,omitempty"`
-	// Deadline (deadline_duration) is deprecated in the TerminalCheckout API —
-	// retained read-only for informational purposes; harmless when set.
+	// Deadline (deadline_duration) is a LIVE TerminalCheckout field: an RFC 3339
+	// duration (e.g. "PT5M") telling the terminal how long the checkout stays
+	// active. Square defaults it to 5 minutes. It is NOT an absolute timestamp
+	// and NOT deprecated. Kept as a string because the app only copies it
+	// through to CheckoutResult.Deadline.
 	Deadline  string `json:"deadline_duration,omitempty"`
 	CreatedAt string `json:"created_at"`
 	UpdatedAt string `json:"updated_at"`
@@ -339,11 +349,38 @@ func validSquareID(id string) bool {
 	return true
 }

+// isTokenLike returns true for Square source_id tokens: cnon:xxx nonces and
+// ccof:xxx card IDs. Raw PANs (all digits) are NOT token-like and are rejected.
+// This is the single source of truth for token validation, shared by the real
+// HTTP client and the dev mock so PCI-DSS parity holds in both builds.
+func isTokenLike(s string) bool {
+	return strings.HasPrefix(s, "cnon:") || strings.HasPrefix(s, "ccof:")
+}
+
+// tokenPrefix returns a PCI-safe abbreviation of a card token for error and
+// log messages: the first 8 characters plus an ellipsis and the total length.
+// The full token (a raw PAN, single-use nonce, or card reference) must NEVER
+// be echoed — handlers log these errors, so embedding the raw value would land
+// client-submitted card data verbatim in server logs.
+func tokenPrefix(s string) string {
+	if len(s) > 8 {
+		return fmt.Sprintf("%s... (len %d)", s[:8], len(s))
+	}
+	return fmt.Sprintf("%s (len %d)", s, len(s))
+}
+
 func createPaymentHTTP(ctx context.Context, req CreatePaymentReq) (*PaymentResult, error) {
 	return createPaymentHTTPWithClient(ctx, req, newHTTPClient())
 }

 func createPaymentHTTPWithClient(ctx context.Context, req CreatePaymentReq, hc *httpClient) (*PaymentResult, error) {
+	// PCI-DSS parity with the dev mock: reject raw PANs before they reach
+	// Square. source_id must be a cnon: nonce or ccof: card ID — anything else
+	// (e.g. a plain card number) is refused client-side so no card data is ever
+	// sent to the API in a non-token form.
+	if !isTokenLike(req.SourceID) {
+		return nil, fmt.Errorf("square: invalid card token %s — use a card nonce (cnon:xxx) or card ID (ccof:xxx)", tokenPrefix(req.SourceID))
+	}
 	body := sqCreatePaymentRequest{
 		SourceID:          req.SourceID,
 		IdempotencyKey:    req.IdempotencyKey,
@@ -452,12 +489,19 @@ func getPaymentHTTPWithClient(ctx context.Context, paymentID string, hc *httpCli
 }

 // squareAPIError wraps a formatted Square API error while exposing the
-// structured Square error code so callers can classify definitive business
-// rejections (e.g. ErrRefundDeclined) vs ambiguous transport/server errors.
+// structured Square error code and the HTTP status code so callers can
+// classify definitive business rejections (e.g. ErrRefundDeclined) vs
+// ambiguous transport/server errors, and distinguish 400/401/429/500
+// structurally without parsing the message. Category/Field are captured from
+// Square's error payload (the wire error carries them; they were previously
+// dropped).
 type squareAPIError struct {
-	Code   string
-	Detail string
-	err    error
+	Code       string
+	Detail     string
+	Category   string
+	Field      string
+	StatusCode int
+	err        error
 }

 func (e *squareAPIError) Error() string { return e.err.Error() }
@@ -486,6 +530,54 @@ func ErrorDetail(err error) string {
 	return ""
 }

+// ErrorStatusCode returns the HTTP status code of the Square response carried
+// by err when err (or any error it wraps) is a *squareAPIError, and 0
+// otherwise. Callers can distinguish 400/401/429/500 structurally instead of
+// substring-matching "HTTP 400" etc.
+func ErrorStatusCode(err error) int {
+	var sqErr *squareAPIError
+	if errors.As(err, &sqErr) {
+		return sqErr.StatusCode
+	}
+	return 0
+}
+
+// ErrorCategory returns the Square error Category carried by err when err (or
+// any error it wraps) is a *squareAPIError, and "" otherwise.
+func ErrorCategory(err error) string {
+	var sqErr *squareAPIError
+	if errors.As(err, &sqErr) {
+		return sqErr.Category
+	}
+	return ""
+}
+
+// ErrorField returns the Square error Field carried by err when err (or any
+// error it wraps) is a *squareAPIError, and "" otherwise.
+func ErrorField(err error) string {
+	var sqErr *squareAPIError
+	if errors.As(err, &sqErr) {
+		return sqErr.Field
+	}
+	return ""
+}
+
+// IsNotFound reports whether err is a Square "not found" condition: the
+// structured NOT_FOUND error code, an HTTP 404 response status, or a plain
+// non-JSON 404 body (doJSON's fallback error message embeds "HTTP 404").
+// Callers use this to treat already-completed/unknown Square resources as
+// idempotent no-ops instead of substring-matching the error message.
+func IsNotFound(err error) bool {
+	if err == nil {
+		return false
+	}
+	var sqErr *squareAPIError
+	if errors.As(err, &sqErr) {
+		return sqErr.Code == "NOT_FOUND" || sqErr.StatusCode == http.StatusNotFound
+	}
+	return strings.Contains(err.Error(), "HTTP 404")
+}
+
 // Definitive Square refund rejection codes — the refund was declined and can
 // never succeed, so retrying is pointless and the refund record should be
 // marked 'failed'. Anything else (transport errors, 5xx) is left ambiguous so
@@ -561,6 +653,11 @@ func createCardOnFileHTTP(ctx context.Context, userID, cardToken, customerID str
 }

 func createCardOnFileHTTPWithClient(ctx context.Context, userID, cardToken, customerID string, hc *httpClient) (*CardOnFile, error) {
+	// PCI-DSS parity with the dev mock: source_id must be a cnon: nonce or
+	// ccof: card ID. A raw PAN is refused client-side before it reaches Square.
+	if !isTokenLike(cardToken) {
+		return nil, fmt.Errorf("square: invalid card token %s — use a card nonce (cnon:xxx) or card ID (ccof:xxx)", tokenPrefix(cardToken))
+	}

 	// Deterministic idempotency key derived from user + card (not time-based)
 	// so that retries with the same details don't create duplicate cards.
@@ -596,11 +693,13 @@ func getCardsOnFileHTTPWithClient(ctx context.Context, userID string, hc *httpCl
 	// Filter by reference_id natively: Square's List Cards API supports the
 	// reference_id query param, and cards are created with reference_id = the
 	// local user ID. customer_id is not used for the filter because a user may
-	// have no provisioned Square customer. List Cards pages at 25 cards, so
-	// loop on the cursor to avoid silently truncating a large saved-card list
-	// (N-10).
+	// have no provisioned Square customer. List Cards has NO limit param and
+	// pages at 25 cards per page, so loop on the cursor to avoid silently
+	// truncating a large saved-card list (N-10). The 20-page guard therefore
+	// caps out at 500 cards before warning.
 	var cards []CardOnFile
 	path := "/v2/cards?reference_id=" + url.QueryEscape(userID)
+	truncated := false
 	for page := 0; page < 20; page++ {
 		var resp sqListCardsResponse
 		if err := hc.doJSON(ctx, http.MethodGet, path, nil, &resp); err != nil {
@@ -612,8 +711,17 @@ func getCardsOnFileHTTPWithClient(ctx context.Context, userID string, hc *httpCl
 		if resp.Cursor == "" {
 			break
 		}
+		if page == 19 {
+			truncated = true
+		}
 		path = "/v2/cards?reference_id=" + url.QueryEscape(userID) + "&cursor=" + url.QueryEscape(resp.Cursor)
 	}
+	if truncated {
+		// 20 pages fetched and a cursor is still present — return what we
+		// collected rather than discarding partial results (mirrors the
+		// listRefunds 20-page guard's behavior).
+		log.Printf("[SQUARE] list cards for %s exceeded 20 pages (infinite-loop guard) — returning partial results: %d cards", userID, len(cards))
+	}
 	if cards == nil {
 		cards = []CardOnFile{}
 	}
@@ -667,11 +775,7 @@ func cancelCheckoutHTTPWithClient(ctx context.Context, checkoutID string, hc *ht
 	if err := hc.doJSON(ctx, http.MethodPost, "/v2/terminals/checkouts/"+checkoutID+"/cancel", nil, &resp); err != nil {
 		// Square returns 404 / NOT_FOUND when the checkout is already
 		// completed or canceled — that is a no-op, not a failure.
-		var sqErr *squareAPIError
-		if errors.As(err, &sqErr) && sqErr.Code == "NOT_FOUND" {
-			return nil
-		}
-		if strings.Contains(err.Error(), "HTTP 404") {
+		if IsNotFound(err) {
 			return nil
 		}
 		return err