feat: build settings by allow-list, node architecture, go_build payload (ADR 0019)

cmd/agent/main.go

@@ -288,15 +288,15 @@ func run(ctx context.Context, logger *slog.Logger, clientset kubernetes.Interfac
 		if err := spool.WriteGoInventory(goInventorySeq, time.Now(), goStore.Coverage(), goStore.Snapshot()); err != nil {
 			logger.Error("spooling go inventory", "error", err)
 		}
-		// Dependency sets are keyed by image digest and immutable for it, so
-		// each is written once and marked only after the write succeeded; a
-		// failed write is retried on the next flush (ADR 0017).
-		for _, d := range goStore.PendingDependencies() {
-			if err := spool.WriteGoDependencies(d); err != nil {
-				logger.Error("spooling go dependencies", "image_digest", d.ImageDigest, "error", err)
+		// Build facts are keyed by image digest and immutable for it, so each is
+		// written once and marked only after the write succeeded; a failed write
+		// is retried on the next flush (ADR 0017).
+		for _, b := range goStore.PendingBuilds() {
+			if err := spool.WriteGoBuild(b); err != nil {
+				logger.Error("spooling go build", "image_digest", b.ImageDigest, "error", err)
 				continue
 			}
-			goStore.MarkDependenciesWritten(d.ImageDigest)
+			goStore.MarkBuildWritten(b.ImageDigest)
 		}
 	}
 

docs/adr/0019-build-settings-by-allow-list.md

@@ -0,0 +1,140 @@
+# 0019. Build settings ship by allow-list, and the payload that carries them is named for the build
+
+Date: 2026-08-22
+Status: Accepted
+
+Extends [ADR 0017](0017-build-facts-keyed-by-digest.md): the payload it
+introduced as `go_dependencies` is renamed `go_build` and gains a `settings`
+object. ADR 0017's decisions — write-once, keyed by image digest, no sequence,
+no capture time — are unchanged and now cover the settings too.
+
+## Context
+
+The scanner has always read the whole build-settings block of every kept binary
+and thrown all but one bit of it away. `hasPGO` looks for a `-pgo` key and
+returns a boolean; everything else the toolchain recorded is discarded on the
+node.
+
+What is discarded decides whether a Go workload can be advised at all.
+`GOAMD64` says which microarchitecture level the binary may use, and a `v1`
+binary on a `v3` fleet is leaving instructions unused. `CGO_ENABLED` decides
+whether the binary is statically linkable at all. `-race` in production costs
+multiples of CPU and memory and is a finding on its own. `vcs.revision` ties a
+running binary to the commit it was built from, and `vcs.modified` says the
+build came from a dirty tree.
+
+Two things make this different from every other fact the agent collects.
+
+**The values are written by people, not by the toolchain.** Everywhere else the
+agent reads a Kubernetes field or a kernel counter. Build settings include
+`-ldflags`, and a real `-ldflags` from this repository's own image looks like
+`-X main.version=a5edd4b-dirty` — with `-X main.buildHost=ci-07.internal.acme.corp`
+being an equally ordinary sight. `-pgo` holds an absolute path on the build
+machine. `-gcflags`, `-asmflags` and `-tags` are free-form by definition. This
+is the one place where a string somebody typed into a CI pipeline can walk into
+a payload.
+
+**The key set is not ours.** The Go toolchain decides what it records and is
+free to add keys in any release. Any rule expressed as "drop these" protects
+only against the keys we thought to name at the time we wrote it, and silently
+stops protecting the moment the toolchain grows a new one.
+
+Separately, the payload introduced by ADR 0017 is named `go_dependencies` while
+already carrying `go_version` and `main_module`. It was facts about a build from
+the day it shipped, and settings make the name plainly wrong. No backend
+consumes it yet, so the rename costs a golden file today and a protocol
+migration later.
+
+## Decision
+
+**Build settings are filtered on the node by an exhaustive allow-list, and
+everything else is discarded before a record is formed.** The list is
+`CGO_ENABLED`, `GOARCH`, `GOAMD64`, `GOARM64`, `GOARM`, `-race`, `-trimpath`,
+`vcs`, `vcs.revision`, `vcs.time`, `vcs.modified`. It lives in
+`internal/nodescan`, which means the filtering happens before the node→controller
+channel: nothing outside it crosses the wire, rather than crossing it and being
+dropped (CLAUDE.md invariant 4).
+
+Allow-list rather than deny-list is the decision, not an implementation detail.
+Everything on the list is a token the toolchain chose from a fixed vocabulary or
+a value git produced; a deny-list would have to keep pace with a key set we do
+not control. The failure mode of an allow-list is a fact we do not collect. The
+failure mode of a deny-list is a build hostname in a payload.
+
+Three consequences follow directly:
+
+**Only the PGO boolean survives, never the profile path.** `-pgo` is not on the
+list, so its value — a directory on the build machine — stays on the node, while
+`GoRecord.PGO` still says whether PGO was applied.
+
+**`PGO` stays on the record even though the settings now imply it.** A record
+whose container has no image digest yet has no `go_build` payload to join to;
+that case is already counted as `facts_undigested` (ADR 0017). PGO is the one
+build fact that survives a missing key.
+
+**An over-long value is dropped whole, never truncated.** Every allowed key holds
+something short and bounded, so a value beyond 128 characters means the binary is
+not what the list assumes. A prefix of an unexpected string is still an
+unexpected string, and would ship looking like a legitimate value.
+
+**The payload is renamed `go_dependencies` → `go_build`**, with `settings`
+alongside `modules`. The file in the spool becomes `go-build-<digest>.json`, and
+the Go type `inventory.BuildDependencies` becomes `inventory.BuildFacts`. ADR
+0018 refers to the old method names `PendingDependencies` /
+`MarkDependenciesWritten`; those are now `PendingBuilds` / `MarkBuildWritten`.
+ADR 0018 is not edited — an accepted ADR describes what was decided when it was
+decided, and this paragraph is the forwarding address.
+
+**`vcs.*` is collected, and its absence is normal.** A commit hash is an
+identifier from the customer's repository. It carries no source, is meaningless
+without access to that repository, and is enumerated in `docs/security.md` §8
+like every other collected field. More importantly it is frequently missing: the
+toolchain stamps it only when the build ran against a VCS working tree, and three
+independent conditions have to hold in a container build — `.git` must reach the
+build context (excluding it is a common and *correct* thing to do, since `.git`
+changes on every commit and invalidates the `COPY` layer cache), the builder
+image must have `git`, and the repository must be owned by the user running the
+build. That last one fails loudly: `go build` with `-buildvcs=auto` silently
+omits the stamp when there is no repository at all, but *errors the build* when
+a repository exists and the VCS command fails, which is what
+`COPY . .` followed by `USER nonroot` produces. The usual response is
+`-buildvcs=false`, after which the stamp is gone for good.
+
+Both halves of this were verified against real images from this repository: the
+agent's own image carries `vcs.revision` and `vcs.modified=true`, while the e2e
+sample image — built from `test/e2e/sample/`, a context with no `.git` — carries
+no `vcs.*` at all.
+
+**`node_metadata` gains `architecture`**, read from `status.nodeInfo.architecture`
+of a node object the agent already watches. This is part of the same decision
+rather than a separate one: `GOARCH` and `GOAMD64` are not findings on their own,
+they are one half of a comparison, and without the node's architecture the other
+half does not exist anywhere in the agent's data.
+
+## Consequences
+
+**Easier.** The build questions that need a settings block — race builds in
+production, microarchitecture left on the table, architecture mismatches, dirty
+builds — become answerable from the payloads, and the allow-list means the answer
+never cost a build-machine path. `security.md` §8 can enumerate the collected
+settings in full, in a table, because the list is finite by construction.
+
+**Harder / given up.** Teams that stamp their revision through
+`-ldflags -X main.version=…` instead of the toolchain's own VCS stamping lose it:
+that flag is dropped whole. This is the correct trade — an allow-list that makes
+an exception for one popular use of a free-form flag is not an allow-list — but
+it is a real loss, and it is the common case in projects older than Go 1.18.
+
+Adding a setting later means editing the list, `security.md` §8, and the
+end-to-end test, which restates the list independently so that widening it in the
+scanner alone cannot widen what leaves a real cluster. That friction is
+deliberate: each addition is a change to what is collected, and CLAUDE.md already
+requires those to be visible in the same PR.
+
+**Not addressed here.** Whether the backend should treat a missing `vcs.*` as a
+prompt to suggest enabling `-buildvcs` is an analysis question, outside the
+agent. Build settings for non-Go workloads do not exist — this payload is
+Go-only, as ADR 0009 established for the scanner as a whole.
+
+This ADR records a decision implemented in the same pull request, per the
+process in [`README.md`](README.md).

docs/adr/README.md

@@ -58,3 +58,4 @@ stays in place.
 - [0016. Prometheus is not a data source — not as an option, not for backfill](0016-no-prometheus-as-a-source.md)
 - [0017. Immutable build facts ship once under the image digest; snapshots say when they were taken](0017-build-facts-keyed-by-digest.md)
 - [0018. The Go inventory forgets: a record lives only while its workload does](0018-inventory-records-live-only-while-their-workload-does.md)
+- [0019. Build settings ship by allow-list, and the payload that carries them is named for the build](0019-build-settings-by-allow-list.md)

docs/backend-requirements.md

@@ -182,14 +182,32 @@ may legitimately appear in one snapshot and not the next; accumulating "has ever
 been a Go workload" across snapshots is the backend's job, and `sequence` and
 `captured_at` are what it orders them by.
 
-**`go_dependencies` is write-once and keyed by image digest.** A build's
-dependency module paths never change, so the payload is immutable given its key
-and the agent sends it once per distinct build. The backend MUST upsert it by
-image digest and MUST NOT expect it on every flush or treat a redelivery (after
-an agent restart) as a change. It carries no sequence and no `captured_at` for
-the same reason. Join it to workloads through `go_inventory.records[].image_digest`.
-It contains module *paths* only, never versions — it is not a dependency-version
-inventory and MUST NOT be presented as one.
+**`go_build` is write-once and keyed by image digest.** What a build is made of
+and how it was built — the Go version, the main module, the dependency module
+paths, and the allow-listed build settings — never changes, so the payload is
+immutable given its key and the agent sends it once per distinct build. The
+backend MUST upsert it by image digest and MUST NOT expect it on every flush or
+treat a redelivery (after an agent restart) as a change. It carries no sequence
+and no `captured_at` for the same reason. Join it to workloads through
+`go_inventory.records[].image_digest`. It contains module *paths* only, never
+versions — it is not a dependency-version inventory and MUST NOT be presented as
+one.
+
+**`go_build.settings` is a bounded allow-list, and its keys are optional**
+([ADR 0019](adr/0019-build-settings-by-allow-list.md)). The agent keeps only the
+settings enumerated in [`security.md`](security.md) §8 and discards the rest on
+the node, so the backend MUST NOT expect any particular key to be present and
+MUST NOT infer anything from an absent one. In particular `vcs.revision`,
+`vcs.time` and `vcs.modified` are recorded by the Go toolchain only when the
+build had a VCS working tree, which containerized builds frequently do not: their
+absence is the common case and MUST NOT be surfaced as missing data or as a
+coverage gap. The whole `settings` object is omitted when nothing was kept.
+
+**`node_metadata.nodes[].architecture` is what `go_build.settings.GOARCH` is
+compared against.** A build's target architecture and microarchitecture level
+mean nothing on their own; the comparison is the finding. The field is the
+kubelet's own value (`amd64`, `arm64`), and is omitted if the kubelet does not
+report one.
 
 ### Delivery semantics
 

docs/security.md

@@ -372,7 +372,7 @@ accumulating history:
   repeating the same `pod` block. It contains no pod names: replicas are
   counted, not listed.
 - **`node_metadata`** — per node: name, size, instance type, capacity type,
-  zone, region, kernel version.
+  zone, region, kernel version, CPU architecture.
 
 Both are limited to what passes your filters — a pod excluded by a namespace
 filter or an opt-out annotation is absent from the snapshot entirely, and its
@@ -447,11 +447,41 @@ applies the filter-early rule on the node, before any record is formed:
 - **Kept — customer workload binaries.** For a Go process whose main module is
   *not* infrastructure, the scanner keeps the Go version, the main module path,
   the dependency module paths (paths only — dependency *versions* are discarded
-  on the node and never leave it), the build settings that matter (notably whether
-  `-pgo` was applied), and the pod UID and container ID parsed from the process
-  cgroup. This is the same "medium sensitivity" metadata class as the rest of
-  the workload metadata above — module and version strings, never source, never
-  environment, never arguments.
+  on the node and never leave it), an allow-listed subset of the build settings
+  (below), and the pod UID and container ID parsed from the process cgroup. This
+  is the same "medium sensitivity" metadata class as the rest of the workload
+  metadata above — module and version strings, never source, never environment,
+  never arguments.
+- **Build settings — an allow-list, enumerated here in full** (ADR 0019). Build
+  settings are the one place where strings written by whoever ran the build
+  enter the agent, so the scanner keeps only these and discards everything else
+  on the node:
+
+  | Setting | What it is |
+  |---|---|
+  | `CGO_ENABLED` | `0` or `1` |
+  | `GOARCH` | target architecture |
+  | `GOAMD64`, `GOARM64`, `GOARM` | target microarchitecture level |
+  | `-race` | built with the race detector |
+  | `-trimpath` | build paths stripped from the binary |
+  | `vcs` | which VCS stamped the build (`git`, …) |
+  | `vcs.revision` | the commit the binary was built from |
+  | `vcs.time` | that commit's timestamp (not the build time — Go does not record it) |
+  | `vcs.modified` | whether the working tree was dirty |
+
+  Everything outside this list is discarded on the node, including
+  **`-ldflags`, `-gcflags`, `-asmflags` and `-tags`** — free-form flags that
+  routinely carry build-machine paths, internal hostnames and injected version
+  strings — and the **value of `-pgo`**, which is a path on the build machine.
+  Whether PGO was applied survives as a boolean; where the profile lived does
+  not. A value longer than 128 characters is dropped whole rather than
+  truncated, since every allowed setting holds a short bounded token.
+
+  `vcs.revision` is a commit identifier from your repository. It reveals no
+  source and is meaningless without access to that repository, and it is
+  frequently absent: the Go toolchain records it only when the build had a VCS
+  working tree, which containerized builds often do not. Its absence is normal
+  and is not treated as a gap.
 - **Dropped — infrastructure.** A main module on the built-in deny-list
   (`k8s.io/`, `sigs.k8s.io/`, the container runtime, the CNI, `go.etcd.io/`,
   `github.com/coredns/`, `github.com/prometheus/`, `github.com/grafana/`, … and
@@ -478,18 +508,19 @@ already collects — into two payloads:
   (ADR 0018). It also carries a `coverage` block: how many of the cluster's
   nodes have delivered a report, and how many facts were received, joined, and
   not joined. Those are cluster-wide counts, and they name nothing.
-- **`go_dependencies`** — the dependency module paths of one build, keyed by its
-  image digest (ADR 0017). One payload per distinct build, sent once, joined
-  back to workloads through the image digest in `go_inventory`. **Module paths
-  only, never versions**: the agent does not collect the version of any
-  dependency, so this is not, and cannot be used as, a vulnerability-scanning
-  feed.
+- **`go_build`** — what one build is made of and how it was built, keyed by its
+  image digest (ADR 0017, ADR 0019): the Go version, the main module, the
+  dependency module paths, and the allow-listed build settings above. One
+  payload per distinct build, sent once, joined back to workloads through the
+  image digest in `go_inventory`. **Module paths only, never versions**: the
+  agent does not collect the version of any dependency, so this is not, and
+  cannot be used as, a vulnerability-scanning feed.
 
 A fact whose pod or container the controller cannot resolve (informer lag, or a
 filtered pod) is counted as *unjoined* and dropped — its pod UID, container ID,
 and module path never appear, only the count. A fact that resolves to a
 container with no image digest yet is counted as *undigested*: its record is
-kept, its dependency set is not, because there is no build to key it to. This is
+kept, its build payload is not, because there is no build to key it to. This is
 the same "medium sensitivity" workload-metadata class as the rest of §8: module
 and version strings, never source, environment, or arguments.