From f0099714ff5f0f9903e2c5839443269889064bf4 Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Mon, 3 Aug 2026 18:08:33 +0100 Subject: [PATCH] Harden Square HTTP client and dev mock: DeleteCustomer, response limits, token redaction Adds SquareClient.DeleteCustomer for GDPR erasure (DELETE /v2/customers/{id}, NOT_FOUND as no-op), bounds doJSON response reads to 1 MiB with rune-safe 500-byte error snippets, adds validCardID guard to the disable-card URL, makes paymentFromSquare card fields consistent when the card ID is empty, redacts ccof/cnon tokens in all log paths, and fixes the idempotency-key-length comment (45 chars for payments/refunds/cards, 64 only for terminal checkouts). --- backend/internal/square/square.go | 4 + backend/internal/square/square_dev.go | 26 +++- backend/internal/square/square_http_client.go | 120 ++++++++++++++++-- backend/internal/square/types.go | 6 + 4 files changed, 143 insertions(+), 13 deletions(-) diff --git a/backend/internal/square/square.go b/backend/internal/square/square.go index b011a60..7f8d314 100644 --- a/backend/internal/square/square.go +++ b/backend/internal/square/square.go @@ -39,6 +39,10 @@ func (p *ProdClient) CreateCustomer(ctx context.Context, name, email string) (*C return createCustomerHTTP(ctx, name, email) } +func (p *ProdClient) DeleteCustomer(ctx context.Context, customerID string) error { + return deleteCustomerHTTP(ctx, customerID) +} + func (p *ProdClient) CancelCheckout(ctx context.Context, checkoutID string) error { return cancelCheckoutHTTP(ctx, checkoutID) } diff --git a/backend/internal/square/square_dev.go b/backend/internal/square/square_dev.go index ccb4a81..3839ba6 100644 --- a/backend/internal/square/square_dev.go +++ b/backend/internal/square/square_dev.go @@ -69,6 +69,10 @@ func (d *devProdClient) GetPayment(ctx context.Context, paymentID string) (*Paym func (d *devProdClient) CreateCustomer(ctx context.Context, name, email string) (*CustomerResult, error) { return createCustomerHTTP(ctx, name, email) } + +func (d *devProdClient) DeleteCustomer(ctx context.Context, customerID string) error { + return deleteCustomerHTTP(ctx, customerID) +} func (d *devProdClient) CancelCheckout(ctx context.Context, checkoutID string) error { return cancelCheckoutHTTP(ctx, checkoutID) } @@ -566,7 +570,7 @@ func (m *MockClient) CreateCustomer(ctx context.Context, name, email string) (*C // the mock mirrors this by deduping on email so a retry returns the // original customer rather than creating a duplicate. if existing, ok := m.customers[email]; ok { - log.Printf("[SQUARE-MOCK] CreateCustomer dedup hit: email=%s → id=%s", redactedEmail(email), existing.ID) + log.Printf("[SQUARE-MOCK] CreateCustomer dedup hit: email=%s → id=%s", redactedEmail(email), tokenPrefix(existing.ID)) return existing, nil } @@ -577,10 +581,28 @@ func (m *MockClient) CreateCustomer(ctx context.Context, name, email string) (*C CreatedAt: clock.Now().UTC().Format(time.RFC3339), } m.customers[email] = customer - log.Printf("[SQUARE-MOCK] Customer created: id=%s, email=%s", customer.ID, redactedEmail(email)) + log.Printf("[SQUARE-MOCK] Customer created: id=%s, email=%s", tokenPrefix(customer.ID), redactedEmail(email)) return customer, nil } +func (m *MockClient) DeleteCustomer(ctx context.Context, customerID string) error { + log.Printf("[SQUARE-MOCK] DeleteCustomer: id=%s", tokenPrefix(customerID)) + + m.mu.Lock() + defer m.mu.Unlock() + + for email, customer := range m.customers { + if customer.ID == customerID { + delete(m.customers, email) + log.Printf("[SQUARE-MOCK] Customer deleted: id=%s", tokenPrefix(customerID)) + return nil + } + } + // Real Square returns 404 / NOT_FOUND for an already-deleted customer — + // mirror the prod semantics of idempotent re-deletion as a no-op. + return nil +} + func (m *MockClient) CancelCheckout(ctx context.Context, checkoutID string) error { log.Printf("[SQUARE-MOCK] CancelCheckout: id=%s", checkoutID) diff --git a/backend/internal/square/square_http_client.go b/backend/internal/square/square_http_client.go index a05e58e..d4e608c 100644 --- a/backend/internal/square/square_http_client.go +++ b/backend/internal/square/square_http_client.go @@ -31,6 +31,16 @@ const ( squareProductionURL = "https://connect.squareup.com" squareAPIVersion = "2026-05-20" defaultHTTPTimeout = 30 * time.Second + + // maxResponseBody caps how many bytes doJSON reads from a response. Square + // responses are normally a few KB; the cap guards against an OOM from a + // compromised/proxied Square endpoint streaming unbounded data. + maxResponseBody = 1 << 20 // 1 MiB + + // maxErrorBody caps the raw response body embedded in error messages. + // Handlers log these errors verbatim, so echoing more than a snippet risks + // leaking PII that Square may have mirrored from the request. + maxErrorBody = 500 ) // --------------------------------------------------------------------------- @@ -87,15 +97,23 @@ func (c *httpClient) doJSON(ctx context.Context, method, path string, body, targ } defer resp.Body.Close() - respBody, err := io.ReadAll(resp.Body) + // Read the response through a limit so a compromised/proxied Square + // endpoint cannot stream unbounded data into memory (OOM guard). If the + // limit is exceeded, the body is cut and callers get a truncation error + // rather than silently parsing a partial response. + respBody, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBody+1)) if err != nil { return fmt.Errorf("square: read response: %w", err) } + truncated := len(respBody) > maxResponseBody + if truncated { + respBody = respBody[:maxResponseBody] + } if resp.StatusCode >= 300 { var errResp struct{ Errors []SquareError `json:"errors"` } 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) + msg := fmt.Sprintf("square: %s %s: [%s/%s] %s (field: %s)", method, path, se.Category, se.Code, capBody(se.Detail), se.Field) return &squareAPIError{ Code: se.Code, Detail: se.Detail, @@ -105,7 +123,24 @@ func (c *httpClient) doJSON(ctx context.Context, method, path string, body, targ err: errors.New(msg), } } - return fmt.Errorf("square: %s %s: HTTP %d: %s", method, path, resp.StatusCode, string(respBody)) + if truncated { + return fmt.Errorf("square: %s %s: HTTP %d: response body exceeds %d bytes (truncated): %s", method, path, resp.StatusCode, maxResponseBody, capBody(string(respBody))) + } + return fmt.Errorf("square: %s %s: HTTP %d: %s", method, path, resp.StatusCode, capBody(string(respBody))) + } + if truncated { + // A truncated 2xx body must never be silently accepted as a partial + // success. Even when the first maxResponseBody bytes are still valid + // JSON (e.g. an array cut at an element boundary), Unmarshal succeeds + // and the caller would otherwise get a partial response with nil error. + // Real Square responses are a few KB, so this only fires against a + // compromised/proxied endpoint. + if target != nil && len(respBody) > 0 { + if err := json.Unmarshal(respBody, target); err != nil { + return fmt.Errorf("square: %s %s: response body exceeds %d bytes (truncated), cannot parse: %w", method, path, maxResponseBody, err) + } + } + return fmt.Errorf("square: %s %s: response body exceeds %d bytes (truncated)", method, path, maxResponseBody) } if target != nil && len(respBody) > 0 { if err := json.Unmarshal(respBody, target); err != nil { @@ -115,6 +150,20 @@ func (c *httpClient) doJSON(ctx context.Context, method, path string, body, targ return nil } +// capBody returns s truncated to maxErrorBody bytes with a truncation marker. +// Error messages are logged verbatim by handlers, so embedding more than a +// snippet of a (possibly echoed) response body risks leaking PII. +func capBody(s string) string { + if len(s) <= maxErrorBody { + return s + } + // Truncate on a rune boundary, not a raw byte slice: s[:maxErrorBody] can + // split a multi-byte UTF-8 sequence, producing invalid UTF-8 in an error + // message logged verbatim (mangles logs feeding UTF-8-sensitive tooling). + // ToValidUTF8 drops the partial rune at the cut. + return strings.ToValidUTF8(s[:maxErrorBody], "") + "... (truncated)" +} + // --------------------------------------------------------------------------- // Square JSON types — exact wire-format match with Square's REST API. // --------------------------------------------------------------------------- @@ -349,6 +398,18 @@ func validSquareID(id string) bool { return true } +// validCardID reports whether a card ID is safe to embed in a Square REST URL +// path segment. Card IDs (ccof:xxx) carry a "ccof:" colon prefix that plain +// Square IDs do not, so the prefix is stripped before the standard +// validSquareID charset check (which rejects ":"); everything after the +// prefix must still pass the same alphanumeric/_/- rule. +func validCardID(id string) bool { + if strings.HasPrefix(id, "ccof:") { + return validSquareID(id[len("ccof:"):]) + } + return validSquareID(id) +} + // 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 @@ -369,6 +430,13 @@ func tokenPrefix(s string) string { return fmt.Sprintf("%s (len %d)", s, len(s)) } +// TokenPrefix is the exported form of tokenPrefix, for packages outside +// internal/square (e.g. handlers) that log ccof:/cnon: card tokens. The full +// token must never reach logs. +func TokenPrefix(s string) string { + return tokenPrefix(s) +} + func createPaymentHTTP(ctx context.Context, req CreatePaymentReq) (*PaymentResult, error) { return createPaymentHTTPWithClient(ctx, req, newHTTPClient()) } @@ -662,8 +730,9 @@ func createCardOnFileHTTPWithClient(ctx context.Context, userID, cardToken, cust // Deterministic idempotency key derived from user + card (not time-based) // so that retries with the same details don't create duplicate cards. // SHA-256 hash prevents recovering the card token from the key itself. - // Truncated to ≤45 chars — Square's documented idempotency-key limit for - // /v2/cards (a full 64-hex hash would be rejected with a 400). + // Truncated to ≤45 chars — Square's idempotency-key limit is 45 chars for + // /v2/cards, /v2/payments, and /v2/refunds (64 only for + // /v2/terminals/checkouts). ikHash := sha256.Sum256([]byte(userID + "|" + cardToken)) body := sqCreateCardRequest{ IdempotencyKey: "card-" + fmt.Sprintf("%x", ikHash)[:38], @@ -729,6 +798,11 @@ func getCardsOnFileHTTPWithClient(ctx context.Context, userID string, hc *httpCl } func deleteCardOnFileHTTP(ctx context.Context, cardID string) error { + if !validCardID(cardID) { + // cardID is a DB-stored ccof: token — never echo the full value in an + // error (handlers log it verbatim). + return fmt.Errorf("square: invalid card id %s", tokenPrefix(cardID)) + } hc := newHTTPClient() var resp sqDisableCardResponse if err := hc.doJSON(ctx, http.MethodPost, "/v2/cards/"+cardID+"/disable", nil, &resp); err != nil { @@ -737,6 +811,25 @@ func deleteCardOnFileHTTP(ctx context.Context, cardID string) error { return nil } +func deleteCustomerHTTP(ctx context.Context, customerID string) error { + return deleteCustomerHTTPWithClient(ctx, customerID, newHTTPClient()) +} + +func deleteCustomerHTTPWithClient(ctx context.Context, customerID string, hc *httpClient) error { + if !validSquareID(customerID) { + return fmt.Errorf("square: invalid customer id %q", customerID) + } + if err := hc.doJSON(ctx, http.MethodDelete, "/v2/customers/"+customerID, nil, nil); err != nil { + // Square returns 404 / NOT_FOUND when the customer is already deleted — + // that is a no-op, not a failure (idempotent re-deletion on GDPR erasure). + if IsNotFound(err) { + return nil + } + return err + } + return nil +} + func createCustomerHTTP(ctx context.Context, name, email string) (*CustomerResult, error) { return createCustomerHTTPWithClient(ctx, name, email, newHTTPClient()) } @@ -745,7 +838,8 @@ func createCustomerHTTPWithClient(ctx context.Context, name, email string, hc *h // Deterministic idempotency key derived from the email (not time-based) // so retries with the same email don't create duplicate customers. SHA-256 // prevents recovering the email from the key. Truncated to ≤45 chars — - // Square's documented idempotency-key limit. + // Square's idempotency-key limit is 45 chars for /v2/cards, /v2/payments, + // and /v2/refunds (64 only for /v2/terminals/checkouts). ikHash := sha256.Sum256([]byte(email)) body := sqCreateCustomerRequest{ IdempotencyKey: "customer-" + fmt.Sprintf("%x", ikHash)[:35], @@ -816,12 +910,16 @@ func paymentFromSquare(sq *sqPayment) *PaymentResult { r.AVSStatus = cd.AVSStatus r.CardBrand = cd.Card.CardBrand r.CardLast4 = cd.Card.Last4 - // exp_month/exp_year ride on the card object — pointer set when present. - expMonth := cd.Card.ExpMonth - expYear := cd.Card.ExpYear - r.ExpMonth = &expMonth - r.ExpYear = &expYear + // exp_month/exp_year/fingerprint ride on the card object. When the + // card object is empty (ID == "") they are meaningless, so leave ALL of + // them nil — nil then consistently means "no card details present" + // (previously ExpMonth/ExpYear became 0/0 pointers while Fingerprint + // stayed nil, which was inconsistent). if cd.Card.ID != "" { + expMonth := cd.Card.ExpMonth + expYear := cd.Card.ExpYear + r.ExpMonth = &expMonth + r.ExpYear = &expYear r.CardFingerprint = cd.Card.Fingerprint } } diff --git a/backend/internal/square/types.go b/backend/internal/square/types.go index ad2be99..ef7232a 100644 --- a/backend/internal/square/types.go +++ b/backend/internal/square/types.go @@ -185,6 +185,12 @@ type SquareClient interface { // idempotency key (derived from email). CreateCustomer(ctx context.Context, name, email string) (*CustomerResult, error) + // DeleteCustomer deletes the Square customer profile (email/name PII) on + // GDPR account erasure. Square returns 404 / NOT_FOUND when the customer is + // already deleted — that is treated as a no-op, so DeleteCustomer returns + // nil. + DeleteCustomer(ctx context.Context, customerID string) error + // CancelCheckout cancels a pending terminal checkout. Square returns a // 404 / NOT_FOUND if the checkout is already completed or canceled — // that is treated as a no-op, so CancelCheckout returns nil.