diff --git a/README.md b/README.md index e920c78..18e32a6 100644 --- a/README.md +++ b/README.md @@ -434,6 +434,11 @@ The keys correspond to resource IDs in `pkg/config/defaults/resources.yaml`. Thi - ✅ Single environment variable to manage - ✅ Easy to add new resources (just add to JSON map) +At scan time, Version Guard verifies each configured report's identity, completed +run status, schedule-based freshness, expected row count, and required CSV +columns. A header-only CSV is accepted only when Wiz reports zero results; stale, +truncated, or schema-incompatible output fails that resource's inventory fetch. + **Logging:** Version Guard uses structured JSON logging via Go's `log/slog` package for production observability: diff --git a/docs/superpowers/plans/2026-08-05-wiz-report-health.md b/docs/superpowers/plans/2026-08-05-wiz-report-health.md new file mode 100644 index 0000000..ae66429 --- /dev/null +++ b/docs/superpowers/plans/2026-08-05-wiz-report-health.md @@ -0,0 +1,410 @@ +# Wiz Saved-Report Health Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make stale, missing, mismatched, incomplete, empty-because-broken, or schema-incompatible Wiz saved reports fail as explicit collector dependency-health errors. + +**Architecture:** Enrich and validate saved-report metadata at the Wiz HTTP boundary, then cross-check the API's expected result count against the parsed CSV before caching it. Keep resource-specific schema validation in the generic parser, but run it before returning a valid zero-resource result and wrap failures with both resource and report identifiers. + +**Tech Stack:** Go 1.24, Wiz GraphQL API, `encoding/json`, `encoding/csv`, `net/http`, `testify`, structured Temporal activity logging. + +## Global Constraints + +- Use `ReportRun.runAt`, whose verified Wiz schema description is “Date this report run (start time).” +- Derive freshness from `runIntervalHours + 6h`; all currently configured reports have a 24-hour cadence. +- Allow at most five minutes of future clock skew. +- Treat an API row count of zero plus a valid header-only CSV as healthy. +- Never include credentials or the presigned report URL in errors or logs. +- Add no configuration knobs, custom metrics, schedule changes, or deployment changes. +- Follow red-green-refactor and run focused Wiz package tests after each production change. + +--- + +### Task 1: Validate Saved-Report Metadata and Freshness + +**Files:** +- Modify: `pkg/inventory/wiz/http_client.go:16-154` +- Modify: `pkg/inventory/wiz/client.go:35-41` +- Test: `pkg/inventory/wiz/http_client_test.go:18-209` +- Test support: `pkg/inventory/wiz/fixtures_test.go:9-69` + +**Interfaces:** +- Consumes: `HTTPClient.GetReport(ctx, accessToken, reportID)` and Wiz's existing GraphQL `report(id:)` query. +- Produces: `Report{ID string, Name string, DownloadURL string, LastRun time.Time, RunIntervalHours int, ExpectedRows int}` for Task 2. + +- [ ] **Step 1: Write failing HTTP metadata tests** + +Add a healthy response with all required metadata, and table-driven failures for null report, mismatched ID, blank name, null last run, non-completed status, blank URL, missing/invalid `runAt`, missing/non-positive `runIntervalHours`, missing/negative row count, stale run, and a run more than five minutes in the future. Build timestamps relative to one captured `now := time.Now().UTC()` so tests remain deterministic enough without adding a production clock abstraction. + +Representative healthy body and assertions: + +```go +runAt := time.Now().UTC().Add(-time.Hour) +body := fmt.Sprintf(`{ + "data": {"report": { + "id": "rep-1", + "name": "Aurora Inventory", + "runIntervalHours": 24, + "lastRun": { + "status": "COMPLETED", + "url": "https://files.example/abc.csv", + "runAt": %q, + "results": {"__typename": "ReportRunResultsCloudResourceV2", "rowCount": 12} + } + }} +}`, runAt.Format(time.RFC3339Nano)) + +rep, err := c.GetReport(context.Background(), "test-token", "rep-1") +require.NoError(t, err) +assert.Equal(t, runAt, rep.LastRun) +assert.Equal(t, 24, rep.RunIntervalHours) +assert.Equal(t, 12, rep.ExpectedRows) +``` + +Inspect the received GraphQL request in the healthy test and assert its query contains `runIntervalHours`, `runAt`, both supported result fragments, and `rowCount` aliases. + +- [ ] **Step 2: Run focused tests and verify RED** + +Run: + +```bash +go test ./pkg/inventory/wiz -run 'TestGetReport_' -count=1 +``` + +Expected: FAIL because the query and response model do not expose cadence, run time, or expected rows, and missing/mismatched metadata is currently accepted. + +- [ ] **Step 3: Extend the report model and GraphQL query** + +Change `Report` to: + +```go +type Report struct { + ID string + Name string + DownloadURL string + LastRun time.Time + RunIntervalHours int + ExpectedRows int +} +``` + +Request verified metadata and normalize supported result counts: + +```graphql +report(id: $reportId) { + id + name + runIntervalHours + lastRun { + status + url + runAt + results { + __typename + ... on ReportRunResultsGraphQuery { rowCount: resultCount } + ... on ReportRunResultsCloudResource { rowCount: count } + ... on ReportRunResultsCloudResourceV2 { rowCount: count } + } + } +} +``` + +Use pointers in the wire response for nullable objects and `rowCount`, so a legitimate zero is distinguishable from missing metadata: + +```go +type reportRunResponse struct { + Status string `json:"status"` + URL string `json:"url"` + RunAt time.Time `json:"runAt"` + Results *struct { + Type string `json:"__typename"` + RowCount *int `json:"rowCount"` + } `json:"results"` +} +``` + +- [ ] **Step 4: Implement minimal metadata validation** + +Add constants: + +```go +const ( + reportFreshnessGrace = 6 * time.Hour + maxFutureClockSkew = 5 * time.Minute +) +``` + +Validate in this order so errors are actionable and do not dereference null data: + +```go +if result.Report == nil { + return nil, errors.Errorf("report %s not found", reportID) +} +if result.Report.ID != reportID { + return nil, errors.Errorf("report identity mismatch: requested %s, received %s", reportID, result.Report.ID) +} +if strings.TrimSpace(result.Report.Name) == "" { + return nil, errors.Errorf("report %s has no name", reportID) +} +if result.Report.LastRun == nil { + return nil, errors.Errorf("report %s has no last run", reportID) +} +``` + +Then enforce completed status, non-empty URL, non-zero run time, positive interval, supported result type, present non-negative row count, five-minute future skew, and age no greater than: + +```go +maxAge := time.Duration(result.Report.RunIntervalHours)*time.Hour + reportFreshnessGrace +``` + +Return errors containing the report ID, observed metadata, and threshold, but never `LastRun.URL`. + +- [ ] **Step 5: Update shared fixtures and verify GREEN** + +Set fixture expected rows to their CSV data-row counts: + +```go +AuroraReport: &Report{ExpectedRows: 5, RunIntervalHours: 24, ...} +ElastiCacheReport: &Report{ExpectedRows: 5, RunIntervalHours: 24, ...} +LambdaReport: &Report{ExpectedRows: 5, RunIntervalHours: 24, ...} +``` + +Run: + +```bash +go test ./pkg/inventory/wiz -run 'TestGetReport_' -count=1 +go test ./pkg/inventory/wiz -count=1 +``` + +Expected: PASS. + +- [ ] **Step 6: Commit metadata health validation** + +```bash +git add pkg/inventory/wiz/http_client.go pkg/inventory/wiz/http_client_test.go \ + pkg/inventory/wiz/client.go pkg/inventory/wiz/fixtures_test.go +git commit -m "feat(wiz): validate saved report freshness" +``` + +### Task 2: Cross-Check CSV Completeness and Preserve Valid Zero Results + +**Files:** +- Modify: `pkg/inventory/wiz/client.go:127-164` +- Test: `pkg/inventory/wiz/client_test.go:59-78` + +**Interfaces:** +- Consumes: Task 1's `Report.ExpectedRows` and downloaded CSV rows. +- Produces: `Client.GetReportData(ctx, reportID) ([][]string, error)` that returns only complete, API-consistent CSV data and caches only validated rows. + +- [ ] **Step 1: Replace the old empty-report test with failing completeness cases** + +Add table-driven tests with per-case `Report.ExpectedRows` and CSV bodies: + +```go +tests := []struct { + name string + expectedRows int + csv string + wantRows int + wantErr string +}{ + {name: "valid zero result", expectedRows: 0, csv: WizAPIFixtures.EmptyCSVData, wantRows: 1}, + {name: "missing header", expectedRows: 0, csv: "", wantErr: "has no header"}, + {name: "broken header only", expectedRows: 5, csv: WizAPIFixtures.EmptyCSVData, wantErr: "expected 5 data rows, downloaded 0"}, + {name: "truncated data", expectedRows: 2, csv: "id,name\n1,one\n", wantErr: "expected 2 data rows, downloaded 1"}, +} +``` + +For each case, copy the fixture report by value before changing `ExpectedRows`, so global fixtures are not mutated across tests. + +- [ ] **Step 2: Run completeness tests and verify RED** + +Run: + +```bash +go test ./pkg/inventory/wiz -run 'TestClient_GetReportData_(CSVCompleteness|EmptyReport)' -count=1 +``` + +Expected: FAIL because empty and mismatched downloads are currently cached and returned without validation. + +- [ ] **Step 3: Implement CSV completeness validation before caching** + +Immediately after `csvReader.ReadAll()`: + +```go +if len(rows) == 0 { + return nil, errors.Errorf("Wiz report %s CSV has no header", reportID) +} + +actualRows := len(rows) - 1 +if actualRows != report.ExpectedRows { + return nil, errors.Errorf( + "Wiz report %s expected %d data rows, downloaded %d", + reportID, + report.ExpectedRows, + actualRows, + ) +} +``` + +Remove the comment claiming all header-only reports are valid. Keep validation before the cache write so unhealthy output is never cached. + +- [ ] **Step 4: Run focused and package tests and verify GREEN** + +Run: + +```bash +go test ./pkg/inventory/wiz -run 'TestClient_GetReportData_' -count=1 +go test ./pkg/inventory/wiz -count=1 +``` + +Expected: PASS. + +- [ ] **Step 5: Commit CSV completeness validation** + +```bash +git add pkg/inventory/wiz/client.go pkg/inventory/wiz/client_test.go +git commit -m "feat(wiz): reject incomplete report CSVs" +``` + +### Task 3: Enforce Schema Health for Zero Results and Add Resource Context + +**Files:** +- Modify: `pkg/inventory/wiz/helpers.go:141-165` +- Modify: `pkg/inventory/wiz/generic.go:69-105` +- Test: `pkg/inventory/wiz/generic_test.go` +- Modify: `README.md:416-438` + +**Interfaces:** +- Consumes: Task 2's guarantee that every returned CSV has a header and matches the API row count. +- Produces: `GenericInventorySource.ListResources` errors containing both `resource ` and `report ` while preserving healthy empty inventories. + +- [ ] **Step 1: Write failing zero-result schema and context tests** + +Configure a generic source with a mock `Report{ExpectedRows: 0}` and a header-only CSV missing one required mapped column. Assert: + +```go +_, err := source.ListResources(context.Background(), cfg.Type) +require.Error(t, err) +assert.Contains(t, err.Error(), `required column "versionDetails.version" not found`) +assert.Contains(t, err.Error(), "resource test-resource") +assert.Contains(t, err.Error(), "report test-report-id") +``` + +Add a healthy header-only case with all required columns and API expected count zero; assert an empty resource slice and no error. + +- [ ] **Step 2: Run generic source tests and verify RED** + +Run: + +```bash +go test ./pkg/inventory/wiz -run 'TestGenericInventorySource_.*Empty' -count=1 +``` + +Expected: FAIL because `parseWizReport` returns before header validation and errors do not include the resource config ID. + +- [ ] **Step 3: Validate headers before returning an empty inventory** + +In `parseWizReport`, rely on Task 2's non-empty-row guarantee, build the column index, validate every required column, then return an empty slice only after schema validation: + +```go +cols := buildColumnIndex(rows[0]) +for _, name := range requiredColumns { + if !cols.hasColumn(name) { + return nil, fmt.Errorf("required column %q not found in CSV header (have: %v)", name, rows[0]) + } +} + +if len(rows) == 1 { + return []*types.Resource{}, nil +} +``` + +- [ ] **Step 4: Wrap dependency-health failures with resource and report IDs** + +Replace the direct `parseWizReport` return in `ListResources`: + +```go +resources, err := parseWizReport(ctx, s.client, reportID, requiredColumns, filterRow, parseRow, s.logger) +if err != nil { + return nil, errors.Wrapf(err, "Wiz dependency unhealthy for resource %s report %s", s.config.ID, reportID) +} +return resources, nil +``` + +This error reaches the existing Temporal workflow's structured `Failed to fetch inventory` log without exposing credentials or the download URL. + +- [ ] **Step 5: Document the runtime report-health contract** + +After the `WIZ_REPORT_IDS` benefits list in `README.md`, add: + +```markdown +At scan time, Version Guard verifies each configured report's identity, completed +run status, schedule-based freshness, expected row count, and required CSV +columns. A header-only CSV is accepted only when Wiz reports zero results; stale, +truncated, or schema-incompatible output fails that resource's inventory fetch. +``` + +- [ ] **Step 6: Run focused and complete verification** + +Run: + +```bash +go test ./pkg/inventory/wiz -count=1 +make fmt-all +make test +make lint +git diff --check +``` + +Expected: all tests and lint pass; formatting produces no unexplained changes; `git diff --check` emits no output. + +- [ ] **Step 7: Commit schema/context behavior and documentation** + +```bash +git add pkg/inventory/wiz/helpers.go pkg/inventory/wiz/generic.go \ + pkg/inventory/wiz/generic_test.go README.md +git commit -m "feat(wiz): surface report dependency health" +``` + +### Task 4: Final Review and Live Contract Check + +**Files:** +- Review: all changes since `origin/main` + +**Interfaces:** +- Consumes: Tasks 1-3. +- Produces: a review-ready branch whose query shape is proven against the live Wiz schema and whose behavior is covered by local tests. + +- [ ] **Step 1: Review the complete branch diff** + +Run: + +```bash +git diff --stat origin/main...HEAD +git diff --check origin/main...HEAD +git diff origin/main...HEAD -- pkg/inventory/wiz README.md docs/superpowers +``` + +Check specifically for presigned URLs in errors, accidental credential material, acceptance of missing metadata, cache writes before validation, and tests that mutate shared fixtures. + +- [ ] **Step 2: Re-run the exact GraphQL query shape read-only** + +Using the existing staging Version Guard service account without printing its credentials or download URL, query one CloudResourceV2 report and the OpenSearch GraphQuery report. Confirm both return `id`, `runIntervalHours`, `lastRun.status`, `lastRun.runAt`, and aliased `lastRun.results.rowCount` without GraphQL errors. + +- [ ] **Step 3: Run final verification from a clean test cache** + +Run: + +```bash +go clean -testcache +make test +make lint +git status --short --branch +``` + +Expected: tests and lint pass, and status shows only intentional committed branch changes. + +- [ ] **Step 4: Commit any review fixes separately** + +If review identifies a defect, write or adjust a failing regression test first, implement the smallest fix, rerun focused and full verification, then commit with a message describing the behavior fixed. If no defect is found, do not create an empty commit. diff --git a/docs/superpowers/specs/2026-08-05-wiz-report-health-design.md b/docs/superpowers/specs/2026-08-05-wiz-report-health-design.md new file mode 100644 index 0000000..aad048a --- /dev/null +++ b/docs/superpowers/specs/2026-08-05-wiz-report-health-design.md @@ -0,0 +1,80 @@ +# Wiz Saved-Report Health Validation + +## Goal + +Fail a configured Wiz inventory dependency explicitly when its saved report is +missing, mismatched, stale, incomplete, schema-incompatible, or inconsistent +with its downloaded CSV. Preserve a legitimate zero-resource report when Wiz +metadata confirms that the completed run returned zero rows. + +## Verified Wiz contract + +Read-only introspection of Block's Wiz GraphQL schema on 2026-08-05 established: + +- `Report.lastRun` is the latest run and has type `ReportRun`. +- `ReportRun.runAt: DateTime!` is the run start time. +- `Report.runIntervalHours: Int` is the configured scheduled cadence. +- `ReportRun.results` is a union. Version Guard's configured reports currently + use `ReportRunResultsCloudResourceV2.count` or + `ReportRunResultsGraphQuery.resultCount` as their CSV row count. +- All eight configured reports have a 24-hour interval. + +The API result count matched parsed CSV data rows for representative reports: +Aurora MySQL (12,216), OpenSearch (279), and Lambda (21,853). + +## Design + +Extend the saved-report query to request the report identity, name, schedule +interval, run status, download URL, run timestamp, result type, and normalized +row count. Normalize the two supported result variants with a GraphQL +`rowCount` alias. + +Validate report metadata before downloading: + +1. The report exists and its returned ID exactly matches the configured ID. +2. Its name and completed-run download URL are non-empty. +3. The latest run status is `COMPLETED`. +4. `runAt`, `runIntervalHours`, and row-count metadata are present and valid. +5. The run timestamp is not materially in the future and is no older than the + configured interval plus six hours. The current 24-hour reports therefore + have a 30-hour freshness window, enough for schedule and collection delay + while still detecting a missed daily run. + +Carry the expected row count into CSV parsing and enforce: + +- a completely empty download is invalid because it has no schema; +- API count zero plus a header-only CSV is a valid empty inventory; +- API count greater than zero plus a header-only CSV is invalid; +- any API/CSV data-row mismatch is invalid. + +Required-column validation must run against the header before returning a valid +zero-resource result. This catches schema drift even when the report contains +no resources. + +Failures continue through the existing inventory/activity error path. Error +messages identify the configured report without exposing credentials or the +presigned download URL. Existing structured activity logs provide the resource +context; no custom application metric is added because Version Guard's +supported metric surface is the Temporal SDK endpoint. + +## Testing + +Use table-driven HTTP and client tests covering: + +- missing and mismatched reports; +- non-completed, stale, and future runs; +- missing or invalid required metadata; +- healthy completed reports; +- valid zero-result CSVs; +- broken header-only and completely empty downloads; +- API/CSV row-count mismatch; and +- required-column drift on an empty report. + +Implementation follows red-green-refactor: each behavior test must fail for the +expected reason before production code changes are added. + +## Non-goals + +- Creating, editing, or rescheduling Wiz reports. +- Adding report-health configuration knobs or new custom metrics. +- Changing Version Guard's Temporal schedule or deployment configuration. diff --git a/pkg/inventory/wiz/client.go b/pkg/inventory/wiz/client.go index 1f7b6ea..6631204 100644 --- a/pkg/inventory/wiz/client.go +++ b/pkg/inventory/wiz/client.go @@ -34,10 +34,12 @@ type WizClient interface { //nolint:govet // field alignment sacrificed for readability type Report struct { - ID string - Name string - DownloadURL string - LastRun time.Time + ID string + Name string + DownloadURL string + LastRun time.Time + RunIntervalHours int + ExpectedRows int } // Client wraps the Wiz API client with caching and CSV parsing. @@ -61,8 +63,9 @@ type Client struct { //nolint:govet // field alignment sacrificed for readability type cachedReport struct { - data [][]string - fetchedAt time.Time + data [][]string + fetchedAt time.Time + freshnessDeadline time.Time } // NewClient creates a new Wiz client with caching @@ -82,9 +85,12 @@ func NewClient(wizClient WizClient, cacheTTL time.Duration) *Client { // rows. Each reportID is cached independently for cacheTTL duration so // parallel scans across different resource types don't evict each // other's data. -func (c *Client) GetReportData(ctx context.Context, reportID string) ([][]string, error) { +func (c *Client) GetReportData(ctx context.Context, reportID string, requiredColumns ...string) ([][]string, error) { // Fast path: read-locked cache lookup. if rows, ok := c.lookup(reportID); ok { + if err := validateRequiredColumns(rows, requiredColumns); err != nil { + return nil, errors.Wrapf(err, "Wiz report %s has invalid CSV schema", reportID) + } return rows, nil } @@ -98,7 +104,7 @@ func (c *Client) GetReportData(ctx context.Context, reportID string) ([][]string if rows, ok := c.lookup(reportID); ok { return rows, nil } - return c.fetchAndCache(ctx, reportID) + return c.fetchAndCache(ctx, reportID, requiredColumns) }) if err != nil { return nil, err @@ -107,6 +113,9 @@ func (c *Client) GetReportData(ctx context.Context, reportID string) ([][]string if !ok { return nil, errors.Errorf("wiz cache returned unexpected type for report %s", reportID) } + if err := validateRequiredColumns(rows, requiredColumns); err != nil { + return nil, errors.Wrapf(err, "Wiz report %s has invalid CSV schema", reportID) + } return rows, nil } @@ -121,6 +130,9 @@ func (c *Client) lookup(reportID string) ([][]string, bool) { if time.Since(cached.fetchedAt) >= c.cacheTTL { return nil, false } + if !cached.freshnessDeadline.IsZero() && !time.Now().Before(cached.freshnessDeadline) { + return nil, false + } return cached.data, true } @@ -128,7 +140,7 @@ func (c *Client) lookup(reportID string) ([][]string, bool) { // for one reportID and writes the parsed rows into the cache before // returning. Called from inside a singleflight slot, so at most one // goroutine per reportID is here at a time. -func (c *Client) fetchAndCache(ctx context.Context, reportID string) ([][]string, error) { +func (c *Client) fetchAndCache(ctx context.Context, reportID string, requiredColumns []string) ([][]string, error) { accessToken, err := c.wizClient.GetAccessToken(ctx) if err != nil { return nil, errors.Wrap(err, "failed to get Wiz access token") @@ -151,18 +163,48 @@ func (c *Client) fetchAndCache(ctx context.Context, reportID string) ([][]string return nil, errors.Wrapf(err, "failed to parse Wiz report CSV for report %s", reportID) } - // Note: Empty reports (header only) are valid - the inventory source will filter them + if len(rows) == 0 { + return nil, errors.Errorf("Wiz report %s CSV has no header", reportID) + } + + actualRows := len(rows) - 1 + if actualRows != report.ExpectedRows { + return nil, errors.Errorf( + "Wiz report %s expected %d data rows, downloaded %d", + reportID, + report.ExpectedRows, + actualRows, + ) + } + + // Required columns are caller-specific. Return invalid rows so every + // singleflight waiter can validate its own requirements, but do not cache + // data that is invalid for the initiating caller. + if err := validateRequiredColumns(rows, requiredColumns); err != nil { + return rows, nil + } c.mu.Lock() c.cache[reportID] = &cachedReport{ - data: rows, - fetchedAt: time.Now(), + data: rows, + fetchedAt: time.Now(), + freshnessDeadline: report.LastRun.Add(time.Duration(report.RunIntervalHours)*time.Hour + reportFreshnessGrace), } c.mu.Unlock() return rows, nil } +func validateRequiredColumns(rows [][]string, requiredColumns []string) error { + cols := buildColumnIndex(rows[0]) + for _, name := range requiredColumns { + if !cols.hasColumn(name) { + return errors.Errorf("required column %q not found in CSV header (have: %v)", name, rows[0]) + } + } + return nil +} + // ParseTags extracts tags from a Wiz Tags column (JSON format) // Example: [{"key":"app","value":"my-app"},{"key":"env","value":"prod"}] func ParseTags(tagsJSON string) (map[string]string, error) { diff --git a/pkg/inventory/wiz/client_test.go b/pkg/inventory/wiz/client_test.go index 40caa9e..a0314f3 100644 --- a/pkg/inventory/wiz/client_test.go +++ b/pkg/inventory/wiz/client_test.go @@ -57,24 +57,42 @@ func TestClient_GetReportData_Success(t *testing.T) { mockWizClient.AssertExpectations(t) } -func TestClient_GetReportData_EmptyReport(t *testing.T) { - ctx := context.Background() - - mockWizClient := new(MockWizClient) - mockWizClient.On("GetAccessToken", mock.Anything).Return(WizAPIFixtures.AccessToken, nil) - mockWizClient.On("GetReport", mock.Anything, mock.Anything, mock.Anything).Return(WizAPIFixtures.AuroraReport, nil) - mockWizClient.On("DownloadReport", mock.Anything, mock.Anything).Return(NewMockReadCloser(WizAPIFixtures.EmptyCSVData), nil) - - client := NewClient(mockWizClient, time.Hour) - - // Execute: Get empty report (only has header row) - rows, err := client.GetReportData(ctx, "empty-report-id") - - // Verify: Returns header row only (this is valid CSV, not an error) - require.NoError(t, err) - require.Len(t, rows, 1, "Should have header row only") +func TestClient_GetReportData_CSVCompleteness(t *testing.T) { + tests := []struct { + csv string + name string + wantErr string + expectedRows int + wantRows int + }{ + {name: "valid zero result", expectedRows: 0, csv: WizAPIFixtures.EmptyCSVData, wantRows: 1}, + {name: "missing header", expectedRows: 0, csv: "", wantErr: "has no header"}, + {name: "broken header only", expectedRows: 5, csv: WizAPIFixtures.EmptyCSVData, wantErr: "expected 5 data rows, downloaded 0"}, + {name: "truncated data", expectedRows: 2, csv: "id,name\n1,one\n", wantErr: "expected 2 data rows, downloaded 1"}, + } - mockWizClient.AssertExpectations(t) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + report := *WizAPIFixtures.AuroraReport + report.ExpectedRows = tt.expectedRows + + mockWizClient := new(MockWizClient) + mockWizClient.On("GetAccessToken", mock.Anything).Return(WizAPIFixtures.AccessToken, nil) + mockWizClient.On("GetReport", mock.Anything, mock.Anything, mock.Anything).Return(&report, nil) + mockWizClient.On("DownloadReport", mock.Anything, mock.Anything).Return(NewMockReadCloser(tt.csv), nil) + + rows, err := NewClient(mockWizClient, time.Hour).GetReportData(context.Background(), "report-id") + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + require.Nil(t, rows) + } else { + require.NoError(t, err) + require.Len(t, rows, tt.wantRows) + } + + mockWizClient.AssertExpectations(t) + }) + } } func TestClient_GetReportData_GetAccessTokenError(t *testing.T) { @@ -139,13 +157,15 @@ func TestClient_GetReportData_DownloadError(t *testing.T) { func TestClient_GetReportData_Caching(t *testing.T) { ctx := context.Background() + report := *WizAPIFixtures.AuroraReport + report.LastRun = time.Now() mockWizClient := new(MockWizClient) // Mock should only be called ONCE due to caching mockWizClient.On("GetAccessToken", mock.Anything).Return(WizAPIFixtures.AccessToken, nil).Once() mockWizClient.On("GetReport", mock.Anything, mock.Anything, "cached-report-id"). - Return(WizAPIFixtures.AuroraReport, nil).Once() + Return(&report, nil).Once() mockWizClient.On("DownloadReport", mock.Anything, mock.Anything). Return(NewMockReadCloser(WizAPIFixtures.AuroraCSVData), nil).Once() @@ -168,6 +188,33 @@ func TestClient_GetReportData_Caching(t *testing.T) { mockWizClient.AssertExpectations(t) } +func TestClient_GetReportData_RefetchesAfterReportFreshnessDeadline(t *testing.T) { + ctx := context.Background() + report := *WizAPIFixtures.AuroraReport + report.LastRun = time.Now() + + mockWizClient := new(MockWizClient) + mockWizClient.On("GetAccessToken", mock.Anything).Return(WizAPIFixtures.AccessToken, nil).Times(2) + mockWizClient.On("GetReport", mock.Anything, mock.Anything, "freshness-report-id"). + Return(&report, nil).Times(2) + mockWizClient.On("DownloadReport", mock.Anything, mock.Anything). + Return(NewMockReadCloser(WizAPIFixtures.AuroraCSVData), nil).Once() + mockWizClient.On("DownloadReport", mock.Anything, mock.Anything). + Return(NewMockReadCloser(WizAPIFixtures.AuroraCSVData), nil).Once() + + client := NewClient(mockWizClient, time.Hour) + _, err := client.GetReportData(ctx, "freshness-report-id") + require.NoError(t, err) + + client.mu.Lock() + client.cache["freshness-report-id"].freshnessDeadline = time.Now().Add(-time.Second) + client.mu.Unlock() + + _, err = client.GetReportData(ctx, "freshness-report-id") + require.NoError(t, err) + mockWizClient.AssertExpectations(t) +} + // TestClient_GetReportData_PerReportIDCache pins the contract that calls // for different reportIDs do NOT evict each other's cache entries. The // Version-Guard server fans out one detection workflow per resource type, @@ -176,13 +223,15 @@ func TestClient_GetReportData_Caching(t *testing.T) { // parallel scans. func TestClient_GetReportData_PerReportIDCache(t *testing.T) { ctx := context.Background() + report := *WizAPIFixtures.AuroraReport + report.LastRun = time.Now() mockWizClient := new(MockWizClient) mockWizClient.On("GetAccessToken", mock.Anything).Return(WizAPIFixtures.AccessToken, nil) mockWizClient.On("GetReport", mock.Anything, mock.Anything, "report-A"). - Return(WizAPIFixtures.AuroraReport, nil).Once() + Return(&report, nil).Once() mockWizClient.On("GetReport", mock.Anything, mock.Anything, "report-B"). - Return(WizAPIFixtures.AuroraReport, nil).Once() + Return(&report, nil).Once() // Each download mock is configured Once() — the test fails if either // is called twice (the symptom of cache eviction). mockWizClient.On("DownloadReport", mock.Anything, mock.Anything). @@ -214,6 +263,8 @@ func TestClient_GetReportData_PerReportIDCache(t *testing.T) { // fetch via singleflight, while remaining correct under the race detector. func TestClient_GetReportData_SingleflightCollapsesConcurrent(t *testing.T) { ctx := context.Background() + report := *WizAPIFixtures.AuroraReport + report.LastRun = time.Now() mockWizClient := new(MockWizClient) // Mock body returns a fresh ReadCloser per call. .Once() on the @@ -221,7 +272,7 @@ func TestClient_GetReportData_SingleflightCollapsesConcurrent(t *testing.T) { // many concurrent callers. mockWizClient.On("GetAccessToken", mock.Anything).Return(WizAPIFixtures.AccessToken, nil).Once() mockWizClient.On("GetReport", mock.Anything, mock.Anything, "concurrent-report"). - Return(WizAPIFixtures.AuroraReport, nil).Once() + Return(&report, nil).Once() mockWizClient.On("DownloadReport", mock.Anything, mock.Anything). Return(NewMockReadCloser(WizAPIFixtures.AuroraCSVData), nil).Once() diff --git a/pkg/inventory/wiz/fixtures_test.go b/pkg/inventory/wiz/fixtures_test.go index 9c98ddf..e02bd03 100644 --- a/pkg/inventory/wiz/fixtures_test.go +++ b/pkg/inventory/wiz/fixtures_test.go @@ -21,10 +21,12 @@ var WizAPIFixtures = struct { AccessToken: "wiz-mock-access-token-12345", AuroraReport: &Report{ - ID: "aurora-report-id-123", - Name: "Aurora Clusters Report", - DownloadURL: "https://wiz-api.example.com/reports/aurora-report-id-123/download", - LastRun: time.Date(2026, 4, 7, 10, 0, 0, 0, time.UTC), + ID: "aurora-report-id-123", + Name: "Aurora Clusters Report", + DownloadURL: "https://wiz-api.example.com/reports/aurora-report-id-123/download", + LastRun: time.Date(2026, 4, 7, 10, 0, 0, 0, time.UTC), + RunIntervalHours: 24, + ExpectedRows: 5, }, // Realistic Wiz CSV export for Aurora clusters @@ -38,10 +40,12 @@ arn:aws:rds:eu-west-1:345678901234:cluster:postgres-11-deprecated,postgres-11-de `, ElastiCacheReport: &Report{ - ID: "elasticache-report-id-456", - Name: "ElastiCache Version Report", - DownloadURL: "https://wiz-api.example.com/reports/elasticache-report-id-456/download", - LastRun: time.Date(2026, 4, 8, 10, 0, 0, 0, time.UTC), + ID: "elasticache-report-id-456", + Name: "ElastiCache Version Report", + DownloadURL: "https://wiz-api.example.com/reports/elasticache-report-id-456/download", + LastRun: time.Date(2026, 4, 8, 10, 0, 0, 0, time.UTC), + RunIntervalHours: 24, + ExpectedRows: 5, }, // Realistic Wiz CSV export for ElastiCache clusters @@ -55,10 +59,12 @@ arn:aws:elasticache:eu-west-1:345678901234:cluster:user-valkey-001,user-valkey-0 `, LambdaReport: &Report{ - ID: "lambda-report-id-789", - Name: "Lambda Functions Report", - DownloadURL: "https://wiz-api.example.com/reports/lambda-report-id-789/download", - LastRun: time.Date(2026, 4, 10, 10, 0, 0, 0, time.UTC), + ID: "lambda-report-id-789", + Name: "Lambda Functions Report", + DownloadURL: "https://wiz-api.example.com/reports/lambda-report-id-789/download", + LastRun: time.Date(2026, 4, 10, 10, 0, 0, 0, time.UTC), + RunIntervalHours: 24, + ExpectedRows: 5, }, // Realistic Wiz CSV export for Lambda functions diff --git a/pkg/inventory/wiz/generic.go b/pkg/inventory/wiz/generic.go index 4f9a240..dcffc67 100644 --- a/pkg/inventory/wiz/generic.go +++ b/pkg/inventory/wiz/generic.go @@ -94,7 +94,7 @@ func (s *GenericInventorySource) ListResources(ctx context.Context, resourceType } // Use shared helper to parse Wiz report - return parseWizReport( + resources, err := parseWizReport( ctx, s.client, reportID, @@ -103,6 +103,10 @@ func (s *GenericInventorySource) ListResources(ctx context.Context, resourceType parseRow, s.logger, ) + if err != nil { + return nil, errors.Wrapf(err, "Wiz dependency unhealthy for resource %s report %s", s.config.ID, reportID) + } + return resources, nil } // GetResource fetches a single resource by ID diff --git a/pkg/inventory/wiz/generic_test.go b/pkg/inventory/wiz/generic_test.go index 081664e..2a7d483 100644 --- a/pkg/inventory/wiz/generic_test.go +++ b/pkg/inventory/wiz/generic_test.go @@ -746,6 +746,78 @@ func TestListResources_ReportIDNotInMap(t *testing.T) { assert.Contains(t, err.Error(), "no report ID configured for resource aurora-postgresql") } +func TestGenericInventorySource_SchemaDriftEmpty(t *testing.T) { + mockWizClient := new(MockWizClient) + report := &Report{ + ID: "test-report-id", + DownloadURL: "https://wiz-api.example.com/reports/test-report-id/download", + LastRun: time.Now(), + RunIntervalHours: 24, + ExpectedRows: 0, + } + mockWizClient.On("GetAccessToken", mock.Anything).Return("test-token", nil).Times(2) + mockWizClient.On("GetReport", mock.Anything, "test-token", "test-report-id").Return(report, nil).Times(2) + mockWizClient.On("DownloadReport", mock.Anything, report.DownloadURL). + Return(NewMockReadCloser("externalId,nativeType\n"), nil).Once() + mockWizClient.On("DownloadReport", mock.Anything, report.DownloadURL). + Return(NewMockReadCloser("externalId,nativeType\n"), nil).Once() + + t.Setenv("WIZ_REPORT_IDS", `{"test-resource":"test-report-id"}`) + cfg := config.ResourceConfig{ + ID: "test-resource", + Type: "aurora", + Inventory: config.InventoryConfig{ + NativeTypePattern: "rds/AmazonAuroraPostgreSQL/cluster", + RequiredMappings: map[string]string{ + "resource_id": "externalId", + "version": "versionDetails.version", + }, + }, + } + source := NewGenericInventorySource(NewClient(mockWizClient, time.Hour), &cfg, nil, nil) + + for range 2 { + _, err := source.ListResources(context.Background(), types.ResourceType(cfg.Type)) + require.Error(t, err) + assert.Contains(t, err.Error(), `required column "versionDetails.version" not found`) + assert.Contains(t, err.Error(), "resource test-resource") + assert.Contains(t, err.Error(), "report test-report-id") + assert.NotContains(t, err.Error(), report.DownloadURL) + } + mockWizClient.AssertNumberOfCalls(t, "DownloadReport", 2) +} + +func TestGenericInventorySource_HealthyEmpty(t *testing.T) { + mockWizClient := new(MockWizClient) + report := &Report{ + ID: "test-report-id", + DownloadURL: "https://wiz-api.example.com/reports/test-report-id/download", + ExpectedRows: 0, + } + mockWizClient.On("GetAccessToken", mock.Anything).Return("test-token", nil) + mockWizClient.On("GetReport", mock.Anything, "test-token", "test-report-id").Return(report, nil) + mockWizClient.On("DownloadReport", mock.Anything, report.DownloadURL). + Return(NewMockReadCloser("externalId,nativeType,versionDetails.version\n"), nil) + + t.Setenv("WIZ_REPORT_IDS", `{"test-resource":"test-report-id"}`) + cfg := config.ResourceConfig{ + ID: "test-resource", + Type: "aurora", + Inventory: config.InventoryConfig{ + NativeTypePattern: "rds/AmazonAuroraPostgreSQL/cluster", + RequiredMappings: map[string]string{ + "resource_id": "externalId", + "version": "versionDetails.version", + }, + }, + } + source := NewGenericInventorySource(NewClient(mockWizClient, time.Hour), &cfg, nil, nil) + + resources, err := source.ListResources(context.Background(), types.ResourceType(cfg.Type)) + require.NoError(t, err) + assert.Empty(t, resources) +} + func TestGetResource(t *testing.T) { // Note: This test would require mocking the Wiz client // For now, we test the error case when ListResources fails diff --git a/pkg/inventory/wiz/helpers.go b/pkg/inventory/wiz/helpers.go index c0a94ee..fe8c702 100644 --- a/pkg/inventory/wiz/helpers.go +++ b/pkg/inventory/wiz/helpers.go @@ -139,24 +139,16 @@ func parseWizReport( logger = slog.Default() } // Fetch report data - rows, err := client.GetReportData(ctx, reportID) + rows, err := client.GetReportData(ctx, reportID, requiredColumns...) if err != nil { return nil, errors.Wrap(err, "failed to fetch Wiz report data") } - if len(rows) < 2 { - // Empty report (only header row or completely empty) - return []*types.Resource{}, nil - } - // Build column index from header row cols := buildColumnIndex(rows[0]) - // Validate that all required columns are present (using alias-aware lookup) - for _, name := range requiredColumns { - if !cols.hasColumn(name) { - return nil, fmt.Errorf("required column %q not found in CSV header (have: %v)", name, rows[0]) - } + if len(rows) == 1 { + return []*types.Resource{}, nil } totalDataRows := len(rows) - 1 diff --git a/pkg/inventory/wiz/http_client.go b/pkg/inventory/wiz/http_client.go index b8bc0ab..ad6a7b0 100644 --- a/pkg/inventory/wiz/http_client.go +++ b/pkg/inventory/wiz/http_client.go @@ -19,15 +19,26 @@ const ( maxRetries = 5 retryBackoff = 3 * time.Second + + reportFreshnessGrace = 6 * time.Hour + maxFutureClockSkew = 5 * time.Minute ) const reportDownloadQuery = `query ReportDownloadUrl($reportId: ID!) { report(id: $reportId) { id name + runIntervalHours lastRun { status url + runAt + results { + __typename + ... on ReportRunResultsGraphQuery { rowCount: resultCount } + ... on ReportRunResultsCloudResource { rowCount: count } + ... on ReportRunResultsCloudResourceV2 { rowCount: count } + } } } }` @@ -74,14 +85,22 @@ type graphQLResponse struct { } `json:"errors"` } +type reportRunResponse struct { + RunAt time.Time `json:"runAt"` + Results *struct { + RowCount *int `json:"rowCount"` + Type string `json:"__typename"` + } `json:"results"` + Status string `json:"status"` + URL string `json:"url"` +} + type reportResponse struct { - Report struct { - ID string `json:"id"` - Name string `json:"name"` - LastRun struct { - Status string `json:"status"` - URL string `json:"url"` - } `json:"lastRun"` + Report *struct { + LastRun *reportRunResponse `json:"lastRun"` + ID string `json:"id"` + Name string `json:"name"` + RunIntervalHours int `json:"runIntervalHours"` } `json:"report"` } @@ -142,14 +161,71 @@ func (c *HTTPClient) GetReport(ctx context.Context, accessToken, reportID string return nil, errors.Wrapf(err, "failed to get report %s", reportID) } - if result.Report.LastRun.Status != "COMPLETED" { - return nil, errors.Errorf("report %s run status is %s", reportID, result.Report.LastRun.Status) + return validateReportMetadata(result.Report, reportID, time.Now().UTC()) +} + +func validateReportMetadata(report *struct { + LastRun *reportRunResponse `json:"lastRun"` + ID string `json:"id"` + Name string `json:"name"` + RunIntervalHours int `json:"runIntervalHours"` +}, reportID string, now time.Time) (*Report, error) { + if report == nil { + return nil, errors.Errorf("report %s not found", reportID) + } + if report.ID != reportID { + return nil, errors.Errorf("report identity mismatch: requested %s, received %s", reportID, report.ID) + } + if strings.TrimSpace(report.Name) == "" { + return nil, errors.Errorf("report %s has no name", reportID) + } + if report.LastRun == nil { + return nil, errors.Errorf("report %s has no last run", reportID) + } + if report.LastRun.Status != "COMPLETED" { + return nil, errors.Errorf("report %s run status is %s", reportID, report.LastRun.Status) + } + if strings.TrimSpace(report.LastRun.URL) == "" { + return nil, errors.Errorf("report %s has no download URL", reportID) + } + if report.LastRun.RunAt.IsZero() { + return nil, errors.Errorf("report %s has no run time", reportID) + } + if report.RunIntervalHours <= 0 { + return nil, errors.Errorf("report %s has invalid run interval %d hours", reportID, report.RunIntervalHours) + } + if report.LastRun.Results == nil { + return nil, errors.Errorf("report %s has no run results", reportID) + } + resultType := report.LastRun.Results.Type + switch resultType { + case "ReportRunResultsGraphQuery", "ReportRunResultsCloudResource", "ReportRunResultsCloudResourceV2": + default: + return nil, errors.Errorf("report %s has unsupported result type %q", reportID, resultType) + } + if report.LastRun.Results.RowCount == nil { + return nil, errors.Errorf("report %s has no row count", reportID) + } + if *report.LastRun.Results.RowCount < 0 { + return nil, errors.Errorf("report %s has invalid row count %d", reportID, *report.LastRun.Results.RowCount) + } + + if report.LastRun.RunAt.After(now.Add(maxFutureClockSkew)) { + return nil, errors.Errorf("report %s run time %s is in the future beyond allowed clock skew %s", reportID, report.LastRun.RunAt, maxFutureClockSkew) + } + maxAge := time.Duration(report.RunIntervalHours)*time.Hour + reportFreshnessGrace + age := now.Sub(report.LastRun.RunAt) + if age > maxAge { + return nil, errors.Errorf("report %s run is stale: age %s exceeds maximum %s", reportID, age, maxAge) } return &Report{ - ID: result.Report.ID, - Name: result.Report.Name, - DownloadURL: result.Report.LastRun.URL, + ID: report.ID, + Name: report.Name, + DownloadURL: report.LastRun.URL, + LastRun: report.LastRun.RunAt, + RunIntervalHours: report.RunIntervalHours, + ExpectedRows: *report.LastRun.Results.RowCount, }, nil } @@ -157,12 +233,12 @@ func (c *HTTPClient) GetReport(ctx context.Context, accessToken, reportID string func (c *HTTPClient) DownloadReport(ctx context.Context, downloadURL string) (io.ReadCloser, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, http.NoBody) if err != nil { - return nil, errors.Wrap(err, "failed to create download request") + return nil, errors.New("failed to create download request") } resp, err := c.httpClient.Do(req) if err != nil { - return nil, errors.Wrap(err, "failed to download report") + return nil, errors.New("failed to download report") } if resp.StatusCode != http.StatusOK { diff --git a/pkg/inventory/wiz/http_client_test.go b/pkg/inventory/wiz/http_client_test.go index 95c3105..d9880bf 100644 --- a/pkg/inventory/wiz/http_client_test.go +++ b/pkg/inventory/wiz/http_client_test.go @@ -2,6 +2,8 @@ package wiz import ( "context" + "encoding/json" + "fmt" "io" "net/http" "net/http/httptest" @@ -101,21 +103,37 @@ func TestGetAccessToken_TransportError(t *testing.T) { // ---------------- GetReport ---------------- func TestGetReport_HappyPath(t *testing.T) { + runAt := time.Now().UTC().Add(-time.Hour) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // Auth header is forwarded. assert.Equal(t, "Bearer test-token", r.Header.Get("Authorization")) assert.Equal(t, "application/json", r.Header.Get("Content-Type")) + var request graphQLRequest + require.NoError(t, json.NewDecoder(r.Body).Decode(&request)) + assert.Contains(t, request.Query, "runIntervalHours") + assert.Contains(t, request.Query, "runAt") + assert.Contains(t, request.Query, "ReportRunResultsGraphQuery") + assert.Contains(t, request.Query, "ReportRunResultsCloudResource") + assert.Contains(t, request.Query, "ReportRunResultsCloudResourceV2") + assert.Equal(t, 3, strings.Count(request.Query, "rowCount:")) + w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{ + _, _ = fmt.Fprintf(w, `{ "data": { "report": { "id": "rep-1", "name": "Aurora Inventory", - "lastRun": {"status": "COMPLETED", "url": "https://files.example/abc.csv"} + "runIntervalHours": 24, + "lastRun": { + "status": "COMPLETED", + "url": "https://files.example/abc.csv", + "runAt": %q, + "results": {"__typename": "ReportRunResultsCloudResourceV2", "rowCount": 12} + } } } - }`)) + }`, runAt.Format(time.RFC3339Nano)) })) defer srv.Close() @@ -125,20 +143,57 @@ func TestGetReport_HappyPath(t *testing.T) { assert.Equal(t, "rep-1", rep.ID) assert.Equal(t, "Aurora Inventory", rep.Name) assert.Equal(t, "https://files.example/abc.csv", rep.DownloadURL) + assert.Equal(t, runAt, rep.LastRun) + assert.Equal(t, 24, rep.RunIntervalHours) + assert.Equal(t, 12, rep.ExpectedRows) } -func TestGetReport_LastRunNotCompleted(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write([]byte(`{ - "data": {"report": {"id":"r","name":"n","lastRun":{"status":"FAILED","url":""}}} - }`)) - })) - defer srv.Close() +func TestGetReport_InvalidMetadata(t *testing.T) { + now := time.Now().UTC() + healthy := func(runAt time.Time) string { + return fmt.Sprintf(`{"data":{"report":{"id":"r","name":"Report","runIntervalHours":24,"lastRun":{"status":"COMPLETED","url":"https://files.example/report.csv","runAt":%q,"results":{"__typename":"ReportRunResultsCloudResourceV2","rowCount":12}}}}}`, runAt.Format(time.RFC3339Nano)) + } - c := newTestHTTPClient("", srv.URL) - _, err := c.GetReport(context.Background(), "tok", "r") - require.Error(t, err) - assert.Contains(t, err.Error(), "FAILED") + tests := []struct { + name string + body string + want string + wantAbsent string + }{ + {name: "null report", body: `{"data":{"report":null}}`, want: "report r not found"}, + {name: "mismatched ID", body: strings.Replace(healthy(now), `"id":"r"`, `"id":"other"`, 1), want: "identity mismatch"}, + {name: "blank name", body: strings.Replace(healthy(now), `"name":"Report"`, `"name":" "`, 1), want: "has no name"}, + {name: "null last run", body: strings.Replace(healthy(now), `"lastRun":{"status":"COMPLETED","url":"https://files.example/report.csv","runAt":`+fmt.Sprintf("%q", now.Format(time.RFC3339Nano))+`,"results":{"__typename":"ReportRunResultsCloudResourceV2","rowCount":12}}`, `"lastRun":null`, 1), want: "has no last run"}, + {name: "non-completed status", body: strings.Replace(healthy(now), `"status":"COMPLETED"`, `"status":"FAILED"`, 1), want: "FAILED"}, + {name: "blank URL", body: strings.Replace(healthy(now), `"url":"https://files.example/report.csv"`, `"url":" "`, 1), want: "download URL", wantAbsent: "files.example"}, + {name: "missing runAt", body: strings.Replace(healthy(now), `"runAt":`+fmt.Sprintf("%q,", now.Format(time.RFC3339Nano)), "", 1), want: "run time"}, + {name: "invalid runAt", body: strings.Replace(healthy(now), fmt.Sprintf("%q", now.Format(time.RFC3339Nano)), `"not-a-time"`, 1), want: "failed to get report r"}, + {name: "missing interval", body: strings.Replace(healthy(now), `"runIntervalHours":24,`, "", 1), want: "run interval"}, + {name: "zero interval", body: strings.Replace(healthy(now), `"runIntervalHours":24`, `"runIntervalHours":0`, 1), want: "run interval"}, + {name: "negative interval", body: strings.Replace(healthy(now), `"runIntervalHours":24`, `"runIntervalHours":-1`, 1), want: "run interval"}, + {name: "missing results", body: strings.Replace(healthy(now), `,"results":{"__typename":"ReportRunResultsCloudResourceV2","rowCount":12}`, "", 1), want: "run results"}, + {name: "unsupported result type", body: strings.Replace(healthy(now), "ReportRunResultsCloudResourceV2", "OtherResults", 1), want: "unsupported result type"}, + {name: "missing row count", body: strings.Replace(healthy(now), `,"rowCount":12`, "", 1), want: "row count"}, + {name: "negative row count", body: strings.Replace(healthy(now), `"rowCount":12`, `"rowCount":-1`, 1), want: "row count"}, + {name: "stale run", body: healthy(now.Add(-30 * time.Hour)), want: "stale"}, + {name: "future run", body: healthy(now.Add(5*time.Minute + time.Second)), want: "future"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(tt.body)) + })) + defer srv.Close() + + _, err := newTestHTTPClient("", srv.URL).GetReport(context.Background(), "tok", "r") + require.Error(t, err) + assert.Contains(t, err.Error(), tt.want) + if tt.wantAbsent != "" { + assert.NotContains(t, err.Error(), tt.wantAbsent) + } + }) + } } func TestGetReport_GraphQLErrorArray(t *testing.T) { @@ -173,15 +228,16 @@ func TestDoGraphQL_RateLimitRetriesThenSucceeds(t *testing.T) { // the rate-limit substring detection AND that a per-attempt success // breaks out of the loop. calls := 0 + runAt := time.Now().UTC().Add(-time.Hour) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { calls++ if calls == 1 { http.Error(w, "Rate limit exceeded — slow down", http.StatusTooManyRequests) return } - _, _ = w.Write([]byte(`{ - "data":{"report":{"id":"r","name":"n","lastRun":{"status":"COMPLETED","url":"u"}}} - }`)) + _, _ = fmt.Fprintf(w, `{ + "data":{"report":{"id":"r","name":"n","runIntervalHours":24,"lastRun":{"status":"COMPLETED","url":"u","runAt":%q,"results":{"__typename":"ReportRunResultsGraphQuery","rowCount":1}}}} + }`, runAt.Format(time.RFC3339Nano)) })) defer srv.Close() @@ -263,3 +319,36 @@ func TestDownloadReport_TransportError(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "download") } + +func TestDownloadReport_MalformedURLRedactedFromError(t *testing.T) { + sensitiveQuery := "X-Amz-Credential=secret\nX-Amz-Signature=token" + downloadURL := "https://files.example/report.csv?" + sensitiveQuery + + c := newTestHTTPClient("", "") + _, err := c.DownloadReport(context.Background(), downloadURL) + require.Error(t, err) + assert.Contains(t, err.Error(), "create download request") + assert.NotContains(t, err.Error(), downloadURL) + assert.NotContains(t, err.Error(), sensitiveQuery) + assert.NotContains(t, err.Error(), "X-Amz-Credential=secret") +} + +func TestDownloadReport_TransportErrorRedactsURL(t *testing.T) { + sensitiveQuery := "X-Amz-Credential=secret&X-Amz-Signature=token" + downloadURL := "https://files.example/report.csv?" + sensitiveQuery + c := newTestHTTPClient("", "") + c.httpClient.Transport = roundTripFunc(func(req *http.Request) (*http.Response, error) { + return nil, fmt.Errorf("transport failed for %s", req.URL.String()) + }) + + _, err := c.DownloadReport(context.Background(), downloadURL) + require.Error(t, err) + assert.NotContains(t, err.Error(), downloadURL) + assert.NotContains(t, err.Error(), sensitiveQuery) +} + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +}