diff --git a/CLAUDE.md b/CLAUDE.md index da07c80..96932c1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -184,7 +184,9 @@ each item completely; `task build` + `go test ./...` green before marking `[x]`. - [x] **7c — tags coverage**: `AuditTags` now also checks ECS clusters, EKS clusters, DynamoDB tables, SNS topics, and SQS queues for missing required tags (was EC2/S3/RDS/Lambda only). New `dynamodbAPI`/`snsAPI` narrow interfaces + `dynamodb`/`sns` clients; `eksAPI` gained `DescribeCluster` and `sqsAPI` gained `ListQueueTags` (extended in place); ECS reuses the existing `ecsAPI` via `DescribeClusters` with `include=TAGS`. Each auditor paginates, derives the resource name (ARN/URL tail), and nil-guards its client so partial test construction doesn't panic. Mock-tested per service. README least-privilege policy + the tags section updated with the new resource types and read/tag IAM actions. - [x] **7d — certs SARIF + Days**: `output.WriteCertsSARIF` + `buildCertRules` (rule id = cert status, level follows severity) wired as `cloudgov certs --output sarif`. `opts.Days` is now authoritative: `ListCertificates` dropped its hardcoded `>180` cap so the certs scanner's `--days` filter is the single window gate (cmd + audit both pass it, default 90) — `--days 365` now surfaces certs expiring in 181+ days, no change at the default. README certs flags table + a SARIF usage example added. Verified by an adversarial review workflow (both correctness dimensions clean; confirmed doc/test-parity nits fixed: README `--output` row, `buildCertRules` in the `TestBuildRules_NonEmpty` map, and `TestWriteCertsSARIF` now asserts the driver rules table levels). - [x] **Honest AWS-only + parity matrix**: an audit workflow found the headline was already AWS-honest; the real overclaims were a handful of command help strings using generic "cloud"/"across providers" (`inventory`, `quota`, `secrets`/`secrets scan` which falsely listed GCP "Cloud Functions" + Azure "App Service" as scan targets, plus `cost`/`orphans`/`drift`). All rewritten to name AWS. README gains a `## Cloud support` section + a command×cloud parity matrix (✅ implemented / ⬡ seam-ready / — n/a): AWS full across all domains, GCP/Azure seam-ready (capability interfaces exist, no provider), k8s for RBAC; offline + `mcp` commands noted as cloud-agnostic. The pluggable-seam framing (the intentional design) is kept; only present-tense multi-cloud claims were removed. Verified by an adversarial review workflow (no overclaim survived, every matrix row accurate; fixed its 4 LOW nits — inventory/drift H3 headings, the cost/orphans/drift Shorts, the Platform footnote which had said "RBAC" instead of IRSA + tenant cluster objects, and `mcp` missing from the matrix note). -- [ ] **Output renderer registry**: `FindingRenderer` so domains self-register (stop editing the two 1000-line `output/{table,json}.go`); move severity into the domain structs (`cloud.QuotaUsage.Severity`). +- **Output renderer cleanup** (split into ordered sub-items): + - [x] **9a — Severity on domain structs**: `cloud.QuotaUsage` gains a `Severity` field (`json:"severity"`), set at construction in the AWS provider's `ListQuotas` (derived from utilization). All readers — `output.WriteQuotas`, `quota.Summarize`, `compare.normalizeQuotas`, and the `report` HTML generator — now read it via a `QuotaUsage.EffectiveSeverity()` accessor that falls back to computing from `Utilization` when unset (back-compat for reports saved before the field + hand-built test data). QuotaUsage was the one struct recomputing severity per-reader; the other findingless structs (OrphanResource/CostDiff/InventoryResource) carry no severity by nature. Mock-tested; verified all read sites converted by grep. + - [ ] **9b — split the monolithic renderer files**: move the per-domain renderers out of the 715-line `output/table.go` (and `json.go`) into per-domain files so adding a domain doesn't edit a shared monolith. NOTE: the original "runtime `FindingRenderer` registry" was reframed after empirical review — commands dispatch type-specifically (compile-time-safe), renderers have heterogeneous signatures (IAM principal counts, audit `*Report`, compare `CompareResult`), and the one generic consumer (`report`) already routes through the centralized `compare.NormalizeReport` switch, so a uniform `any`-typed registry would trade type safety for indirection with no caller that needs it. The achievable win is the per-domain file split (pure code-org, behavior-preserving). - [ ] **Integration tests + CI floors**: cmd→scanner→provider→output tests with fixtures; per-package coverage floors + `golangci-lint` in `ci.yml` (folds in section 6). ## how to run a single improvement pass (headless) diff --git a/internal/cloud/aws/quota.go b/internal/cloud/aws/quota.go index 0d4587f..0bfa85b 100644 --- a/internal/cloud/aws/quota.go +++ b/internal/cloud/aws/quota.go @@ -77,6 +77,12 @@ func (p *Provider) ListQuotas(ctx context.Context) ([]cloud.QuotaUsage, error) { quotas = append(quotas, rdsQuotas...) } + // Severity is derived from utilization; set it on the struct so it travels with + // the finding (JSON output, comparison) rather than being recomputed per reader. + for i := range quotas { + quotas[i].Severity = cloud.QuotaSeverity(quotas[i].Utilization) + } + return quotas, nil } diff --git a/internal/cloud/aws/quota_test.go b/internal/cloud/aws/quota_test.go index 4b85248..a179a3a 100644 --- a/internal/cloud/aws/quota_test.go +++ b/internal/cloud/aws/quota_test.go @@ -14,6 +14,7 @@ import ( s3types "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/aws/aws-sdk-go-v2/service/servicequotas" sqtypes "github.com/aws/aws-sdk-go-v2/service/servicequotas/types" + "github.com/nanohype/cloudgov/internal/cloud" ) type quotaMockEC2 struct { @@ -214,6 +215,15 @@ func TestListQuotas_AggregatesAll(t *testing.T) { if len(got) != 9 { t.Errorf("expected 9 quotas, got %d", len(got)) } + // ListQuotas sets Severity on every quota (derived from utilization). + for _, q := range got { + if q.Severity == "" { + t.Errorf("quota %s/%s has no Severity set", q.Service, q.QuotaName) + } + if q.Severity != cloud.QuotaSeverity(q.Utilization) { + t.Errorf("quota %s: Severity=%q, want %q", q.QuotaName, q.Severity, cloud.QuotaSeverity(q.Utilization)) + } + } } type mockServiceQuotas struct { diff --git a/internal/cloud/provider_test.go b/internal/cloud/provider_test.go index 00b6b33..e6abf6d 100644 --- a/internal/cloud/provider_test.go +++ b/internal/cloud/provider_test.go @@ -2,6 +2,29 @@ package cloud import "testing" +func TestQuotaEffectiveSeverity(t *testing.T) { + // Stored Severity wins, even when it disagrees with Utilization. + stored := QuotaUsage{Utilization: 10, Severity: SeverityCritical} + if got := stored.EffectiveSeverity(); got != SeverityCritical { + t.Errorf("stored: got %q, want CRITICAL", got) + } + // Unset Severity falls back to computing from Utilization. + for _, tc := range []struct { + util float64 + want Severity + }{ + {95, SeverityCritical}, + {85, SeverityHigh}, + {60, SeverityMedium}, + {10, SeverityLow}, + } { + q := QuotaUsage{Utilization: tc.util} + if got := q.EffectiveSeverity(); got != tc.want { + t.Errorf("util %.0f: got %q, want %q", tc.util, got, tc.want) + } + } +} + func TestSeverityRank(t *testing.T) { tests := []struct { sev Severity diff --git a/internal/cloud/quota.go b/internal/cloud/quota.go index 7d8fc87..c1b4888 100644 --- a/internal/cloud/quota.go +++ b/internal/cloud/quota.go @@ -4,13 +4,24 @@ import "context" // QuotaUsage represents a single service quota and its current utilization. type QuotaUsage struct { - Provider string `json:"provider"` - Service string `json:"service"` - QuotaName string `json:"quota_name"` - Used float64 `json:"used"` - Limit float64 `json:"limit"` - Utilization float64 `json:"utilization"` - Region string `json:"region"` + Provider string `json:"provider"` + Service string `json:"service"` + QuotaName string `json:"quota_name"` + Used float64 `json:"used"` + Limit float64 `json:"limit"` + Utilization float64 `json:"utilization"` + Region string `json:"region"` + Severity Severity `json:"severity"` +} + +// EffectiveSeverity returns the stored Severity, falling back to computing it from +// Utilization when unset — e.g. a report saved before Severity was a field, or a +// QuotaUsage built without going through a provider's ListQuotas. +func (q QuotaUsage) EffectiveSeverity() Severity { + if q.Severity != "" { + return q.Severity + } + return QuotaSeverity(q.Utilization) } // QuotaProvider lists service quota utilization. diff --git a/internal/compare/normalize.go b/internal/compare/normalize.go index dde73bf..93a0b92 100644 --- a/internal/compare/normalize.go +++ b/internal/compare/normalize.go @@ -312,7 +312,7 @@ func normalizeQuotas(data []byte) ([]NormalizedFinding, error) { Type: q.Service, ResourceID: q.QuotaName, Detail: fmt.Sprintf("%.0f/%.0f (%.1f%%)", q.Used, q.Limit, q.Utilization), - Severity: string(cloud.QuotaSeverity(q.Utilization)), + Severity: string(q.EffectiveSeverity()), }) } return result, nil diff --git a/internal/output/json.go b/internal/output/json.go index 59a339e..1e0ef30 100644 --- a/internal/output/json.go +++ b/internal/output/json.go @@ -172,7 +172,7 @@ type quotaReport struct { func WriteQuotas(w io.Writer, quotas []cloud.QuotaUsage) error { var crit, high, med int for _, q := range quotas { - switch cloud.QuotaSeverity(q.Utilization) { + switch q.EffectiveSeverity() { case cloud.SeverityCritical: crit++ case cloud.SeverityHigh: diff --git a/internal/quota/scanner.go b/internal/quota/scanner.go index 624cf3b..a6f1e35 100644 --- a/internal/quota/scanner.go +++ b/internal/quota/scanner.go @@ -48,7 +48,7 @@ type Summary struct { func Summarize(quotas []cloud.QuotaUsage) Summary { s := Summary{Total: len(quotas)} for _, q := range quotas { - switch cloud.QuotaSeverity(q.Utilization) { + switch q.EffectiveSeverity() { case cloud.SeverityCritical: s.Critical++ case cloud.SeverityHigh: diff --git a/internal/report/report.go b/internal/report/report.go index c01a5fd..b8f33f3 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -329,7 +329,7 @@ func buildQuotasReport(data []byte, td *TemplateData) (*TemplateData, error) { td.ByDomain["quotas"] = report.Total for _, q := range report.Quotas { - switch cloud.QuotaSeverity(q.Utilization) { + switch q.EffectiveSeverity() { case cloud.SeverityCritical: td.Critical++ case cloud.SeverityHigh: