From 37c3cb0fee4b9daafee48a58af3fbe66dc3508c6 Mon Sep 17 00:00:00 2001 From: qiuz-cz Date: Thu, 30 Apr 2026 21:00:41 -0400 Subject: [PATCH 1/5] Add gitleaks secret scanning to CI Scans full git history on every PR and push to main via gitleaks-action v2. Action pinned to immutable commit SHA per existing workflow conventions. --- .github/workflows/test.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4a2d229..acdb6e7 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,6 +10,17 @@ permissions: contents: read jobs: + secret-scan: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + fetch-depth: 0 + - uses: gitleaks/gitleaks-action@dcedce43c6f43de0b836d1fe38946645c9c638dc # v2 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + build: runs-on: ubuntu-latest steps: From d757cc30d1fdc7c62662d1711c7568c7fd54c1d5 Mon Sep 17 00:00:00 2001 From: qiuz-cz Date: Fri, 1 May 2026 13:07:35 -0400 Subject: [PATCH 2/5] Fix security findings M1-M5 from sec-audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M1: Cap HTTP response bodies at 64 KiB and strip control characters before embedding in error messages to prevent log injection and unbounded diagnostics output. M2: Validate the host provider attribute with url.Parse — require https:// (http:// allowed for localhost only), reject any path or query component. Add explicit TLS MinVersion=1.2 to http.Transport. M3: Restrict 5xx retries to idempotent methods (GET/HEAD/PUT/DELETE) so non-idempotent POSTs never duplicate on transient server errors. Honor Retry-After header on 429 responses. M4: Replace tflog.Warn in aws_account Delete with resp.Diagnostics.AddWarning so the deregistration notice is visible in terraform destroy output without requiring TF_LOG=WARN. M5: Pin tfplugindocs from @latest to @v0.25.0 in the generate CI job to restore supply-chain pinning lost in eafcff5. --- .github/workflows/test.yml | 2 +- internal/client/aws_account.go | 17 +-- internal/client/client.go | 115 +++++++++++++++------ internal/provider/provider.go | 30 ++++++ internal/resources/aws_account/resource.go | 13 ++- 5 files changed, 126 insertions(+), 51 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index acdb6e7..74f6da5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -57,7 +57,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: | diff --git a/internal/client/aws_account.go b/internal/client/aws_account.go index 1a76f90..5754e19 100644 --- a/internal/client/aws_account.go +++ b/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 -} diff --git a/internal/client/client.go b/internal/client/client.go index a343010..2ae7072 100644 --- a/internal/client/client.go +++ b/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,10 +34,15 @@ 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, @@ -41,34 +50,75 @@ func New(host, apiKey, testKey, version string) *Client { } } +// 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 { diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 0e80921..6f426ce 100644 --- a/internal/provider/provider.go +++ b/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,27 @@ 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" + if u.Scheme != "https" && !(u.Scheme == "http" && isLocalhost) { + return "", fmt.Errorf("must use https:// (http:// is only allowed for localhost); 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 +145,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 diff --git a/internal/resources/aws_account/resource.go b/internal/resources/aws_account/resource.go index c7d5f56..d1ad88e 100644 --- a/internal/resources/aws_account/resource.go +++ b/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.", + ) } From 35a5a85b2e9e6c6e5dbe58daa53e5216f1558bd5 Mon Sep 17 00:00:00 2001 From: qiuz-cz Date: Fri, 1 May 2026 13:10:59 -0400 Subject: [PATCH 3/5] Fix staticcheck QF1001: apply De Morgan's law in validateHost --- internal/provider/provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 6f426ce..a1da830 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -87,7 +87,7 @@ func validateHost(host string) (string, error) { 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" - if u.Scheme != "https" && !(u.Scheme == "http" && isLocalhost) { + if !(u.Scheme == "https" || (u.Scheme == "http" && isLocalhost)) { return "", fmt.Errorf("must use https:// (http:// is only allowed for localhost); got scheme %q", u.Scheme) } if u.Path != "" && u.Path != "/" { From 4ec3822fc4dfe0e2fc4bd2656b4ba038af7e901b Mon Sep 17 00:00:00 2001 From: qiuz-cz Date: Fri, 1 May 2026 13:13:58 -0400 Subject: [PATCH 4/5] Simplify validateHost scheme check to avoid staticcheck QF1001 --- internal/provider/provider.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/provider/provider.go b/internal/provider/provider.go index a1da830..65630d6 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -87,8 +87,15 @@ func validateHost(host string) (string, error) { 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" - if !(u.Scheme == "https" || (u.Scheme == "http" && isLocalhost)) { - return "", fmt.Errorf("must use https:// (http:// is only allowed for localhost); got scheme %q", u.Scheme) + 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) From 722fd32b9798105579bbe9b6f85723841191a9ae Mon Sep 17 00:00:00 2001 From: qiuz-cz Date: Mon, 4 May 2026 10:44:49 -0400 Subject: [PATCH 5/5] Switch secret-scan to gitleaks CLI to remove org license requirement --- .github/workflows/test.yml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 74f6da5..b0d6fb4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,9 +16,13 @@ jobs: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: fetch-depth: 0 - - uses: gitleaks/gitleaks-action@dcedce43c6f43de0b836d1fe38946645c9c638dc # v2 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - 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: