From 3afc4ab0129174d85f8e528ef17ce210691e5541 Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Thu, 6 Aug 2026 11:27:37 +0100 Subject: [PATCH] docs: workload registry plan; remove tests from both plans Both plans now verify by build, vet and manual checks written into the tasks. Spec verification sections updated to match so they no longer describe tests that will not be written. --- ...8-06-package-inventory-and-cve-findings.md | 1296 +++++--------- .../plans/2026-08-06-workload-registry.md | 1489 +++++++++++++++++ ...ckage-inventory-and-cve-findings-design.md | 48 +- .../2026-08-06-workload-registry-design.md | 35 +- 4 files changed, 1916 insertions(+), 952 deletions(-) create mode 100644 docs/superpowers/plans/2026-08-06-workload-registry.md 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. ---