Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .coverage-floors
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Per-package statement-coverage floors (percent). A package below its floor fails
# CI. Floors are set a few points below current coverage to lock it in without
# flapping on small refactors; RAISE a floor when you raise its coverage (ratchet).
# Packages absent here have no floor yet (e.g. internal/integration is fixture/
# test-only with no coverable statements of its own).
cmd 15
internal/audit 60
internal/baseline 70
internal/certs 90
internal/cloud 55
internal/cloud/aws 80
internal/cloud/k8s 45
internal/compare 60
internal/compliance 65
internal/cost 95
internal/drift 85
internal/fix 18
internal/iam 50
internal/inventory 85
internal/network 90
internal/orphans 90
internal/output 65
internal/output/sinks 80
internal/platform 80
internal/providers 50
internal/quota 95
internal/report 40
internal/secrets 90
internal/storage 28
internal/tags 90
19 changes: 5 additions & 14 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,16 @@ jobs:
- name: go build
run: go build ./...

- name: go test with coverage
run: go test ./... -coverprofile=coverage.out -covermode=atomic
- name: test with per-package coverage floors
# Runs the suite with coverage and enforces .coverage-floors (each package
# has its own floor, ratcheting up as coverage lands). Produces coverage.out.
run: bash scripts/coverage.sh

- name: go vet
run: go vet ./...

- name: enforce coverage floor
# Phase 2 lifted internal/cloud/aws from 0% to ~65-85%. The 50% floor is
# the floor — it'll ratchet up as more code lands under test.
run: |
total=$(go tool cover -func=coverage.out | grep total: | awk '{print $3}' | sed 's/%//')
echo "total coverage: ${total}%"
# bash arithmetic on decimals via awk
if awk "BEGIN {exit !(${total} < 50)}"; then
echo "::error::coverage ${total}% is below the 50% floor"
exit 1
fi

- name: upload coverage artifact
if: always() # keep the profile even when a floor fails, for debugging
uses: actions/upload-artifact@v4
with:
name: coverage
Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ each item completely; `task build` + `go test ./...` green before marking `[x]`.
- **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.
- [x] **9b — split the monolithic renderer files**: the 715-line `output/table.go` + 252-line `output/json.go` are split into per-domain files — `output/<domain>.go` (iam, storage, orphans, cost, network, certs, tags, drift, compliance, lambda, k8s, secrets, audit, inventory, quota, compare, platform) each owns that domain's table renderer + JSON report struct + writer; shared infra lives in `style.go` (lipgloss styles, `colorSeverity`, `formatTags`, `truncate`) and `jsoncore.go` (`writeJSON`). Adding a domain now adds one file instead of editing two shared monoliths. (`sarif.go` left cohesive as the SARIF concern.) Pure move — verified behavior-preserving by line-conservation (869 code lines in == 869 out, sorted-identical) plus the unchanged `output` test suite (table/json/sarif) passing. 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 `compare.NormalizeReport`, so a uniform `any`-typed registry would trade type safety for indirection with no caller that needs it.
- [ ] **Integration tests + CI floors**: cmd→scanner→provider→output tests with fixtures; per-package coverage floors + `golangci-lint` in `ci.yml` (folds in section 6).
- [x] **Integration tests + CI floors**: `internal/integration` runs provider→scanner→output end-to-end — a fixture provider registered through the real `providers.NewRegistry`/`Capable`, resolved by capability, run through each domain scanner, rendered to JSON+table, asserting multiple fields per domain + a severity-filter-discriminates case (the cmd `RunE` shell resolves via `Default()` and is intentionally un-injectable, so the cobra layer is covered by `cmd` unit tests). Writing the `cmd` helper tests (`cmd/remediate_test.go`) surfaced + fixed a latent bug: remediate's bare-array fallback was unreachable (a bare array errored the envelope unmarshal before the fallback ran). `.coverage-floors` (per-package floors, ratcheting) enforced by `scripts/coverage.sh` in `ci.yml` — fails on below-floor, on a floored package with no coverage line (stale name), and on a tested package with no floor (ungated new code); `golangci-lint` + coverage profiling were already in CI. Verified by an adversarial review workflow (12 findings; bugfix + floor logic verified correct, the rest — honest seam labeling, multi-field assertions, filter discrimination, the two new floor guards, bugfix edge-case tests — addressed).

