fix(infra): GKE control plane could be public with no allowlist - #93
Merged
Conversation
Closing out ADR-046 meant re-reading the justification for the one surviving
suppression, GCP-0061. 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 nothing in the repo sets the variable —
no overlay, no tfvars. It defaults to [], so the `dynamic`
master_authorized_networks_config block never rendered, and GKE with no
authorized-networks block applies NO restriction.
That is harmless while enable_private_endpoint = true, which is the default.
It is a publicly reachable control plane with no allowlist the moment an
adopter takes the documented dev opt-out and sets it false.
So the finding was not a false positive. It was pointing at a real gap, held
open for two releases by a compensating control that had never been built.
Fixed in the module rather than re-suppressed:
- The block is now unconditional. An empty list means "enabled, no external
CIDR allowed" — the restrictive reading — instead of "not configured",
which GKE reads permissively. This also makes the configuration
statically visible, which is why GCP-0061 clears.
- A precondition on google_container_cluster.gke rejects
enable_private_endpoint = false together with an empty list, at plan time
rather than at apply. It lives on the resource rather than in a
`validation` block because the condition spans two variables — variable
validation could not reference other variables until Terraform 1.9, and
this module targets >= 1.7.
Verified across all three combinations in an isolated config: private
endpoint passes, public with CIDRs passes, public without CIDRs fails with
the intended message. terraform validate and fmt clean; trivy config reports
0 HIGH/CRITICAL across all five root modules with no ignore file at all.
.security-baselines/trivy-config.trivyignore is now empty. Suppressions
across the 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 — the step that would have
caught this a review earlier.
Adopter-visible, both rows in MIGRATION.md. A plan that starts failing with
"public control plane must be constrained" is the fix working.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You asked for the module change that eliminates
GCP-0061. Doing it turned up that the finding was not a false positive.What the justification claimed, and what was there
The suppression — inherited from tfsec's
google-gke-enable-master-networksand carried for two releases — rested on this:There is no such validation rule.
variables.tfhas threevalidationblocks —network_mode,log_retention_days, one more — and none for this variable. And nothing in the repo sets it: no overlay, no tfvars. It defaults to[].So the
dynamicblock, gated onlength(...) > 0, never rendered. And GKE with no authorized-networks block applies no restriction.That is harmless while
enable_private_endpoint = true— the default. It is a publicly reachable control plane with no allowlist the moment an adopter takes the documented dev opt-out:Rely on a variable that defaults to empty, with nothing enforcing otherwise.
The fix, in the module
1. The block is unconditional now. An empty list means "enabled, no external CIDR allowed" — the restrictive reading — instead of "not configured", which GKE reads permissively. This inverts the failure mode, and as a side effect makes the configuration statically visible, which is why
GCP-0061clears.2. A
preconditionmakes the dangerous pairing impossible:It lives on the resource rather than in a
validationblock because the condition spans two variables — variable validation could not reference other variables until Terraform 1.9, and this module targets>= 1.7. It fails at plan time, not apply.Verified, not assumed
The precondition logic, exercised in an isolated config across all three combinations:
enable_private_endpointmaster_authorized_networkstrue(default)[]false[]falseSuppressions across the repo: zero
.security-baselines/trivy-config.trivyignoreis now empty, which its header documents as the intended steady state. The 2027-01-01 expiry no longer exists — there is nothing left to re-justify.Three suppressions → one (#90) → none.
What this says about the review process
docs/audit/baseline-review.mdcarries a third dated review, and it is candid about the miss. The 2026-09-04 review recorded all three as "tool limitations, not accepted risks". Two of them were. The third's justification asserted an enforcement that did not exist, and no review caught it — because reviewing a suppression had meant reading its rationale, not verifying it.Step 3 of the review procedure now reads: confirm the compensating control still exists, in the file the justification names. That is the step that would have caught this a review earlier, and it is why it is worded that way.
Adopter impact
Both rows are in
MIGRATION.md. For anyone on the default private endpoint, nothing observable changes. For anyone who took the dev opt-out,terraform planwill now stop them — and that failure is the fix working, not a regression. ADR-046 is amended with the finding rather than rewritten.🤖 Generated with Claude Code