From 0aa35cf747c76ca007bf9f55cde2359e588ecb67 Mon Sep 17 00:00:00 2001
From: Bernard <barjebernard@gmail.com>
Date: Wed, 20 May 2026 17:33:12 +0100
Subject: [PATCH] Restrict API retries to safe methods

---
 internal/api/request.go            | 11 +++++++-
 internal/api/request_test.go       |  1 +
 internal/api/retry_circuit_test.go | 43 ++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/internal/api/request.go b/internal/api/request.go
index 368851f..81b1cf3 100644
--- a/internal/api/request.go
+++ b/internal/api/request.go
@@ -53,7 +53,7 @@ func (c *Client) Do(ctx context.Context, method string, path string, clientID st
 			return nil, apiErr
 		}
 
-		if attempt == c.retryMaxAttempts {
+		if !isRetryableMethod(method) || attempt == c.retryMaxAttempts {
 			return nil, apiErr
 		}
 
@@ -193,6 +193,15 @@ func isTransientError(err *Error) bool {
 	return false
 }
 
+func isRetryableMethod(method string) bool {
+	switch method {
+	case http.MethodGet, http.MethodHead, http.MethodOptions:
+		return true
+	default:
+		return false
+	}
+}
+
 func (c *Client) retryDelay(attempt int, err *Error) time.Duration {
 	if err != nil && err.Type == ErrorTypeRateLimit && err.RetryAfter > 0 {
 		return time.Duration(err.RetryAfter) * time.Second
diff --git a/internal/api/request_test.go b/internal/api/request_test.go
index 755c7a8..d0d3eab 100644
--- a/internal/api/request_test.go
+++ b/internal/api/request_test.go
@@ -238,6 +238,7 @@ func TestClient_Do(t *testing.T) {
 			// Create client with test server URL
 			client := NewClientWithOptions(Options{
 				BaseURL: server.URL,
+				Sleep:   func(_ context.Context, _ time.Duration) error { return nil },
 			})
 			ctx := context.Background()
 
diff --git a/internal/api/retry_circuit_test.go b/internal/api/retry_circuit_test.go
index c2c17a1..d6e1a73 100644
--- a/internal/api/retry_circuit_test.go
+++ b/internal/api/retry_circuit_test.go
@@ -66,6 +66,49 @@ func TestClientDo_RetriesTransientErrorAndSucceeds(t *testing.T) {
 	}
 }
 
+func TestClientDo_DoesNotRetryMutatingMethods(t *testing.T) {
+	t.Parallel()
+
+	methods := []string{http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodDelete}
+	for _, method := range methods {
+		method := method
+		t.Run(method, func(t *testing.T) {
+			t.Parallel()
+
+			var calls int32
+			httpClient := &http.Client{
+				Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) {
+					atomic.AddInt32(&calls, 1)
+					return &http.Response{
+						StatusCode: http.StatusInternalServerError,
+						Status:     "500 Internal Server Error",
+						Body:       io.NopCloser(strings.NewReader(`{"error":"temporary failure"}`)),
+						Header:     make(http.Header),
+						Request:    req,
+					}, nil
+				}),
+			}
+
+			client := NewClientWithOptions(Options{
+				BaseURL:                 "https://api.test",
+				HTTPClient:              httpClient,
+				RetryMaxAttempts:        3,
+				CircuitFailureThreshold: 10,
+				CircuitOpenDuration:     time.Second,
+				Sleep:                   func(_ context.Context, _ time.Duration) error { return nil },
+			})
+
+			_, err := client.Do(context.Background(), method, "/api/test", "client", "secret", []byte(`{"ok":true}`))
+			if err == nil {
+				t.Fatal("expected transient error")
+			}
+			if got := atomic.LoadInt32(&calls); got != 1 {
+				t.Fatalf("unexpected call count: got=%d want=1", got)
+			}
+		})
+	}
+}
+
 func TestClientDo_CircuitBreakerOpensAfterFailures(t *testing.T) {
 	t.Parallel()