test: the chart's absences are asserted, not reviewed

charts/runtime-agent/templates/controller-configmap.yaml

@@ -4,7 +4,7 @@
 # told to do.
 #
 # The agent parses it strictly: a key it does not know is a startup error rather
-# than a setting silently ignored. charts/runtime-agent/render_test.go feeds this
+# than a setting silently ignored. internal/chartrender/chart_test.go feeds this
 # rendered text to the agent's own parser, so a chart that would fail at startup
 # fails in CI instead.
 #

charts/runtime-agent/templates/controller-rbac.yaml

@@ -14,9 +14,14 @@ imagePullSecrets:
 ---
 # The controller's access is read-only. Every rule here is get/list/watch, and
 # there is no create, update, patch, delete or deletecollection anywhere — a
-# property docs/security.md §1 promises and charts/runtime-agent/render_test.go
+# property docs/security.md §1 promises and internal/chartrender/chart_test.go
 # enforces mechanically rather than by review.
 #
+# This list is closed, not a floor: internal/chartrender/guardrail_test.go names
+# the resources permitted here and fails on anything else, so a word added to a
+# rule below is a red build rather than a quiet widening of what the agent can
+# read.
+#
 # The two nonResourceURL GETs are OIDC discovery and JWKS, used to validate node
 # tokens locally (ADR 0010). They are reads, and specifically NOT
 # `create tokenreviews`.

docs/development.md

@@ -74,6 +74,15 @@ A PR is mergeable when:
   emptyDir. It also feeds the rendered configuration to the agent's own strict
   parser, so a chart that would CrashLoop at startup fails here instead
   (ADR 0036). `make chart-lint` adds the helm CLI's own opinion, per profile.
+- **And the absences are asserted too.** A test that enumerates what must be
+  present is indifferent to an addition standing next to it: `secrets` added to
+  an existing read-only rule, `hostPID` on the controller, a capability added
+  beside `drop: [ALL]`, a privileged init container — all five measured passing
+  the whole suite green before `internal/chartrender/guardrail_test.go` existed.
+  It names the complete permitted set instead, per rendered pod, so an addition
+  has to be argued for in a diff that changes a test. When you widen the chart,
+  expect to widen a list there; that is the mechanism working, not a test in the
+  way.
 - **End-to-end on kind installs the chart.** The agent runs against a local kind
   cluster with synthetic workloads; assertions are made on the spool contents,
   not on logs. The suites install `charts/runtime-agent` rather than a copy of

docs/security.md

@@ -74,6 +74,11 @@ runtime-agent node         # DaemonSet (optional) — eBPF profiling and Go bina
   stable name, and a StatefulSet would hold a replacement back while a node is
   unreachable, stopping collection cluster-wide (ADR 0026).
 - The node role sends data to the controller over the cluster network only.
+- The host boundary runs between the two roles, and only the node role crosses
+  it. The controller shares no namespace with the node it lands on, mounts no
+  host path, and adds no capability — asserted per rendered pod in
+  `internal/chartrender/guardrail_test.go`, so §7's blast-radius paragraphs stay
+  paragraphs about the DaemonSet.
 - The controller exposes an in-cluster-only HTTP receiver for those node
   reports (ADR 0010): it accepts data pushes from the node DaemonSet and
   answers ack/error only. It is not reachable from outside the cluster and
@@ -948,7 +953,10 @@ now say which one they are (ADR 0039).
 
 - No `secrets`, no `configmaps` — no RBAC for either exists anywhere, so the
   controller cannot read one through the API server, and the API server is what
