DuqueOM · DuqueOM · Sep 5, 2026 · Sep 5, 2026
@@ -1,28 +1,20 @@
 # Trivy config (Terraform IaC) baseline — ADR-046.
 #
-# Same contract as every other file in this directory: each accepted finding
-# is explicit, dated and reviewable, and `scripts/check_baselines_expiry.py`
-# fails CI once an entry is past its `# expiry:`.
+# Currently EMPTY, and that is the intended steady state: no accepted
+# misconfiguration findings. An empty baseline is valid — see
+# `scripts/check_baselines_expiry.py`, which treats a file with no entries as
+# nothing to expire.
 #
-# Why the expiry lives in a comment rather than trivy's own `expiredAt:`
-# field: trivy 0.71.0 accepts `expiredAt` in the YAML ignore format and does
-# NOT honour it. Measured on this repo — an entry dated 2020-01-01 still
-# suppressed its finding, in all three date formats tried, with no warning on
-# stderr. Delegating expiry to the tool would have produced exactly the
-# failure this directory exists to prevent: a suppression that outlives its
-# justification while the gate reports green.
-
-# GKE master authorized networks.
+# It held one entry, GCP-0061 (GKE master authorized networks), inherited
+# from tfsec's google-gke-enable-master-networks. That suppression was
+# justified on the grounds that the HCL was correct and only the scanner
+# could not see it, because the block was `dynamic`. The justification also
+# claimed the invariant was "enforced by the variable validation rule in
+# variables.tf" — there was no such rule. The finding was pointing at a real
+# gap in the `enable_private_endpoint = false` path, and the module was fixed
+# rather than the finding re-suppressed.
 #
