diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4a2d229..b0d6fb4 100644 --- a/.github/workflows/test.yml +++ b/.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: | 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..65630d6 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,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 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.", + ) }