Add gitleaks secret scanning to CI

.github/workflows/test.yml

@@ -10,6 +10,21 @@ permissions:
   contents: read
 
 jobs:
+  secret-scan:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5  # v4
+        with:
+          fetch-depth: 0
+      - name: Install gitleaks
+        run: |
+          curl -sSfL https://github.com/gitleaks/gitleaks/releases/download/v8.30.1/gitleaks_8.30.1_linux_x64.tar.gz \
+            | tar -xz gitleaks
+          sudo mv gitleaks /usr/local/bin/
+      - name: Scan for secrets
+        run: gitleaks git . --redact --verbose
+
+
   build:
     runs-on: ubuntu-latest
     steps:
@@ -46,7 +61,7 @@ jobs:
       - uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff  # v5
         with:
           go-version-file: go.mod
-      - run: go install github.com/hashicorp/terraform-plugin-docs/cmd/tfplugindocs@latest
+      - run: go install github.com/hashicorp/terraform-plugin-docs/cmd/tfplugindocs@v0.25.0
       - run: tfplugindocs generate --provider-dir . --provider-name cloudzero
       - name: Verify no generated changes
         run: |

internal/client/aws_account.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"strings"
 	"time"
 )
 
@@ -86,9 +87,9 @@ func (c *Client) RegisterAWSAccount(ctx context.Context, req AWSAccountLinkReque
 			return &resp, nil
 		}
 
-		// Retry on AssumeRole errors (IAM propagation delay)
+		// Retry on AssumeRole/AccessDenied errors (IAM propagation delay after role creation)
 		errMsg := err.Error()
-		if attempt < maxAttempts && (contains(errMsg, "AssumeRole") || contains(errMsg, "AccessDenied")) {
+		if attempt < maxAttempts && (strings.Contains(errMsg, "AssumeRole") || strings.Contains(errMsg, "AccessDenied")) {
 			select {
 			case <-ctx.Done():
 				return nil, ctx.Err()
@@ -103,15 +104,3 @@ func (c *Client) RegisterAWSAccount(ctx context.Context, req AWSAccountLinkReque
 	return nil, fmt.Errorf("max registration attempts exceeded")
 }
 
-func contains(s, substr string) bool {
-	return len(s) >= len(substr) && searchString(s, substr)
-}
-
-func searchString(s, substr string) bool {
-	for i := 0; i <= len(s)-len(substr); i++ {
-		if s[i:i+len(substr)] == substr {
-			return true
-		}
-	}
-	return false
-}

internal/client/client.go

@@ -6,12 +6,16 @@ package client
 import (
 	"bytes"
 	"context"
+	"crypto/tls"
 	"encoding/json"
 	"fmt"
 	"io"
 	"math"
 	"net/http"
+	"strconv"
+	"strings"
 	"time"
+	"unicode"
 )
 
 // Client is the CloudZero API client used by all resources and data sources.
@@ -30,45 +34,91 @@ type Client struct {
 // New creates a new CloudZero API client.
 func New(host, apiKey, testKey, version string) *Client {
 	return &Client{
-		host:       host,
-		apiKey:     apiKey,
-		testKey:    testKey,
-		httpClient: &http.Client{Timeout: 30 * time.Second},
+		host:    host,
+		apiKey:  apiKey,
+		testKey: testKey,
+		httpClient: &http.Client{
+			Timeout: 30 * time.Second,
+			Transport: &http.Transport{
+				TLSClientConfig: &tls.Config{MinVersion: tls.VersionTLS12},
+			},
+		},
 		userAgent:  fmt.Sprintf("terraform-provider-cloudzero/%s", version),
 		maxRetries: 3,
 		retryBase:  1 * time.Second,
 		retryMax:   30 * time.Second,
 	}
 }
 
+// isIdempotent reports whether an HTTP method is safe to retry on server errors
+// without risk of duplicate side effects.
+func isIdempotent(method string) bool {
+	switch method {
+	case http.MethodGet, http.MethodHead, http.MethodPut, http.MethodDelete:
+		return true
+	}
+	return false
+}
+
+// retryDelay returns the backoff duration before the next attempt.
+// For 429 responses it honors the Retry-After header (delta-seconds) when present.
+func retryDelay(resp *http.Response, attempt int, base, max time.Duration) time.Duration {
+	if resp != nil && resp.StatusCode == http.StatusTooManyRequests {
+		if v := resp.Header.Get("Retry-After"); v != "" {
+			if secs, err := strconv.Atoi(v); err == nil && secs > 0 {
+				if d := time.Duration(secs) * time.Second; d < max {
+					return d
+				}
+				return max
+			}
+		}
+	}
+	return time.Duration(math.Min(
+		float64(base)*math.Pow(2, float64(attempt-1)),
+		float64(max),
+	))
+}
+
+// sanitizeErrBody caps a response body and strips control characters before
+// embedding it in an error message, preventing log injection and excessive output.
+func sanitizeErrBody(b []byte) string {
+	const maxBytes = 512
+	if len(b) > maxBytes {
+		b = b[:maxBytes]
+	}
+	return strings.Map(func(r rune) rune {
+		if unicode.IsPrint(r) || r == '\n' || r == '\t' {
+			return r
+		}
+		return -1
+	}, string(b))
+}
+
 func (c *Client) do(ctx context.Context, method, path string, body interface{}, result interface{}) error {
-	var bodyReader io.Reader
+	var bodyBytes []byte
 	if body != nil {
-		b, err := json.Marshal(body)
+		var err error
+		bodyBytes, err = json.Marshal(body)
 		if err != nil {
 			return fmt.Errorf("marshaling request body: %w", err)
 		}
-		bodyReader = bytes.NewReader(b)
 	}
 
 	var lastErr error
+	var lastResp *http.Response
 	for attempt := 0; attempt <= c.maxRetries; attempt++ {
 		if attempt > 0 {
-			backoff := time.Duration(math.Min(
-				float64(c.retryBase)*math.Pow(2, float64(attempt-1)),
-				float64(c.retryMax),
-			))
+			delay := retryDelay(lastResp, attempt, c.retryBase, c.retryMax)
 			select {
 			case <-ctx.Done():
 				return ctx.Err()
-			case <-time.After(backoff):
+			case <-time.After(delay):
 			}
+		}
 
-			// Reset body reader for retry
-			if body != nil {
-				b, _ := json.Marshal(body)
-				bodyReader = bytes.NewReader(b)
-			}
+		var bodyReader io.Reader
+		if bodyBytes != nil {
+			bodyReader = bytes.NewReader(bodyBytes)
 		}
 
 		req, err := http.NewRequestWithContext(ctx, method, c.host+path, bodyReader)
@@ -87,27 +137,34 @@ func (c *Client) do(ctx context.Context, method, path string, body interface{},
 		resp, err := c.httpClient.Do(req)
 		if err != nil {
 			lastErr = err
+			lastResp = nil
 			continue
 		}
 
-		respBody, err := io.ReadAll(resp.Body)
+		respBody, err := io.ReadAll(io.LimitReader(resp.Body, 64*1024))
 		_ = resp.Body.Close()
 		if err != nil {
 			return fmt.Errorf("reading response: %w", err)
 		}
 
-		// Retry on 429 and 5xx
-		if resp.StatusCode == http.StatusTooManyRequests || resp.StatusCode >= 500 {
-			lastErr = fmt.Errorf("HTTP %d: %s", resp.StatusCode, string(respBody))
+		switch {
+		case resp.StatusCode == http.StatusTooManyRequests:
+			// Retry 429 for any method; retryDelay will honor Retry-After.
+			lastErr = fmt.Errorf("HTTP %d: %s", resp.StatusCode, sanitizeErrBody(respBody))
+			lastResp = resp
 			continue
-		}
-
-		if resp.StatusCode == http.StatusNotFound {
-			return &NotFoundError{Message: string(respBody)}
-		}
-
-		if resp.StatusCode >= 400 {
-			return fmt.Errorf("HTTP %d: %s", resp.StatusCode, string(respBody))
+		case resp.StatusCode >= 500 && isIdempotent(method):
+			// Retry 5xx only for idempotent methods to avoid duplicate side effects.
+			lastErr = fmt.Errorf("HTTP %d: %s", resp.StatusCode, sanitizeErrBody(respBody))
+			lastResp = resp
+			continue
+		case resp.StatusCode >= 500:
+			// Non-idempotent 5xx: return immediately — do not risk a duplicate write.
+			return fmt.Errorf("HTTP %d: %s", resp.StatusCode, sanitizeErrBody(respBody))
+		case resp.StatusCode == http.StatusNotFound:
+			return &NotFoundError{Message: sanitizeErrBody(respBody)}
+		case resp.StatusCode >= 400:
+			return fmt.Errorf("HTTP %d: %s", resp.StatusCode, sanitizeErrBody(respBody))
 		}
 
 		if result != nil {

internal/provider/provider.go

@@ -5,7 +5,10 @@ package provider
 
 import (
 	"context"
+	"fmt"
+	"net/url"
 	"os"
+	"strings"
 
 	"github.com/hashicorp/terraform-plugin-framework/datasource"
 	"github.com/hashicorp/terraform-plugin-framework/path"
@@ -75,6 +78,34 @@ func (p *CloudZeroProvider) Schema(_ context.Context, _ provider.SchemaRequest,
 	}
 }
 
+// validateHost parses and normalises the provider host value.
+// It requires https:// (http:// is only allowed for localhost to support local testing).
+// Any path or query component is rejected to prevent accidental credential exfiltration.
+func validateHost(host string) (string, error) {
+	u, err := url.Parse(host)
+	if err != nil || u.Host == "" {
+		return "", fmt.Errorf("must be a valid URL (e.g. https://api.cloudzero.com), got: %q", host)
+	}
+	isLocalhost := u.Hostname() == "localhost" || u.Hostname() == "127.0.0.1"
+	switch u.Scheme {
+	case "https":
+		// valid
+	case "http":
+		if !isLocalhost {
+			return "", fmt.Errorf("must use https:// (http:// is only allowed for localhost); got host %q", u.Host)
+		}
+	default:
+		return "", fmt.Errorf("must use https:// (got scheme %q)", u.Scheme)
+	}
+	if u.Path != "" && u.Path != "/" {
+		return "", fmt.Errorf("must not include a path — use only the scheme and hostname (e.g. https://api.cloudzero.com), got path %q", u.Path)
+	}
+	if u.RawQuery != "" {
+		return "", fmt.Errorf("must not include a query string, got: %q", u.RawQuery)
+	}
+	return strings.TrimRight(u.Scheme+"://"+u.Host, "/"), nil
+}
+
 func (p *CloudZeroProvider) Configure(ctx context.Context, req provider.ConfigureRequest, resp *provider.ConfigureResponse) {
 	var config CloudZeroProviderModel
 	resp.Diagnostics.Append(req.Config.Get(ctx, &config)...)
@@ -121,6 +152,12 @@ func (p *CloudZeroProvider) Configure(ctx context.Context, req provider.Configur
 		host = "https://api.cloudzero.com"
 	}
 
+	host, err := validateHost(host)
+	if err != nil {
+		resp.Diagnostics.AddAttributeError(path.Root("host"), "Invalid CloudZero API Host", err.Error())
+		return
+	}
+
 	c := client.New(host, apiKey, testKey, p.version)
 	resp.DataSourceData = c
 	resp.ResourceData = c

internal/resources/aws_account/resource.go

@@ -307,11 +307,10 @@ func (r *AWSAccountResource) Delete(ctx context.Context, req resource.DeleteRequ
 		return
 	}
 
-	tflog.Warn(ctx, "CloudZero does not currently support programmatic account deregistration. "+
-		"The account connection will remain in CloudZero. Remove it manually from the CloudZero UI "+
-		"at Organization > Connected Accounts if needed.", map[string]interface{}{
-		"cloud_account_id": state.CloudAccountID.ValueString(),
-	})
-
-	// State is automatically removed by Terraform on successful Delete
+	resp.Diagnostics.AddWarning(
+		"Account deregistration not supported",
+		"CloudZero does not currently support programmatic account deregistration. "+
+			"The account connection for "+state.CloudAccountID.ValueString()+" will remain active in CloudZero. "+
+			"Remove it manually at Organization > Connected Accounts in the CloudZero UI if needed.",
+	)
 }