Add tests for Square client hardening and GDPR deletion

Covers DeleteCustomer (success + NOT_FOUND no-op), doJSON truncation (oversized bodies + rune-safe capBody), validCardID rejection without leaking the token, paymentFromSquare empty-card consistency, and mock-side customer-ID redaction in logs.
2026-08-22 00:34:49 +01:00
parent f0099714ff
commit 8ff77bc39b
2 changed files with 347 additions and 2 deletions
@@ -797,6 +797,58 @@ func TestDevClient_CreateCustomer_EmptyEmail(t *testing.T) {
 	assert.Contains(t, err.Error(), "email")
 }

+func TestDevClient_DeleteCustomer(t *testing.T) {
+	client := NewDevClient().(*MockClient)
+	ctx := context.Background()
+
+	cust, err := client.CreateCustomer(ctx, "Jane Doe", "jane@example.com")
+	require.NoError(t, err)
+
+	err = client.DeleteCustomer(ctx, cust.ID)
+	require.NoError(t, err)
+
+	client.mu.RLock()
+	defer client.mu.RUnlock()
+	assert.Len(t, client.customers, 0, "deleted customer must be removed from the mock store")
+}
+
+func TestDevClient_DeleteCustomer_NotFoundIsNoop(t *testing.T) {
+	client := NewDevClient().(*MockClient)
+	ctx := context.Background()
+
+	// Deleting a customer the mock never created mirrors Square's NOT_FOUND —
+	// idempotent re-deletion must return nil (GDPR re-runs are safe).
+	err := client.DeleteCustomer(ctx, "cus_missing")
+	require.NoError(t, err)
+}
+
+// TestDevClient_CustomerID_RedactedInLogs verifies the mock never logs a full
+// customer ID (S-2 convention): DeleteCustomer's entry/success lines and
+// CreateCustomer's dedup-hit/created lines all use the tokenPrefix redaction,
+// mirroring how prod redacts ccof: tokens.
+func TestDevClient_CustomerID_RedactedInLogs(t *testing.T) {
+	client := NewDevClient().(*MockClient)
+	ctx := context.Background()
+
+	cust, err := client.CreateCustomer(ctx, "Jane Doe", "jane@example.com")
+	require.NoError(t, err)
+
+	var buf bytes.Buffer
+	log.SetOutput(&buf)
+	defer log.SetOutput(os.Stderr)
+
+	err = client.DeleteCustomer(ctx, cust.ID)
+	require.NoError(t, err)
+
+	logs := buf.String()
+	if strings.Contains(logs, cust.ID) {
+		t.Errorf("full customer id %q leaked into mock logs: %q", cust.ID, logs)
+	}
+	if !strings.Contains(logs, tokenPrefix(cust.ID)) {
+		t.Errorf("expected redacted customer id %q in logs, got %q", tokenPrefix(cust.ID), logs)
+	}
+}
+
 func TestDevClient_CancelCheckout_CancelsPending(t *testing.T) {
 	client := NewDevClient().(*MockClient)
 	client.HoldCheckouts = true
@@ -13,6 +13,7 @@ import (
 	"strings"
 	"testing"
 	"time"
+	"unicode/utf8"
 )

 func TestPaymentFromSquare_ElseBranch_SurfacesBrandWithoutCardID(t *testing.T) {
@@ -25,6 +26,8 @@ func TestPaymentFromSquare_ElseBranch_SurfacesBrandWithoutCardID(t *testing.T) {
 				ID:        "",
 				CardBrand: "VISA",
 				Last4:     "4242",
+				ExpMonth:  12,
+				ExpYear:   2030,
 			},
 		},
 	}
@@ -36,6 +39,18 @@ func TestPaymentFromSquare_ElseBranch_SurfacesBrandWithoutCardID(t *testing.T) {
 	if result.CardLast4 != "4242" {
 		t.Errorf("expected CardLast4 4242, got %s", result.CardLast4)
 	}
+	// When the card object is empty (ID == "") the card-details fields must be
+	// left nil consistently — a 0/0 expiry with a nil fingerprint was the old
+	// inconsistent behaviour.
+	if result.ExpMonth != nil {
+		t.Errorf("expected nil ExpMonth when card ID is empty, got %d", *result.ExpMonth)
+	}
+	if result.ExpYear != nil {
+		t.Errorf("expected nil ExpYear when card ID is empty, got %d", *result.ExpYear)
+	}
+	if result.CardFingerprint != "" {
+		t.Errorf("expected empty CardFingerprint when card ID is empty, got %s", result.CardFingerprint)
+	}
 }

 func TestPaymentFromSquare_NilCardDetails(t *testing.T) {
@@ -1236,3 +1251,281 @@ func TestIsTokenLike(t *testing.T) {
 		}
 	}
 }