-  enforces that rather than agent configuration. This is a statement about the
+  enforces that rather than agent configuration. The ClusterRole's resource list
+  is closed rather than merely reviewed: `internal/chartrender/guardrail_test.go`
+  names every resource the chart may request and fails on anything else, in every
+  install profile. This is a statement about the
   API path only: secret *contents* remain reachable by the controller's identity
   through `nodes/proxy` (below), and on a node by the DaemonSet's `/proc` access
   ([§7.1](#71-the-go-binary-scanner-the-current-node-role)). What keeps them out

internal/chartrender/chart_test.go

@@ -220,23 +220,22 @@ func TestNoContainerIsPrivilegedAndCapabilitiesAreExactlyWhatIsPromised(t *testi
 	}
 }
 
-// No host path is ever writable, in any profile. This is the difference between
-// reading a node and being able to change one.
+// No host path is ever writable, in any profile, in any pod the chart renders.
+// This is the difference between reading a node and being able to change one.
 func TestEveryHostMountIsReadOnly(t *testing.T) {
 	for name, values := range profiles() {
 		t.Run(name, func(t *testing.T) {
-			docs := render(t, values)
-			for _, ds := range decode[appsv1.DaemonSet](t, docs, "DaemonSet") {
+			for _, pod := range allPodSpecs(t, render(t, values)) {
 				hostPaths := map[string]bool{}
-				for _, volume := range ds.Spec.Template.Spec.Volumes {
+				for _, volume := range pod.spec.Volumes {
 					if volume.HostPath != nil {
 						hostPaths[volume.Name] = true
 					}
 				}
-				for _, container := range ds.Spec.Template.Spec.Containers {
+				for _, container := range pod.containers() {
 					for _, mount := range container.VolumeMounts {
 						if hostPaths[mount.Name] && !mount.ReadOnly {
-							t.Errorf("host mount %q is writable in %q", mount.Name, container.Name)
+							t.Errorf("%s: host mount %q is writable in %q", pod, mount.Name, container.Name)
 						}
 					}
 				}
@@ -566,14 +565,43 @@ func loadControllerConfig(t *testing.T, cm corev1.ConfigMap) config.Config {
 	return cfg
 }
 
-func allContainers(t *testing.T, docs []string) []corev1.Container {
+// podSpec is one rendered pod template with enough identity to name it in a
+// failure. Every workload the chart renders is one of these, so a test written
+// against allPodSpecs cannot miss a component the chart grows later.
+type podSpec struct {
+	kind string // Deployment or DaemonSet
+	name string
+	spec corev1.PodSpec
+}
+
+func (p podSpec) String() string { return p.kind + " " + p.name }
+
+func allPodSpecs(t *testing.T, docs []string) []podSpec {
 	t.Helper()
-	var out []corev1.Container
+	var out []podSpec
 	for _, deploy := range decode[appsv1.Deployment](t, docs, "Deployment") {
-		out = append(out, deploy.Spec.Template.Spec.Containers...)
+		out = append(out, podSpec{"Deployment", deploy.Name, deploy.Spec.Template.Spec})
 	}
 	for _, ds := range decode[appsv1.DaemonSet](t, docs, "DaemonSet") {
-		out = append(out, ds.Spec.Template.Spec.Containers...)
+		out = append(out, podSpec{"DaemonSet", ds.Name, ds.Spec.Template.Spec})
+	}
+	return out
+}
+
+// containers returns the init containers as well as the regular ones. An init
+// container runs with its own securityContext and can hold privileges the
+// containers beside it do not, so a check that walks only Spec.Containers is a
+// check with a hole in it. The chart renders none today; this is what makes the
+// first one that appears subject to the same assertions as everything else.
+func (p podSpec) containers() []corev1.Container {
+	return append(append([]corev1.Container{}, p.spec.InitContainers...), p.spec.Containers...)
+}
+
+func allContainers(t *testing.T, docs []string) []corev1.Container {
+	t.Helper()
+	var out []corev1.Container
+	for _, p := range allPodSpecs(t, docs) {
+		out = append(out, p.containers()...)
 	}
 	return out
 }

internal/chartrender/guardrail_test.go

@@ -0,0 +1,247 @@
+package chartrender_test
+
+import (
+	"sort"
+	"strings"
+	"testing"
+
+	rbacv1 "k8s.io/api/rbac/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/yaml"
+)
+
+// What this file is for.
+//
+// chart_test.go asserts that the chart contains what it must: every grant the
+// agent's informers need, the capabilities the profiler asks for, the policy on
+// the receiver's port. Those tests enumerate what is *present*, and a test that
+// enumerates presence is indifferent to an addition standing next to it.
+//
+// The promises this product makes are mostly about absence. "No secrets, no
+// configmaps" (docs/security.md §9), "the controller is an ordinary workload"
+// (§7), "no host access outside the node role" (§2). None of those survives a
+// presence test, and each was measured passing a whole green suite:
+//
+//	resources: [pods, namespaces, nodes, secrets, configmaps]   PASS
+//	hostPID: true, hostNetwork: true on the controller          PASS
+//	capabilities.add: [NET_ADMIN, SYS_ADMIN] on the controller  PASS
+//	a privileged initContainer in the Deployment                PASS
+//	hostPath: / mounted writable into the controller            PASS
+//
+// So the tests here are written the other way round: they name the complete
+// permitted set and fail on anything outside it. An addition has to be argued
+// for in a diff that changes a test, which is the point — the failure mode these
+// guard against is not someone writing a wrong rule, it is a right-looking word
+// added to a right rule and nobody noticing.
+
+// The controller's grant is a closed list, not a floor.
+// TestTheClusterRoleGrantsEveryCacheTheAgentOpens checks that nothing needed is
+// missing; this checks that nothing else is there. Both are necessary: one word
+// added to an existing rule keeps every verb read-only, keeps every required
+// grant in place, and quietly breaks the promise of docs/security.md §9.
+//
+// The permitted set is spelled out here rather than derived from the chart,
+// because a test that reads its expectation from the thing it is testing asserts
+// nothing.
+func TestTheClusterRoleNamesNothingBeyondThisList(t *testing.T) {
+	permitted := map[string][]string{
+		"":                  {"pods", "namespaces", "nodes", "nodes/proxy", "limitranges", "resourcequotas", "persistentvolumeclaims"},
+		"apps":              {"replicasets", "deployments", "statefulsets", "daemonsets"},
+		"batch":             {"jobs", "cronjobs"},
+		"policy":            {"poddisruptionbudgets"},
+		"autoscaling":       {"horizontalpodautoscalers"},
+		"scheduling.k8s.io": {"priorityclasses"},
+		"storage.k8s.io":    {"storageclasses"},
+	}
+	want := map[string]bool{}
+	for group, resources := range permitted {
+		for _, resource := range resources {
+			want[group+"/"+resource] = true
+		}
+	}
+
+	for name, values := range profiles() {
+		t.Run(name, func(t *testing.T) {
+			role := only(t, decode[rbacv1.ClusterRole](t, render(t, values), "ClusterRole"), "ClusterRole")
+			for _, rule := range role.Rules {
+				for _, group := range rule.APIGroups {
+					for _, resource := range rule.Resources {
+						if !want[group+"/"+resource] {
+							t.Errorf("the ClusterRole names %q in apiGroup %q, which is not on the permitted list; "+
+								"the agent opens no informer on it and docs/security.md §9 says it is not requested",
+								resource, group)
+						}
+					}
+				}
+			}
+		})
+	}
+}
+
+// The two non-resource GETs are OIDC discovery and JWKS, and they exist to
+// validate node tokens locally instead of calling TokenReview (ADR 0010). A
+// third URL would be a capability nothing in the agent uses, and the profile
+// with no node must ask for neither.
+func TestTheOnlyNonResourceURLsAreTheOnesNodeTokensNeed(t *testing.T) {
+	oidc := []string{"/.well-known/openid-configuration", "/openid/v1/jwks"}
+	cases := map[string][]string{
+		"metrics-only": nil, // no node, no token to validate, no reason to ask
+		"inventory":    oidc,
+		"ebpf":         oidc,
+	}
+	for name, want := range cases {
+		t.Run(name, func(t *testing.T) {
+			role := only(t, decode[rbacv1.ClusterRole](t, render(t, profiles()[name]), "ClusterRole"), "ClusterRole")
+			var got []string
+			for _, rule := range role.Rules {
+				got = append(got, rule.NonResourceURLs...)
+			}
+			sort.Strings(got)
+			if strings.Join(got, " ") != strings.Join(want, " ") {
+				t.Errorf("nonResourceURLs = %v, want exactly %v", got, want)
+			}
+		})
+	}
+}
+
+// docs/security.md §2 draws the boundary between the two roles at the host: the
+// node role reaches it, the controller is an ordinary workload that could run
+// anywhere. hostPID is what the scanner needs to see other containers' /proc
+// (ADR 0009), so it is asserted in both directions — absent from the controller,
+// and present on the node, because losing it there makes the scanner collect
+// nothing and say nothing about why.
+//
+// hostNetwork and hostIPC are on no pod in any profile. Neither has ever been
+// needed, and hostNetwork in particular would put the receiver's port on the
+// node's address, where the NetworkPolicy that restricts it does not reach.
+func TestOnlyTheNodeSharesTheHostsNamespaces(t *testing.T) {
+	for name, values := range profiles() {
+		t.Run(name, func(t *testing.T) {
+			for _, pod := range allPodSpecs(t, render(t, values)) {
+				if pod.spec.HostNetwork {
+					t.Errorf("%s sets hostNetwork; no component of this agent uses the node's network namespace", pod)
+				}
+				if pod.spec.HostIPC {
+					t.Errorf("%s sets hostIPC; no component of this agent uses the node's IPC namespace", pod)
+				}
+				switch pod.kind {
+				case "Deployment":
+					if pod.spec.HostPID {
+						t.Errorf("%s sets hostPID; the controller reads the API server, not the node (docs/security.md §2)", pod)
+					}
+				case "DaemonSet":
+					if !pod.spec.HostPID {
+						t.Errorf("%s does not set hostPID; without it the scanner sees only its own /proc and collects nothing (ADR 0009)", pod)
+					}
+				}
+			}
+		})
+	}
+}
+
+// docs/security.md §7 promises the added capabilities belong to the node role
+// alone. TestNoContainerIsPrivilegedAndCapabilitiesAreExactlyWhatIsPromised
+// pins the node's set exactly; every other container must add nothing at all.
+//
+// `drop: [ALL]` does not cover this, and the reason it looks like it should is
+// what makes the hole worth a test. The runtime applies the drop and then the
+// add, so the add wins. Measured on Kubernetes 1.36, one container with
+// `drop: [ALL]` and `add: [SYS_ADMIN, NET_ADMIN]`:
+//
+//	CapEff: 0000000000201000   — bits 21 and 12, exactly the two added
+//
+// A securityContext that reads as careful, holding CAP_SYS_ADMIN.
+func TestOnlyTheNodeContainerAddsACapability(t *testing.T) {
+	for name, values := range profiles() {
+		t.Run(name, func(t *testing.T) {
+			for _, pod := range allPodSpecs(t, render(t, values)) {
+				if pod.kind == "DaemonSet" {
+					continue // its exact set is asserted in chart_test.go
+				}
+				for _, container := range pod.containers() {
+					sc := container.SecurityContext
+					if sc == nil || sc.Capabilities == nil {
+						continue // the privileged/drop-ALL assertions cover an absent context
+					}
+					if len(sc.Capabilities.Add) != 0 {
+						t.Errorf("%s: container %q adds %v; only the node role adds capabilities (docs/security.md §7)",
+							pod, container.Name, sc.Capabilities.Add)
+					}
+				}
+			}
+		})
+	}
+}
+
+// The controller touches no part of the node it runs on. TestEveryHostMountIsReadOnly
+// bounds how a host path may be mounted; this says the controller may not mount
+// one at all, read-only included — a read-only mount of the host filesystem is
+// still a read of the host filesystem, and §7's blast-radius paragraphs are
+// written about the DaemonSet on the understanding that the controller has no
+// such reach.
+//
+// The same goes for the volume types that read the cluster around the pod: the
+// controller's config comes from its own ConfigMap and nothing else, and a
+// projected serviceAccountToken belongs to the node pod, which is the side that
+// authenticates (ADR 0010).
+func TestTheControllerReachesNothingOutsideItsOwnPod(t *testing.T) {
+	for name, values := range profiles() {
+		t.Run(name, func(t *testing.T) {
+			for _, pod := range allPodSpecs(t, render(t, values)) {
+				if pod.kind != "Deployment" {
+					continue
+				}
+				for _, volume := range pod.spec.Volumes {
+					switch {
+					case volume.HostPath != nil:
+						t.Errorf("%s mounts host path %q; the controller reads the API server, not a node (docs/security.md §7)",
+							pod, volume.HostPath.Path)
+					case volume.Secret != nil:
+						t.Errorf("%s mounts Secret %q; the agent reads no Secret (docs/security.md §9)",
+							pod, volume.Secret.SecretName)
+					case volume.Projected != nil:
+						t.Errorf("%s mounts a projected volume; the token side of the node channel is the node's (ADR 0010)", pod)
+					}
+				}
+			}
+		})
+	}
+}
+
+// A guard on the guards. Every assertion above is written per pod spec, so it is
+// only as complete as allPodSpecs, and allPodSpecs knows the two workload kinds
+// the chart renders today. If a third appears — a Job, a CronJob, a second
+// Deployment — it would be silently exempt from all of them.
+//
+// A rendered kind is either a workload this file must walk or it is not one, and
+// that is a decision to make deliberately rather than discover later.
+func TestNoWorkloadKindEscapesThePodSpecWalk(t *testing.T) {
+	walked := map[string]bool{"Deployment": true, "DaemonSet": true}
+	notWorkloads := map[string]bool{
+		"ServiceAccount": true, "ClusterRole": true, "ClusterRoleBinding": true,
+		"ConfigMap": true, "Service": true, "NetworkPolicy": true,
+	}
+	for name, values := range profiles() {
+		t.Run(name, func(t *testing.T) {
+			for _, doc := range render(t, values) {
+				kind := kindOf(t, doc)
+				if !walked[kind] && !notWorkloads[kind] {
+					t.Errorf("the chart renders a %s, which no guardrail in this file walks; "+
+						"add it to allPodSpecs if it carries a pod, or to this test's list if it does not", kind)
+				}
+			}
+		})
+	}
+}
+
+func kindOf(t *testing.T, doc string) string {
+	t.Helper()
+	var meta metav1.TypeMeta
+	if err := yaml.Unmarshal([]byte(doc), &meta); err != nil {
+		t.Fatalf("decoding TypeMeta: %v\n%s", err, doc)
+	}
+	if meta.Kind == "" {
+		t.Fatalf("document has no kind:\n%s", doc)
+	}
+	return meta.Kind
+}