fix(infra): GKE nodes ran as the default Compute Engine service account

[DuqueOM]

The MEDIUM triage ADR-046 deferred as "separate work". Three of the five findings were real, and two of them were security defects in the module.

GCP-0050 ×2 — nodes ran as the default Compute Engine service account

Neither node pool set node_config.service_account, so GKE fell back to the default Compute Engine service account — typically roles/editor across the whole project.

That is D-31 violated inside the module that implements D-31's other five identities (ci, deploy, runtime, drift, retrain). The identity split covered everything except the machines.

Bounded rather than harmless, and worth being precise: Workload Identity means pods impersonate runtime/drift/retrain, so this is what an attacker inherits from a compromised node, not from a compromised pod.

A sixth identity, nodes, now carries Google's documented minimum — logging.logWriter, monitoring.metricWriter, monitoring.viewer, stackdriver.resourceMetadata.writer — plus artifactregistry.reader scoped to the repository rather than granted project-wide.

A second-order catch: node_oauth_scopes defaulted to logging + monitoring only. A scope is an upper bound on what a service account may do, so the new Artifact Registry grant would have been unusable and private image pulls would fail. Added devstorage.read_only — the narrow scope, not cloud-platform, which the variable's own description tells you to avoid. Granting a role the scopes cannot reach would have looked like hardening and done nothing.

GCP-0011 — CI could impersonate every service account in the project

resource "google_project_iam_member" "ci_sa_user" {
  # Scoped via condition (only acting on SAs in this project).
  role   = "roles/iam.serviceAccountUser"

There is no condition block. The grant was project-wide and unconditional, so the CI identity could act as runtime, drift and retrain — exactly the blast radius ADR-017 exists to bound.

Replaced with three per-account bindings: deploy, runtime, and nodes (attaching a service account to a node pool requires serviceAccountUser on it). drift and retrain are reached by workloads through Workload Identity, never by CI, and are deliberately absent rather than carried over.

The two that were accepted, not fixed

GCP-0078 and AWS-0090 are object versioning on the access-log buckets, GCP and AWS. Append-only sinks, already lifecycle-bounded. Versioning guards against overwrite and modification of existing objects — neither is a failure mode for a log sink, and it multiplies cost on the highest-volume bucket in the project. The buckets holding state, models and MLflow artifacts do have versioning, which is where the control matters.

Baselined with a dated justification (2027-03-05) and a review question written into the entry: are those buckets still access-log-only? The acceptance rests entirely on that, so it is a property to verify in storage.tf, not a rationale to re-read.

This is the distinction the previous review learned to make. The first two are defects; these are an accepted risk where the check is correct in general and does not apply to this object lifecycle. Only the third kind belongs in a baseline.

The gate now runs at MEDIUM

	Before	After
Template repo	CRITICAL,HIGH	CRITICAL,HIGH,MEDIUM
Scaffolded service	CRITICAL,HIGH	CRITICAL,HIGH — deliberately

At HIGH those five findings were visible in the log and enforced by nothing, which is the same as not looking.

The service stays at HIGH on purpose: the higher bar is affordable here because this repo has .security-baselines/ plus an expiry gate, so an accepted finding can be recorded, dated and reviewed. A generated service has neither, and a MEDIUM gate with nowhere to write down an acceptance trains adopters to delete the step — which costs more than the two findings it would catch.

LOW stays out. Eight findings whose triage has not been done, and an unenforced threshold is honest about that in a way a suppressed finding is not.

Third instance of the same pattern

GCP-0011 makes three this session — a comment asserting a control that was never built, surviving because reviewing meant reading it:

1. the tfsec suppression cited a variables.tf validation rule — none existed (fix(infra): GKE control plane could be public with no allowlist #93);
2. check_baselines_expiry.py claimed to read exclude: blocks — it filtered on uppercase ids and saw none (fix(security,docs): the baseline gate saw no entries, and Art. 11 cited a missing index #88);
3. ci_sa_user claimed an IAM condition — the block was absent.

A comment asserting that a control exists is a checkable claim, in exactly the way a path is. Nothing here verifies that class of claim. docs/audit/baseline-review.md records it as the open structural gap, and it is a bigger finding than any of the three.

Adopter impact — read this one

service_account is replace-forcing. Terraform will propose recreating both node pools; workloads reschedule. MIGRATION.md says to read the plan, do it in a window, and confirm PodDisruptionBudgets (D-27) are in place first. Two further rows cover the scope change and the IAM move.

Verification

$ trivy config templates/service/infra/terraform          # before
  {'LOW': 8, 'MEDIUM': 5, 'HIGH': 0}
$ trivy config templates/service/infra/terraform          # after the two fixes
  {'LOW': 8, 'MEDIUM': 2}

$ # exactly what CI will run
$ trivy config ... --severity CRITICAL,HIGH,MEDIUM \
    --ignorefile .security-baselines/trivy-config.trivyignore --exit-code 1
  exit=0

$ terraform validate  →  Success!   ;   terraform fmt -check  →  clean
$ pytest templates/service/tests/test_iam_least_privilege.py  →  11 passed
$ make verify  →  ✓ every gate green

ADR-017 and D-31 are amended to six identities; ADR-046's MEDIUM count is corrected from 4 to 5 — it was mine to get right.

🤖 Generated with Claude Code