sec(ci): destroy and rollback workflows have no environment binding, so no reviewer gate applies

[cristim]

Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code, since a finding may have been fixed or moved.

Where

• .github/workflows/cleanup-staging.yml:47, :117, :187, :252 - four terraform destroy -auto-approve jobs, none declaring environment:
• .github/workflows/destroy-fargate-dev.yml:38 - no environment:
• .github/workflows/rollback.yml:45 (validate) and :381 (summary) - no environment:
• Contrast: .github/workflows/deploy-aws-lambda.yml:124 and database-migration.yml:170,246,313 do declare environment:
• terraform/environments/aws/ci-cd-permissions/role.tf:28-33 - trust policy accepts repo:<org/repo>:ref:refs/heads/main
• Typed-string guards: cleanup-staging.yml:41, destroy-fargate-dev.yml:33
• cleanup-staging.yml:228-232 - the Azure job breaks the state lease before destroying

What

The repo's destructive workflows have no GitHub Environment binding, so none of the protection that Environments provide - required reviewers, wait timers, branch restrictions - ever applies to them. The only guard is a typed-string confirmation in a preceding job in the same run, which the same person who dispatched the workflow types. A second human is never involved.

This is not mitigated by the OIDC trust policy, because the AWS deploy role's trust policy accepts ref:refs/heads/main independently of the environment:{dev,staging,prod} subjects. A job with no environment: binding running on main can still mint an OIDC token and assume cudly-terraform-deploy, the full deploy role. The environment approval gate is therefore architecturally bypassable, not merely unconfigured on these workflows.

Failure scenario

Any repository collaborator with write access dispatches Cleanup Staging Resources from main, types destroy into the confirmation input, and four terraform destroy -auto-approve runs delete the entire staging estate across AWS, Azure and GCP. The Azure job first breaks the state lease (cleanup-staging.yml:228-232), so a concurrent apply loses its lock as well. No second human is ever consulted, no reviewer is notified, and there is no wait timer in which to cancel. The same shape applies to destroy-fargate-dev.yml and to rollback.yml's unbound validate / summary jobs, which additionally hold id-token: write (see #1542).

Fix direction

Two changes, and both are needed - either alone leaves the gate bypassable:

1. Bind every destroy / rollback job to a protected GitHub Environment with required reviewers.
2. Remove repo:...:ref:refs/heads/main from the AWS role's sub allow-list in terraform/environments/aws/ci-cd-permissions/role.tf so deploy credentials are obtainable only through an environment-gated job. Note the bootstrap/runtime split: ci-cd-permissions/ is applied manually by a privileged human, so this change needs a bootstrap re-apply, not a workflow apply.

Related

• sec(ci): rollback.yml interpolates the free-text reason input into run blocks in a job holding id-token write #1542 - the rollback.yml reason injection whose escalation path depends on exactly this missing binding on the validate job.
• ci: prod down migration with steps>=total bypasses confirmation gate (down -all equivalent) cloud-commitments-platform#68 (ci: prod down migration with steps>=total bypasses confirmation gate) - the same "typed-string guard is the only gate" pattern in database-migration.yml.

[cristim]

Partial resolution for the rollback.yml portion: PR #1641 (fixing #1542) drops id-token: write from workflow level to job level, granting it only to the four environment-bound rollback-* jobs. It also deletes the verify-image job, which authenticated to AWS/GCP/Azure with no environment: binding and was the workflow's real gate bypass.

So validate (:45) and summary (:381) still have no environment: binding, but they now hold only contents: read — with no id-token: write there is no OIDC token for them to mint, so the missing binding on those two is no longer a privilege issue.

Untouched by #1641 and still open here: cleanup-staging.yml:47,117,187,252, destroy-fargate-dev.yml:38, and the repo:<org/repo>:ref:refs/heads/main subject in role.tf:28-33 (which cannot be removed until those jobs get environment bindings — #1641's PR body documents the dependency).

Related new finding: the <cloud>-<env>-rollback environment names are not in the trust policy's sub allowlist at all, filed as LeanerCloud/cloud-commitments-platform#139.