+
+// TestTokenPrefix_Redacts verifies the PCI-safe abbreviation: the full token
+// never appears, only the first 8 chars plus the length.
+func TestTokenPrefix_Redacts(t *testing.T) {
+	token := "ccof:secret_token_abc123"
+	redacted := TokenPrefix(token)
+	if strings.Contains(redacted, "ccof:secret") {
+		t.Errorf("TokenPrefix leaked more than the first 8 chars: %q", redacted)
+	}
+	if !strings.HasPrefix(redacted, "ccof:sec") {
+		t.Errorf("expected first 8 chars prefix, got %q", redacted)
+	}
+	if !strings.Contains(redacted, "len 24") {
+		t.Errorf("expected total length in abbreviation, got %q", redacted)
+	}
+}
+
+// TestDoJSON_OversizedResponseTruncated covers the doJSON response-body limit:
+// an oversized body is cut at maxResponseBody and errors surface truncation
+// instead of embedding garbage or an unbounded body.
+func TestDoJSON_OversizedResponseTruncated(t *testing.T) {
+	t.Run("oversized_non_json_error_is_capped", func(t *testing.T) {
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusInternalServerError)
+			_, _ = w.Write([]byte(strings.Repeat("A", maxResponseBody+100)))
+		}))
+		defer srv.Close()
+
+		hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()}
+		err := hc.doJSON(context.Background(), http.MethodPost, "/v2/payments", nil, nil)
+		if err == nil {
+			t.Fatal("expected error")
+		}
+		msg := err.Error()
+		if !strings.Contains(msg, "HTTP 500") {
+			t.Errorf("expected HTTP 500 in error, got %q", msg)
+		}
+		if !strings.Contains(msg, "truncated") {
+			t.Errorf("expected truncation note in error, got %q", msg)
+		}
+		// The embedded body snippet must be capped well below the full body.
+		if len(msg) > maxErrorBody*3 {
+			t.Errorf("error message embeds too much of the response body (%d bytes)", len(msg))
+		}
+	})
+
+	t.Run("oversized_success_json_unmarshal_fails_with_truncation", func(t *testing.T) {
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(`{"payment":{"id":"` + strings.Repeat("x", maxResponseBody) + `"}}`))
+		}))
+		defer srv.Close()
+
+		hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()}
+		var target sqCreatePaymentResponse
+		err := hc.doJSON(context.Background(), http.MethodPost, "/v2/payments", nil, &target)
+		if err == nil {
+			t.Fatal("expected error for truncated oversized body")
+		}
+		if !strings.Contains(err.Error(), "truncated") {
+			t.Errorf("expected truncation error, got %q", err.Error())
+		}
+	})
+
+	t.Run("valid_json_prefix_is_still_truncation_error", func(t *testing.T) {
+		// A 2xx body over maxResponseBody whose first 1 MiB prefix is itself
+		// valid JSON (e.g. {"ok":true} padded to the cap, then a huge trailing
+		// JSON object) must return a truncation error, NEVER a silent partial
+		// success. Unmarshal would succeed on the prefix alone, so the
+		// truncation check must fire independently of the unmarshal result.
+		prefix := `{"ok":true}`
+		padding := strings.Repeat(" ", maxResponseBody-len(prefix))
+		trailer := `{"pad":"` + strings.Repeat("x", maxResponseBody) + `"}`
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(prefix + padding + trailer))
+		}))
+		defer srv.Close()
+
+		hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()}
+		var target sqCreatePaymentResponse
+		err := hc.doJSON(context.Background(), http.MethodPost, "/v2/payments", nil, &target)
+		if err == nil {
+			t.Fatal("expected truncation error for oversized body whose prefix is valid JSON")
+		}
+		if !strings.Contains(err.Error(), "truncated") {
+			t.Errorf("expected truncation error, got %q", err.Error())
+		}
+	})
+
+	t.Run("structured_error_detail_capped_in_message_only", func(t *testing.T) {
+		fullDetail := "echoed-input-" + strings.Repeat("A", 600)
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusBadRequest)
+			_, _ = w.Write([]byte(`{"errors":[{"category":"PAYMENT_METHOD_ERROR","code":"REFUND_DECLINED","detail":` + fmt.Sprintf("%q", fullDetail) + `,"field":"payment_id"}]}`))
+		}))
+		defer srv.Close()
+
+		hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()}
+		err := hc.doJSON(context.Background(), http.MethodPost, "/v2/refunds", nil, nil)
+		if err == nil {
+			t.Fatal("expected error")
+		}
+		var sqErr *squareAPIError
+		if !errors.As(err, &sqErr) {
+			t.Fatalf("expected *squareAPIError, got %T", err)
+		}
+		// The message (what handlers log) must be capped...
+		if len(sqErr.err.Error()) > maxErrorBody*3 {
+			t.Errorf("error message embeds too much of the detail (%d bytes)", len(sqErr.err.Error()))
+		}
+		// ...while the structured Detail stays intact for ErrorDetail() callers.
+		if sqErr.Detail != fullDetail {
+			t.Errorf("expected full Detail preserved for ErrorDetail, got %d bytes", len(sqErr.Detail))
+		}
+	})
+}
+
+// TestCapBody_RuneSafeTruncation verifies capBody never splits a multi-byte
+// UTF-8 rune at the maxErrorBody cut: the result must always be valid UTF-8
+// even when the byte cut lands mid-rune (handlers log these messages verbatim,
+// and a half-encoded rune would mangle logs feeding UTF-8-sensitive tooling).
+func TestCapBody_RuneSafeTruncation(t *testing.T) {
+	t.Run("cut_lands_mid_rune", func(t *testing.T) {
+		// 499 ASCII bytes + a 3-byte rune (€) starting at byte 499: a raw
+		// s[:maxErrorBody] cut would slice the rune in half.
+		s := strings.Repeat("a", maxErrorBody-1) + "€" + strings.Repeat("b", maxErrorBody)
+		got := capBody(s)
+		if !utf8.ValidString(got) {
+			t.Errorf("capBody result is invalid UTF-8: %q", got)
+		}
+		if !strings.HasSuffix(got, "... (truncated)") {
+			t.Errorf("expected truncation marker, got %q", got)
+		}
+		prefix := strings.TrimSuffix(got, "... (truncated)")
+		if len(prefix) > maxErrorBody {
+			t.Errorf("capped prefix is %d bytes, exceeds cap %d", len(prefix), maxErrorBody)
+		}
+		// The split rune must be dropped at the cut, not mangled.
+		if strings.Contains(got, "€") {
+			t.Errorf("expected the split rune to be dropped, got %q", got)
+		}
+	})
+
+	t.Run("cut_on_clean_rune_edge_is_preserved", func(t *testing.T) {
+		s := strings.Repeat("a", maxErrorBody) + "rest"
+		got := capBody(s)
+		if got != strings.Repeat("a", maxErrorBody)+"... (truncated)" {
+			t.Errorf("expected exact %d-byte prefix preserved, got %q", maxErrorBody, got)
+		}
+	})
+
+	t.Run("within_cap_is_unchanged", func(t *testing.T) {
+		s := "short body with €"
+		if got := capBody(s); got != s {
+			t.Errorf("expected short input unchanged, got %q", got)
+		}
+	})
+}
+
+// TestDeleteCardOnFileHTTP_RejectsInvalidID verifies the deleteCardOnFileHTTP
+// path-segment guard: an invalid card id errors before any HTTP call, and the
+// error redacts the full ID (it is a DB-stored ccof: token) rather than
+// echoing it verbatim.
+func TestDeleteCardOnFileHTTP_RejectsInvalidID(t *testing.T) {
+	invalidID := "bad/card/id/path/traversal" // long enough that tokenPrefix must truncate it
+	err := deleteCardOnFileHTTP(context.Background(), invalidID)
+	if err == nil || !strings.Contains(err.Error(), "invalid card id") {
+		t.Fatalf("expected invalid card id error, got %v", err)
+	}
+	if strings.Contains(err.Error(), invalidID) {
+		t.Errorf("error must not embed the full invalid card id: %q", err.Error())
+	}
+}
+
+// TestValidCardID covers the ccof:-aware card-id check used by
+// deleteCardOnFileHTTP: the ccof: prefix is stripped before the standard
+// charset rule, so legitimate card IDs pass while path-traversal rejects.
+func TestValidCardID(t *testing.T) {
+	valid := []string{
+		"ccof:abc_123",
+		"ccof:ABC-def",
+		"plain_id_1",
+	}
+	invalid := []string{
+		"",
+		"ccof:",
+		"ccof:bad/id",
+		"ccof:bad..id",
+		"bad/id",
+		"bad?id",
+		strings.Repeat("a", 65),
+	}
+	for _, id := range valid {
+		if !validCardID(id) {
+			t.Errorf("expected %q to be a valid card ID", id)
+		}
+	}
+	for _, id := range invalid {
+		if validCardID(id) {
+			t.Errorf("expected %q to be rejected", id)
+		}
+	}
+}
+
+// TestDeleteCustomerHTTP covers the DELETE /v2/customers/{id} endpoint:
+// success, idempotent NOT_FOUND no-ops, propagated failures, and the
+// invalid-id guard.
+func TestDeleteCustomerHTTP(t *testing.T) {
+	t.Run("success_deletes_customer", func(t *testing.T) {
+		var gotMethod, gotPath string
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			gotMethod = r.Method
+			gotPath = r.URL.Path
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(`{"customer":{"id":"cus_1","email_address":"jane@example.com","given_name":"Jane","created_at":"2026-07-31T00:00:00Z"}}`))
+		}))
+		defer srv.Close()
+
+		hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()}
+		if err := deleteCustomerHTTPWithClient(context.Background(), "cus_1", hc); err != nil {
+			t.Fatalf("deleteCustomerHTTP failed: %v", err)
+		}
+		if gotMethod != http.MethodDelete {
+			t.Errorf("expected DELETE, got %s", gotMethod)
+		}
+		if gotPath != "/v2/customers/cus_1" {
+			t.Errorf("expected /v2/customers/cus_1, got %s", gotPath)
+		}
+	})
+
+	t.Run("structured_not_found_is_noop", func(t *testing.T) {
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusNotFound)
+			_, _ = w.Write([]byte(`{"errors":[{"category":"INVALID_REQUEST_ERROR","code":"NOT_FOUND","detail":"Customer not found"}]}`))
+		}))
+		defer srv.Close()
+		hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()}
+		if err := deleteCustomerHTTPWithClient(context.Background(), "cus_missing", hc); err != nil {
+			t.Fatalf("expected nil for NOT_FOUND (idempotent re-deletion), got %v", err)
+		}
+	})
+
+	t.Run("plain_404_is_noop", func(t *testing.T) {
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusNotFound)
+			_, _ = w.Write([]byte("customer not found"))
+		}))
+		defer srv.Close()
+		hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()}
+		if err := deleteCustomerHTTPWithClient(context.Background(), "cus_404", hc); err != nil {
+			t.Fatalf("expected nil for plain HTTP 404, got %v", err)
+		}
+	})
+
+	t.Run("other_error_propagates", func(t *testing.T) {
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusBadRequest)
+			_, _ = w.Write([]byte(`{"errors":[{"category":"INVALID_REQUEST_ERROR","code":"INVALID_VALUE","detail":"bad"}]}`))
+		}))
+		defer srv.Close()
+		hc := &httpClient{baseURL: srv.URL, token: "t", http: srv.Client()}
+		if err := deleteCustomerHTTPWithClient(context.Background(), "cus_bad", hc); err == nil {
+			t.Fatal("expected error for genuine failure")
+		}
+	})
+
+	t.Run("invalid_id_rejected_before_http", func(t *testing.T) {
+		hc := &httpClient{baseURL: "http://unused", token: "t", http: &http.Client{}}
+		err := deleteCustomerHTTPWithClient(context.Background(), "bad/id", hc)
+		if err == nil || !strings.Contains(err.Error(), "invalid customer id") {
+			t.Fatalf("expected invalid customer id error, got %v", err)
+		}
+	})
+}