## how to run a single improvement pass (headless)

Expand Down
55 changes: 31 additions & 24 deletions cmd/remediate.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,43 +121,50 @@ type orphansEnvelope struct {

func unmarshalStorageReport(data []byte) ([]cloud.BucketFinding, error) {
var env storageEnvelope
if err := json.Unmarshal(data, &env); err != nil {
return nil, fmt.Errorf("parse storage report: %w", err)
}
if len(env.Findings) == 0 {
// Fall back to a bare-array shape some users hand-craft.
var bare []cloud.BucketFinding
if err := json.Unmarshal(data, &bare); err == nil && len(bare) > 0 {
return bare, nil
}
envErr := json.Unmarshal(data, &env)
if envErr == nil && len(env.Findings) > 0 {
return env.Findings, nil
}
return env.Findings, nil
// Fall back to a bare-array shape some users hand-craft. A bare array fails the
// envelope unmarshal above, so this must be tried regardless of envErr.
var bare []cloud.BucketFinding
if err := json.Unmarshal(data, &bare); err == nil {
return bare, nil
}
if envErr != nil {
return nil, fmt.Errorf("parse storage report: %w", envErr)
}
return env.Findings, nil // valid envelope with no findings
}

func unmarshalNetworkReport(data []byte) ([]cloud.NetworkFinding, error) {
var env networkEnvelope
if err := json.Unmarshal(data, &env); err != nil {
return nil, fmt.Errorf("parse network report: %w", err)
envErr := json.Unmarshal(data, &env)
if envErr == nil && len(env.Findings) > 0 {
return env.Findings, nil
}
if len(env.Findings) == 0 {
var bare []cloud.NetworkFinding
if err := json.Unmarshal(data, &bare); err == nil && len(bare) > 0 {
return bare, nil
}
var bare []cloud.NetworkFinding
if err := json.Unmarshal(data, &bare); err == nil {
return bare, nil
}
if envErr != nil {
return nil, fmt.Errorf("parse network report: %w", envErr)
}
return env.Findings, nil
}

func unmarshalOrphansReport(data []byte) ([]cloud.OrphanResource, error) {
var env orphansEnvelope
if err := json.Unmarshal(data, &env); err != nil {
return nil, fmt.Errorf("parse orphans report: %w", err)
envErr := json.Unmarshal(data, &env)
if envErr == nil && len(env.Resources) > 0 {
return env.Resources, nil
}
if len(env.Resources) == 0 {
var bare []cloud.OrphanResource
if err := json.Unmarshal(data, &bare); err == nil && len(bare) > 0 {
return bare, nil
}
var bare []cloud.OrphanResource
if err := json.Unmarshal(data, &bare); err == nil {
return bare, nil
}
if envErr != nil {
return nil, fmt.Errorf("parse orphans report: %w", envErr)
}
return env.Resources, nil
}
Expand Down
110 changes: 110 additions & 0 deletions cmd/remediate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
package cmd

import (
"testing"

"github.com/nanohype/cloudgov/internal/cloud"
)

func TestUnmarshalStorageReport(t *testing.T) {
// Envelope shape.
env := []byte(`{"findings":[{"bucket":"b1","severity":"HIGH"}],"total":1}`)
got, err := unmarshalStorageReport(env)
if err != nil {
t.Fatalf("envelope: %v", err)
}
if len(got) != 1 || got[0].Bucket != "b1" {
t.Errorf("envelope: got %+v", got)
}
// Bare-array fallback.
bare := []byte(`[{"bucket":"b2","severity":"LOW"}]`)
got, err = unmarshalStorageReport(bare)
if err != nil {
t.Fatalf("bare: %v", err)
}
if len(got) != 1 || got[0].Bucket != "b2" {
t.Errorf("bare: got %+v", got)
}
// Invalid JSON errors.
if _, err := unmarshalStorageReport([]byte("not json")); err == nil {
t.Error("expected error on invalid JSON")
}
// Valid envelope with no findings returns empty, not an error (the fallback's
// final path).
got, err = unmarshalStorageReport([]byte(`{"findings":[],"total":0}`))
if err != nil || len(got) != 0 {
t.Errorf("empty envelope: got %+v err %v, want empty/nil", got, err)
}
}

func TestUnmarshalNetworkReport(t *testing.T) {
got, err := unmarshalNetworkReport([]byte(`{"findings":[{"resource":"sg-1","severity":"HIGH"}]}`))
if err != nil || len(got) != 1 || got[0].Resource != "sg-1" {
t.Fatalf("envelope: got %+v err %v", got, err)
}
got, err = unmarshalNetworkReport([]byte(`[{"resource":"sg-2"}]`))
if err != nil || len(got) != 1 || got[0].Resource != "sg-2" {
t.Fatalf("bare: got %+v err %v", got, err)
}
if _, err := unmarshalNetworkReport([]byte("not json")); err == nil {
t.Error("expected error on invalid JSON")
}
got, err = unmarshalNetworkReport([]byte(`{"findings":[]}`))
if err != nil || len(got) != 0 {
t.Errorf("empty envelope: got %+v err %v, want empty/nil", got, err)
}
}

func TestUnmarshalOrphansReport(t *testing.T) {
// Orphans use a "resources" envelope (not "findings").
got, err := unmarshalOrphansReport([]byte(`{"resources":[{"Kind":"disk","ID":"vol-1"}],"total":1}`))
if err != nil || len(got) != 1 || got[0].ID != "vol-1" {
t.Fatalf("envelope: got %+v err %v", got, err)
}
got, err = unmarshalOrphansReport([]byte(`[{"Kind":"ip","ID":"eip-1"}]`))
if err != nil || len(got) != 1 || got[0].ID != "eip-1" {
t.Fatalf("bare: got %+v err %v", got, err)
}
if _, err := unmarshalOrphansReport([]byte("not json")); err == nil {
t.Error("expected error on invalid JSON")
}
got, err = unmarshalOrphansReport([]byte(`{"resources":[],"total":0}`))
if err != nil || len(got) != 0 {
t.Errorf("empty envelope: got %+v err %v, want empty/nil", got, err)
}
}

func TestFilterStorageBySeverity(t *testing.T) {
in := []cloud.BucketFinding{
{Bucket: "crit", Severity: cloud.SeverityCritical},
{Bucket: "low", Severity: cloud.SeverityLow},
}
got := filterStorageBySeverity(in, cloud.SeverityHigh)
if len(got) != 1 || got[0].Bucket != "crit" {
t.Errorf("got %+v, want only crit", got)
}
}

func TestFilterNetworkBySeverity(t *testing.T) {
in := []cloud.NetworkFinding{
{Resource: "high", Severity: cloud.SeverityHigh},
{Resource: "low", Severity: cloud.SeverityLow},
}
got := filterNetworkBySeverity(in, cloud.SeverityMedium)
if len(got) != 1 || got[0].Resource != "high" {
t.Errorf("got %+v, want only high", got)
}
}

func TestAnnounceFiles(t *testing.T) {
// Exercises both the "wrote" and the "no remediable findings" branches, plus
// the quiet short-circuit. Output goes to stderr; we assert it doesn't panic
// and that quiet suppresses cleanly.
orig := quiet
defer func() { quiet = orig }()
quiet = false
announceFiles([]string{"fix-aws.sh"}, 1)
announceFiles(nil, 3)
quiet = true
announceFiles([]string{"fix-aws.sh"}, 1)
}
13 changes: 13 additions & 0 deletions internal/integration/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Package integration holds tests that exercise the provider → scanner → output
// layers together. A fixture provider is registered through the real provider
// registry (providers.NewRegistry / Capable), resolved by capability, run through
// the domain scanner, and rendered by the output package.
//
// This is the substance of what a command does, but not the command shell itself:
// a command's RunE resolves providers via providers.Resolve (→ Default()), which is
// AWS-backed and intentionally has no test-injection seam, so the cobra layer
// (flag→ScanOptions threading, the output-format switch, gate/exit-code) is covered
// by the unit tests in package cmd, not here. These tests catch composition breaks
// the per-layer unit tests miss: a scanner that resolves or filters wrong, or a
// renderer that drops a field.
package integration
Loading
Loading