[cristim]

PR #1674 adds the environment binding this issue asks for. It does not add a reviewer gate, and this issue closing should not be read as "destroy is now gated."

Stating that plainly because the title's framing invites the wrong conclusion. environment: buys OIDC/secret scoping and nothing more until protection rules exist. Re-confirmed against the live API today:

$ gh api repos/LeanerCloud/CUDly/environments
total_count: 5
aws-fargate-dev: protection_rules=[] | aws-fargate-staging: [] | azure-: [] | dev: [] | gcp-: []

No environment in this repo has any protection rules, and staging does not exist at all. GitHub auto-creates a referenced environment bare on first use, so the first cleanup-staging dispatch mints staging with no reviewers and runs straight through. A binding alone, on these workflows, is exactly the reassuring-looking control that is not one.

Which half is code and which is manual

Code (PR #1674, done): environment: staging on the four cleanup-staging destroy jobs and environment: dev on destroy-fargate-dev; id-token: write dropped from workflow level to only the five jobs that assume a cloud role; permissions: {} on both confirmation guards; per-environment Azure federated credentials; the last ${{ }} removed from a run: block.

Manual — a privileged human, in repo settings or via the API. None of it is code:

1. Create the staging environment. dev exists. Not declarable in this repo today: there is no github_repository_environment resource anywhere under terraform/ or iac/, and no GitHub Terraform provider is configured. Adding that provider belongs to sec(ci): deployment environments have no protection rules, so environment-bound credentialed jobs run unapproved cloud-commitments-platform#141, not to sec(ci): bind destroy jobs to an environment and narrow id-token on destroy workflows #1674.
2. Required reviewers on dev and staging — the actual gate. Without this, nothing in sec(ci): bind destroy jobs to an environment and narrow id-token on destroy workflows #1674 stops anything.
3. Deployment branch policy — custom_branch_policies with pattern main. Load-bearing, see below.
4. Wait timer on staging if a cancellation window is wanted.
5. can_admins_bypass = false, at least on anything production-facing.
6. Re-apply terraform/environments/azure/ci-cd-permissions (bootstrap-only per CLAUDE.md). Until this runs, the Azure destroy job fails with AADSTS70021 — pre-existing per fix(iac/aws): rollback environments are absent from the OIDC trust allowlist, so AWS/Azure rollback cannot authenticate cloud-commitments-platform#139, and sec(ci): bind destroy jobs to an environment and narrow id-token on destroy workflows #1674 knowingly merges it in that state.

Item 3 is not optional

An OIDC sub is either …:ref:refs/heads/<branch> or …:environment:<name>, never both. So the binding replaces the ref-scoped subject with a ref-agnostic one, and since neither environment carries a branch policy, the binding by itself trades main-only for any-branch — a widening, on the workflows this exists to narrow.

#1674 adds a Restrict to main step, but it is defense-in-depth against accidents and not a security boundary: workflow_dispatch runs the workflow file as it exists on the dispatched ref, so anyone who can push a branch can delete the step and still present the environment:<name> subject. Only the deployment branch policy survives that, because GitHub evaluates it before the job starts and before the token is minted. It does not require main to be a protected branch — custom_branch_policies takes an explicit pattern and works on an unprotected one.

Scope correction

rollback.yml is named in this issue but needs no change: PR #1641 already bound it. Workflow level is contents: read, id-token: write sits only on the four environment-bound rollback-* jobs, and validate/summary are contents: read. Verified against current main. Its remaining gap is the missing protection rules, which is LeanerCloud/cloud-commitments-platform#141.

So after #1674 merges, the residue of this issue is entirely items 1-6 above. Cross-referencing rather than restating: #1660 owns environments-have-no-protection-rules and main-is-unprotected; #1648 owns the trust-policy sub allowlist gaps; #1659 owns the sibling deploy workflows. Filing nothing new — this family already has three overlapping issues.

Suggest this issue stays open until at least items 1-3 are done, or is closed with an explicit note that it covered the binding only and the gate lives in LeanerCloud/cloud-commitments-platform#141.