-# `master_authorized_networks_config` IS configured, as a `dynamic` block at
-# templates/service/infra/terraform/gcp/compute.tf:38, gated on
-# `var.master_authorized_networks`. Static analysis does not evaluate dynamic
-# blocks, so the finding fires against the cluster resource at compute.tf:10
-# even though the HCL is correct. Staging and prod MUST supply a non-empty
-# `master_authorized_networks`, enforced by a variable validation rule in
-# variables.tf.
-#
-# tfsec reported the same false positive as google-gke-enable-master-networks;
-# it is the one suppression of three that did NOT dissolve in the migration.
-# expiry: 2027-01-01  reason: ADR-046 — dynamic block not evaluated by static analysis
-GCP-0061
+# Contract for anything added here: explicit, dated and reviewable. Each
+# entry carries `# expiry: YYYY-MM-DD` on the line above it or inline, and
+# CI fails once past that date. Do NOT use trivy's own `expiredAt:` field —
+# 0.71.0 accepts it and does not honour it (measured; see ADR-046).
@@ -10,6 +10,43 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and [Sem
 
 ## [Unreleased]
 
+### Fixed — the GKE control plane could be public with no allowlist, and the suppression cited a control that did not exist
+
+- `master_authorized_networks_config` was a `dynamic` block gated on the list
+  being non-empty. The variable defaults to `[]` and **nothing in the repo
+  sets it** — no overlay, no tfvars — so the block never rendered, and GKE
+  with no authorized-networks block applies **no restriction**.
+- Harmless while `enable_private_endpoint = true`, which is the default. A
+  **publicly reachable control plane with no allowlist** the moment an
+  adopter takes the documented dev opt-out and sets it false.
+- `GCP-0061` — and `google-gke-enable-master-networks` before it — had been
+  suppressed for two releases as a scanner limitation, on a justification
+  that read: *"enforced by the variable validation rule in `variables.tf`"*.
+  **There is no such rule.** The finding was not a false positive; it was
+  pointing at a real gap held open by a compensating control that was never
+  built.
+- **Fixed in the module, not re-suppressed:**
+  - the block is now unconditional, so an empty list means *"enabled, no
+    external CIDR allowed"* — the restrictive reading — rather than *"not
+    configured"*;
+  - a `precondition` on `google_container_cluster.gke` rejects
+    `enable_private_endpoint = false` together with an empty list, at **plan**
+    time. It lives on the resource rather than in a `validation` block
+    because the condition spans two variables, which variable validation
+    could not do before Terraform 1.9 and this module targets `>= 1.7`.
+  - Verified across all three combinations: private endpoint passes, public
+    with CIDRs passes, public without CIDRs fails.
+- `.security-baselines/trivy-config.trivyignore` is now **empty**.
+  Suppressions across the whole repo: **zero**, and the 2027-01-01 expiry no
+  longer exists. `docs/audit/baseline-review.md` carries the third dated
+  review, including what the review procedure itself missed: reviewing a
+  suppression had meant reading its rationale, not verifying it. Step 3 now
+  says to confirm the compensating control exists in the file the
+  justification names.
+- Adopter-visible; both rows are in `MIGRATION.md`. A plan that starts
+  failing with *"public control plane must be constrained"* is the fix
+  working.
+
 ### Changed — the security check is named for what it does, not for its tools
 
 - `Self-audit (gitleaks + tfsec + checkov + trivy fs)` →

@@ -17,6 +17,17 @@ contract that prevents future versions from breaking adopters silently.
 
 ---
 
+## v0.26.0 → next (unreleased)
+
+| Change | Manual action required |
+|--------|------------------------|
+| **GKE `master_authorized_networks_config` is now always emitted** | Usually none. Previously the block was `dynamic` and rendered only when `master_authorized_networks` was non-empty; with the default empty list GKE received no authorized-networks configuration, which it reads as **no restriction**. The block is now unconditional, so an empty list means "enabled, no external CIDR allowed" — the restrictive reading. If your control plane is private (`enable_private_endpoint = true`, the default) this changes nothing you can observe. If it is public and you were relying on unrestricted access, `terraform plan` will now stop you — see the next row. |
+| **`enable_private_endpoint = false` now requires `master_authorized_networks`** | A `precondition` on `google_container_cluster.gke` fails at **plan** time if you expose the control plane publicly without listing the CIDRs allowed to reach it. If your plan starts failing with *"public control plane must be constrained"*, that is the intended behaviour and it is telling you the cluster was reachable from anywhere. Either revert to the private endpoint, or supply `master_authorized_networks` with your VPN/office/CI ranges. The old suppression's justification claimed this pairing was already "enforced by the variable validation rule in `variables.tf`"; no such rule existed (ADR-046 §Amendment). |
+| **tfsec replaced by `trivy config`** (ADR-046) | None for existing infrastructure — this is a scanner swap, not a resource change. Scaffolded services get `aquasecurity/trivy-action` in `ci-infra.yml` instead of `aquasecurity/tfsec-action`; tfsec is archived upstream. If you added your own entries to the old .security-baselines/tfsec.yml, port them to `.security-baselines/trivy-config.trivyignore` using Trivy check ids. **Do not use Trivy's `expiredAt:` field** — 0.71.0 accepts and ignores it; keep the `# expiry:` comment form the repo's own gate enforces. |
+| **`make scaffold-update` requires `TEMPLATE_REF`** | Run `make scaffold-update TEMPLATE_REF=<tag>`. A bare invocation now refuses to run instead of updating from whatever tag sorts highest — and it previously reused `REF`, which `ci-green` defaults to `main`, so it could pull the moving development branch. |
+
+---
+
 ## v0.25.0 → v0.26.0 (2026-08-08)
 
 | Change | Manual action required |

@@ -92,18 +92,60 @@ finding, silently. `scripts/check_baselines_expiry.py` remains the expiry
 authority, and it now discovers baseline files instead of naming three of
 them literally, so a future format swap cannot leave a file unwatched.
 
+## Review — 2026-09-05 (second entry, same day): the last suppression is gone
+
+Closing ADR-046 out meant re-reading the one surviving justification, and a
+clause in it did not hold:
+
+> Environment overlays MUST supply non-empty `master_authorized_networks`
+> for staging/prod (enforced by the variable validation rule in
+> `variables.tf`).
+
+There was no such rule, and nothing sets the variable anywhere — it defaults
+to `[]`. The `dynamic` block therefore never rendered, and GKE with no
+authorized-networks block applies *no restriction*. Safe while
+`enable_private_endpoint = true` (the default); a public control plane with
+no allowlist as soon as an adopter takes the documented dev opt-out.
+
+`GCP-0061` was not a false positive. It was pointing at a real gap, held open
+by a compensating control that had never been built.
+
+**Fixed in the module rather than re-suppressed:**
+
+- the authorized-networks block is now unconditional, so an empty list means
+  "enabled, no external CIDR allowed" instead of "not configured";
+- a `precondition` on `google_container_cluster.gke` rejects
+  `enable_private_endpoint = false` together with an empty list, at plan
+  time. Verified across all three combinations, including that the safe ones
+  still pass.
+
+**Suppressions in force: zero.**
+`.security-baselines/trivy-config.trivyignore` is empty, which is the
+intended steady state.
+
+### What this says about the review itself
+
+The 2026-09-04 review recorded all three as "tool limitations, not accepted
+risks" and I wrote that in good faith from the justifications on file. Two of
+the three were. The third's justification asserted an enforcement that did
+not exist, and no review had checked it — because reviewing a suppression had
+meant reading its rationale, not verifying it.
+
+Step 3 of the procedure below now says to confirm the compensating control
+exists **in the file the justification names**. That step is what would have
+caught this, and it is why it is written the way it is.
+
 ## Next review
 
-**Due 2027-01-01**, when the single remaining suppression (`GCP-0061`)
-expires and CI fails until it is re-justified or removed.
+**No suppression is due.** Both baseline files are empty of accepted
+findings, so `check_baselines_expiry.py` has nothing to expire and the
+2027-01-01 date no longer exists.
 
-It will almost certainly still be true: no static analyser evaluates
-Terraform `dynamic` blocks, so the finding is structural rather than a gap
-in one tool. The way to actually close it is to stop needing the dynamic
-block — for example by making `master_authorized_networks` a required
-variable with a non-empty default for staging and prod, so the block can be
-static. That is a module design decision, not a scanner decision, and it is
-the question the 2027-01-01 review should answer.
+The next review is therefore calendar-driven rather than deadline-driven:
+**due 2026-12-05**, one quarter out, to confirm the state is still zero. A
+review with nothing to renew is the point of the exercise, not a reason to
+skip it — the failure mode this document exists to prevent is a suppression
+nobody remembers making.
 
 ## How to run a review
 

@@ -58,6 +58,33 @@ modules. They do not block, and triaging them is separate work.
 
 **Two of three suppressions dissolve. Three become one.**
 
+### Amendment (2026-09-05): the third one dissolved too, and it was not a false positive
+
+The surviving suppression was carried on the grounds that the HCL is correct
+and only the scanner cannot see it. Re-reading its justification while
+closing this ADR out, one clause did not hold:
+
+> Environment overlays MUST supply non-empty `master_authorized_networks`
+> for staging/prod (enforced by the variable validation rule in
+> `variables.tf`).
+
+**There is no such validation rule**, and no overlay or tfvars file sets the
+variable anywhere. It defaults to `[]`, the `dynamic` block therefore never
+rendered, and GKE's default with no authorized-networks block is *no
+restriction*. Harmless while `enable_private_endpoint = true` — the default —
+and a publicly reachable control plane the moment an adopter takes the
+documented dev opt-out and sets it false.
+
+So `GCP-0061` was pointing at a real gap in that path, justified by a
+compensating control that had never been built. The module was fixed rather
+than the finding re-suppressed: the block is now unconditional (an empty list
+means "enabled, nothing allowed" instead of "not configured"), and a
+`precondition` on the cluster rejects the dangerous pairing at plan time.
+
+`.security-baselines/trivy-config.trivyignore` is now empty. **Three
+suppressions become zero**, and the 2027-01-01 review has nothing left to
+re-justify.
+
 ## Decision
 
 Replace tfsec with `trivy config` in the `Self-audit` job.

@@ -35,15 +35,30 @@ resource "google_container_cluster" "gke" {
     master_ipv4_cidr_block  = "172.16.0.0/28"
   }
 
-  dynamic "master_authorized_networks_config" {
-    for_each = length(var.master_authorized_networks) > 0 ? [1] : []
-    content {
-      dynamic "cidr_blocks" {
-        for_each = var.master_authorized_networks
-        content {
-          cidr_block   = cidr_blocks.value.cidr_block
-          display_name = cidr_blocks.value.display_name
-        }
+  # Always emitted, never conditional.
+  #
+  # This used to be a `dynamic` block gated on the list being non-empty, so
+  # with the default empty list the cluster got NO authorized-networks
+  # configuration at all — and GKE's default is then "no restriction". That
+  # is harmless while `enable_private_endpoint = true` (the default), and it
+  # is a publicly reachable control plane the moment an adopter takes the
+  # documented dev opt-out and sets it false without also supplying a list.
+  #
+  # Emitting the block unconditionally inverts the failure mode: an empty
+  # list now means "authorized networks enabled, no external CIDR allowed" —
+  # the restrictive reading rather than the permissive one. The precondition
+  # below makes the dangerous combination impossible outright.
+  #
+  # It also makes the configuration statically visible. GCP-0061 (and tfsec's
+  # google-gke-enable-master-networks before it) fired here because no static
+  # analyser evaluates `dynamic` blocks. The finding was suppressed for two
+  # releases on the grounds that the HCL was correct anyway; it was not.
+  master_authorized_networks_config {
+    dynamic "cidr_blocks" {
+      for_each = var.master_authorized_networks
+      content {
+        cidr_block   = cidr_blocks.value.cidr_block
+        display_name = cidr_blocks.value.display_name
       }
     }
   }
@@ -56,6 +71,27 @@ resource "google_container_cluster" "gke" {
   release_channel {
     channel = "REGULAR"
   }
+
+  lifecycle {
+    # The invariant the old suppression's justification claimed was "enforced
+    # by the variable validation rule in variables.tf". No such rule existed.
+    #
+    # A public control plane is acceptable only when it is constrained. This
+    # is a cross-variable condition, so it cannot live in a `validation`
+    # block on either variable — those could not reference other variables
+    # until Terraform 1.9, and this module targets >= 1.7. A resource
+    # precondition can, and it fails at plan time rather than at apply.
+    precondition {
+      condition     = var.enable_private_endpoint || length(var.master_authorized_networks) > 0
+      error_message = <<-EOT
+        enable_private_endpoint = false exposes the GKE control plane on a
+        public endpoint, so master_authorized_networks must list the CIDRs
+        allowed to reach it. Either keep the private endpoint (the default),
+        or supply master_authorized_networks. Leaving both unset would put an
+        unrestricted public control plane in front of the cluster.
+      EOT
+    }
+  }
 }
 
 # ============================================================================

@@ -149,8 +149,17 @@ variable "node_oauth_scopes" {
 
 variable "master_authorized_networks" {
   description = <<-EOT
-    CIDR blocks allowed to reach the GKE control plane. Empty list = no
-    public access (only relevant when enable_private_endpoint=false).
+    CIDR blocks allowed to reach the GKE control plane.
+
+    The authorized-networks block is always emitted, so an empty list means
+    "enabled, no external CIDR allowed" — the restrictive reading. It used to
+    mean the block was omitted entirely, which GKE reads as no restriction at
+    all.
+
+    REQUIRED when enable_private_endpoint = false: a public control plane has
+    to be constrained. That pairing is enforced by a precondition on
+    google_container_cluster.gke and fails at plan time.
+
     Format: list of objects with cidr_block + display_name.
   EOT
   type = list(object({