From 076b819c71958cab3df379cd5d4042337ec4c6f5 Mon Sep 17 00:00:00 2001 From: Brad Edwards Date: Wed, 23 Sep 2026 19:20:31 +0000 Subject: [PATCH 1/2] fix(gcp): reject destroy trust tuples off protected dev/main gcp-dev-destroy.yml rejects every dispatch ref except refs/heads/dev and refs/heads/main before auth, but the cicd-oidc input contract accepted a destroy purpose tuple on any branch. A tenant bootstrapped with its destroy tuple on the tenant branch could never be torn down. Validate destroy tuples in both the cicd-oidc root and the identity module, and in gcp_foundation bootstrap before any command runs. A test pins the workflow guard, both Terraform validations, and the example inputs to one Python constant so they cannot drift. Correct the bootstrap README, which said destroy uses the tenant branch. --- docs/dev/deploy-secrets.md | 6 +- .../gcp/global/cicd-oidc/inventory.tf | 7 +++ .../cicd-oidc/tests/inventory.tftest.hcl | 43 +++++++++++++ .../modules/cicd-oidc-identity/inventory.tf | 7 +++ scripts/bootstrap/README.md | 8 ++- scripts/bootstrap/gcp_foundation.py | 15 +++++ .../bootstrap/tests/test_gcp_foundation.py | 62 ++++++++++++++++--- 7 files changed, 135 insertions(+), 13 deletions(-) diff --git a/docs/dev/deploy-secrets.md b/docs/dev/deploy-secrets.md index a2ed9b8c7..fa77653ba 100644 --- a/docs/dev/deploy-secrets.md +++ b/docs/dev/deploy-secrets.md @@ -185,9 +185,9 @@ The destroy workflow must be dispatched from protected `dev` (or `main`) and rejects a tenant branch such as `gcp-dev`. Configure the `-destroy` Environment branch policy and its foundation `purpose_contexts.destroy` tuple for the same protected branch; the standard -non-production choice is `dev`. A tenant-branch policy or WIF tuple leaves the -destroy job waiting at the Environment gate or rejected by the provider's exact -attribute condition. +non-production choice is `dev`. Foundation bootstrap and the `cicd-oidc` root +reject a destroy tuple on any other ref before writes. A tenant-branch +Environment policy blocks the destroy job at the Environment gate. GCP may report Cloud SQL, Memorystore, or GKE deleted before Service Networking and load-balancer dependencies observe the release. The destroy workflow retries diff --git a/platform/terraform/gcp/global/cicd-oidc/inventory.tf b/platform/terraform/gcp/global/cicd-oidc/inventory.tf index 439021ead..ed1859a61 100644 --- a/platform/terraform/gcp/global/cicd-oidc/inventory.tf +++ b/platform/terraform/gcp/global/cicd-oidc/inventory.tf @@ -61,6 +61,13 @@ variable "purpose_contexts" { ])) error_message = "Trust tuples require exact repository, Environment, branch and workflow contexts." } + validation { + condition = alltrue([ + for context in lookup(var.purpose_contexts, "destroy", []) : + contains(["refs/heads/dev", "refs/heads/main"], context.ref) && endswith(context.workflow_ref, "@${context.ref}") + ]) + error_message = "Destroy trust tuples must bind protected refs/heads/dev or refs/heads/main; gcp-dev-destroy.yml rejects every other dispatch ref." + } } variable "name_prefix" { diff --git a/platform/terraform/gcp/global/cicd-oidc/tests/inventory.tftest.hcl b/platform/terraform/gcp/global/cicd-oidc/tests/inventory.tftest.hcl index eb68632b0..6e46e8b11 100644 --- a/platform/terraform/gcp/global/cicd-oidc/tests/inventory.tftest.hcl +++ b/platform/terraform/gcp/global/cicd-oidc/tests/inventory.tftest.hcl @@ -95,3 +95,46 @@ run "case_insensitive_environment_ownership" { } expect_failures = [var.purpose_contexts] } + +# gcp-dev-destroy.yml rejects every dispatch ref except protected dev/main before +# auth, so a destroy tuple on the tenant deploy branch can never be satisfied. +run "destroy_on_protected_dev_with_tenant_deploy_branch" { + command = plan + variables { + purpose_contexts = { + deploy = [{ + environment = "customer" + ref = "refs/heads/customer" + workflow_ref = "example/product/.github/workflows/deploy.yml@refs/heads/customer" + reusable_workflow_ref = "" + }] + destroy = [{ + environment = "customer-destroy" + ref = "refs/heads/dev" + workflow_ref = "example/product/.github/workflows/gcp-dev-destroy.yml@refs/heads/dev" + reusable_workflow_ref = "" + }] + } + } +} + +run "destroy_on_tenant_branch" { + command = plan + variables { + purpose_contexts = { + deploy = [{ + environment = "customer" + ref = "refs/heads/customer" + workflow_ref = "example/product/.github/workflows/deploy.yml@refs/heads/customer" + reusable_workflow_ref = "" + }] + destroy = [{ + environment = "customer-destroy" + ref = "refs/heads/customer" + workflow_ref = "example/product/.github/workflows/gcp-dev-destroy.yml@refs/heads/customer" + reusable_workflow_ref = "" + }] + } + } + expect_failures = [var.purpose_contexts] +} diff --git a/platform/terraform/gcp/modules/cicd-oidc-identity/inventory.tf b/platform/terraform/gcp/modules/cicd-oidc-identity/inventory.tf index 895a70c69..cddbea1fb 100644 --- a/platform/terraform/gcp/modules/cicd-oidc-identity/inventory.tf +++ b/platform/terraform/gcp/modules/cicd-oidc-identity/inventory.tf @@ -65,6 +65,13 @@ variable "purpose_contexts" { ])) error_message = "Trust tuples require exact repository, Environment, branch and workflow contexts." } + validation { + condition = alltrue([ + for context in lookup(var.purpose_contexts, "destroy", []) : + contains(["refs/heads/dev", "refs/heads/main"], context.ref) && endswith(context.workflow_ref, "@${context.ref}") + ]) + error_message = "Destroy trust tuples must bind protected refs/heads/dev or refs/heads/main; gcp-dev-destroy.yml rejects every other dispatch ref." + } } variable "project_number" { diff --git a/scripts/bootstrap/README.md b/scripts/bootstrap/README.md index 9e60b1dc5..32280c3ca 100644 --- a/scripts/bootstrap/README.md +++ b/scripts/bootstrap/README.md @@ -144,8 +144,12 @@ operator-owned file outside the repository. Supply the project ID and number, numeric GitHub repository and owner IDs, bucket names, and exact purpose Environment/branch/workflow tuples. Obtain IDs with `gcloud projects describe` and `gh api repos//`; bootstrap verifies them before writes. Keep -image build and validation on protected `dev`/`main` refs. Deploy and destroy -use the selected tenant branch. No secret payload belongs in this file. +image build and validation on protected `dev`/`main` refs. Deploy uses the +selected tenant branch. Destroy must bind protected `refs/heads/dev` or +`refs/heads/main`, because `gcp-dev-destroy.yml` rejects every other dispatch +ref before authentication; bootstrap rejects any other destroy tuple before +writes. Configure the `-destroy` GitHub Environment branch policy +to allow that same protected branch. No secret payload belongs in this file. ```bash ./scripts/bootstrap/deploy.py gcp-foundation --inputs /path/to/foundation.tfvars.json --dry-run diff --git a/scripts/bootstrap/gcp_foundation.py b/scripts/bootstrap/gcp_foundation.py index 596ba3e8d..f7731eff5 100644 --- a/scripts/bootstrap/gcp_foundation.py +++ b/scripts/bootstrap/gcp_foundation.py @@ -6,6 +6,20 @@ from bootstrap_core import confirm, gcloud_resource_exists, get_repo_root, run_cmd +# gcp-dev-destroy.yml rejects every other dispatch ref before auth; the +# cicd-oidc purpose_contexts validation enforces the same set. +DESTROY_PROTECTED_REFS = ("refs/heads/dev", "refs/heads/main") + + +def _require_protected_destroy_refs(purpose_contexts: dict) -> None: + for context in purpose_contexts.get("destroy", []): + ref = context.get("ref") + if ref not in DESTROY_PROTECTED_REFS or not str(context.get("workflow_ref", "")).endswith(f"@{ref}"): + raise ValueError( + f"Destroy purpose tuple must bind {' or '.join(DESTROY_PROTECTED_REFS)} " + f"(gcp-dev-destroy.yml rejects other dispatch refs); got ref={ref!r}" + ) + def bootstrap_gcp_foundation(inputs_path: str, *, dry_run: bool = False) -> None: """Apply explicit foundation inputs before runners, image bakes, or platform.""" @@ -27,6 +41,7 @@ def bootstrap_gcp_foundation(inputs_path: str, *, dry_run: bool = False) -> None missing = sorted(required - inputs.keys()) if missing: raise ValueError("Missing foundation inputs: " + ", ".join(missing)) + _require_protected_destroy_refs(inputs["purpose_contexts"]) project = inputs["project_id"] bucket = inputs["terraform_state_bucket_name"] region = inputs.get("region", "us-central1") diff --git a/scripts/bootstrap/tests/test_gcp_foundation.py b/scripts/bootstrap/tests/test_gcp_foundation.py index fc5fd2dfe..bfa390ad7 100644 --- a/scripts/bootstrap/tests/test_gcp_foundation.py +++ b/scripts/bootstrap/tests/test_gcp_foundation.py @@ -6,13 +6,24 @@ import pytest -from bootstrap_core import set_assume_yes -from gcp_foundation import bootstrap_gcp_foundation +from bootstrap_core import get_repo_root, set_assume_yes +from gcp_foundation import DESTROY_PROTECTED_REFS, bootstrap_gcp_foundation -@pytest.fixture -def inputs(tmp_path): - path = tmp_path / "foundation.json" +def _destroy(ref, workflow_ref=None): + return { + "destroy": [ + { + "environment": "example-destroy", + "ref": ref, + "workflow_ref": workflow_ref or f"example/shifter/.github/workflows/gcp-dev-destroy.yml@{ref}", + "reusable_workflow_ref": "", + } + ] + } + + +def _write_inputs(path, purpose_contexts): path.write_text( json.dumps( { @@ -24,7 +35,7 @@ def inputs(tmp_path): "github_repo": "shifter", "github_repository_id": "456", "github_owner_id": "789", - "purpose_contexts": {}, + "purpose_contexts": purpose_contexts, "terraform_state_bucket_name": "example-project-state", "release_evidence_bucket_name": "example-project-evidence", } @@ -33,6 +44,11 @@ def inputs(tmp_path): return path +@pytest.fixture +def inputs(tmp_path): + return _write_inputs(tmp_path / "foundation.json", _destroy("refs/heads/dev")) + + def test_new_foundation_creates_backend_before_saved_plan_apply(inputs, monkeypatch): calls = [] @@ -84,8 +100,38 @@ def test_incomplete_inputs_fail_before_commands(tmp_path, monkeypatch): assert calls == [] -def test_dry_run_executes_no_commands(inputs, monkeypatch): +@pytest.mark.parametrize("ref", DESTROY_PROTECTED_REFS) +def test_dry_run_executes_no_commands(ref, tmp_path, monkeypatch): + calls = [] + monkeypatch.setattr(subprocess, "run", lambda *args, **kwargs: calls.append(args)) + bootstrap_gcp_foundation(str(_write_inputs(tmp_path / "foundation.json", _destroy(ref))), dry_run=True) + assert calls == [] + + +@pytest.mark.parametrize( + "purpose_contexts", + [ + _destroy("refs/heads/balrog"), + _destroy("refs/heads/dev", "example/shifter/.github/workflows/gcp-dev-destroy.yml@refs/heads/balrog"), + ], +) +def test_unprotected_destroy_tuple_fails_before_commands(purpose_contexts, tmp_path, monkeypatch): calls = [] monkeypatch.setattr(subprocess, "run", lambda *args, **kwargs: calls.append(args)) - bootstrap_gcp_foundation(str(inputs), dry_run=True) + with pytest.raises(ValueError, match="Destroy purpose tuple"): + bootstrap_gcp_foundation(str(_write_inputs(tmp_path / "foundation.json", purpose_contexts))) assert calls == [] + + +def test_destroy_refs_match_workflow_guard_and_terraform_contract(): + root = get_repo_root() + workflow = (root / ".github/workflows/gcp-dev-destroy.yml").read_text() + assert f"{'|'.join(DESTROY_PROTECTED_REFS)}) ;;" in workflow + allowed = "contains([" + ", ".join(f'"{ref}"' for ref in DESTROY_PROTECTED_REFS) + "], context.ref)" + for inventory in ( + "platform/terraform/gcp/global/cicd-oidc/inventory.tf", + "platform/terraform/gcp/modules/cicd-oidc-identity/inventory.tf", + ): + assert allowed in (root / inventory).read_text() + example = json.loads((root / "scripts/bootstrap/gcp-foundation.example.tfvars.json").read_text()) + assert all(context["ref"] in DESTROY_PROTECTED_REFS for context in example["purpose_contexts"]["destroy"]) From ff0defadff500a63607925f7af77cd7a3bde87a4 Mon Sep 17 00:00:00 2001 From: Brad Edwards Date: Wed, 23 Sep 2026 19:59:11 +0000 Subject: [PATCH 2/2] fix(gcp): document and type destroy ref validation helper --- scripts/bootstrap/gcp_foundation.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/bootstrap/gcp_foundation.py b/scripts/bootstrap/gcp_foundation.py index f7731eff5..7408681cb 100644 --- a/scripts/bootstrap/gcp_foundation.py +++ b/scripts/bootstrap/gcp_foundation.py @@ -11,7 +11,8 @@ DESTROY_PROTECTED_REFS = ("refs/heads/dev", "refs/heads/main") -def _require_protected_destroy_refs(purpose_contexts: dict) -> None: +def _require_protected_destroy_refs(purpose_contexts: dict[str, list[dict[str, str]]]) -> None: + """Reject destroy tuples the destroy workflow's protected-ref guard can never satisfy.""" for context in purpose_contexts.get("destroy", []): ref = context.get("ref") if ref not in DESTROY_PROTECTED_REFS or not str(context.get("workflow_ref", "")).endswith(f"@{ref}"):