diff --git a/docs/superpowers/plans/2026-08-06-package-inventory-and-cve-findings.md b/docs/superpowers/plans/2026-08-06-package-inventory-and-cve-findings.md index 3b61ff6..b0c41e4 100644 --- a/docs/superpowers/plans/2026-08-06-package-inventory-and-cve-findings.md +++ b/docs/superpowers/plans/2026-08-06-package-inventory-and-cve-findings.md @@ -4,7 +4,7 @@ **Goal:** Agents report the packages installed on each Linux server; the control plane matches them against distribution security feeds (`trivy-db`) and raises findings that link to the existing `ApplyUpdatesCmd` patching path. -**Architecture:** The agent collects packages on its existing hourly loop and offers a hash first, sending the ~150KB list only when the server says it differs. `ReportPackages` does **not** match — it upserts the list and sets `scan_pending: true`. A new `vulnsched` loop, running inside the existing `bus.RunAsLeader("housekeeping", …)`, pulls `trivy-db` to an ephemeral directory, scans pending servers, diffs findings and emits one batched digest per tick. All matching logic is pure functions in `server/internal/vulndb` testable without a database, a network or a real database file. +**Architecture:** The agent collects packages on its existing hourly loop and offers a hash first, sending the ~150KB list only when the server says it differs. `ReportPackages` does **not** match — it upserts the list and sets `scan_pending: true`. A new `vulnsched` loop, running inside the existing `bus.RunAsLeader("housekeeping", …)`, pulls `trivy-db` to an ephemeral directory, scans pending servers, diffs findings and emits one batched digest per tick. Matching logic lives in `server/internal/vulndb` as pure functions behind a small interface. **Tech Stack:** Go 1.26, gin, mongo-driver v2, gRPC, bbolt, Next.js 16 + TanStack Query, Tailwind 3. @@ -12,9 +12,8 @@ ## Global Constraints +- **Do not write tests.** No `*_test.go`, no npm test files, no test scaffolding. This is a deliberate instruction from the repository owner, not an oversight. Verification in this plan is `go build`, `go vet`, `npm run build` and manual checks. - **Linux only.** Windows agents must not collect packages and must not appear in findings. Do not add an MSRC source, a `Get-HotFix` collector or KB supersedence logic — that is a separate spec. -- **This repository has no Go tests today.** `git ls-files | grep _test.go` returns nothing. Tests added here are the first. They must run under plain `go test ./...` from the repo root with **no database, no network, no build tags and no fixtures downloaded at test time**. -- CLAUDE.md claims `shared/mail/render_test.go` exists. It does not. Task 12 creates it. - Every service query is scoped by `instance_id`. No unscoped lookups, ever. - Every mutating API path writes an audit event via `services.LogEvent(instanceID, eventType, actor, serverID, keyID, details string)`. - Licence features are read with `lic.HasFeature("vuln_scanning")`. **Never switch on `Tier`.** @@ -26,6 +25,18 @@ - Severity strings are lowercase and fixed: `critical`, `high`, `medium`, `low`, `unknown`. - Finding states are exactly `open`, `fixed`, `accepted`. +## Correctness risks carried without tests + +These are the places where a mistake produces a **wrong answer rather than a crash**, so nothing will surface them automatically. Read the code twice at each: + +1. **Version comparison (Task 1).** `dpkg` ordering has epochs and sorts `~` before the empty string, so `3.0.2-0ubuntu1.15~rc1` precedes `3.0.2-0ubuntu1.15`. `rpmvercmp` has its own segment rules. Any fallback to string comparison orders `1.10` before `1.9`. Every one of those mistakes reports a vulnerable fleet as clean. +2. **The backport case (Task 11).** Installed `1:3.0.2-0ubuntu1.15` against advisory fixed-in `1:3.0.2-0ubuntu1.15` must resolve to **not vulnerable**. Equal must not compare as less-than. +3. **`first_seen` preservation (Task 12).** An upsert that overwrites it makes every finding look discovered today. +4. **Fixed-before-reopen ordering (Task 12).** A finding both absent from the scan and past its acceptance expiry must settle `fixed`, not reopen. +5. **Log/line caps** do not apply here, but the **200k-row equivalent** does: `server_packages` is one document per server and must stay one, or the 16MB limit becomes reachable. + +Manual verification for 1 and 2 is described inline in those tasks. + ## Dependencies to add Run from the repo root; `go.work` covers all modules. @@ -49,31 +60,24 @@ go get go.etcd.io/bbolt@latest **Create — agent:** - `agent/internal/packages/packages.go` — `Collect()` dispatch by package manager - `agent/internal/packages/parse.go` — pure output parsers, one per manager -- `agent/internal/packages/parse_test.go` — parser tests from captured output - `agent/internal/packages/osrelease.go` — `/etc/os-release` parsing -- `agent/internal/packages/osrelease_test.go` **Create — server:** - `server/internal/models/packages.go` — `ServerPackages`, `InstalledPackage`, `OSRelease` - `server/internal/models/vuln.go` — `VulnFinding`, `Acceptance`, `VulnDBMeta`, `VulnAlertRule` - `server/internal/vulndb/version.go` — comparator dispatch by OS family -- `server/internal/vulndb/version_test.go` — **the highest-value test in the feature** - `server/internal/vulndb/ecosystem.go` — OS family/version → trivy-db bucket, or unsupported -- `server/internal/vulndb/ecosystem_test.go` - `server/internal/vulndb/pull.go` — OCI fetch to temp dir - `server/internal/vulndb/db.go` — advisory lookup over the BoltDB - `server/internal/vulndb/match.go` — pure matching -- `server/internal/vulndb/match_test.go` -- `server/internal/vulndb/source.go` — binary → source name resolution - `server/internal/services/packages.go` — store, hash compare, package search - `server/internal/services/findings.go` — finding diff/state machine + acceptance -- `server/internal/services/findings_test.go` — pure state-machine tests -- `server/internal/services/vulnrules.go` — alert rule CRUD, digest recipients +- `server/internal/services/vulnrules.go` — alert rule CRUD, digest dispatch +- `server/internal/services/vulnindexes.go` — index builder - `server/internal/vulnsched/sched.go` — the leader-owned loop - `server/internal/api/vulnerabilities.go` — REST handlers - `shared/mail/vuln.go` — `SendVulnDigest` - `shared/mail/templates/vuln_digest.html.tmpl`, `vuln_digest.txt.tmpl` -- `shared/mail/render_test.go` — renders every template; fails on any not covered **Create — web:** - `web/app/(app)/vulnerabilities/page.tsx` — fleet board grouped by CVE @@ -85,8 +89,7 @@ go get go.etcd.io/bbolt@latest **Modify:** - `proto/vantage/v1/vantage.proto` — `ReportPackages` RPC and messages; `SyncResponse.collect_packages` - `agent/internal/sync/sync.go:370-411` — `runUpdateCheck` also reports packages -- `server/internal/grpc/server.go` — `ReportPackages` handler -- `server/internal/services/servers.go` — `SyncKeys` sets `collect_packages` +- `server/internal/grpc/server.go` — `ReportPackages` handler; `SyncKeys` sets the flag - `server/cmd/main.go:184-206` — start `vulnsched` and the sweeper inside `RunAsLeader` - `server/cmd/main.go` (schema setup, near the other `Ensure*Indexes` calls) — `EnsureVulnIndexes` - `server/internal/api/handlers.go` — register routes @@ -103,94 +106,25 @@ go get go.etcd.io/bbolt@latest ### Task 1: Version comparators -The single most important task. Its failure mode is silent: a wrong comparison reports a vulnerable fleet as clean. +The most correctness-critical code in the feature. Its failure mode is silent: a wrong comparison reports a vulnerable fleet as clean. **Files:** - Create: `server/internal/vulndb/version.go` -- Test: `server/internal/vulndb/version_test.go` **Interfaces:** - Consumes: nothing. - Produces: `vulndb.LessThan(family, a, b string) (bool, error)` and `vulndb.ErrUnsupportedFamily`. -- [ ] **Step 1: Write the failing test** +- [ ] **Step 1: Add the dependencies** -Create `server/internal/vulndb/version_test.go`: - -```go -package vulndb - -import "testing" - -func TestLessThan(t *testing.T) { - cases := []struct { - name string - family string - a, b string - want bool - }{ - // The backport trap. An Ubuntu package patched in place carries the - // same upstream version as the vulnerable one; only the revision - // differs. Equal versions must NOT compare as less than, or a patched - // host reports as vulnerable. - {"deb equal backport", "ubuntu", "1:3.0.2-0ubuntu1.15", "1:3.0.2-0ubuntu1.15", false}, - {"deb revision older", "ubuntu", "1:3.0.2-0ubuntu1.14", "1:3.0.2-0ubuntu1.15", true}, - {"deb revision newer", "ubuntu", "1:3.0.2-0ubuntu1.16", "1:3.0.2-0ubuntu1.15", false}, - - // Tilde sorts BEFORE the empty string: 1.0~rc1 precedes 1.0. - {"deb tilde is older", "debian", "1.0~rc1", "1.0", true}, - {"deb tilde reversed", "debian", "1.0", "1.0~rc1", false}, - - // Epoch dominates everything to its right. - {"deb epoch dominates", "debian", "2.0", "1:1.0", true}, - - // Numeric segments compare numerically, not lexically. - {"deb numeric not lexical", "debian", "1.9", "1.10", true}, - - {"rpm equal", "redhat", "1.2.3-4.el9", "1.2.3-4.el9", false}, - {"rpm release older", "redhat", "1.2.3-3.el9", "1.2.3-4.el9", true}, - {"rpm epoch dominates", "redhat", "2:1.0-1", "1:9.0-1", false}, - {"rpm numeric not lexical", "rocky", "1.9-1", "1.10-1", true}, - - {"apk equal", "alpine", "1.2.3-r0", "1.2.3-r0", false}, - {"apk release older", "alpine", "1.2.3-r0", "1.2.3-r1", true}, - {"apk numeric not lexical", "alpine", "1.9-r0", "1.10-r0", true}, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - got, err := LessThan(tc.family, tc.a, tc.b) - if err != nil { - t.Fatalf("LessThan(%q, %q, %q): unexpected error %v", tc.family, tc.a, tc.b, err) - } - if got != tc.want { - t.Errorf("LessThan(%q, %q, %q) = %v, want %v", tc.family, tc.a, tc.b, got, tc.want) - } - }) - } -} - -func TestLessThanUnsupportedFamily(t *testing.T) { - if _, err := LessThan("arch", "1.0", "2.0"); err == nil { - t.Fatal("expected an error for an unsupported family, got nil") - } -} - -func TestLessThanUnparseableVersion(t *testing.T) { - // An unparseable version must error, never silently answer false — - // false here means "not vulnerable", which is the dangerous direction. - if _, err := LessThan("ubuntu", "not a version!!", "1.0"); err == nil { - t.Fatal("expected an error for an unparseable version, got nil") - } -} +```bash +cd server +go get github.com/knqyf263/go-deb-version@latest +go get github.com/knqyf263/go-rpm-version@latest +go get github.com/knqyf263/go-apk-version@latest ``` -- [ ] **Step 2: Run the test to verify it fails** - -Run: `cd server && go test ./internal/vulndb/ -run TestLessThan -v` -Expected: build failure — package `vulndb` does not exist. - -- [ ] **Step 3: Write the implementation** +- [ ] **Step 2: Write the implementation** Create `server/internal/vulndb/version.go`: @@ -226,6 +160,9 @@ var ErrUnsupportedFamily = errors.New("unsupported OS family") func LessThan(family, a, b string) (bool, error) { switch family { case "debian", "ubuntu": + if a == "" || b == "" { + return false, fmt.Errorf("empty deb version (a=%q b=%q)", a, b) + } va, err := deb.NewVersion(a) if err != nil { return false, fmt.Errorf("parse deb version %q: %w", a, err) @@ -236,7 +173,7 @@ func LessThan(family, a, b string) (bool, error) { } return va.LessThan(vb), nil - case "redhat", "centos", "rocky", "alma", "amazon", "oracle": + case "redhat", "centos", "rocky", "alma", "amazon", "oracle", "suse", "opensuse", "sles": // go-rpm-version does not error; rpmvercmp is defined over arbitrary // strings. Guard empties so a missing version cannot read as equal. if a == "" || b == "" { @@ -245,6 +182,9 @@ func LessThan(family, a, b string) (bool, error) { return rpm.NewVersion(a).LessThan(rpm.NewVersion(b)), nil case "alpine": + if a == "" || b == "" { + return false, fmt.Errorf("empty apk version (a=%q b=%q)", a, b) + } va, err := apk.NewVersion(a) if err != nil { return false, fmt.Errorf("parse apk version %q: %w", a, err) @@ -255,29 +195,45 @@ func LessThan(family, a, b string) (bool, error) { } return va.LessThan(vb), nil - case "suse", "opensuse", "sles": - if a == "" || b == "" { - return false, fmt.Errorf("empty rpm version (a=%q b=%q)", a, b) - } - return rpm.NewVersion(a).LessThan(rpm.NewVersion(b)), nil - default: return false, fmt.Errorf("%w: %s", ErrUnsupportedFamily, family) } } ``` -- [ ] **Step 4: Run the test to verify it passes** +- [ ] **Step 3: Verify the ordering by hand** -Run: `cd server && go test ./internal/vulndb/ -v` -Expected: PASS, all subtests. +This replaces the test that would normally guard it. Write a scratch `main.go` outside the repo (or use `go run` on a temp file), call `LessThan` with each pair below, and confirm every result. **Do not skip this** — these are the exact cases that go wrong silently. -If `TestLessThanUnparseableVersion` fails because `go-deb-version` accepts the input, replace the input with an empty string and add an explicit empty-string guard to the `debian`/`ubuntu` branch matching the rpm one. Do not delete the test. +| family | a | b | expected | +| ------ | - | - | -------- | +| ubuntu | `1:3.0.2-0ubuntu1.15` | `1:3.0.2-0ubuntu1.15` | `false` — the backport case; equal is not less-than | +| ubuntu | `1:3.0.2-0ubuntu1.14` | `1:3.0.2-0ubuntu1.15` | `true` | +| ubuntu | `1:3.0.2-0ubuntu1.16` | `1:3.0.2-0ubuntu1.15` | `false` | +| debian | `1.0~rc1` | `1.0` | `true` — tilde sorts before empty | +| debian | `1.0` | `1.0~rc1` | `false` | +| debian | `2.0` | `1:1.0` | `true` — epoch dominates | +| debian | `1.9` | `1.10` | `true` — numeric, not lexical | +| redhat | `1.2.3-4.el9` | `1.2.3-4.el9` | `false` | +| redhat | `1.2.3-3.el9` | `1.2.3-4.el9` | `true` | +| redhat | `2:1.0-1` | `1:9.0-1` | `false` | +| rocky | `1.9-1` | `1.10-1` | `true` | +| alpine | `1.2.3-r0` | `1.2.3-r0` | `false` | +| alpine | `1.2.3-r0` | `1.2.3-r1` | `true` | +| alpine | `1.9-r0` | `1.10-r0` | `true` | +| arch | `1.0` | `2.0` | error wrapping `ErrUnsupportedFamily` | + +Delete the scratch file afterwards. If any row disagrees, the bug is in this file or in the chosen library — resolve it before continuing, because everything downstream inherits it. + +- [ ] **Step 4: Build** + +Run: `cd server && go build ./... && go vet ./...` +Expected: exit 0. - [ ] **Step 5: Commit** ```bash -git add server/internal/vulndb/version.go server/internal/vulndb/version_test.go server/go.mod server/go.sum +git add server/internal/vulndb/version.go server/go.mod server/go.sum git commit -m "feat: version comparators for distro package ordering" ``` @@ -287,71 +243,12 @@ git commit -m "feat: version comparators for distro package ordering" **Files:** - Create: `server/internal/vulndb/ecosystem.go` -- Test: `server/internal/vulndb/ecosystem_test.go` **Interfaces:** - Consumes: `vulndb.ErrUnsupportedFamily` from Task 1. - Produces: `vulndb.Bucket(family, versionID string) (string, error)`. -- [ ] **Step 1: Write the failing test** - -Create `server/internal/vulndb/ecosystem_test.go`: - -```go -package vulndb - -import ( - "errors" - "testing" -) - -func TestBucket(t *testing.T) { - cases := []struct { - family, versionID, want string - }{ - {"ubuntu", "22.04", "ubuntu 22.04"}, - {"ubuntu", "24.04", "ubuntu 24.04"}, - {"debian", "12", "debian 12"}, - {"alpine", "3.19.1", "alpine 3.19"}, // apk advisories are keyed on major.minor - {"rocky", "9.3", "redhat 9"}, // rebuilds share Red Hat's feed, major only - {"alma", "9.3", "redhat 9"}, - {"redhat", "9", "redhat 9"}, - } - for _, tc := range cases { - got, err := Bucket(tc.family, tc.versionID) - if err != nil { - t.Fatalf("Bucket(%q, %q): unexpected error %v", tc.family, tc.versionID, err) - } - if got != tc.want { - t.Errorf("Bucket(%q, %q) = %q, want %q", tc.family, tc.versionID, got, tc.want) - } - } -} - -func TestBucketUnsupported(t *testing.T) { - // Arch has no feed in trivy-db. It must be reported unsupported, never - // scanned to zero findings — claiming clean when the truth is unknown is - // the same class of lie as a silently stale database. - if _, err := Bucket("arch", ""); !errors.Is(err, ErrUnsupportedFamily) { - t.Fatalf("Bucket(arch) error = %v, want ErrUnsupportedFamily", err) - } -} - -func TestBucketMissingVersion(t *testing.T) { - // Ubuntu 22.04 and 24.04 publish different fixed versions for the same - // CVE, so a bucket without a version is guesswork. - if _, err := Bucket("ubuntu", ""); err == nil { - t.Fatal("expected an error when the version is missing, got nil") - } -} -``` - -- [ ] **Step 2: Run the test to verify it fails** - -Run: `cd server && go test ./internal/vulndb/ -run TestBucket -v` -Expected: FAIL — `undefined: Bucket`. - -- [ ] **Step 3: Write the implementation** +- [ ] **Step 1: Write the implementation** Create `server/internal/vulndb/ecosystem.go`: @@ -419,15 +316,17 @@ func majorMinor(v string) string { } ``` -- [ ] **Step 4: Run the test to verify it passes** +Expected behaviour, for reference while reading it back: `("ubuntu","22.04")` → `ubuntu 22.04`; `("debian","12")` → `debian 12`; `("alpine","3.19.1")` → `alpine 3.19`; `("rocky","9.3")` → `redhat 9`; `("arch","")` → `ErrUnsupportedFamily`; `("ubuntu","")` → error, because Ubuntu 22.04 and 24.04 publish different fixed versions for the same CVE. -Run: `cd server && go test ./internal/vulndb/ -v` -Expected: PASS. +- [ ] **Step 2: Build** -- [ ] **Step 5: Commit** +Run: `cd server && go build ./...` +Expected: exit 0. + +- [ ] **Step 3: Commit** ```bash -git add server/internal/vulndb/ecosystem.go server/internal/vulndb/ecosystem_test.go +git add server/internal/vulndb/ecosystem.go git commit -m "feat: map OS family and version to trivy-db advisory buckets" ``` @@ -437,85 +336,12 @@ git commit -m "feat: map OS family and version to trivy-db advisory buckets" **Files:** - Create: `agent/internal/packages/osrelease.go` -- Test: `agent/internal/packages/osrelease_test.go` **Interfaces:** - Consumes: nothing. -- Produces: `packages.OSRelease{Family, VersionID, Arch string}` and `packages.ParseOSRelease(r io.Reader) (OSRelease, error)`. +- Produces: `packages.OSRelease{Family, VersionID, Arch string}`, `packages.ParseOSRelease(r io.Reader) (OSRelease, error)`, `packages.DetectOS() (OSRelease, error)`. -- [ ] **Step 1: Write the failing test** - -Create `agent/internal/packages/osrelease_test.go`: - -```go -package packages - -import ( - "strings" - "testing" -) - -const ubuntuOSRelease = `PRETTY_NAME="Ubuntu 22.04.4 LTS" -NAME="Ubuntu" -VERSION_ID="22.04" -VERSION="22.04.4 LTS (Jammy Jellyfish)" -ID=ubuntu -ID_LIKE=debian -HOME_URL="https://www.ubuntu.com/" -` - -const rockyOSRelease = `NAME="Rocky Linux" -VERSION="9.3 (Blue Onyx)" -ID="rocky" -ID_LIKE="rhel centos fedora" -VERSION_ID="9.3" -` - -const alpineOSRelease = `NAME="Alpine Linux" -ID=alpine -VERSION_ID=3.19.1 -PRETTY_NAME="Alpine Linux v3.19" -` - -func TestParseOSRelease(t *testing.T) { - cases := []struct { - name string - input string - family, versionID string - }{ - {"ubuntu", ubuntuOSRelease, "ubuntu", "22.04"}, - {"rocky quoted id", rockyOSRelease, "rocky", "9.3"}, - {"alpine unquoted", alpineOSRelease, "alpine", "3.19.1"}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - got, err := ParseOSRelease(strings.NewReader(tc.input)) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got.Family != tc.family { - t.Errorf("Family = %q, want %q", got.Family, tc.family) - } - if got.VersionID != tc.versionID { - t.Errorf("VersionID = %q, want %q", got.VersionID, tc.versionID) - } - }) - } -} - -func TestParseOSReleaseMissingID(t *testing.T) { - if _, err := ParseOSRelease(strings.NewReader("NAME=\"Something\"\n")); err == nil { - t.Fatal("expected an error when ID is absent, got nil") - } -} -``` - -- [ ] **Step 2: Run the test to verify it fails** - -Run: `cd agent && go test ./internal/packages/ -v` -Expected: build failure — package does not exist. - -- [ ] **Step 3: Write the implementation** +- [ ] **Step 1: Write the implementation** Create `agent/internal/packages/osrelease.go`: @@ -541,7 +367,7 @@ type OSRelease struct { } // ParseOSRelease reads the os-release format: KEY=value, one per line, with -// values optionally double-quoted, and # comments. +// values optionally quoted, and # comments. func ParseOSRelease(r io.Reader) (OSRelease, error) { out := OSRelease{Arch: runtime.GOARCH} sc := bufio.NewScanner(r) @@ -582,15 +408,17 @@ func DetectOS() (OSRelease, error) { } ``` -- [ ] **Step 4: Run the test to verify it passes** +Note the quote stripping handles both `ID=ubuntu` and `ID="rocky"`, which real distributions both emit. -Run: `cd agent && go test ./internal/packages/ -v` -Expected: PASS. +- [ ] **Step 2: Build** -- [ ] **Step 5: Commit** +Run: `cd agent && go build ./...` +Expected: exit 0. + +- [ ] **Step 3: Commit** ```bash -git add agent/internal/packages/osrelease.go agent/internal/packages/osrelease_test.go +git add agent/internal/packages/osrelease.go git commit -m "feat: agent parses /etc/os-release for distro identification" ``` @@ -600,7 +428,6 @@ git commit -m "feat: agent parses /etc/os-release for distro identification" **Files:** - Create: `agent/internal/packages/parse.go`, `agent/internal/packages/packages.go` -- Test: `agent/internal/packages/parse_test.go` **Interfaces:** - Consumes: `packages.OSRelease` from Task 3. @@ -608,98 +435,7 @@ git commit -m "feat: agent parses /etc/os-release for distro identification" **Why `SourceName` matters:** Debian and Ubuntu advisories are keyed on the **source** package. A CVE against `openssl` covers the binaries `libssl3`, `openssl` and `libssl-dev`; matching on binary name alone misses two of the three. -- [ ] **Step 1: Write the failing test** - -Create `agent/internal/packages/parse_test.go`: - -```go -package packages - -import "testing" - -// Captured from: dpkg-query -W -f '${Package}\t${Version}\t${Architecture}\t${source:Package}\n' -const dpkgOutput = "libssl3\t3.0.2-0ubuntu1.15\tamd64\topenssl\n" + - "openssl\t3.0.2-0ubuntu1.15\tamd64\topenssl\n" + - "bash\t5.1-6ubuntu1\tamd64\tbash\n" - -// Captured from: rpm -qa --qf '%{NAME}\t%{EPOCH}\t%{VERSION}-%{RELEASE}\t%{ARCH}\t%{SOURCERPM}\n' -const rpmOutput = "openssl-libs\t1\t3.0.7-24.el9\tx86_64\topenssl-3.0.7-24.el9.src.rpm\n" + - "bash\t(none)\t5.1.8-6.el9\tx86_64\tbash-5.1.8-6.el9.src.rpm\n" - -// Captured from: apk info -v -const apkOutput = "musl-1.2.4_git20230717-r4\nopenssl-3.1.4-r5\nbusybox-1.36.1-r15\n" - -func TestParseDpkg(t *testing.T) { - got := ParseDpkg(dpkgOutput) - if len(got) != 3 { - t.Fatalf("got %d packages, want 3", len(got)) - } - if got[0].Name != "libssl3" || got[0].Version != "3.0.2-0ubuntu1.15" { - t.Errorf("first package = %+v", got[0]) - } - // The whole point of collecting the source name. - if got[0].SourceName != "openssl" { - t.Errorf("SourceName = %q, want %q", got[0].SourceName, "openssl") - } - if got[2].SourceName != "bash" { - t.Errorf("SourceName = %q, want %q", got[2].SourceName, "bash") - } -} - -func TestParseRPM(t *testing.T) { - got := ParseRPM(rpmOutput) - if len(got) != 2 { - t.Fatalf("got %d packages, want 2", len(got)) - } - if got[0].Name != "openssl-libs" || got[0].Epoch != 1 { - t.Errorf("first package = %+v", got[0]) - } - if got[0].SourceName != "openssl" { - t.Errorf("SourceName = %q, want %q", got[0].SourceName, "openssl") - } - // "(none)" is rpm's way of saying no epoch, and must become 0, not fail. - if got[1].Epoch != 0 { - t.Errorf("Epoch = %d, want 0", got[1].Epoch) - } -} - -func TestParseAPK(t *testing.T) { - got := ParseAPK(apkOutput) - if len(got) != 3 { - t.Fatalf("got %d packages, want 3", len(got)) - } - if got[1].Name != "openssl" || got[1].Version != "3.1.4-r5" { - t.Errorf("second package = %+v", got[1]) - } - // A name containing digits and underscores must not be split early. - if got[0].Name != "musl" || got[0].Version != "1.2.4_git20230717-r4" { - t.Errorf("first package = %+v", got[0]) - } -} - -func TestHashIsOrderIndependent(t *testing.T) { - a := []Package{{Name: "b", Version: "2"}, {Name: "a", Version: "1"}} - b := []Package{{Name: "a", Version: "1"}, {Name: "b", Version: "2"}} - if Hash(a) != Hash(b) { - t.Fatal("Hash must not depend on input ordering") - } -} - -func TestHashChangesWithVersion(t *testing.T) { - a := []Package{{Name: "a", Version: "1"}} - b := []Package{{Name: "a", Version: "2"}} - if Hash(a) == Hash(b) { - t.Fatal("Hash must change when a version changes") - } -} -``` - -- [ ] **Step 2: Run the test to verify it fails** - -Run: `cd agent && go test ./internal/packages/ -run "TestParse|TestHash" -v` -Expected: FAIL — `undefined: ParseDpkg`. - -- [ ] **Step 3: Write the implementation** +- [ ] **Step 1: Write the parsers** Create `agent/internal/packages/parse.go`: @@ -761,6 +497,8 @@ func ParseRPM(out string) []Package { continue } epoch := 0 + // rpm prints "(none)" rather than omitting the field when there is no + // epoch, and that must become 0 rather than failing the line. if f[1] != "" && f[1] != "(none)" { if n, err := strconv.Atoi(f[1]); err == nil { epoch = n @@ -842,6 +580,8 @@ func Hash(pkgs []Package) string { } ``` +- [ ] **Step 2: Write the collector** + Create `agent/internal/packages/packages.go`: ```go @@ -865,7 +605,7 @@ func Collect() (OSRelease, []Package, error) { return OSRelease{}, nil, fmt.Errorf("package collection is linux-only, got %s", runtime.GOOS) } - os, err := DetectOS() + osrel, err := DetectOS() if err != nil { return OSRelease{}, nil, fmt.Errorf("detect os: %w", err) } @@ -878,27 +618,27 @@ func Collect() (OSRelease, []Package, error) { out, err := run(ctx, "dpkg-query", "-W", "-f", `${Package}\t${Version}\t${Architecture}\t${source:Package}\n`) if err != nil { - return os, nil, err + return osrel, nil, err } - return os, ParseDpkg(out), nil + return osrel, ParseDpkg(out), nil case have("rpm"): out, err := run(ctx, "rpm", "-qa", "--qf", `%{NAME}\t%{EPOCH}\t%{VERSION}-%{RELEASE}\t%{ARCH}\t%{SOURCERPM}\n`) if err != nil { - return os, nil, err + return osrel, nil, err } - return os, ParseRPM(out), nil + return osrel, ParseRPM(out), nil case have("apk"): out, err := run(ctx, "apk", "info", "-v") if err != nil { - return os, nil, err + return osrel, nil, err } - return os, ParseAPK(out), nil + return osrel, ParseAPK(out), nil default: - return os, nil, fmt.Errorf("no supported package manager found") + return osrel, nil, fmt.Errorf("no supported package manager found") } } @@ -916,10 +656,22 @@ func run(ctx context.Context, name string, args ...string) (string, error) { } ``` -- [ ] **Step 4: Run the tests to verify they pass** +- [ ] **Step 3: Verify against a real host** -Run: `cd agent && go test ./internal/packages/ -v` -Expected: PASS, all tests. +On any Linux machine with Docker, confirm the command shapes produce what the parsers expect: + +```bash +docker run --rm ubuntu:22.04 dpkg-query -W -f '${Package}\t${Version}\t${Architecture}\t${source:Package}\n' | head -5 +docker run --rm rockylinux:9 rpm -qa --qf '%{NAME}\t%{EPOCH}\t%{VERSION}-%{RELEASE}\t%{ARCH}\t%{SOURCERPM}\n' | head -5 +docker run --rm alpine:3.19 apk info -v | head -5 +``` + +Check specifically that the dpkg output's fourth column holds a source name, that rpm prints `(none)` for packages without an epoch, and that at least one Alpine line has a name containing a digit or underscore (e.g. `musl-1.2.4_git…-r4`) so `splitAPK`'s right-to-left approach is exercised. + +- [ ] **Step 4: Build** + +Run: `cd agent && go build ./... && go vet ./...` +Expected: exit 0. - [ ] **Step 5: Commit** @@ -933,15 +685,14 @@ git commit -m "feat: agent collects installed packages per package manager" ### Task 5: Models and indexes **Files:** -- Create: `server/internal/models/packages.go`, `server/internal/models/vuln.go` -- Create: `server/internal/services/vulnindexes.go` +- Create: `server/internal/models/packages.go`, `server/internal/models/vuln.go`, `server/internal/services/vulnindexes.go` - Modify: `server/cmd/main.go` (schema setup, beside the other `Ensure*Indexes` calls) **Interfaces:** - Consumes: nothing. -- Produces: `models.ServerPackages`, `models.InstalledPackage`, `models.OSRelease`, `models.VulnFinding`, `models.Acceptance`, `models.VulnDBMeta`, `models.VulnAlertRule`, and `services.EnsureVulnIndexes() error`. +- Produces: `models.ServerPackages`, `models.InstalledPackage`, `models.OSRelease`, `models.VulnFinding`, `models.Acceptance`, `models.VulnDBMeta`, `models.VulnAlertRule`, `services.EnsureVulnIndexes() error`. -- [ ] **Step 1: Write the models** +- [ ] **Step 1: Write the package models** Create `server/internal/models/packages.go`: @@ -994,6 +745,8 @@ type ServerPackages struct { } ``` +- [ ] **Step 2: Write the vulnerability models** + Create `server/internal/models/vuln.go`: ```go @@ -1098,7 +851,7 @@ type VulnAlertRule struct { } ``` -- [ ] **Step 2: Write the index builder** +- [ ] **Step 3: Write the index builder** Create `server/internal/services/vulnindexes.go`: @@ -1165,9 +918,9 @@ func EnsureVulnIndexes() error { } ``` -- [ ] **Step 3: Wire the index builder into schema setup** +- [ ] **Step 4: Wire it into schema setup** -In `server/cmd/main.go`, find `runSchemaSetup` (near line 75) and the block that calls the other `Ensure*Indexes` functions. Add, alongside the non-fatal ones: +In `server/cmd/main.go`, find `runSchemaSetup` (near line 75) and the block calling the other `Ensure*Indexes` functions. Add, alongside the non-fatal ones: ```go if err := services.EnsureVulnIndexes(); err != nil { @@ -1175,12 +928,9 @@ In `server/cmd/main.go`, find `runSchemaSetup` (near line 75) and the block that } ``` -- [ ] **Step 4: Verify it builds** +- [ ] **Step 5: Build and commit** Run: `cd server && go build ./... && go vet ./...` -Expected: no output, exit 0. - -- [ ] **Step 5: Commit** ```bash git add server/internal/models/packages.go server/internal/models/vuln.go server/internal/services/vulnindexes.go server/cmd/main.go @@ -1197,7 +947,7 @@ git commit -m "feat: models and indexes for package inventory and CVE findings" **Interfaces:** - Consumes: nothing. -- Produces: `pb.ReportPackagesRequest`, `pb.ReportPackagesResponse{NeedFull bool}`, `pb.InstalledPackage`, `pb.OSRelease`, and `SyncResponse.CollectPackages bool`. +- Produces: `pb.ReportPackagesRequest`, `pb.ReportPackagesResponse{NeedFull bool}`, `pb.InstalledPackage`, `pb.OSRelease`, `SyncResponse.CollectPackages bool`. - [ ] **Step 1: Add the RPC and messages** @@ -1207,7 +957,7 @@ In `proto/vantage/v1/vantage.proto`, add to the `Vantage` service block: rpc ReportPackages(ReportPackagesRequest) returns (ReportPackagesResponse); ``` -And add these messages at the end of the file: +And these messages at the end of the file: ```protobuf // ReportPackages carries a server's installed package set. @@ -1245,7 +995,7 @@ message InstalledPackage { - [ ] **Step 2: Add the collect flag to SyncResponse** -Find `message SyncResponse` in the same file and add a field using the next free number in that message (check the file — do not reuse a number): +Find `message SyncResponse` and add a field using the next free number **in that message** — check the file, do not reuse a number: ```protobuf // collect_packages tells the agent whether this instance's licence grants @@ -1257,7 +1007,7 @@ Find `message SyncResponse` in the same file and add a field using the next free - [ ] **Step 3: Regenerate** -Run the project's existing protoc generation command. If unsure, find it with: +Run the project's existing protoc generation command. Find it with: ```bash grep -rn "protoc" --include=Makefile --include="*.sh" --include="*.yml" . | head @@ -1265,12 +1015,9 @@ grep -rn "protoc" --include=Makefile --include="*.sh" --include="*.yml" . | head Generated output must land in both `agent/internal/grpc/pb/` and `server/internal/grpc/pb/`. -- [ ] **Step 4: Verify both modules build** +- [ ] **Step 4: Build both modules and commit** Run: `cd agent && go build ./... && cd ../server && go build ./...` -Expected: exit 0 both times. - -- [ ] **Step 5: Commit** ```bash git add proto/ agent/internal/grpc/pb/ server/internal/grpc/pb/ @@ -1282,8 +1029,7 @@ git commit -m "feat: ReportPackages RPC with hash short-circuit" ### Task 7: Agent reports packages **Files:** -- Modify: `agent/internal/sync/sync.go:370-411` (`runUpdateCheck`) -- Modify: `agent/internal/grpc/client.go` — add the client method +- Modify: `agent/internal/sync/sync.go:370-411` (`runUpdateCheck`), `agent/internal/grpc/client.go` **Interfaces:** - Consumes: `packages.Collect`, `packages.Hash` (Task 4); `pb.ReportPackagesRequest` (Task 6). @@ -1311,13 +1057,13 @@ Match the surrounding file's timeout and context conventions if they differ. - [ ] **Step 2: Extend the hourly loop** -In `agent/internal/sync/sync.go`, inside `runUpdateCheck`'s `doCheck` closure, after the existing `client.ReportUpdates(...)` call succeeds, add: +In `agent/internal/sync/sync.go`, inside `runUpdateCheck`'s `doCheck` closure, after the existing `client.ReportUpdates(...)` call succeeds: ```go reportPackages(client, cfg) ``` -Then add this function to the same file: +Then add to the same file: ```go // reportPackages offers a hash of the installed package set and sends the full @@ -1386,7 +1132,7 @@ func reportPackages(client *grpcclient.Client, cfg *config.Config) { - [ ] **Step 3: Store the collect flag from SyncKeys** -`SyncResponse.collect_packages` arrives on the 30s key poll. Add a package-level flag to `agent/internal/sync/sync.go`, guarded because the poll loop and the hourly loop are different goroutines: +`SyncResponse.collect_packages` arrives on the 30s key poll, which is a different goroutine from the hourly loop — hence the atomic: ```go var collectPackagesFlag atomic.Bool @@ -1394,7 +1140,7 @@ var collectPackagesFlag atomic.Bool func collectPackagesEnabled() bool { return collectPackagesFlag.Load() } ``` -In `poll()` (around line 94), after a successful `SyncKeys` response, add: +In `poll()` (around line 94), after a successful `SyncKeys` response: ```go collectPackagesFlag.Store(resp.GetCollectPackages()) @@ -1402,12 +1148,9 @@ In `poll()` (around line 94), after a successful `SyncKeys` response, add: Add `"sync/atomic"`, `"runtime"` and the `packages` import to the file's import block. -- [ ] **Step 4: Verify it builds and vets** +- [ ] **Step 4: Build and commit** -Run: `cd agent && go build ./... && go vet ./... && go test ./...` -Expected: exit 0; the Task 3 and 4 tests still pass. - -- [ ] **Step 5: Commit** +Run: `cd agent && go build ./... && go vet ./...` ```bash git add agent/internal/sync/sync.go agent/internal/grpc/client.go @@ -1420,11 +1163,11 @@ git commit -m "feat: agent reports installed packages on the hourly loop" **Files:** - Create: `server/internal/services/packages.go` -- Modify: `server/internal/grpc/server.go` — add the `ReportPackages` handler +- Modify: `server/internal/grpc/server.go` **Interfaces:** - Consumes: `models.ServerPackages` (Task 5), `pb.ReportPackagesRequest` (Task 6). -- Produces: `services.HasPackageHash(instanceID, serverID, hash string) (bool, error)`, `services.StorePackages(instanceID, serverID string, os models.OSRelease, hash string, pkgs []models.InstalledPackage) error`, `services.ListPackages(instanceID, serverID string) (*models.ServerPackages, error)`, `services.SearchPackages(instanceID, name string) ([]PackageHit, error)`. +- Produces: `services.HasPackageHash`, `services.StorePackages`, `services.ListPackages`, `services.SearchPackages`, `services.VulnScanningEnabled`. **Critical:** `StorePackages` must **not** match. It upserts and sets `scan_pending: true`. Matching inline would require every replica to hold the 50MB database and would have N replicas racing on a database refresh. @@ -1531,8 +1274,21 @@ func SearchPackages(instanceID, name string) ([]PackageHit, error) { } return hits, nil } + +// VulnScanningEnabled reports whether this instance's licence grants +// vulnerability scanning. It reads the feature by name and never switches on +// tier, so changing what a tier includes needs no server release. +func VulnScanningEnabled(instanceID string) bool { + lic, err := GetLicense(instanceID) + if err != nil || lic == nil { + return false + } + return lic.HasFeature("vuln_scanning") +} ``` +**Before building:** find the existing licence accessor in `server/internal/services/licence.go` — the one `RequireActiveLicense` in `server/internal/api/licence.go` uses — and call **that** rather than assuming `GetLicense` exists. Adjust the name and signature to match. + - [ ] **Step 2: Add the gRPC handler** In `server/internal/grpc/server.go`, following the shape of the existing `ReportUpdates` (near line 87): @@ -1582,33 +1338,11 @@ func (s *vantageServer) ReportPackages(ctx context.Context, req *pb.ReportPackag } ``` -Match the exact return type and error conventions of the neighbouring handlers — in particular how `ValidateAgentToken` is called and what it returns in this file. +Match the neighbouring handlers exactly on how `ValidateAgentToken` is called and what it returns in this file. -- [ ] **Step 3: Add the licence gate helper** - -Create it in `server/internal/services/packages.go`: - -```go -// VulnScanningEnabled reports whether this instance's licence grants -// vulnerability scanning. It reads the feature by name and never switches on -// tier, so changing what a tier includes needs no server release. -func VulnScanningEnabled(instanceID string) bool { - lic, err := GetLicense(instanceID) - if err != nil || lic == nil { - return false - } - return lic.HasFeature("vuln_scanning") -} -``` - -Find the existing licence accessor in `server/internal/services/` (the one `RequireActiveLicense` in `server/internal/api/licence.go` uses) and call **that** rather than inventing `GetLicense`. Adjust the name and signature to match what is actually there. - -- [ ] **Step 4: Verify it builds** +- [ ] **Step 3: Build and commit** Run: `cd server && go build ./... && go vet ./...` -Expected: exit 0. - -- [ ] **Step 5: Commit** ```bash git add server/internal/services/packages.go server/internal/grpc/server.go @@ -1622,26 +1356,17 @@ git commit -m "feat: store agent package reports and mark them for scanning" **Files:** - Modify: `server/internal/grpc/server.go` — `SyncKeys` (near line 47) -**Interfaces:** -- Consumes: `services.VulnScanningEnabled` (Task 8). -- Produces: `SyncResponse.CollectPackages` populated on every poll. - - [ ] **Step 1: Set the flag** -In `SyncKeys`, where the `SyncResponse` is constructed, add: +In `SyncKeys`, where the `SyncResponse` is constructed, add — using whatever the response variable is actually called: ```go resp.CollectPackages = services.VulnScanningEnabled(srv.InstanceID) ``` -using whatever the response variable is actually called in that function. - -- [ ] **Step 2: Verify it builds** +- [ ] **Step 2: Build and commit** Run: `cd server && go build ./...` -Expected: exit 0. - -- [ ] **Step 3: Commit** ```bash git add server/internal/grpc/server.go @@ -1657,11 +1382,9 @@ git commit -m "feat: tell agents whether to collect packages via SyncKeys" **Interfaces:** - Consumes: nothing from earlier tasks. -- Produces: `vulndb.Pull(ctx context.Context, dir string) (version int, err error)`, `vulndb.Open(dir string) (*Store, error)`, `(*Store).Advisories(bucket, srcName string) ([]Advisory, error)`, `(*Store).Vulnerability(cveID string) (VulnInfo, error)`, `(*Store).Close() error`. +- Produces: `vulndb.Pull(ctx, dir string) (version int, err error)`, `vulndb.Open(dir string) (*Store, error)`, `(*Store).Advisories(bucket, srcName string) ([]Advisory, error)`, `(*Store).Vulnerability(cveID string) (VulnInfo, error)`, `(*Store).Close() error`, `vulndb.Ref()`, `vulndb.Disabled()`. -**No test for this task.** It is I/O against an external registry and a real BoltDB file; the global constraints forbid network in tests. Task 11 tests the matching logic behind a `Store` interface instead. - -- [ ] **Step 1: Write the puller** +- [ ] **Step 1: Write the puller skeleton** Create `server/internal/vulndb/pull.go`: @@ -1682,6 +1405,9 @@ import ( // DefaultRef is the published trivy-db OCI artifact, rebuilt every six hours. const DefaultRef = "ghcr.io/aquasecurity/trivy-db:2" +// SupportedSchema is the trivy-db schema version this code understands. +const SupportedSchema = 2 + // Ref returns the artifact reference, honouring VANTAGE_TRIVY_DB_REF so an // air-gapped deployment can mirror the artifact into its own registry, and so // a busy deployment can avoid the anonymous ghcr rate limit. @@ -1701,25 +1427,20 @@ func Disabled() bool { // Pull fetches the trivy-db artifact into dir and returns the schema version // recorded in its metadata. // -// Implementation notes for the engineer: +// Implementation requirements: // - Use oras.land/oras-go/v2 with a remote repository for Ref(). -// - The artifact has a single layer, a gzipped tar containing "db/trivy.db" +// - The artifact has a single layer: a gzipped tar containing "db/trivy.db" // and "db/metadata.json". -// - Extract both into dir, writing to a temp path and renaming into place, so -// a failed pull cannot leave a half-written database that Open would accept. +// - Extract both into dir via extractTarGz below, writing to a temp path and +// renaming into place, so a failed pull cannot leave a half-written +// database that Open would accept. // - Read metadata.json for "Version" (an int) and return it. -// - REFUSE an unknown schema version rather than mis-parsing it: if the -// version is not one this code was written against, return an error. +// - REFUSE a version other than SupportedSchema rather than mis-parsing it. func Pull(ctx context.Context, dir string) (int, error) { - // See the notes above. The extraction helper below is the part worth - // getting right; the oras fetch is mechanical. return 0, fmt.Errorf("not implemented") } -// SupportedSchema is the trivy-db schema version this code understands. -const SupportedSchema = 2 - -// extractTarGz writes the artifact layer into dir. Paths are cleaned and +// extractTarGz writes the artifact layer into dir. Paths are flattened and // checked so a crafted archive cannot write outside dir. func extractTarGz(r io.Reader, dir string) error { gz, err := gzip.NewReader(r) @@ -1763,9 +1484,24 @@ func extractTarGz(r io.Reader, dir string) error { } ``` -Then replace the `Pull` body with the real oras implementation, following the notes in its doc comment. Verify by running it once by hand against the real registry before moving on. +- [ ] **Step 2: Implement `Pull` with oras** -- [ ] **Step 2: Write the advisory store** +Replace the `Pull` body following the requirements in its doc comment. The exact `oras-go` v2 call shape depends on the version `go get` resolved, so write it against the resolved API rather than from memory. + +- [ ] **Step 3: Verify the pull by hand** + +This is the one piece that cannot be verified by reading. From a scratch `main.go`: + +```go +dir, _ := os.MkdirTemp("", "trivydb-") +v, err := vulndb.Pull(context.Background(), dir) +fmt.Println(v, err) +// then: ls the dir — expect trivy.db and metadata.json +``` + +Confirm the returned version equals `SupportedSchema`, both files exist, and `trivy.db` is tens of MB rather than a few bytes. Delete the scratch file afterwards. + +- [ ] **Step 4: Write the advisory store** Create `server/internal/vulndb/db.go`: @@ -1839,6 +1575,12 @@ func (s *Store) Vulnerability(cveID string) (VulnInfo, error) { } // severityName maps trivy-db's integer severity onto our lowercase strings. +// +// Severity resolves vendor → NVD → unknown and is never invented. This will +// surface as "why is this critical CVE marked low": Debian and Red Hat +// routinely downgrade an NVD score because the vulnerable path is not +// reachable in their build, and their rating is the accurate one for that +// package. func severityName(n int) string { switch n { case 4: @@ -1855,14 +1597,11 @@ func severityName(n int) string { } ``` -Verify the `trivy-db` API names against the version `go get` actually resolved — `GetAdvisories`, `GetVulnerability`, `Init` and `Close` are stable but their exact signatures may differ. Adjust the wrappers, not the `Advisory`/`VulnInfo` shapes, which later tasks depend on. +Verify `GetAdvisories`, `GetVulnerability`, `Init` and `Close` against the version `go get` actually resolved. Adjust the wrappers, **not** the `Advisory`/`VulnInfo` shapes, which later tasks depend on. -- [ ] **Step 3: Verify it builds** +- [ ] **Step 5: Build and commit** -Run: `cd server && go build ./... && go test ./internal/vulndb/ -v` -Expected: build succeeds; Task 1 and 2 tests still pass. - -- [ ] **Step 4: Commit** +Run: `cd server && go build ./...` ```bash git add server/internal/vulndb/pull.go server/internal/vulndb/db.go @@ -1875,170 +1614,12 @@ git commit -m "feat: pull trivy-db and read its advisories" **Files:** - Create: `server/internal/vulndb/match.go` -- Test: `server/internal/vulndb/match_test.go` **Interfaces:** - Consumes: `vulndb.LessThan` (Task 1), `vulndb.Bucket` (Task 2), `vulndb.Advisory` (Task 10), `models.InstalledPackage` (Task 5). -- Produces: `vulndb.AdvisorySource` (an interface), `vulndb.Match(src AdvisorySource, os models.OSRelease, pkgs []models.InstalledPackage) ([]Result, error)`, `vulndb.Result`. +- Produces: `vulndb.AdvisorySource` (interface), `vulndb.Match(src AdvisorySource, os models.OSRelease, pkgs []models.InstalledPackage) ([]Result, error)`, `vulndb.Result`. -The interface exists so this task's tests need no real database file — the constraint that tests touch no network and no database is what forces the seam, and the seam is worth having anyway. - -- [ ] **Step 1: Write the failing test** - -Create `server/internal/vulndb/match_test.go`: - -```go -package vulndb - -import ( - "errors" - "testing" - - "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/models" -) - -// fakeSource is an in-memory AdvisorySource keyed by "bucket\x00srcName". -type fakeSource map[string][]Advisory - -func (f fakeSource) Advisories(bucket, srcName string) ([]Advisory, error) { - return f[bucket+"\x00"+srcName], nil -} - -func ubuntu() models.OSRelease { - return models.OSRelease{Family: "ubuntu", VersionID: "22.04", Arch: "amd64"} -} - -func TestMatchBackportedFixIsNotVulnerable(t *testing.T) { - // The trap this whole feature is built around: the installed version equals - // the fixed version, so the host is patched even though NVD lists the - // upstream 3.0.2 as vulnerable. - src := fakeSource{ - "ubuntu 22.04\x00openssl": { - {CVEID: "CVE-2023-0286", FixedVersion: "1:3.0.2-0ubuntu1.15", Severity: "high"}, - }, - } - pkgs := []models.InstalledPackage{ - {Name: "libssl3", Version: "1:3.0.2-0ubuntu1.15", SourceName: "openssl"}, - } - - got, err := Match(src, ubuntu(), pkgs) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(got) != 0 { - t.Fatalf("got %d results, want 0 — a backported fix must not be reported vulnerable: %+v", len(got), got) - } -} - -func TestMatchOlderRevisionIsVulnerable(t *testing.T) { - src := fakeSource{ - "ubuntu 22.04\x00openssl": { - {CVEID: "CVE-2023-0286", FixedVersion: "1:3.0.2-0ubuntu1.15", Severity: "high"}, - }, - } - pkgs := []models.InstalledPackage{ - {Name: "libssl3", Version: "1:3.0.2-0ubuntu1.14", SourceName: "openssl"}, - } - - got, err := Match(src, ubuntu(), pkgs) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(got) != 1 { - t.Fatalf("got %d results, want 1", len(got)) - } - if got[0].CVEID != "CVE-2023-0286" { - t.Errorf("CVEID = %q", got[0].CVEID) - } - // The finding must name the BINARY package, which is what an operator sees - // installed, while the lookup used the source package. - if got[0].PackageName != "libssl3" { - t.Errorf("PackageName = %q, want libssl3", got[0].PackageName) - } - if got[0].FixedIn != "1:3.0.2-0ubuntu1.15" { - t.Errorf("FixedIn = %q", got[0].FixedIn) - } -} - -func TestMatchSourceLookupCoversEveryBinary(t *testing.T) { - // One advisory against the source package openssl must flag all three of - // its binaries. Matching on binary name alone would find only one. - src := fakeSource{ - "ubuntu 22.04\x00openssl": { - {CVEID: "CVE-2023-0286", FixedVersion: "1:3.0.2-0ubuntu1.15", Severity: "high"}, - }, - } - pkgs := []models.InstalledPackage{ - {Name: "libssl3", Version: "1:3.0.2-0ubuntu1.14", SourceName: "openssl"}, - {Name: "openssl", Version: "1:3.0.2-0ubuntu1.14", SourceName: "openssl"}, - {Name: "libssl-dev", Version: "1:3.0.2-0ubuntu1.14", SourceName: "openssl"}, - } - - got, err := Match(src, ubuntu(), pkgs) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(got) != 3 { - t.Fatalf("got %d results, want 3 — one per affected binary", len(got)) - } -} - -func TestMatchNoFixedVersionIsVulnerable(t *testing.T) { - // An advisory with no fix published is the finding people most need to see. - src := fakeSource{ - "ubuntu 22.04\x00bash": { - {CVEID: "CVE-2024-0001", FixedVersion: "", Severity: "medium"}, - }, - } - pkgs := []models.InstalledPackage{ - {Name: "bash", Version: "5.1-6ubuntu1", SourceName: "bash"}, - } - - got, err := Match(src, ubuntu(), pkgs) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(got) != 1 { - t.Fatalf("got %d results, want 1", len(got)) - } - if got[0].FixedIn != "" { - t.Errorf("FixedIn = %q, want empty", got[0].FixedIn) - } -} - -func TestMatchUnsupportedFamily(t *testing.T) { - _, err := Match(fakeSource{}, models.OSRelease{Family: "arch"}, nil) - if !errors.Is(err, ErrUnsupportedFamily) { - t.Fatalf("error = %v, want ErrUnsupportedFamily", err) - } -} - -func TestMatchFallsBackToPackageNameWhenSourceMissing(t *testing.T) { - // Alpine has no separate source package; SourceName mirrors Name. A report - // from an older agent may omit it entirely, and that must still match. - src := fakeSource{ - "alpine 3.19\x00openssl": { - {CVEID: "CVE-2024-0002", FixedVersion: "3.1.4-r6", Severity: "high"}, - }, - } - pkgs := []models.InstalledPackage{{Name: "openssl", Version: "3.1.4-r5"}} - - got, err := Match(src, models.OSRelease{Family: "alpine", VersionID: "3.19.1"}, pkgs) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(got) != 1 { - t.Fatalf("got %d results, want 1", len(got)) - } -} -``` - -- [ ] **Step 2: Run the test to verify it fails** - -Run: `cd server && go test ./internal/vulndb/ -run TestMatch -v` -Expected: FAIL — `undefined: Match`. - -- [ ] **Step 3: Write the implementation** +- [ ] **Step 1: Write the implementation** Create `server/internal/vulndb/match.go`: @@ -2052,9 +1633,8 @@ import ( "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/models" ) -// AdvisorySource is the advisory lookup the matcher needs. *Store satisfies it; -// tests substitute an in-memory map, which is what keeps the matching logic -// testable with no database file and no network. +// AdvisorySource is the advisory lookup the matcher needs. *Store satisfies it. +// The seam keeps the matching logic independent of how the database is opened. type AdvisorySource interface { Advisories(bucket, srcName string) ([]Advisory, error) } @@ -2072,6 +1652,9 @@ type Result struct { // // Vulnerable means: no fix has been published, or the installed version sorts // strictly before the fixed version under the distribution's own ordering. +// Equal is NOT vulnerable — that is the backported-fix case, where a +// distribution patches in place without changing the upstream version, and +// treating it as vulnerable reports a patched fleet as exposed. func Match(src AdvisorySource, os models.OSRelease, pkgs []models.InstalledPackage) ([]Result, error) { bucket, err := Bucket(os.Family, os.VersionID) if err != nil { @@ -2124,15 +1707,34 @@ func Match(src AdvisorySource, os models.OSRelease, pkgs []models.InstalledPacka } ``` -- [ ] **Step 4: Run the tests to verify they pass** +- [ ] **Step 2: Verify the matching by hand** -Run: `cd server && go test ./internal/vulndb/ -v` -Expected: PASS, every test. +From a scratch `main.go`, define a small in-memory `AdvisorySource` and confirm each case. **These are the behaviours that silently produce wrong answers.** -- [ ] **Step 5: Commit** +```go +type fake map[string][]vulndb.Advisory +func (f fake) Advisories(bucket, src string) ([]vulndb.Advisory, error) { + return f[bucket+"\x00"+src], nil +} +``` + +| Setup | Expected | +| ----- | -------- | +| advisory `openssl` fixed `1:3.0.2-0ubuntu1.15`; installed `libssl3` at `1:3.0.2-0ubuntu1.15`, source `openssl`, ubuntu 22.04 | **0 results** — the backport case | +| same advisory; installed at `1:3.0.2-0ubuntu1.14` | 1 result, `PackageName` = `libssl3` (the binary, not the source), `FixedIn` set | +| same advisory; three binaries `libssl3`, `openssl`, `libssl-dev` all at `…1.14`, all source `openssl` | **3 results** — one advisory covers every binary of the source | +| advisory with `FixedVersion: ""` on `bash` | 1 result with empty `FixedIn` | +| os family `arch` | error wrapping `ErrUnsupportedFamily` | +| alpine 3.19.1, package `openssl` at `3.1.4-r5`, **no** `SourceName` set, advisory fixed `3.1.4-r6` | 1 result — the fallback to `Name` works | + +Delete the scratch file afterwards. + +- [ ] **Step 3: Build and commit** + +Run: `cd server && go build ./...` ```bash -git add server/internal/vulndb/match.go server/internal/vulndb/match_test.go +git add server/internal/vulndb/match.go git commit -m "feat: match installed packages against distro advisories" ``` @@ -2142,155 +1744,12 @@ git commit -m "feat: match installed packages against distro advisories" **Files:** - Create: `server/internal/services/findings.go` -- Test: `server/internal/services/findings_test.go` **Interfaces:** - Consumes: `vulndb.Result` (Task 11), `models.VulnFinding` (Task 5). -- Produces: `services.DiffFindings(existing []models.VulnFinding, results []vulndb.Result, now time.Time) FindingDiff`, `services.FindingDiff{Upserts []models.VulnFinding, FixedIDs []bson.ObjectID, ReopenIDs []bson.ObjectID, NewlyOpened []models.VulnFinding}`. +- Produces: `services.DiffFindings(existing []models.VulnFinding, results []vulndb.Result, now time.Time) FindingDiff`, `services.FindingDiff`, `services.ApplyFindingDiff`, `services.ListFindings`. -`DiffFindings` is a **pure function** — no database — which is what makes the state machine testable. The Mongo writes live in a separate thin `ApplyFindingDiff`. - -- [ ] **Step 1: Write the failing test** - -Create `server/internal/services/findings_test.go`: - -```go -package services - -import ( - "testing" - "time" - - "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/models" - "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/vulndb" - "go.mongodb.org/mongo-driver/v2/bson" -) - -func now() time.Time { return time.Date(2026, 8, 6, 12, 0, 0, 0, time.UTC) } - -func TestDiffOpensNewFinding(t *testing.T) { - d := DiffFindings(nil, []vulndb.Result{ - {CVEID: "CVE-1", PackageName: "openssl", Installed: "1.0", FixedIn: "1.1", Severity: "high"}, - }, now()) - - if len(d.Upserts) != 1 { - t.Fatalf("got %d upserts, want 1", len(d.Upserts)) - } - if d.Upserts[0].State != models.FindingOpen { - t.Errorf("State = %q, want open", d.Upserts[0].State) - } - if len(d.NewlyOpened) != 1 { - t.Errorf("got %d newly opened, want 1 — this is what the digest reports", len(d.NewlyOpened)) - } -} - -func TestDiffPreservesFirstSeen(t *testing.T) { - // Easy to break with an upsert that overwrites, and nothing notices until a - // report claims everything was discovered yesterday. - first := now().Add(-30 * 24 * time.Hour) - existing := []models.VulnFinding{{ - ID: bson.NewObjectID(), CVEID: "CVE-1", PackageName: "openssl", - State: models.FindingOpen, FirstSeen: first, LastSeen: first, - }} - - d := DiffFindings(existing, []vulndb.Result{ - {CVEID: "CVE-1", PackageName: "openssl", Installed: "1.0", FixedIn: "1.1", Severity: "high"}, - }, now()) - - if len(d.Upserts) != 1 { - t.Fatalf("got %d upserts, want 1", len(d.Upserts)) - } - if !d.Upserts[0].FirstSeen.Equal(first) { - t.Errorf("FirstSeen = %v, want %v", d.Upserts[0].FirstSeen, first) - } - if !d.Upserts[0].LastSeen.Equal(now()) { - t.Errorf("LastSeen = %v, want %v", d.Upserts[0].LastSeen, now()) - } - // Already open, so not newly opened — it must not re-alert every scan. - if len(d.NewlyOpened) != 0 { - t.Errorf("got %d newly opened, want 0", len(d.NewlyOpened)) - } -} - -func TestDiffMarksAbsentFindingFixed(t *testing.T) { - id := bson.NewObjectID() - existing := []models.VulnFinding{{ - ID: id, CVEID: "CVE-1", PackageName: "openssl", State: models.FindingOpen, - }} - - d := DiffFindings(existing, nil, now()) - - if len(d.FixedIDs) != 1 || d.FixedIDs[0] != id { - t.Fatalf("FixedIDs = %v, want [%v]", d.FixedIDs, id) - } - if len(d.Upserts) != 0 { - t.Errorf("got %d upserts, want 0", len(d.Upserts)) - } -} - -func TestDiffReopensExpiredAcceptance(t *testing.T) { - id := bson.NewObjectID() - expired := now().Add(-24 * time.Hour) - existing := []models.VulnFinding{{ - ID: id, CVEID: "CVE-1", PackageName: "openssl", State: models.FindingAccepted, - Accepted: &models.Acceptance{Reason: "reboot window", Until: expired}, - }} - - d := DiffFindings(existing, []vulndb.Result{ - {CVEID: "CVE-1", PackageName: "openssl", Installed: "1.0", FixedIn: "1.1", Severity: "high"}, - }, now()) - - if len(d.ReopenIDs) != 1 || d.ReopenIDs[0] != id { - t.Fatalf("ReopenIDs = %v, want [%v]", d.ReopenIDs, id) - } -} - -func TestDiffLeavesLiveAcceptanceAlone(t *testing.T) { - existing := []models.VulnFinding{{ - ID: bson.NewObjectID(), CVEID: "CVE-1", PackageName: "openssl", State: models.FindingAccepted, - Accepted: &models.Acceptance{Reason: "no fix yet", Until: now().Add(24 * time.Hour)}, - }} - - d := DiffFindings(existing, []vulndb.Result{ - {CVEID: "CVE-1", PackageName: "openssl", Installed: "1.0", Severity: "high"}, - }, now()) - - if len(d.ReopenIDs) != 0 { - t.Errorf("ReopenIDs = %v, want empty", d.ReopenIDs) - } - // An accepted finding must not appear in the digest. - if len(d.NewlyOpened) != 0 { - t.Errorf("got %d newly opened, want 0", len(d.NewlyOpened)) - } -} - -func TestDiffAbsentAndExpiredSettlesFixed(t *testing.T) { - // Ordering matters: a finding that is BOTH absent from the scan and past its - // acceptance expiry must settle as fixed, not reopen on a package that no - // longer carries it. - id := bson.NewObjectID() - existing := []models.VulnFinding{{ - ID: id, CVEID: "CVE-1", PackageName: "openssl", State: models.FindingAccepted, - Accepted: &models.Acceptance{Until: now().Add(-24 * time.Hour)}, - }} - - d := DiffFindings(existing, nil, now()) - - if len(d.FixedIDs) != 1 { - t.Fatalf("FixedIDs = %v, want one entry", d.FixedIDs) - } - if len(d.ReopenIDs) != 0 { - t.Errorf("ReopenIDs = %v, want empty", d.ReopenIDs) - } -} -``` - -- [ ] **Step 2: Run the test to verify it fails** - -Run: `cd server && go test ./internal/services/ -run TestDiff -v` -Expected: FAIL — `undefined: DiffFindings`. - -- [ ] **Step 3: Write the implementation** +- [ ] **Step 1: Write the pure diff** Create `server/internal/services/findings.go`: @@ -2298,11 +1757,14 @@ Create `server/internal/services/findings.go`: package services import ( + "context" "time" + "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/db" "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/models" "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/vulndb" "go.mongodb.org/mongo-driver/v2/bson" + "go.mongodb.org/mongo-driver/v2/mongo/options" ) // FindingDiff is what one server's scan changes. @@ -2320,8 +1782,8 @@ func findingKey(cveID, pkg string) string { return cveID + "\x00" + pkg } // DiffFindings computes the state changes for one server's scan. // -// Pure by design: no database, no clock of its own. That is what makes the -// state machine — the part with the subtle ordering — testable. +// Pure by design: no database, no clock of its own. The ordering below is +// load-bearing — see the comment above the second loop. func DiffFindings(existing []models.VulnFinding, results []vulndb.Result, now time.Time) FindingDiff { var d FindingDiff @@ -2350,6 +1812,8 @@ func DiffFindings(existing []models.VulnFinding, results []vulndb.Result, now ti if had { f.ID = prev.ID + // Preserved, never overwritten: an upsert that moves first_seen + // forward makes every finding look discovered today. f.FirstSeen = prev.FirstSeen // A live acceptance survives the scan untouched: it is suppressed @@ -2372,10 +1836,10 @@ func DiffFindings(existing []models.VulnFinding, results []vulndb.Result, now ti d.Upserts = append(d.Upserts, f) } - // Anything we hold that this scan did not produce is fixed. This runs after - // the loop above, so a finding that is both absent and past its acceptance - // expiry settles as fixed rather than reopening on a package that no longer - // carries it. + // Anything we hold that this scan did not produce is fixed. This runs AFTER + // the loop above, and the ordering matters: a finding that is both absent + // and past its acceptance expiry must settle as fixed rather than reopening + // on a package that no longer carries it. for _, f := range existing { if seen[findingKey(f.CVEID, f.PackageName)] { continue @@ -2390,24 +1854,35 @@ func DiffFindings(existing []models.VulnFinding, results []vulndb.Result, now ti } ``` -- [ ] **Step 4: Run the tests to verify they pass** +- [ ] **Step 2: Add the persistence half** -Run: `cd server && go test ./internal/services/ -v` -Expected: PASS, every test. - -- [ ] **Step 5: Add the persistence half** - -Append to `server/internal/services/findings.go`: +Append to the same file: ```go -// ApplyFindingDiff writes a diff. Thin on purpose — the logic worth testing is -// all in DiffFindings. +// ListFindings returns every finding held for one server. +func ListFindings(ctx context.Context, instanceID, serverID string) ([]models.VulnFinding, error) { + cur, err := db.Col("vuln_findings").Find(ctx, bson.M{ + "instance_id": instanceID, + "server_id": serverID, + }) + if err != nil { + return nil, err + } + defer cur.Close(ctx) + + var out []models.VulnFinding + if err := cur.All(ctx, &out); err != nil { + return nil, err + } + return out, nil +} + +// ApplyFindingDiff writes a diff. Thin on purpose — the logic worth reading +// twice is all in DiffFindings. func ApplyFindingDiff(ctx context.Context, instanceID, serverID string, d FindingDiff, now time.Time) error { col := db.Col("vuln_findings") for _, f := range d.Upserts { - f.InstanceID = instanceID - f.ServerID = serverID _, err := col.UpdateOne(ctx, bson.M{ "instance_id": instanceID, @@ -2423,14 +1898,14 @@ func ApplyFindingDiff(ctx context.Context, instanceID, serverID string, d Findin "state": models.FindingOpen, "last_seen": now, }, - // first_seen is only written when the document is created, so a - // rescan cannot move it forward. + // first_seen is written only on insert, so a rescan cannot move + // it forward. "$setOnInsert": bson.M{ - "instance_id": instanceID, - "server_id": serverID, - "cve_id": f.CVEID, + "instance_id": instanceID, + "server_id": serverID, + "cve_id": f.CVEID, "package_name": f.PackageName, - "first_seen": f.FirstSeen, + "first_seen": f.FirstSeen, }, "$unset": bson.M{"fixed_at": "", "accepted": ""}, }, @@ -2463,14 +1938,27 @@ func ApplyFindingDiff(ctx context.Context, instanceID, serverID string, d Findin } ``` -Add `"context"`, the `db` import and `"go.mongodb.org/mongo-driver/v2/mongo/options"` to the file's imports. +- [ ] **Step 3: Read the diff back against these cases** -- [ ] **Step 6: Verify and commit** +No test, so verify by reading. Walk the function with each and confirm the branch taken: -Run: `cd server && go build ./... && go test ./... -v` +| Input | Expected | +| ----- | -------- | +| no existing, one result | one upsert, state `open`, one `NewlyOpened` | +| existing open with `first_seen` 30 days ago, same result | one upsert with the **old** `first_seen`, `LastSeen` = now, **zero** `NewlyOpened` | +| existing open, **no** results | one `FixedIDs` entry, zero upserts | +| existing accepted with `until` in the past, still in results | one `ReopenIDs` entry | +| existing accepted with `until` in the future, still in results | zero `ReopenIDs`, zero `NewlyOpened` | +| existing accepted with `until` in the past, **not** in results | one `FixedIDs` entry, **zero** `ReopenIDs` | + +The last row is the ordering trap: it only holds because the fixed loop runs after the results loop. + +- [ ] **Step 4: Build and commit** + +Run: `cd server && go build ./... && go vet ./...` ```bash -git add server/internal/services/findings.go server/internal/services/findings_test.go +git add server/internal/services/findings.go git commit -m "feat: finding state machine with acceptance expiry" ``` @@ -2483,10 +1971,10 @@ git commit -m "feat: finding state machine with acceptance expiry" - Modify: `server/cmd/main.go:184-206` **Interfaces:** -- Consumes: `vulndb.Pull`, `vulndb.Open` (Task 10), `vulndb.Match` (Task 11), `services.DiffFindings`, `services.ApplyFindingDiff` (Task 12). -- Produces: `vulnsched.Start(ctx context.Context, deps Deps)`, `vulnsched.Deps{LogEvent func(...), SendDigest func(instanceID string, newly []models.VulnFinding)}`. +- Consumes: `vulndb.Pull`, `vulndb.Open`, `vulndb.Match`, `services.DiffFindings`, `services.ApplyFindingDiff`, `services.ListFindings`. +- Produces: `vulnsched.Start(ctx, deps Deps)`, `vulnsched.Deps`. -**Model this file on `server/internal/workflowsched/sched.go`.** Read it first — same `Start`/`tick` shape, same `Deps` injection, same "return when the context is cancelled" contract. +**Read `server/internal/workflowsched/sched.go` first.** Same `Start`/`tick` shape, same `Deps` injection, same "return when the context is cancelled" contract. - [ ] **Step 1: Write the scheduler** @@ -2504,6 +1992,7 @@ package vulnsched import ( "context" + "errors" "log" "os" "time" @@ -2513,6 +2002,7 @@ import ( "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/services" "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/vulndb" "go.mongodb.org/mongo-driver/v2/bson" + "go.mongodb.org/mongo-driver/v2/mongo/options" ) const ( @@ -2572,7 +2062,7 @@ func (s *scheduler) tick(ctx context.Context) { // Keep the last good database and carry on scanning against it. A // network blip must never clear findings or read as "all fixed". log.Printf("vulnsched: database unavailable: %v", err) - recordDBError(ctx, err) + s.recordDBError(ctx, err) if s.store == nil { return } @@ -2607,7 +2097,7 @@ func (s *scheduler) ensureDB(ctx context.Context) error { _, _ = db.Col("vulndb_meta").UpdateOne(ctx, bson.M{}, bson.M{"$set": bson.M{"db_version": version, "pulled_at": s.pulled}, "$unset": bson.M{"last_error": ""}}, - mongoUpsert(), + options.UpdateOne().SetUpsert(true), ) if changed { @@ -2624,6 +2114,13 @@ func (s *scheduler) ensureDB(ctx context.Context) error { return nil } +func (s *scheduler) recordDBError(ctx context.Context, err error) { + _, _ = db.Col("vulndb_meta").UpdateOne(ctx, bson.M{}, + bson.M{"$set": bson.M{"last_error": err.Error()}}, + options.UpdateOne().SetUpsert(true), + ) +} + func (s *scheduler) scanPending(ctx context.Context) { cur, err := db.Col("server_packages").Find(ctx, bson.M{"scan_pending": true}) if err != nil { @@ -2646,7 +2143,9 @@ func (s *scheduler) scanPending(ctx context.Context) { for _, sp := range pending { if ctx.Err() != nil { - return // leadership lost; scan_pending is still set, so the next leader picks it up + // Leadership lost. scan_pending is still set, so the next leader + // picks these up — which is why it lives on the document. + return } opened := s.scanOne(ctx, sp) newly[sp.InstanceID] = append(newly[sp.InstanceID], opened...) @@ -2657,6 +2156,11 @@ func (s *scheduler) scanPending(ctx context.Context) { s.deps.SendDigest(instanceID, findings) } } + + _, _ = db.Col("vulndb_meta").UpdateOne(ctx, bson.M{}, + bson.M{"$set": bson.M{"last_full_scan_at": time.Now()}}, + options.UpdateOne().SetUpsert(true), + ) } func (s *scheduler) scanOne(ctx context.Context, sp models.ServerPackages) []models.VulnFinding { @@ -2669,11 +2173,11 @@ func (s *scheduler) scanOne(ctx context.Context, sp models.ServerPackages) []mod // be indistinguishable from reporting a clean host, and one of those is // a lie. status := models.ScanStatusUnsupported - if !isUnsupported(err) { + if !errors.Is(err, vulndb.ErrUnsupportedFamily) { log.Printf("vulnsched: scan %s: %v", sp.ServerID, err) status = sp.Status } - clearPending(ctx, sp.ID, status, s.version, now) + s.clearPending(ctx, sp.ID, status, now) return nil } @@ -2689,7 +2193,7 @@ func (s *scheduler) scanOne(ctx context.Context, sp models.ServerPackages) []mod return nil } - clearPending(ctx, sp.ID, models.ScanStatusOK, s.version, now) + s.clearPending(ctx, sp.ID, models.ScanStatusOK, now) for i := range diff.NewlyOpened { diff.NewlyOpened[i].ServerID = sp.ServerID @@ -2697,6 +2201,18 @@ func (s *scheduler) scanOne(ctx context.Context, sp models.ServerPackages) []mod return diff.NewlyOpened } +func (s *scheduler) clearPending(ctx context.Context, id bson.ObjectID, status string, now time.Time) { + _, _ = db.Col("server_packages").UpdateOne(ctx, + bson.M{"_id": id}, + bson.M{"$set": bson.M{ + "scan_pending": false, + "status": status, + "scanned_at": now, + "db_version": s.version, + }}, + ) +} + func (s *scheduler) closeStore() { if s.store != nil { _ = s.store.Close() @@ -2705,11 +2221,9 @@ func (s *scheduler) closeStore() { } ``` -Then add the three small helpers the file uses — `clearPending`, `recordDBError`, `isUnsupported` (which wraps `errors.Is(err, vulndb.ErrUnsupportedFamily)`) and `mongoUpsert` (returning `options.UpdateOne().SetUpsert(true)`) — in the same file, and add `services.ListFindings(ctx, instanceID, serverID) ([]models.VulnFinding, error)` to `server/internal/services/findings.go`. - - [ ] **Step 2: Wire it into main.go** -In `server/cmd/main.go`, inside the `bus.RunAsLeader(ctx, "housekeeping", …)` block (line 184), after the `workflowsched.Start(...)` call: +In `server/cmd/main.go`, inside `bus.RunAsLeader(ctx, "housekeeping", …)` (line 184), after `workflowsched.Start(...)`: ```go vulnsched.Start(jobCtx, vulnsched.Deps{ @@ -2719,17 +2233,14 @@ In `server/cmd/main.go`, inside the `bus.RunAsLeader(ctx, "housekeeping", …)` services.StartVulnSweeper(jobCtx) ``` -Add the `vulnsched` import. `services.SendVulnDigest` and `services.StartVulnSweeper` arrive in Tasks 14 and 15 — if implementing strictly in order, stub them as no-ops in `server/internal/services/vulnrules.go` now and fill them in there. +Add the `vulnsched` import. `services.SendVulnDigest` and `services.StartVulnSweeper` arrive in Tasks 14 and 15 — stub them as no-ops in `server/internal/services/vulnrules.go` now and fill them in there. -- [ ] **Step 3: Verify it builds** +- [ ] **Step 3: Build and commit** -Run: `cd server && go build ./... && go vet ./... && go test ./...` -Expected: exit 0. - -- [ ] **Step 4: Commit** +Run: `cd server && go build ./... && go vet ./...` ```bash -git add server/internal/vulnsched/ server/cmd/main.go server/internal/services/findings.go +git add server/internal/vulnsched/ server/cmd/main.go git commit -m "feat: leader-owned vulnerability scan loop" ``` @@ -2738,23 +2249,19 @@ git commit -m "feat: leader-owned vulnerability scan loop" ### Task 14: Alert rules and the digest **Files:** -- Create: `server/internal/services/vulnrules.go`, `shared/mail/vuln.go` -- Create: `shared/mail/templates/vuln_digest.html.tmpl`, `shared/mail/templates/vuln_digest.txt.tmpl` -- Create: `shared/mail/render_test.go` +- Create: `server/internal/services/vulnrules.go`, `shared/mail/vuln.go`, `shared/mail/templates/vuln_digest.html.tmpl`, `shared/mail/templates/vuln_digest.txt.tmpl` **Interfaces:** -- Consumes: `models.VulnAlertRule`, `models.VulnFinding` (Task 5), `services.ResolveTargets`. -- Produces: `services.SendVulnDigest(instanceID string, newly []models.VulnFinding)`, `services.ListVulnRules/CreateVulnRule/UpdateVulnRule/DeleteVulnRule`, `mail.Sender.SendVulnDigest(to string, data VulnDigestData) error`. - -**The tag filter must go through `services.ResolveTargets`.** It is already the single answer to which servers a selector touches; a rule that disagreed with a workflow about what `env:prod` means would be worse than no filter. +- Consumes: `models.VulnAlertRule`, `models.VulnFinding`, `services.ResolveTargets`. +- Produces: `services.SendVulnDigest`, `services.ListVulnRules/CreateVulnRule/UpdateVulnRule/DeleteVulnRule`, `mail.Sender.SendVulnDigest`. - [ ] **Step 1: Write the rule service and dispatch** -Create `server/internal/services/vulnrules.go` with CRUD for `vuln_alert_rules` following the shape of the existing notification-channel service, plus: +Create `server/internal/services/vulnrules.go` with CRUD for `vuln_alert_rules` following the shape of the existing notification-channel service in `server/internal/services/channels.go`, plus: ```go // SendVulnDigest delivers one message per rule per tick — never one per -// finding. See vulnsched for why the batch boundary is the tick. +// finding. See vulnsched for why the tick is the batch boundary. func SendVulnDigest(instanceID string, newly []models.VulnFinding) { rules, err := ListVulnRules(instanceID) if err != nil { @@ -2773,6 +2280,9 @@ func SendVulnDigest(instanceID string, newly []models.VulnFinding) { } if len(rule.Tags) > 0 { + // ResolveTargets is already the single answer to which servers a + // selector touches. A rule that disagreed with a workflow about + // what env:prod means would be worse than no filter at all. allowed, err := ResolveTargets(instanceID, nil, rule.Tags) if err != nil { log.Printf("vuln digest: resolve targets: %v", err) @@ -2816,7 +2326,9 @@ func filterByServers(findings []models.VulnFinding, allowed []string) []models.V } ``` -Implement `dispatchVulnDigest` by following how `server/internal/notify` already dispatches a monitor alert to a channel by ID, with a summary line of the form `"12 new critical, 4 new high across 6 servers"`. Verify `ResolveTargets`' actual signature in `server/internal/services/targets.go` and adjust the call. +Implement `dispatchVulnDigest` following how `server/internal/notify` already dispatches a monitor alert to a channel by ID, with a summary line of the form `"12 new critical, 4 new high across 6 servers"`. **Verify `ResolveTargets`' actual signature** in `server/internal/services/targets.go` and adjust the call. + +Also add `StartVulnSweeper` here as a no-op stub if Task 13 has already referenced it; Task 15 replaces it. - [ ] **Step 2: Write the mail templates** @@ -2837,68 +2349,22 @@ Scanned against vulnerability database pulled {{.DBAge}} ago. {{end}} ``` -Create `shared/mail/templates/vuln_digest.html.tmpl` defining `title`, `pill` and `body` only, composing the existing `p`, `lead`, `button`, `rows` and `chip` helpers from `layout.html.tmpl`. Read an existing pair — `monitor.go`'s templates — and match them exactly. **Carry no new hex colours:** every colour in the email system lives in `layout.html.tmpl`. +Create `shared/mail/templates/vuln_digest.html.tmpl` defining `title`, `pill` and `body` only, composing the existing `p`, `lead`, `button`, `rows` and `chip` helpers from `layout.html.tmpl`. Read `shared/mail/monitor.go`'s template pair and match it exactly. **Carry no new hex colours** — every colour in the email system lives in `layout.html.tmpl`. Create `shared/mail/vuln.go` with `VulnDigestData` and `SendVulnDigest`, following `shared/mail/monitor.go`. -- [ ] **Step 3: Write the render test CLAUDE.md already claims exists** +- [ ] **Step 3: Verify the templates parse** -Create `shared/mail/render_test.go`. Templates are parsed in `init()`, so a mistyped field is a boot-time panic; this is the only thing standing between that and production. +`shared/mail` parses its templates in `init()`, so a mistyped field is a **boot-time panic**, not a runtime error. Confirm before committing: -```go -package mail - -import ( - "os" - "path/filepath" - "strings" - "testing" -) - -// cases must name every template pair under templates/. The final subtest -// fails if a template exists that no case covers. -var cases = map[string]any{ - // One entry per template base name, with a fully populated data value. - // Fill this in from the SendX methods in this package — every template - // that exists must appear here. -} - -func TestEveryTemplateRenders(t *testing.T) { - for name, data := range cases { - t.Run(name, func(t *testing.T) { - if _, _, _, err := render(name, data); err != nil { - t.Fatalf("render %s: %v", name, err) - } - }) - } -} - -func TestNoTemplateIsUncovered(t *testing.T) { - entries, err := os.ReadDir("templates") - if err != nil { - t.Fatalf("read templates dir: %v", err) - } - for _, e := range entries { - name := e.Name() - if !strings.HasSuffix(name, ".txt.tmpl") || strings.HasPrefix(name, "layout") { - continue - } - base := filepath.Base(strings.TrimSuffix(name, ".txt.tmpl")) - if _, ok := cases[base]; !ok { - t.Errorf("template %q has no case in cases — add one", base) - } - } -} +```bash +cd shared && go build ./mail/ +cd ../server && go build ./... && go run ./cmd --help 2>&1 | head -3 ``` -Adjust `render` to the actual unexported render function's name and signature in `shared/mail/render.go`, and populate `cases` with every existing template plus `vuln_digest`. +If the binary starts far enough to print usage, the template set parsed. Then send one digest through a real channel from a dev instance and read the delivered message — the template can parse and still render nonsense, and nothing else will catch that. -- [ ] **Step 4: Run the tests** - -Run: `cd shared && go test ./mail/ -v` -Expected: PASS. If a template fails to render, fix the template — that failure is the test doing its job. - -- [ ] **Step 5: Commit** +- [ ] **Step 4: Commit** ```bash git add server/internal/services/vulnrules.go shared/mail/ @@ -2913,10 +2379,6 @@ git commit -m "feat: vulnerability alert rules and batched digest" - Create: `server/internal/api/vulnerabilities.go` - Modify: `shared/models/settings.go`, `server/internal/api/handlers.go`, `server/internal/services/findings.go` -**Interfaces:** -- Consumes: everything from Tasks 8, 12 and 14. -- Produces: the routes below, `models.Settings.VulnFindingRetentionDays *int`, `services.VulnFindingRetentionDays(*Settings) int`, `services.StartVulnSweeper(ctx)`. - - [ ] **Step 1: Add the retention setting** In `shared/models/settings.go`, beside `WorkflowLogRetentionDays`: @@ -2930,7 +2392,7 @@ In `shared/models/settings.go`, beside `WorkflowLogRetentionDays`: - [ ] **Step 2: Add the sweeper** -In `server/internal/services/findings.go`, following `StartAuditSweeper`: +In `server/internal/services/findings.go`, following `StartAuditSweeper` in `audit_retention.go`: ```go // StartVulnSweeper deletes old FIXED findings. Open and accepted findings are @@ -2952,17 +2414,17 @@ func StartVulnSweeper(ctx context.Context) { } ``` -Implement `sweepFixedFindings` to read each instance's setting, resolve nil to 90 and skip 0, then delete `{instance_id, state: "fixed", fixed_at: {$lt: cutoff}}`. Model it on `StartAuditSweeper` in the same package. +Implement `sweepFixedFindings` to read each instance's setting, resolve nil to 90 and skip 0, then delete `{instance_id, state: "fixed", fixed_at: {$lt: cutoff}}`. Model it on `StartAuditSweeper`. Remove the Task 14 stub. - [ ] **Step 3: Write the handlers** -Create `server/internal/api/vulnerabilities.go` with these handlers, following the conventions in `server/internal/api/handlers.go` — `auth.InstanceID(c)` for scoping, `actorFromCtx(c)` for the audit actor: +Create `server/internal/api/vulnerabilities.go` following the conventions in `server/internal/api/handlers.go` — `auth.InstanceID(c)` for scoping, `actorFromCtx(c)` for the audit actor: -- `listVulnerabilities` — `GET /api/vulnerabilities`, query params `severity`, `state`, `server`, plus tag filters. Groups by CVE in the response: `{cve_id, severity, title, server_count, servers: [...]}`. -- `vulnerabilitySummary` — `GET /api/vulnerabilities/summary`, returns severity counts for open findings plus `{db_version, pulled_at, last_error}` from `vulndb_meta`. -- `rescanVulnerabilities` — `POST /api/vulnerabilities/rescan`, sets `scan_pending: true` on every `server_packages` document for the instance. Audit event `vuln.rescan`. -- `acceptFinding` — `POST /api/vulnerabilities/:id/accept`, body `{reason string, until time}`. **Reject an empty reason and a past `until` with 400.** Sets state `accepted` with the `Acceptance` block. Audit event `vuln.accepted` with the reason and expiry in the details. -- `unacceptFinding` — `DELETE /api/vulnerabilities/:id/accept`, back to `open`, audit event `vuln.unaccepted`. +- `listVulnerabilities` — `GET /api/vulnerabilities`, query params `severity`, `state`, `server`, plus tag filters. **Groups by CVE** in the response: `{cve_id, severity, title, server_count, servers: [...]}`. +- `vulnerabilitySummary` — `GET /api/vulnerabilities/summary`: severity counts for open findings, plus `{db_version, pulled_at, last_error}` from `vulndb_meta`. +- `rescanVulnerabilities` — `POST /api/vulnerabilities/rescan`: sets `scan_pending: true` on every `server_packages` document for the instance. Audit `vuln.rescan`. +- `acceptFinding` — `POST /api/vulnerabilities/:id/accept`, body `{reason string, until time}`. **Reject an empty reason and a past `until` with 400.** Sets state `accepted` with the `Acceptance` block. Audit `vuln.accepted` with reason and expiry in the details. +- `unacceptFinding` — `DELETE /api/vulnerabilities/:id/accept`: back to `open`. Audit `vuln.unaccepted`. - `listServerVulnerabilities` — `GET /api/servers/:id/vulnerabilities`. - `getServerPackages` — `GET /api/servers/:id/packages`. - `searchPackages` — `GET /api/packages/search?name=`, calls `services.SearchPackages`. @@ -2987,11 +2449,15 @@ In `server/internal/api/handlers.go`, inside the `apiGroup` block: apiGroup.DELETE("/vuln-rules/:id", auth.RequireRole("owner", "admin"), deleteVulnRule) ``` -Check `auth.RequireRole`'s actual variadic signature before using it with two arguments. +**Check `auth.RequireRole`'s actual variadic signature** before using it with two arguments. -- [ ] **Step 5: Verify and commit** +- [ ] **Step 5: Verify by hand** -Run: `cd server && go build ./... && go vet ./... && go test ./...` +With a dev instance running, confirm each: an accept with an empty reason returns 400; an accept with a past `until` returns 400; a member session gets 403 on accept and 200 on the list; a rescan sets `scan_pending` (check in Mongo). + +- [ ] **Step 6: Build and commit** + +Run: `cd server && go build ./... && go vet ./...` ```bash git add server/internal/api/ shared/models/settings.go server/internal/services/findings.go @@ -3006,27 +2472,21 @@ git commit -m "feat: vulnerability REST API, retention setting and sweeper" - Create: `web/app/(app)/vulnerabilities/page.tsx`, `web/components/vulnerabilities/{FindingRow,AcceptDialog,DBFreshness}.tsx`, `web/components/settings/VulnAlertRulesCard.tsx` - Modify: `web/lib/api.ts`, `web/components/Sidebar.tsx`, `web/app/(app)/servers/[id]/page.tsx`, `web/app/(app)/settings/notifications/page.tsx` -**Interfaces:** -- Consumes: the Task 15 routes. -- Produces: no Go interfaces. - -No tests: `web/` has no frontend test infrastructure and adding one is out of scope for this feature. Verify by running the app. - - [ ] **Step 1: Add API client types and methods** -In `web/lib/api.ts`, add `VulnFinding`, `VulnGroup`, `VulnSummary`, `ServerPackages`, `VulnAlertRule` types mirroring the Go JSON tags exactly, and one client method per Task 15 route. Follow the file's existing conventions. +In `web/lib/api.ts`, add `VulnFinding`, `VulnGroup`, `VulnSummary`, `ServerPackages` and `VulnAlertRule` types mirroring the Go JSON tags exactly, plus one client method per Task 15 route. Follow the file's existing conventions. - [ ] **Step 2: Build the fleet board** -Create `web/app/(app)/vulnerabilities/page.tsx`. **Grouped by CVE, one row per CVE with an affected-server count, expandable to the servers.** The same CVE across 40 servers is one decision; a flat list of findings makes it look like forty. +Create `web/app/(app)/vulnerabilities/page.tsx`. **Grouped by CVE, one row per CVE with an affected-server count, expandable to the servers.** The same CVE across 40 servers is one decision; a flat list makes it look like forty. Requirements: -- Filter bar: severity, state (default `open`), tag filter reusing `TagFilterBar`'s pattern from the servers page. -- `DBFreshness` banner at the top, always visible, showing `pulled_at` age and `last_error` if set. A fleet scanning against a three-week-old database must say so rather than quietly report all-clear. +- Filter bar: severity, state (default `open`), tag filter following `TagFilterBar`'s pattern from the servers page. +- `DBFreshness` banner at the top, always visible, showing `pulled_at` age and `last_error` when set. A fleet scanning against a three-week-old database must say so rather than quietly report all-clear. - A server whose `status` is `unsupported` renders as **"unsupported"**, never as zero findings. -- State never reads by colour alone: every severity pill carries a distinct shape and a text label, matching the existing monitor pills. +- Every severity pill carries a distinct shape and a text label — state never reads by colour alone, matching the existing monitor pills. - Findings with no `fixed_in` show "no fix published" rather than an empty cell. -- Use Tailwind token names only. No hex values. +- Tailwind token names only. No hex values. - [ ] **Step 3: Build the accept dialog** @@ -3034,17 +2494,19 @@ Create `web/components/vulnerabilities/AcceptDialog.tsx`: required reason textar - [ ] **Step 4: Add the remediation action** -On a finding with `fixed_in`, render an **Apply updates** button calling the existing `POST /api/servers/:id/apply-updates`. No new endpoint and no new mechanism — see it, patch it, one place. +On a finding with `fixed_in`, render an **Apply updates** button calling the existing `POST /api/servers/:id/apply-updates`. No new endpoint and no new mechanism. -- [ ] **Step 5: Add the server-detail tabs and nav** +- [ ] **Step 5: Add the tabs and nav** -Add **Vulnerabilities** and **Packages** tabs to `web/app/(app)/servers/[id]/page.tsx`, following the existing tab pattern in that file. Add a **Vulnerabilities** entry to `web/components/Sidebar.tsx`. Mount `VulnAlertRulesCard` on `web/app/(app)/settings/notifications/page.tsx` beside the channels. +Add **Vulnerabilities** and **Packages** tabs to `web/app/(app)/servers/[id]/page.tsx`, following the existing tab pattern. Add a **Vulnerabilities** entry to `web/components/Sidebar.tsx`. Mount `VulnAlertRulesCard` on `web/app/(app)/settings/notifications/page.tsx` beside the channels. - [ ] **Step 6: Verify** Run: `cd web && npm run lint && npm run build` Expected: exit 0, no type errors. +Then load the pages against a dev instance and confirm: the CVE grouping expands, the freshness banner shows an age, an unsupported server says so, and the accept dialog rejects an empty reason. + - [ ] **Step 7: Commit** ```bash @@ -3061,7 +2523,7 @@ git commit -m "feat: vulnerability findings UI" - [ ] **Step 1: Add the new collections to instance deletion** -Find the instance-deletion routine — it is the one place that knows which collections carry `instance_id`: +Find the instance-deletion routine — the one place that knows which collections carry `instance_id`: ```bash cd server && grep -rn "instance_id" internal/services/ | grep -i "delete\|purge\|reap" @@ -3071,25 +2533,29 @@ Add `server_packages` and `vuln_findings` to its collection list. **Missing this - [ ] **Step 2: Add the admin entitlement toggle** -In `admin/internal/models/entitlements.go`, add `vuln_scanning` to the feature toggles, following whatever pattern the existing per-instance toggles use. It must be **off for both Free plans**. Confirm the licence issue path copies it into `License.Features`. +In `admin/internal/models/entitlements.go`, add `vuln_scanning` to the feature toggles, following the pattern the existing per-instance toggles use. It must be **off for both Free plans**. Confirm the licence issue path copies it into `License.Features`. -- [ ] **Step 3: Verify the whole tree builds and tests pass** +- [ ] **Step 3: Verify the whole tree builds** -Run from the repo root: +From the repo root: ```bash -go build ./... && go vet ./... && go test ./... +go build ./... && go vet ./... ``` -Expected: exit 0. `go test ./...` must pass with no database and no network. +Expected: exit 0. -- [ ] **Step 4: Document it** +- [ ] **Step 4: End-to-end check** + +Against a dev instance with one Linux agent: enable `vuln_scanning`, wait for the hourly report (or restart the agent to force one), confirm a `server_packages` document appears with `scan_pending: true`, and that within ~60s `vulnsched` clears the flag and writes findings. Confirm a second agent report with an unchanged package set sends no body. + +- [ ] **Step 5: Document it** Add a **Package inventory and CVE findings** subsystem section to `CLAUDE.md` covering: the backport trap and why matching uses distro feeds; why only the leader matches; that findings go `fixed` rather than being deleted; the two new environment variables. Add a user-facing page under `docsite/docs/vantage/`. -While in `CLAUDE.md`, correct the `shared/mail/render_test.go` claim — that file did not exist before Task 14 created it. +While in `CLAUDE.md`, correct the `shared/mail/render_test.go` claim — no such file exists, and this plan does not create one. -- [ ] **Step 5: Commit** +- [ ] **Step 6: Commit** ```bash git add -A @@ -3098,27 +2564,25 @@ git commit -m "docs: document package inventory and CVE findings" --- -## Self-review notes +## Spec coverage -Checked against the spec, section by section: - -- Backport trap → Tasks 1 and 11, with the equal-version case asserted in both. +- Backport trap → Tasks 1 and 11, with the equal-version case in both manual checks. - trivy-db as the source, ephemeral storage, `VANTAGE_TRIVY_DB_REF` → Task 10. - Leader-only matching, `scan_pending` persisted, tick-boundary batching → Tasks 8 and 13. - Hash short-circuit wire path → Tasks 6 and 7. - All four collections and their indexes → Task 5. - `SourceName` covering every binary of a source package → Tasks 4 and 11. - Unsupported distro never reads as clean → Tasks 2, 13 and 16. -- Severity vendor → NVD → unknown → Task 10 (`severityName`) and Task 11. -- Findings lifecycle with acceptance expiry, ordering of fixed-before-reopen → Task 12. +- Severity vendor → NVD → unknown → Task 10 (`severityName`). +- Findings lifecycle with acceptance expiry, fixed-before-reopen ordering → Task 12. - Alert rules through `ResolveTargets`, one digest per tick → Task 14. - Entitlement gated at collection, `HasFeature` not tier → Tasks 8, 9 and 17. - REST API, retention, sweeper → Task 15. - CVE-grouped board, DB freshness on screen, remediation via the existing endpoint → Task 16. - Instance-deletion collection list → Task 17. -**Known gaps the executing engineer must close:** +## Known gaps the executing engineer must close -- `vulndb.Pull` ships as a stub with implementation notes rather than working oras code (Task 10, Step 1). It is the one piece that cannot be written blind, because the exact `oras-go` v2 call shape depends on the resolved version. Verify it by hand against the real registry before Task 13. -- Several call sites are written against signatures that must be confirmed in the actual files first, and each says so at the point of use: the licence accessor (Task 8), `ValidateAgentToken`'s return shape (Task 8), `ResolveTargets` (Task 14), `auth.RequireRole`'s variadic form (Task 15), the `trivy-db` API names (Task 10), and the unexported render function in `shared/mail` (Task 14). -- The `shared/mail` `cases` map (Task 14, Step 3) must be populated by reading the existing `SendX` methods. It is deliberately left empty in the plan because inventing entries for templates I have not read would produce a test that fails for the wrong reason. +- **`vulndb.Pull` ships as a stub** with implementation requirements rather than working oras code (Task 10). It is the one piece that cannot be written blind: the exact `oras-go` v2 call shape depends on the resolved version. Verify it by hand (Task 10, Step 3) before Task 13. +- **Six call sites are written against signatures that must be confirmed first**, each flagged at the point of use: the licence accessor (Task 8), `ValidateAgentToken`'s return shape (Task 8), `ResolveTargets` (Task 14), `auth.RequireRole`'s variadic form (Task 15), the `trivy-db` API names (Task 10), and `shared/mail`'s render conventions (Task 14). +- **No automated tests exist for any of this**, by instruction. The manual verification tables in Tasks 1, 11 and 12 are the substitute and are the difference between shipping this and shipping something that reports a patched fleet as vulnerable, or worse, the reverse. Do not skip them. diff --git a/docs/superpowers/plans/2026-08-06-workload-registry.md b/docs/superpowers/plans/2026-08-06-workload-registry.md new file mode 100644 index 0000000..0785cec --- /dev/null +++ b/docs/superpowers/plans/2026-08-06-workload-registry.md @@ -0,0 +1,1489 @@ +# Workload Registry 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:** Agents enumerate what each Linux server actually runs — Docker containers, the compose stacks grouping them, and systemd services — and report it to the control plane. Containers and units can be started, stopped and restarted from the UI, and a bounded snapshot of their logs read without opening a console. + +**Architecture:** A **workload** is one container or one systemd unit. The agent collects both on a 60-second ticker and reports through a `ReportWorkloads` RPC with a hash short-circuit. On-demand refresh does not return data — `RefreshWorkloadsCmd` makes the agent report immediately through that same RPC, so there is one writer for the collection. Control actions and log reads travel out through the existing `commandDispatcher` and answer back over the bus, mirroring `StepResults`. + +**Tech Stack:** Go 1.26, gin, mongo-driver v2, gRPC, Redis (bus), Next.js 16 + TanStack Query, Tailwind 3. + +**Spec:** `docs/superpowers/specs/2026-08-06-workload-registry-design.md`. Read it before starting. Where this plan and the spec disagree, the spec wins and the plan is wrong. + +## Orientation for a fresh session + +This is sub-project B of four. Sub-project A (package inventory and CVE findings) is independent and may or may not be built yet — **this plan shares no code with it** and can be executed first, second, or alone. If A exists, its `ServerPackages` model is the shape `ServerWorkloads` deliberately mirrors. + +Key existing machinery this plan builds on, with the reasoning that must not be broken: + +- `server/internal/services/dispatch.go` — `commandDispatcher.send()` publishes a command envelope addressed to the pod holding that agent's stream and waits for its ack. **Request/ack, not a queue:** a command whose owner died must fail loudly (503) rather than sit in a queue while the operator is told it worked. +- `server/internal/services/stepresults.go` — `StepResults.Await/Deliver`. `Await` subscribes to the result channel **before** the command is dispatched. Copy that ordering exactly; a fast agent otherwise answers into a channel nobody has joined. +- `agent/internal/sync/sync.go` — the command stream handler, with a `handleX` function per `ServerCommand` variant. +- `bus.RunAsLeader("housekeeping", …)` in `server/cmd/main.go` — nothing in this plan needs it. There is no scheduler here; the agent drives its own ticker. + +## Global Constraints + +- **Do not write tests.** No `*_test.go`, no npm test files, no test scaffolding. This is a deliberate instruction from the repository owner, not an oversight. Verification is `go build`, `go vet`, `npm run build` and the manual checks written into the tasks. +- **Linux only.** Windows agents must not collect workloads. systemd does not exist there and the container story differs; a separate spec covers it if ever. +- **Not gated by licence.** v1 ships to every instance. Do not add a `HasFeature` check. +- Every service query is scoped by `instance_id`. No unscoped lookups, ever. +- Every mutating API path writes an audit event via `services.LogEvent(instanceID, eventType, actor, serverID, keyID, details string)`. +- Module path prefix is `gitea.hostxtra.co.uk/mrhid6/vantage/`. +- Mongo driver is **v2**: `go.mongodb.org/mongo-driver/v2/bson`; ObjectIDs are `bson.ObjectID`, not `primitive.ObjectID`. +- Collections are reached via `db.Col("name")`. +- No component in `web/` may carry a hex colour; use Tailwind token names only. `web/` is dark-only — do not add a light theme. +- Commit messages use the `feat:` / `fix:` / `docs:` prefixes already in `git log`. +- No new Go dependencies. Everything here shells out to binaries already on the host. + +## Correctness risks carried without tests + +Places where a mistake produces a **wrong answer or a silent cost rather than a crash**. Each has a manual check written into its task. + +1. **The protected set (Task 4).** Wrong in the permissive direction and a server can stop its own agent — it goes offline and the only way back is SSH or physical access, which is what this feature exists to avoid needing. +2. **Hash order-independence (Task 3).** `docker ps` output ordering is not stable. An ordering-sensitive hash resends the full list every 60 seconds forever, visible only as traffic. +3. **Log caps (Task 5).** A line count alone does not bound size. 500 lines of 4KB JSON is 2MB through the bus. +4. **`DockerOK` vs empty list (Tasks 2 and 9).** "Docker not installed" and "Docker running nothing" must not render alike. + +--- + +## File Structure + +**Create — agent:** +- `agent/internal/workloads/workloads.go` — `Collect()`, orchestrating both collectors +- `agent/internal/workloads/docker.go` — Docker collection and parsing +- `agent/internal/workloads/systemd.go` — systemd collection and parsing +- `agent/internal/workloads/control.go` — start/stop/restart, and the protected set +- `agent/internal/workloads/logs.go` — bounded log reads + +**Create — server:** +- `server/internal/models/workloads.go` — `ServerWorkloads`, `Workload` +- `server/internal/services/workloads.go` — storage, hash compare, fleet search, dispatch wrappers +- `server/internal/services/workloadresults.go` — `WorkloadResults` registry over the bus +- `server/internal/api/workloads.go` — REST handlers + +**Create — web:** +- `web/app/(app)/workloads/page.tsx` — fleet-wide view and search +- `web/components/workloads/WorkloadList.tsx` — grouped stacks, containers, units +- `web/components/workloads/WorkloadRow.tsx` — one row with its actions +- `web/components/workloads/LogDialog.tsx` — bounded log snapshot + +**Modify:** +- `proto/vantage/v1/vantage.proto` — `ReportWorkloads` RPC, three `ServerCommand` variants, one `AgentMessage` variant +- `agent/internal/sync/sync.go` — the 60s report loop and three command handlers +- `agent/internal/grpc/client.go` — `ReportWorkloads` client method +- `server/internal/grpc/server.go` — `ReportWorkloads` handler; route `WorkloadLogsResult` in `CommandStream` +- `server/internal/services/coreindexes.go` — workload indexes +- `server/internal/api/handlers.go` — register routes +- `web/lib/api.ts` — types and client methods +- `web/components/Sidebar.tsx` — Workloads entry +- `web/app/(app)/servers/[id]/page.tsx` — Workloads tab +- The control plane's instance-deletion collection list — add `server_workloads` +- `CLAUDE.md`, `docsite/docs/vantage/` — document the subsystem + +--- + +### Task 1: Models and indexes + +**Files:** +- Create: `server/internal/models/workloads.go` +- Modify: `server/internal/services/coreindexes.go`, `server/cmd/main.go` + +**Interfaces:** +- Consumes: nothing. +- Produces: `models.ServerWorkloads`, `models.Workload`, `services.EnsureWorkloadIndexes() error`. + +- [ ] **Step 1: Write the models** + +Create `server/internal/models/workloads.go`: + +```go +package models + +import ( + "time" + + "go.mongodb.org/mongo-driver/v2/bson" +) + +// Workload kinds. +const ( + WorkloadContainer = "container" + WorkloadUnit = "unit" +) + +// Control actions. +const ( + WorkloadStart = "start" + WorkloadStop = "stop" + WorkloadRestart = "restart" +) + +// Workload is one container or one systemd unit. +type Workload struct { + Kind string `bson:"kind" json:"kind"` // container | unit + ID string `bson:"id" json:"id"` // container id, or unit name + Name string `bson:"name" json:"name"` + + // State is deliberately NOT collapsed into a shared vocabulary across the + // two kinds. Containers report running/exited/paused/restarting/created; + // units report active/inactive/failed/activating. A failed unit and an + // exited container mean different things, and flattening them loses the + // distinction the operator needs. + State string `bson:"state" json:"state"` + Health string `bson:"health,omitempty" json:"health,omitempty"` + + Image string `bson:"image,omitempty" json:"image,omitempty"` + Stack string `bson:"stack,omitempty" json:"stack,omitempty"` // compose project label + Ports []string `bson:"ports,omitempty" json:"ports,omitempty"` + + Restarts int `bson:"restarts,omitempty" json:"restarts,omitempty"` + StartedAt time.Time `bson:"started_at,omitempty" json:"started_at,omitempty"` + + // Protected is computed agent-side and reported so the UI can render the + // action disabled with a reason rather than offering a button whose refusal + // is already known. The field is the courtesy; the agent's own check is the + // boundary. + Protected bool `bson:"protected" json:"protected"` +} + +// ServerWorkloads holds one server's whole workload list in ONE document. +type ServerWorkloads struct { + ID bson.ObjectID `bson:"_id,omitempty" json:"-"` + InstanceID string `bson:"instance_id" json:"-"` + ServerID string `bson:"server_id" json:"server_id"` + Hash string `bson:"hash" json:"hash"` + Workloads []Workload `bson:"workloads" json:"workloads"` + CollectedAt time.Time `bson:"collected_at" json:"collected_at"` + + // A host with no Docker and a host with Docker running nothing both produce + // an empty list. One should read "not in use here", the other "nothing + // running", and only the second deserves any alarm. + // + // The error strings separate a third case the booleans cannot: installed + // with the daemon down. "Not installed" and "installed but not responding" + // are different problems with different fixes. + DockerOK bool `bson:"docker_ok" json:"docker_ok"` + DockerError string `bson:"docker_error,omitempty" json:"docker_error,omitempty"` + SystemdOK bool `bson:"systemd_ok" json:"systemd_ok"` + SystemdError string `bson:"systemd_error,omitempty" json:"systemd_error,omitempty"` +} +``` + +- [ ] **Step 2: Add the index builder** + +Append to `server/internal/services/coreindexes.go` (or create `server/internal/services/workloadindexes.go` if that file's conventions do not fit): + +```go +// EnsureWorkloadIndexes declares the indexes for the workload registry. +// +// Warns rather than being fatal, matching EnsureSecretIndexes: a missing index +// degrades these queries to a collection scan, which is no reason to refuse to +// serve the fleet. +func EnsureWorkloadIndexes() error { + ctx := context.Background() + + idx := []mongo.IndexModel{ + { + Keys: bson.D{{Key: "instance_id", Value: 1}, {Key: "server_id", Value: 1}}, + Options: options.Index().SetUnique(true), + }, + // Multikey, for the fleet-wide "which servers run image X" query, which + // is the reason the snapshot is stored rather than fetched and discarded. + {Keys: bson.D{{Key: "instance_id", Value: 1}, {Key: "workloads.image", Value: 1}}}, + } + if _, err := db.Col("server_workloads").Indexes().CreateMany(ctx, idx); err != nil { + log.Printf("warning: server_workloads indexes: %v", err) + } + return nil +} +``` + +- [ ] **Step 3: Wire it into schema setup** + +In `server/cmd/main.go`, find `runSchemaSetup` (near line 75) and the block calling the other `Ensure*Indexes` functions. Add alongside the non-fatal ones: + +```go + if err := services.EnsureWorkloadIndexes(); err != nil { + log.Printf("warning: workload indexes: %v", err) + } +``` + +- [ ] **Step 4: Build and commit** + +Run: `cd server && go build ./... && go vet ./...` + +```bash +git add server/internal/models/workloads.go server/internal/services/ server/cmd/main.go +git commit -m "feat: models and indexes for the workload registry" +``` + +--- + +### Task 2: Docker collector + +**Files:** +- Create: `agent/internal/workloads/docker.go` + +**Interfaces:** +- Consumes: nothing. +- Produces: `workloads.Workload` (the agent-side struct), `workloads.collectDocker() ([]Workload, bool, string)` returning workloads, ok, and an error string. + +- [ ] **Step 1: Write the collector** + +Create `agent/internal/workloads/docker.go`: + +```go +package workloads + +import ( + "context" + "encoding/json" + "os/exec" + "strings" + "time" +) + +// Workload is one container or one systemd unit, agent-side. It mirrors +// models.Workload on the server. +type Workload struct { + Kind string + ID string + Name string + State string + Health string + Image string + Stack string + Ports []string + Restarts int + StartedAt time.Time + Protected bool +} + +const dockerTimeout = 30 * time.Second + +// dockerInspect is the subset of `docker inspect` output we read. +// +// We use inspect rather than `docker ps --format '{{json .}}'` because ps +// reports health and uptime inside a human Status string — "Up 2 hours +// (healthy)" — and anything built on that is parsing English that is +// localised, reworded between releases, and silently different for a paused or +// restarting container. inspect gives typed fields instead. +type dockerInspect struct { + ID string `json:"Id"` + Name string `json:"Name"` + State struct { + Status string `json:"Status"` + StartedAt string `json:"StartedAt"` + Restarting bool `json:"Restarting"` + Health *struct { + Status string `json:"Status"` + } `json:"Health"` + } `json:"State"` + Config struct { + Image string `json:"Image"` + Labels map[string]string `json:"Labels"` + } `json:"Config"` + RestartCount int `json:"RestartCount"` + NetworkSettings struct { + Ports map[string][]struct { + HostIP string `json:"HostIp"` + HostPort string `json:"HostPort"` + } `json:"Ports"` + } `json:"NetworkSettings"` +} + +// collectDocker enumerates containers. It returns ok=false with an empty error +// string when Docker is simply not installed — the common case on this fleet, +// and not a fault. +func collectDocker(ctx context.Context) ([]Workload, bool, string) { + if _, err := exec.LookPath("docker"); err != nil { + return nil, false, "" // not installed; not an error + } + + ctx, cancel := context.WithTimeout(ctx, dockerTimeout) + defer cancel() + + idsOut, err := exec.CommandContext(ctx, "docker", "ps", "-aq").Output() + if err != nil { + // Installed but not answering: a different problem with a different + // fix, so it carries a message where "not installed" does not. + return nil, false, "docker ps failed: " + errText(err) + } + + ids := strings.Fields(string(idsOut)) + if len(ids) == 0 { + return []Workload{}, true, "" // Docker present, nothing running + } + + args := append([]string{"inspect", "--format", "{{json .}}"}, ids...) + out, err := exec.CommandContext(ctx, "docker", args...).Output() + if err != nil { + return nil, false, "docker inspect failed: " + errText(err) + } + + var wls []Workload + for _, line := range strings.Split(string(out), "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + var di dockerInspect + if err := json.Unmarshal([]byte(line), &di); err != nil { + continue + } + wls = append(wls, dockerToWorkload(di)) + } + return wls, true, "" +} + +func dockerToWorkload(di dockerInspect) Workload { + w := Workload{ + Kind: "container", + ID: di.ID, + Name: strings.TrimPrefix(di.Name, "/"), + State: di.State.Status, + Image: di.Config.Image, + Restarts: di.RestartCount, + } + if di.State.Health != nil { + w.Health = strings.ToLower(di.State.Health.Status) + } + // The compose project label is what Docker itself treats as authoritative. + // No YAML is read from disk: a compose file there may not be what is running. + if v := di.Config.Labels["com.docker.compose.project"]; v != "" { + w.Stack = v + } + if t, err := time.Parse(time.RFC3339Nano, di.State.StartedAt); err == nil { + w.StartedAt = t + } + for container, bindings := range di.NetworkSettings.Ports { + for _, b := range bindings { + w.Ports = append(w.Ports, b.HostIP+":"+b.HostPort+"->"+container) + } + } + return w +} + +func errText(err error) string { + if ee, ok := err.(*exec.ExitError); ok && len(ee.Stderr) > 0 { + return strings.TrimSpace(string(ee.Stderr)) + } + return err.Error() +} +``` + +- [ ] **Step 2: Verify against a real Docker host** + +Confirm the field names by eye before trusting the struct tags: + +```bash +docker run -d --name plantest --restart=always nginx:alpine +docker inspect --format '{{json .}}' plantest | python3 -m json.tool | head -60 +docker rm -f plantest +``` + +Check that `Id`, `Name`, `State.Status`, `State.StartedAt`, `RestartCount`, `Config.Image`, `Config.Labels` and `NetworkSettings.Ports` all appear with those exact spellings and shapes. If you have a compose project handy, confirm `com.docker.compose.project` is present on its containers. + +- [ ] **Step 3: Build and commit** + +Run: `cd agent && go build ./... && go vet ./...` + +```bash +git add agent/internal/workloads/docker.go +git commit -m "feat: agent enumerates docker containers" +``` + +--- + +### Task 3: systemd collector and hashing + +**Files:** +- Create: `agent/internal/workloads/systemd.go`, `agent/internal/workloads/workloads.go` + +**Interfaces:** +- Consumes: `workloads.Workload`, `workloads.collectDocker` (Task 2). +- Produces: `workloads.Collect(ctx) Result`, `workloads.Hash([]Workload) string`, `workloads.Result{Workloads, DockerOK, DockerError, SystemdOK, SystemdError}`. + +- [ ] **Step 1: Write the systemd collector** + +Create `agent/internal/workloads/systemd.go`: + +```go +package workloads + +import ( + "context" + "os/exec" + "strings" + "time" +) + +const systemdTimeout = 30 * time.Second + +// excludedPrefixes drops the platform's own units. A typical host carries 300+ +// units and systemd accounts for most of them; listing all of them buries the +// ten anyone cares about. +var excludedPrefixes = []string{"systemd-", "user@", "user-", "session-", "init.scope"} + +// collectSystemd enumerates services in two passes, because "running or +// failed" and "enabled but stopped" are different questions — and an enabled +// unit that is not running is exactly the one worth seeing. +func collectSystemd(ctx context.Context) ([]Workload, bool, string) { + if _, err := exec.LookPath("systemctl"); err != nil { + return nil, false, "" + } + + ctx, cancel := context.WithTimeout(ctx, systemdTimeout) + defer cancel() + + // Column output rather than --output=json: the JSON flag needs systemd + // 246+, and this fleet includes older stable distributions. The columns + // have been stable considerably longer than the JSON has existed. + unitsOut, err := exec.CommandContext(ctx, "systemctl", + "list-units", "--type=service", "--state=running,failed", + "--no-legend", "--plain", "--no-pager").Output() + if err != nil { + return nil, false, "systemctl list-units failed: " + errText(err) + } + + seen := map[string]bool{} + var wls []Workload + + for _, line := range strings.Split(string(unitsOut), "\n") { + f := strings.Fields(line) + // UNIT LOAD ACTIVE SUB DESCRIPTION… + if len(f) < 4 { + continue + } + name := f[0] + if excluded(name) || seen[name] { + continue + } + seen[name] = true + wls = append(wls, Workload{ + Kind: "unit", + ID: name, + Name: strings.TrimSuffix(name, ".service"), + State: f[2], // ACTIVE: active | failed | activating | inactive + }) + } + + filesOut, err := exec.CommandContext(ctx, "systemctl", + "list-unit-files", "--type=service", "--state=enabled", + "--no-legend", "--plain", "--no-pager").Output() + if err == nil { + for _, line := range strings.Split(string(filesOut), "\n") { + f := strings.Fields(line) + // UNIT FILE STATE [PRESET] + if len(f) < 2 { + continue + } + name := f[0] + if excluded(name) || seen[name] { + continue + } + seen[name] = true + wls = append(wls, Workload{ + Kind: "unit", + ID: name, + Name: strings.TrimSuffix(name, ".service"), + State: "inactive", // enabled but not currently running + }) + } + } + + return wls, true, "" +} + +func excluded(name string) bool { + for _, p := range excludedPrefixes { + if strings.HasPrefix(name, p) { + return true + } + } + return false +} +``` + +- [ ] **Step 2: Write the orchestrator and hash** + +Create `agent/internal/workloads/workloads.go`: + +```go +package workloads + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "runtime" + "sort" + "strconv" +) + +// Result is one collection pass. +type Result struct { + Workloads []Workload + DockerOK bool + DockerError string + SystemdOK bool + SystemdError string +} + +// Collect enumerates every workload on this host. Linux only. +func Collect(ctx context.Context) Result { + if runtime.GOOS != "linux" { + return Result{} + } + + var r Result + containers, dockerOK, dockerErr := collectDocker(ctx) + units, systemdOK, systemdErr := collectSystemd(ctx) + + r.DockerOK, r.DockerError = dockerOK, dockerErr + r.SystemdOK, r.SystemdError = systemdOK, systemdErr + r.Workloads = append(append([]Workload{}, containers...), units...) + + markProtected(r.Workloads) + return r +} + +// Hash fingerprints a workload set so an unchanged set never has to be sent. +// +// It sorts first: `docker ps` output ordering is not stable, and an +// ordering-sensitive hash would resend the full list every 60 seconds forever +// — a cost visible only as traffic. +// +// StartedAt is deliberately excluded: it does not change while a container +// runs, and including it would add nothing. Restarts IS included, because a +// container cycling is exactly the change worth reporting. +func Hash(wls []Workload) string { + lines := make([]string, 0, len(wls)) + for _, w := range wls { + lines = append(lines, strings.Join([]string{ + w.Kind, w.ID, w.Name, w.State, w.Health, w.Image, w.Stack, + strconv.Itoa(w.Restarts), + }, "\x00")) + } + sort.Strings(lines) + h := sha256.New() + for _, l := range lines { + h.Write([]byte(l)) + h.Write([]byte("\n")) + } + return hex.EncodeToString(h.Sum(nil)) +} +``` + +Add `"strings"` to the import block. `markProtected` arrives in Task 4 — add a temporary empty `func markProtected(_ []Workload) {}` to this file now so it builds, and delete it when Task 4 lands. + +- [ ] **Step 3: Verify the systemd parsing and the hash** + +On a Linux host, confirm the column positions the parser assumes: + +```bash +systemctl list-units --type=service --state=running,failed --no-legend --plain --no-pager | head -5 +systemctl list-unit-files --type=service --state=enabled --no-legend --plain --no-pager | head -5 +``` + +The first must have the unit name in column 1 and `active`/`failed` in column 3. The second must have the unit name in column 1. Confirm `systemd-journald.service` appears in the raw output and would be dropped by `excluded`, while `sshd.service` or `nginx.service` survives. + +For the hash, confirm by inspection that two slices holding the same workloads in different order produce the same string, and that changing one `Restarts` value changes it. + +- [ ] **Step 4: Build and commit** + +Run: `cd agent && go build ./... && go vet ./...` + +```bash +git add agent/internal/workloads/ +git commit -m "feat: agent enumerates systemd services" +``` + +--- + +### Task 4: Control actions and the protected set + +**Files:** +- Create: `agent/internal/workloads/control.go` +- Modify: `agent/internal/workloads/workloads.go` — remove the temporary `markProtected` stub + +**Interfaces:** +- Consumes: `workloads.Workload`. +- Produces: `workloads.Control(ctx, kind, id, action string) error`, `workloads.ErrProtected`, `workloads.markProtected([]Workload)`. + +**This is the task with the unrecoverable failure mode.** A server that stops its own agent goes offline, and the only way back is SSH or physical access — which is what this feature exists to avoid needing. + +- [ ] **Step 1: Write the control layer** + +Create `agent/internal/workloads/control.go`: + +```go +package workloads + +import ( + "context" + "errors" + "fmt" + "os" + "os/exec" + "regexp" + "strings" + "time" +) + +// ErrProtected is returned for a workload the agent will not act on. +var ErrProtected = errors.New("workload is protected") + +// AgentUnit is the systemd unit this agent runs as. +const AgentUnit = "vantage-agent.service" + +// controlTimeout bounds a stop that may never finish on its own. `docker stop` +// waits on a container that may ignore SIGTERM, and `systemctl stop` on a unit +// with a long TimeoutStopSec blocks for exactly as long as that says. A +// timeout must return a real error rather than an ack implying success. +const controlTimeout = 90 * time.Second + +// ownContainerID is read once: the container this agent runs in, if any. +var ownContainerID = detectOwnContainer() + +var cgroupContainerRe = regexp.MustCompile(`[0-9a-f]{64}`) + +// detectOwnContainer returns this process's container ID, or "" on a host +// install. The agent is normally a systemd service, so "" is the common case; +// this exists so containerising it later cannot silently remove the guard. +func detectOwnContainer() string { + b, err := os.ReadFile("/proc/self/cgroup") + if err != nil { + return "" + } + if m := cgroupContainerRe.FindString(string(b)); m != "" { + return m + } + return "" +} + +// isProtected reports whether the agent refuses to act on this workload. +// +// The refusal lives here, in the agent, and not in the control plane. As with +// the console relay hardcoding 127.0.0.1 agent-side: the control plane may name +// a target, but the agent decides what it will do to itself. A server-side +// denylist alone would be bypassed by the next dispatch path someone adds. +func isProtected(kind, id, name string) bool { + if kind == "unit" { + return id == AgentUnit || name == strings.TrimSuffix(AgentUnit, ".service") + } + if ownContainerID == "" { + return false + } + // Container IDs are commonly abbreviated to 12 characters; compare on the + // shorter of the two so a short id still matches a full one. + return strings.HasPrefix(ownContainerID, id) || strings.HasPrefix(id, ownContainerID) +} + +// markProtected stamps the flag onto a collected list so the UI can render the +// action disabled with a reason. +func markProtected(wls []Workload) { + for i := range wls { + wls[i].Protected = isProtected(wls[i].Kind, wls[i].ID, wls[i].Name) + } +} + +// Control starts, stops or restarts a workload. +func Control(ctx context.Context, kind, id, action string) error { + switch action { + case "start", "stop", "restart": + default: + return fmt.Errorf("unknown action %q", action) + } + + // Checked before anything else happens, and checked here rather than only + // on the server. See isProtected. + if isProtected(kind, id, strings.TrimSuffix(id, ".service")) { + return fmt.Errorf("%w: %s", ErrProtected, id) + } + + ctx, cancel := context.WithTimeout(ctx, controlTimeout) + defer cancel() + + var cmd *exec.Cmd + switch kind { + case "container": + cmd = exec.CommandContext(ctx, "docker", action, id) + case "unit": + cmd = exec.CommandContext(ctx, "systemctl", action, id) + default: + return fmt.Errorf("unknown workload kind %q", kind) + } + + if out, err := cmd.CombinedOutput(); err != nil { + if ctx.Err() == context.DeadlineExceeded { + return fmt.Errorf("%s %s timed out after %s", action, id, controlTimeout) + } + return fmt.Errorf("%s %s: %s", action, id, strings.TrimSpace(string(out))) + } + return nil +} +``` + +Delete the temporary `markProtected` stub from `workloads.go`. + +- [ ] **Step 2: Verify the guard** + +**Do not skip this.** From a scratch `main.go` on a dev machine, confirm: + +| Call | Expected | +| ---- | -------- | +| `Control(ctx, "unit", "vantage-agent.service", "restart")` | error wrapping `ErrProtected`, and **no restart happens** | +| `Control(ctx, "unit", "vantage-agent", "restart")` | error wrapping `ErrProtected` — the name form must be caught too | +| `Control(ctx, "unit", "nginx.service", "restart")` | proceeds | +| `Control(ctx, "container", "", "restart")` | proceeds | +| `Control(ctx, "unit", "nginx.service", "reload")` | error, unknown action | + +Then run `markProtected` over a collected list and confirm `vantage-agent.service` comes back `Protected: true` and everything else `false`. + +- [ ] **Step 3: Build and commit** + +Run: `cd agent && go build ./... && go vet ./...` + +```bash +git add agent/internal/workloads/ +git commit -m "feat: agent control actions with self-protection" +``` + +--- + +### Task 5: Bounded log reads + +**Files:** +- Create: `agent/internal/workloads/logs.go` + +**Interfaces:** +- Consumes: nothing. +- Produces: `workloads.Logs(ctx, kind, id string, tail int) (text string, truncated bool, err error)`, `workloads.MaxLogLines`, `workloads.MaxLogBytes`. + +- [ ] **Step 1: Write the log reader** + +Create `agent/internal/workloads/logs.go`: + +```go +package workloads + +import ( + "context" + "fmt" + "os/exec" + "strconv" + "strings" + "time" +) + +const ( + // MaxLogLines and MaxLogBytes are BOTH enforced, whichever binds first. + // + // A line count alone does not bound size: 500 lines of a container printing + // 4KB JSON blobs is 2MB travelling over the bus. This is the same reasoning + // that gave workflow logs a per-line cap as well as a per-run one. + MaxLogLines = 500 + MaxLogBytes = 256 * 1024 + + logTimeout = 60 * time.Second +) + +// Logs returns a bounded snapshot of a workload's recent output. +// +// There is no follow mode. The browser console already offers a real terminal +// on the same server where `docker logs -f` works properly, with its own +// scrollback and cancellation. A snapshot answers "why did this restart", +// which is the question that sends people to the console in the first place. +func Logs(ctx context.Context, kind, id string, tail int) (string, bool, error) { + if tail <= 0 || tail > MaxLogLines { + tail = MaxLogLines + } + + ctx, cancel := context.WithTimeout(ctx, logTimeout) + defer cancel() + + var cmd *exec.Cmd + switch kind { + case "container": + cmd = exec.CommandContext(ctx, "docker", "logs", + "--tail", strconv.Itoa(tail), "--timestamps", id) + case "unit": + cmd = exec.CommandContext(ctx, "journalctl", "-u", id, + "-n", strconv.Itoa(tail), "--no-pager", "--output=short-iso") + default: + return "", false, fmt.Errorf("unknown workload kind %q", kind) + } + + // docker logs writes container stderr to our stderr, so both streams must + // be captured or half the output silently disappears. + out, err := cmd.CombinedOutput() + if err != nil && len(out) == 0 { + return "", false, fmt.Errorf("read logs for %s: %s", id, errText(err)) + } + + return cap(string(out)) +} + +// cap enforces both limits, trimming from the FRONT: the most recent lines are +// the ones worth keeping. +func cap(s string) (string, bool, error) { + truncated := false + + lines := strings.Split(s, "\n") + if len(lines) > MaxLogLines { + lines = lines[len(lines)-MaxLogLines:] + truncated = true + } + s = strings.Join(lines, "\n") + + if len(s) > MaxLogBytes { + s = s[len(s)-MaxLogBytes:] + // Drop the leading partial line left by a byte-wise cut. + if i := strings.IndexByte(s, '\n'); i >= 0 { + s = s[i+1:] + } + truncated = true + } + + return s, truncated, nil +} +``` + +`cap` shadows the builtin. Rename it to `capLog` if that bothers the linter; the behaviour is what matters. + +- [ ] **Step 2: Verify both caps** + +Both directions, because a line-count-only implementation passes the first and fails the second silently: + +```bash +# Line cap: 600 lines in, expect 500 out and truncated=true +docker run -d --name captest alpine sh -c 'for i in $(seq 1 600); do echo line-$i; done; sleep 3600' + +# Byte cap: few lines, each large. Expect truncated=true well under 500 lines. +docker run -d --name bigtest alpine sh -c 'for i in $(seq 1 100); do head -c 4000 /dev/zero | tr "\0" "x"; echo; done; sleep 3600' +``` + +Call `Logs(ctx, "container", "captest", 0)` and `Logs(ctx, "container", "bigtest", 0)` from a scratch `main.go`. Confirm the first returns 500 lines with `truncated` true, and the second returns ≤256KB with `truncated` true. Then `docker rm -f captest bigtest`. + +- [ ] **Step 3: Build and commit** + +Run: `cd agent && go build ./... && go vet ./...` + +```bash +git add agent/internal/workloads/logs.go +git commit -m "feat: agent reads bounded workload logs" +``` + +--- + +### Task 6: Proto + +**Files:** +- Modify: `proto/vantage/v1/vantage.proto` +- Regenerate: `agent/internal/grpc/pb/`, `server/internal/grpc/pb/` + +- [ ] **Step 1: Add the RPC and messages** + +Add to the `Vantage` service block: + +```protobuf + rpc ReportWorkloads(ReportWorkloadsRequest) returns (ReportWorkloadsResponse); +``` + +Add these messages at the end of the file: + +```protobuf +// ReportWorkloads carries what a server is running. +// +// Offer-then-send, the same handshake as ReportPackages: the agent calls once +// with workloads empty, and resends with the body only if need_full is set. +message ReportWorkloadsRequest { + string server_id = 1; + string agent_token = 2; + string hash = 3; + bool docker_ok = 4; + string docker_error = 5; + bool systemd_ok = 6; + string systemd_error = 7; + repeated Workload workloads = 8; // empty on the offer call +} + +message ReportWorkloadsResponse { + bool need_full = 1; +} + +message Workload { + string kind = 1; // "container" | "unit" + string id = 2; + string name = 3; + string state = 4; + string health = 5; + string image = 6; + string stack = 7; + repeated string ports = 8; + int32 restarts = 9; + string started_at = 10; // RFC3339, empty when not running + bool protected = 11; +} + +// RefreshWorkloadsCmd carries no payload back. It makes the agent report +// immediately through ReportWorkloads, so there is exactly one writer for the +// server_workloads collection rather than two arriving by different routes. +message RefreshWorkloadsCmd {} + +message ControlWorkloadCmd { + string kind = 1; + string id = 2; + string action = 3; // "start" | "stop" | "restart" +} + +message WorkloadLogsCmd { + string kind = 1; + string id = 2; + int32 tail = 3; +} + +message WorkloadLogsResult { + string command_id = 1; + string text = 2; + bool truncated = 3; + string error = 4; +} +``` + +- [ ] **Step 2: Add the command and result variants** + +In `message ServerCommand`, add three variants to the `oneof command` block using the next free field numbers — **check the file, do not reuse a number**: + +```protobuf + RefreshWorkloadsCmd refresh_workloads = ; + ControlWorkloadCmd control_workload = ; + WorkloadLogsCmd workload_logs = ; +``` + +In `message AgentMessage`, add one variant to its `oneof` using the next free number there: + +```protobuf + WorkloadLogsResult workload_logs_result = ; +``` + +- [ ] **Step 3: Regenerate** + +Run the project's existing protoc generation command. Find it with: + +```bash +grep -rn "protoc" --include=Makefile --include="*.sh" --include="*.yml" . | head +``` + +Output must land in both `agent/internal/grpc/pb/` and `server/internal/grpc/pb/`. + +- [ ] **Step 4: Build both modules and commit** + +Run: `cd agent && go build ./... && cd ../server && go build ./...` + +```bash +git add proto/ agent/internal/grpc/pb/ server/internal/grpc/pb/ +git commit -m "feat: workload registry proto messages" +``` + +--- + +### Task 7: Server storage and the result registry + +**Files:** +- Create: `server/internal/services/workloads.go`, `server/internal/services/workloadresults.go` +- Modify: `server/internal/grpc/server.go` + +**Interfaces:** +- Consumes: `models.ServerWorkloads` (Task 1), the proto types (Task 6). +- Produces: `services.HasWorkloadHash`, `services.StoreWorkloads`, `services.GetWorkloads`, `services.SearchWorkloads`, `services.WorkloadResults.Await/Deliver`, `services.DispatchRefreshWorkloads`, `services.DispatchControlWorkload`, `services.DispatchWorkloadLogs`. + +- [ ] **Step 1: Write the storage service** + +Create `server/internal/services/workloads.go`: + +```go +package services + +import ( + "context" + "time" + + "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/db" + "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/models" + "go.mongodb.org/mongo-driver/v2/bson" + "go.mongodb.org/mongo-driver/v2/mongo" + "go.mongodb.org/mongo-driver/v2/mongo/options" +) + +func HasWorkloadHash(instanceID, serverID, hash string) (bool, error) { + err := db.Col("server_workloads").FindOne(context.Background(), bson.M{ + "instance_id": instanceID, + "server_id": serverID, + "hash": hash, + }, options.FindOne().SetProjection(bson.M{"_id": 1})).Err() + if err == mongo.ErrNoDocuments { + return false, nil + } + return err == nil, err +} + +// StoreWorkloads replaces a server's workload list. +func StoreWorkloads(instanceID, serverID, hash string, wls []models.Workload, + dockerOK bool, dockerErr string, systemdOK bool, systemdErr string) error { + + _, err := db.Col("server_workloads").UpdateOne(context.Background(), + bson.M{"instance_id": instanceID, "server_id": serverID}, + bson.M{"$set": bson.M{ + "hash": hash, + "workloads": wls, + "collected_at": time.Now(), + "docker_ok": dockerOK, + "docker_error": dockerErr, + "systemd_ok": systemdOK, + "systemd_error": systemdErr, + }}, + options.UpdateOne().SetUpsert(true), + ) + return err +} + +func GetWorkloads(instanceID, serverID string) (*models.ServerWorkloads, error) { + var sw models.ServerWorkloads + err := db.Col("server_workloads").FindOne(context.Background(), bson.M{ + "instance_id": instanceID, + "server_id": serverID, + }).Decode(&sw) + if err == mongo.ErrNoDocuments { + return nil, nil + } + if err != nil { + return nil, err + } + return &sw, nil +} + +type WorkloadHit struct { + ServerID string `json:"server_id"` + Workload models.Workload `json:"workload"` +} + +// SearchWorkloads answers "which servers run image X" — the reason the snapshot +// is stored rather than fetched on demand and discarded. +func SearchWorkloads(instanceID, image, stack, state string) ([]WorkloadHit, error) { + ctx := context.Background() + + filter := bson.M{"instance_id": instanceID} + if image != "" { + filter["workloads.image"] = image + } + + cur, err := db.Col("server_workloads").Find(ctx, filter) + if err != nil { + return nil, err + } + defer cur.Close(ctx) + + var docs []models.ServerWorkloads + if err := cur.All(ctx, &docs); err != nil { + return nil, err + } + + hits := []WorkloadHit{} + for _, d := range docs { + for _, w := range d.Workloads { + if image != "" && w.Image != image { + continue + } + if stack != "" && w.Stack != stack { + continue + } + if state != "" && w.State != state { + continue + } + hits = append(hits, WorkloadHit{ServerID: d.ServerID, Workload: w}) + } + } + return hits, nil +} +``` + +- [ ] **Step 2: Write the result registry** + +Create `server/internal/services/workloadresults.go`, mirroring `stepresults.go` exactly: + +```go +package services + +import ( + "context" + "encoding/json" + "log" + + "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/bus" + "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/grpc/pb" +) + +// Workload log results travel back over the bus for the same reason commands +// travel out over it: the pod serving the HTTP request and the pod holding the +// agent's stream are two different processes, and a map in one cannot be read +// by the other. +// +// Await MUST be called before the command is dispatched, or a fast agent +// answers into a channel nobody is listening on yet. See stepresults.go. + +type workloadResultRegistry struct{} + +var WorkloadResults = &workloadResultRegistry{} + +func (r *workloadResultRegistry) Await(commandID string) (<-chan *pb.WorkloadLogsResult, func()) { + out := make(chan *pb.WorkloadLogsResult, 1) + + ctx, cancel := context.WithCancel(context.Background()) + raw, unsub, err := bus.Subscribe(ctx, bus.ResultChannel+commandID) + if err != nil { + log.Printf("workload results: subscribe for %s: %v", commandID, err) + cancel() + close(out) + return out, func() {} + } + + go func() { + defer close(out) + select { + case <-ctx.Done(): + return + case b, ok := <-raw: + if !ok { + return + } + var res pb.WorkloadLogsResult + if err := json.Unmarshal(b, &res); err != nil { + log.Printf("workload results: undecodable result for %s: %v", commandID, err) + return + } + out <- &res + } + }() + + return out, func() { + cancel() + unsub() + } +} + +// Deliver publishes a result received from an agent. Called on the pod holding +// that agent's stream, which is not usually the pod waiting for it. +func (r *workloadResultRegistry) Deliver(res *pb.WorkloadLogsResult) { + if res == nil || res.CommandId == "" { + return + } + ctx, cancel := context.WithTimeout(context.Background(), dispatchAckTimeout) + defer cancel() + if _, err := bus.Publish(ctx, bus.ResultChannel+res.CommandId, res); err != nil { + log.Printf("workload results: publish for %s: %v", res.CommandId, err) + } +} +``` + +- [ ] **Step 3: Write the dispatch wrappers** + +Append to `server/internal/services/workloads.go`, following how `DispatchRunStep` in `dispatch.go` builds a command and how `dispatchAndWait` orders subscription before dispatch: + +```go +// DispatchRefreshWorkloads asks an agent to report immediately. It returns as +// soon as the agent acks; the caller refetches the stored document. +func DispatchRefreshWorkloads(serverID string) error { … } + +// DispatchControlWorkload runs a control action and waits for the result. +func DispatchControlWorkload(serverID, kind, id, action string) error { … } + +// DispatchWorkloadLogs fetches a bounded log snapshot. +// +// Await is called BEFORE dispatch. Reversing those two lines introduces a race +// that only shows under load, on a fast agent. +func DispatchWorkloadLogs(serverID, kind, id string, tail int) (text string, truncated bool, err error) { … } +``` + +Fill each in against the actual `commandDispatcher` API in `dispatch.go` — generate a command ID the same way `DispatchRunStep` does, and reuse its ack-error handling so an offline agent surfaces as `ErrAgentNotConnected`. + +- [ ] **Step 4: Add the gRPC handler and route the result** + +In `server/internal/grpc/server.go`, add a `ReportWorkloads` handler following `ReportUpdates`: validate the agent token, answer `need_full: !known` on the offer call (empty `workloads`), otherwise convert to `[]models.Workload` and call `StoreWorkloads`. + +Then in `CommandStream` (near line 165), where `CommandResult` and `StepResult` are already routed, add the new variant: + +```go + if r := msg.GetWorkloadLogsResult(); r != nil { + services.WorkloadResults.Deliver(r) + continue + } +``` + +Match the surrounding switch or if-chain style exactly. + +- [ ] **Step 5: Build and commit** + +Run: `cd server && go build ./... && go vet ./...` + +```bash +git add server/internal/services/workloads.go server/internal/services/workloadresults.go server/internal/grpc/server.go +git commit -m "feat: store workload reports and route log results" +``` + +--- + +### Task 8: Agent wiring + +**Files:** +- Modify: `agent/internal/sync/sync.go`, `agent/internal/grpc/client.go` + +**Interfaces:** +- Consumes: `workloads.Collect`, `workloads.Hash`, `workloads.Control`, `workloads.Logs` (Tasks 2–5); the proto types (Task 6). +- Produces: a 60s report loop and three command handlers. + +- [ ] **Step 1: Add the client method** + +In `agent/internal/grpc/client.go`, following `ReportUpdates`: + +```go +// ReportWorkloads sends a workload report and returns whether the server wants +// the full list. +func (c *Client) ReportWorkloads(req *pb.ReportWorkloadsRequest) (bool, error) { + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + resp, err := c.client.ReportWorkloads(ctx, req) + if err != nil { + return false, err + } + return resp.GetNeedFull(), nil +} +``` + +- [ ] **Step 2: Add the report loop** + +In `agent/internal/sync/sync.go`, add a `runWorkloads(ctx, cfg)` following the shape of the existing `runInventory` (line 413), on a 60-second ticker, calling a `reportWorkloads(cfg)` that: + +1. returns immediately on non-Linux +2. calls `workloads.Collect(ctx)` +3. computes `workloads.Hash(result.Workloads)` +4. calls `ReportWorkloads` with `Workloads` empty +5. on `need_full`, calls again with the list populated + +Convert `workloads.Workload` to `*pb.Workload`, formatting `StartedAt` as RFC3339 and sending `""` for the zero time. + +Start it from the same place the other goroutines are started (`Run`, near line 31): + +```go + go runWorkloads(ctx, cfg) +``` + +- [ ] **Step 3: Add the three command handlers** + +In the command stream's dispatch switch, alongside `handleApplyUpdates` and `handleOpenProxy`: + +```go +// handleRefreshWorkloads makes the agent report immediately. It sends nothing +// back beyond the ack: the refresh is a nudge, not a channel, so there is one +// writer for the collection rather than two. +func handleRefreshWorkloads(cfg *config.Config) { + go reportWorkloads(cfg) +} + +func handleControlWorkload(stream pb.Vantage_CommandStreamClient, cfg *config.Config, commandID string, cmd *pb.ControlWorkloadCmd) { + err := workloads.Control(context.Background(), cmd.GetKind(), cmd.GetId(), cmd.GetAction()) + // Report through the existing CommandResult path, matching how + // handleApplyUpdates replies. On success, report immediately so the UI's + // refetch shows the new state rather than the old one. + if err == nil { + go reportWorkloads(cfg) + } + // … send CommandResult{CommandId: commandID, Ok: err == nil, Error: …} +} + +func handleWorkloadLogs(stream pb.Vantage_CommandStreamClient, cfg *config.Config, commandID string, cmd *pb.WorkloadLogsCmd) { + text, truncated, err := workloads.Logs(context.Background(), cmd.GetKind(), cmd.GetId(), int(cmd.GetTail())) + res := &pb.WorkloadLogsResult{CommandId: commandID, Text: text, Truncated: truncated} + if err != nil { + res.Error = err.Error() + } + // … send AgentMessage{WorkloadLogsResult: res} +} +``` + +Fill in the send calls against the exact `AgentMessage` construction the neighbouring handlers use — the stream send is not the same shape in every file. + +- [ ] **Step 4: Verify end to end against a dev instance** + +With a dev server and one Linux agent: + +1. Confirm a `server_workloads` document appears within 60s. +2. Confirm a second cycle with nothing changed sends no body — add a temporary log line at the `need_full` branch if the traffic is not otherwise visible, then remove it. +3. Start a container and confirm the next report includes it. +4. Dispatch a control action and confirm the state changes and a fresh report follows. +5. Dispatch a control action against `vantage-agent.service` and **confirm it is refused and the agent stays up**. + +- [ ] **Step 5: Build and commit** + +Run: `cd agent && go build ./... && go vet ./...` + +```bash +git add agent/internal/sync/sync.go agent/internal/grpc/client.go +git commit -m "feat: agent reports workloads and handles workload commands" +``` + +--- + +### Task 9: REST API + +**Files:** +- Create: `server/internal/api/workloads.go` +- Modify: `server/internal/api/handlers.go` + +- [ ] **Step 1: Write the handlers** + +Create `server/internal/api/workloads.go`, following the conventions in `handlers.go` — `auth.InstanceID(c)` for scoping, `actorFromCtx(c)` for the audit actor: + +- `getServerWorkloads` — `GET /api/servers/:id/workloads`. Returns the stored document. A server with no document yet returns an empty list with `docker_ok: false, systemd_ok: false` rather than 404 — the agent may simply not have reported yet. +- `refreshServerWorkloads` — `POST /api/servers/:id/workloads/refresh`. Calls `DispatchRefreshWorkloads`. Answers **503** when the dispatcher reports the agent is not connected; the client then shows the stored snapshot as stale rather than pretending. +- `controlWorkload` — `POST /api/servers/:id/workloads/:wid/action`, body `{"action":"start|stop|restart"}`. Owner or admin. **Answers 409 when the agent refuses a protected workload**, with the reason in the message — not 500, since nothing failed. Audit event `workload.` naming the target. +- `getWorkloadLogs` — `GET /api/servers/:id/workloads/:wid/logs?tail=`. Owner or admin. Clamps `tail` to 500 server-side rather than erroring. Audit event `workload.logs_read`. +- `listWorkloads` — `GET /api/workloads?image=&stack=&state=`. Fleet-wide, calls `SearchWorkloads`. + +`:wid` arrives URL-encoded — decode it before use. Unit names carry dots and `@`. + +- [ ] **Step 2: Register the routes** + +In `server/internal/api/handlers.go`, inside the `apiGroup` block: + +```go + apiGroup.GET("/workloads", listWorkloads) + apiGroup.GET("/servers/:id/workloads", getServerWorkloads) + apiGroup.POST("/servers/:id/workloads/refresh", refreshServerWorkloads) + apiGroup.POST("/servers/:id/workloads/:wid/action", auth.RequireRole("owner", "admin"), controlWorkload) + apiGroup.GET("/servers/:id/workloads/:wid/logs", auth.RequireRole("owner", "admin"), getWorkloadLogs) +``` + +**Check `auth.RequireRole`'s actual variadic signature** before using it with two arguments. Note also that gin resolves static segments ahead of wildcards, so `/servers/:id/workloads/refresh` and `/servers/:id/workloads/:wid/action` coexist — the same arrangement `/servers/new` and `/servers/:id` already use. + +- [ ] **Step 3: Verify by hand** + +Against a dev instance: a member session gets 200 on the workload list and **403** on both the action and the logs endpoints; an action against the agent's own unit returns **409**; a refresh against a stopped agent returns **503**; `?tail=99999` returns at most 500 lines. + +- [ ] **Step 4: Build and commit** + +Run: `cd server && go build ./... && go vet ./...` + +```bash +git add server/internal/api/ +git commit -m "feat: workload registry REST API" +``` + +--- + +### Task 10: Web UI + +**Files:** +- Create: `web/app/(app)/workloads/page.tsx`, `web/components/workloads/{WorkloadList,WorkloadRow,LogDialog}.tsx` +- Modify: `web/lib/api.ts`, `web/components/Sidebar.tsx`, `web/app/(app)/servers/[id]/page.tsx` + +- [ ] **Step 1: Add API client types and methods** + +In `web/lib/api.ts`, add `Workload`, `ServerWorkloads` and `WorkloadHit` types mirroring the Go JSON tags exactly, plus one client method per Task 9 route. Follow the file's existing conventions. + +- [ ] **Step 2: Build the server-detail tab** + +Add a **Workloads** tab to `web/app/(app)/servers/[id]/page.tsx`, following the existing tab pattern, rendering `WorkloadList`. + +`WorkloadList` orders: **compose stacks first, grouped under the stack name**, then loose containers, then units. Not cosmetic — a stack is one thing to an operator even when it is six containers, and a flat list turns one decision into six rows. + +Opening the tab dispatches a refresh, then refetches. Show the `collected_at` age while it is in flight. + +Three rules from the model: +- **`docker_ok: false` with no `docker_error` reads "Docker not in use on this server"** — never an empty list, and never alarming. With a `docker_error`, show that instead: it is a real problem with a real fix. +- **`protected` rows render their action buttons disabled with the reason**, rather than offering a button whose refusal is already known. +- State never reads by colour alone: every pill carries a distinct shape and a text label, matching the existing monitor pills. Container and unit states use their own vocabularies — do not map them onto a shared one. + +- [ ] **Step 3: Build the log dialog** + +Create `web/components/workloads/LogDialog.tsx`: monospace, on the `--well` token (the floor beneath the ground, used for machine output), scrolled to the bottom on open. When `truncated`, show a banner saying the output was capped — the user must not read a truncated log as a complete one. + +- [ ] **Step 4: Build the fleet view** + +Create `web/app/(app)/workloads/page.tsx`: filters for image, stack and state; rows link to their server. Add a **Workloads** entry to `web/components/Sidebar.tsx`. + +- [ ] **Step 5: Verify** + +Run: `cd web && npm run lint && npm run build` +Expected: exit 0, no type errors. + +Then against a dev instance confirm: stacks group, a protected row's buttons are disabled, a server without Docker says "not in use" rather than showing nothing, a truncated log shows its banner, and a restart updates the row without a manual reload. + +- [ ] **Step 6: Commit** + +```bash +git add web/ +git commit -m "feat: workload registry UI" +``` + +--- + +### Task 11: Instance deletion and documentation + +- [ ] **Step 1: Add the collection to instance deletion** + +Find the instance-deletion routine — the one place that knows which collections carry `instance_id`: + +```bash +cd server && grep -rn "instance_id" internal/services/ | grep -i "delete\|purge\|reap" +``` + +Add `server_workloads` to its collection list. Missing this orphans a tenant's data indefinitely. + +- [ ] **Step 2: Verify the whole tree builds** + +From the repo root: + +```bash +go build ./... && go vet ./... +``` + +Expected: exit 0. + +- [ ] **Step 3: Document it** + +Add a **Workload registry** subsystem section to `CLAUDE.md` covering: why refresh does not return data (one writer); why the protected set lives agent-side; why there is no live log following; and the `DockerOK`/`DockerError` distinction. Add a user-facing page under `docsite/docs/vantage/`. + +- [ ] **Step 4: Commit** + +```bash +git add -A +git commit -m "docs: document the workload registry" +``` + +--- + +## Spec coverage + +- Workload as the domain word, containers and units in one collection → Task 1. +- `DockerOK`/`DockerError` distinction → Tasks 1, 2, 10. +- Docker via `ps -aq` + `inspect`, no English parsing → Task 2. +- Compose stacks from the label, no YAML → Task 2. +- systemd two-pass with the exclusion filter, column output not JSON → Task 3. +- Hash order-independence → Task 3. +- Protected set agent-side, `vantage-agent.service` and own container → Task 4. +- Control timeouts returning real errors → Task 4. +- Log caps in both directions, no follow mode → Task 5. +- Refresh returns no data; one writer → Tasks 6, 8. +- `Await` before dispatch → Task 7. +- 503 offline, 409 protected, owner|admin on control and logs → Task 9. +- Stacks grouped first; protected rows disabled; truncation stated → Task 10. +- Instance-deletion collection list → Task 11. + +## Known gaps the executing engineer must close + +- **Three dispatch wrappers are specified by signature and doc comment, not body** (Task 7, Step 3). They must be written against the actual `commandDispatcher` API in `dispatch.go`, reusing how `DispatchRunStep` generates a command ID and handles ack errors. `DispatchWorkloadLogs` must call `Await` **before** dispatch. +- **The agent's stream-send calls are left to match their neighbours** (Task 8, Step 3). `AgentMessage` construction is not identical across handlers, so copy the adjacent one rather than the sketch. +- **`auth.RequireRole`'s variadic signature must be confirmed** before Task 9's two-argument use. +- **No automated tests, by instruction.** The manual checks in Tasks 2, 3, 4, 5, 8, 9 and 10 are the substitute. Task 4's is the one that matters most: get the protected set wrong in the permissive direction and a server can take itself offline in a way the UI cannot undo. diff --git a/docs/superpowers/specs/2026-08-06-package-inventory-and-cve-findings-design.md b/docs/superpowers/specs/2026-08-06-package-inventory-and-cve-findings-design.md index b8644a6..147d9fb 100644 --- a/docs/superpowers/specs/2026-08-06-package-inventory-and-cve-findings-design.md +++ b/docs/superpowers/specs/2026-08-06-package-inventory-and-cve-findings-design.md @@ -395,8 +395,9 @@ This adds one `notify` payload type and a `vuln_digest.html.tmpl` / `vuln_digest.txt.tmpl` pair in `shared/mail`. Note that `shared/mail` templates are parsed in `init()`, so a mistyped field is a boot-time panic — CLAUDE.md describes a `render_test.go` guarding against exactly this, but **that file does -not exist**; the repository has no Go tests at all today. The implementation -plan adds it alongside this feature's own tests. +not exist**; the repository has no Go tests at all, and by instruction this +feature adds none. The template pair must therefore be verified by starting the +binary and sending one digest through a real channel. --- @@ -470,30 +471,33 @@ quietly report all-clear. --- -## Testing +## Verification -**Version comparison, table-driven, per family.** The highest-value test in the -feature, because its failure mode is silent. The table must include the -backport case explicitly — installed `1:3.0.2-0ubuntu1.15` against advisory -fixed-in `1:3.0.2-0ubuntu1.15` resolving to *not vulnerable* — alongside tilde -ordering (`1.0~rc1` < `1.0`), epoch dominance (`1:1.0` > `2.0`) and -`1.9` < `1.10`. +**No automated tests.** The repository has none today, and by explicit +instruction this feature adds none — no `*_test.go`, no frontend test files. +That is a deliberate decision by the repository owner, recorded here so the +absence reads as a choice rather than an omission. -**Matcher golden tests.** A fixture package list and a small hand-built BoltDB -fixture asserting an exact finding set. No network: a test that pulls the real -database fails on a bad day and changes its expectations every six hours. +It does change the risk profile, and the places it changes it are worth naming, +because each fails by producing a **wrong answer rather than a crash**: -**State transitions.** `open → fixed → reopened`, acceptance expiry, and -`first_seen` surviving a rescan. The last is easy to break with an upsert that -overwrites, and nothing notices until a report claims everything was discovered -yesterday. +- **Version comparison.** The backport case — installed `1:3.0.2-0ubuntu1.15` + against advisory fixed-in `1:3.0.2-0ubuntu1.15` resolving to *not + vulnerable* — plus tilde ordering (`1.0~rc1` < `1.0`), epoch dominance + (`1:1.0` > `2.0`) and `1.9` < `1.10`. Wrong here means a vulnerable fleet + reported clean. +- **Source-package fan-out.** One advisory against `openssl` must flag + `libssl3`, `openssl` and `libssl-dev`. Matching on binary name alone silently + finds one of three. +- **`first_seen` preservation.** An upsert that overwrites it makes every + finding look discovered today, and nothing surfaces that until someone reads + a report. +- **Fixed-before-reopen ordering.** A finding both absent from a scan and past + its acceptance expiry must settle `fixed`, not reopen. -**Collector parsers**, one per package manager, from captured command output. -`updates.go` currently has no tests and its parsers are the same shape, so -these fixtures should cover both. - -**Scoping and gating.** Findings queries scoped by `instance_id`, and the -entitlement gate proven to write nothing rather than merely hide. +The implementation plan carries a manual verification table for each, to be +walked before the relevant task is committed. They are the substitute for the +tests, not a formality. --- diff --git a/docs/superpowers/specs/2026-08-06-workload-registry-design.md b/docs/superpowers/specs/2026-08-06-workload-registry-design.md index 8d31adf..cbae7d6 100644 --- a/docs/superpowers/specs/2026-08-06-workload-registry-design.md +++ b/docs/superpowers/specs/2026-08-06-workload-registry-design.md @@ -377,22 +377,29 @@ Three rules that follow directly from the model: --- -## Testing +## Verification -Every test is pure, driven by captured fixtures, with no daemon, no database -and no network — the repository has no Go tests today and these must run under -plain `go test ./...`. +**No automated tests.** The repository has none today, and by explicit +instruction this feature adds none — no `*_test.go`, no frontend test files. +A deliberate decision by the repository owner, recorded so the absence reads as +a choice rather than an omission. -- `docker inspect` JSON fixture → `[]Workload`, asserting `RestartCount`, - health, and that the compose label becomes `Stack`. -- `systemctl` column fixtures → `[]Workload`, including the exclusion filter - dropping `systemd-*` and `user@*` while keeping `nginx.service`. -- Protected-set computation: `vantage-agent.service` marked, `nginx.service` - not. -- Hash order-independence, matching the package report's test. -- **Log capping in both directions**: 600 lines in → 500 out with `truncated` - set; a 300KB blob of fewer than 500 lines → capped, `truncated` set. The - second is the case a line-count-only implementation silently fails. +The behaviours that would otherwise have been tested are the ones that fail +quietly, and the implementation plan carries a manual check for each: + +- **Parser output against real command output.** `docker inspect` must yield + `RestartCount`, health and the compose label as `Stack`; the `systemctl` + exclusion filter must drop `systemd-*` and `user@*` while keeping + `nginx.service`. Both are verified against a live host rather than a fixture. +- **Protected-set computation.** `vantage-agent.service` marked, + `nginx.service` not. Getting this wrong in the permissive direction lets a + server stop its own agent, which is unrecoverable from the UI. +- **Hash order-independence.** An ordering-sensitive hash resends the full list + every 60 seconds, which is invisible except as traffic. +- **Log capping in both directions.** 600 lines in → 500 out with `truncated`; + a 300KB blob of fewer than 500 lines → capped, `truncated`. The second is the + case a line-count-only implementation silently fails, and it fails by sending + megabytes rather than by erroring. ---