sec(ci): bind destroy jobs to an environment and narrow id-token on destroy workflows

.github/workflows/cleanup-staging.yml

@@ -21,8 +21,32 @@ on:
         description: 'Type "destroy" to confirm destruction of ALL staging resources'
         required: true
 
+# Least privilege: `id-token: write` is granted per job, only to the four jobs
+# that assume a cloud deploy role, and each of those is bound to the `staging`
+# deployment environment. `guard` authenticates to nothing and must not hold it.
+#
+# IMPORTANT — what the `environment:` binding does and does not buy. It scopes
+# secrets/vars and sets the OIDC subject to `repo:<org/repo>:environment:staging`.
+# What each cloud does with that subject differs; see the per-job comments.
+#
+# It is NOT a reviewer gate. GitHub only blocks a job once required-reviewer
+# protection rules are configured on the environment, and at the time of writing
+# no environment in this repo has any. Note GitHub AUTO-CREATES a referenced
+# environment bare on first use — no reviewers, no branch policy — so a binding
+# to an environment nobody has configured runs straight through. Until that is
+# configured out-of-band these destroy jobs still run unapproved. See #1660 for
+# the live state; deliberately not restated here so this comment cannot rot into
+# false reassurance.
+#
+# The environment subject is ref-agnostic, so the binding does not by itself keep
+# this workflow on `main`. The `guard` job below checks the ref, but that check is
+# defense-in-depth against ACCIDENTS ONLY and is NOT a security boundary:
+# `workflow_dispatch` runs the workflow file as it exists on the dispatched ref, so
+# anyone able to push a branch can delete the check and still present the
+# `environment:staging` subject. The only control that survives that is a
+# deployment branch policy, which GitHub evaluates before the job starts and
+# before the token is minted. See #1660.
 permissions:
-  id-token: write
   contents: read
 
 env:
@@ -33,7 +57,33 @@ jobs:
   guard:
     name: Confirm Destruction
     runs-on: ubuntu-latest
+    permissions: {}
     steps:
+      # An OIDC `sub` is EITHER `…:ref:refs/heads/<branch>` OR
+      # `…:environment:<name>` -- never both (absent a custom sub-claim
+      # template, and this repo sets none). So binding the destroy jobs to an
+      # environment (below) REPLACES the ref-scoped subject with a ref-agnostic
+      # one, and neither `dev` nor `staging` carries a deployment_branch_policy.
+      # Without a ref check that trades a main-only restriction for any-branch
+      # access -- a widening, on the workflow this change exists to narrow.
+      #
+      # This step catches the accidental case only. It is 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 this step and
+      # still present the `environment:staging` subject. The control that
+      # survives that is a deployment branch policy on the environment
+      # (custom_branch_policies + a `main` pattern), which GitHub evaluates
+      # before the job starts and before the token is minted, and which does NOT
+      # require `main` to be a protected branch. Tracked in #1660.
+      - name: Restrict to main
+        env:
+          REF: ${{ github.ref }}
+        run: |
+          if [ "$REF" != "refs/heads/main" ]; then
+            echo "::error::Refusing to destroy from '$REF'; this workflow may only be dispatched from refs/heads/main"
+            exit 1
+          fi
+
       - name: Check confirmation
         env:
           CONFIRM: ${{ inputs.confirm }}
@@ -48,6 +98,13 @@ jobs:
     name: Destroy AWS Lambda (staging)
     runs-on: ubuntu-24.04-arm
     needs: guard
+    permissions:
+      id-token: write
+      contents: read
+    # AWS: role.tf's sub allowlist includes `environment:staging`, so this is
+    # the subject the trust policy matches. Not a reviewer gate until
+    # protection rules exist -- see the note at the top of this file.
+    environment: staging
     env:
       AWS_REGION: ${{ vars.AWS_REGION || 'us-east-1' }}
 
@@ -118,6 +175,13 @@ jobs:
     name: Destroy AWS Fargate (staging)
     runs-on: ubuntu-24.04-arm
     needs: guard
+    permissions:
+      id-token: write
+      contents: read
+    # AWS: role.tf's sub allowlist includes `environment:staging`, so this is
+    # the subject the trust policy matches. Not a reviewer gate until
+    # protection rules exist -- see the note at the top of this file.
+    environment: staging
     env:
       AWS_REGION: ${{ vars.AWS_REGION || 'us-east-1' }}
 
@@ -188,6 +252,16 @@ jobs:
     name: Destroy Azure (staging)
     runs-on: ubuntu-latest
     needs: guard
+    permissions:
+      id-token: write
+      contents: read
+    # Azure: REQUIRES the bootstrap module
+    # terraform/environments/azure/ci-cd-permissions to have been re-applied
+    # with `github_environments` including "staging". Until that manual apply
+    # happens this job fails with AADSTS70021, because the only federated
+    # credentials that exist are ref:refs/heads/main and pull_request (#1648).
+    # Not a reviewer gate either -- see the note at the top of this file.
+    environment: staging
     env:
       ARM_USE_OIDC: "true"
       ARM_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
@@ -253,6 +327,16 @@ jobs:
     name: Destroy GCP (staging)
     runs-on: ubuntu-latest
     needs: guard
+    permissions:
+      id-token: write
+      contents: read
+    # GCP: the WIF provider keys on assertion.repository and assertion.ref
+    # only (gcp/ci-cd-permissions/github_oidc.tf:37), and the impersonation
+    # binding is on attribute.repository -- nothing reads the subject. So this
+    # binding does NOT affect whether the token mints; it is here for parity
+    # and so protection rules can apply once they exist. GCP therefore stays
+    # main-only via assertion.ref regardless.
+    environment: staging
     env:
       GCP_REGION: ${{ vars.GCP_REGION || 'us-central1' }}
 
@@ -282,8 +366,18 @@ jobs:
       - name: Delete Cloud SQL instance directly (avoids user/DB ordering deadlock)
         env:
           TF_VAR_project_id: ${{ vars.GCP_PROJECT_ID }}
+          GCP_PROJECT_ID: ${{ vars.GCP_PROJECT_ID }}
         run: |
-          PROJECT="${{ vars.GCP_PROJECT_ID }}"
+          # Routed via env: rather than interpolated into this script's source,
+          # per the zero-expression-interpolation-in-run-blocks rule from #1641.
+          PROJECT="$GCP_PROJECT_ID"
+          # Validate the shape, not merely non-emptiness: a whitespace-only or
+          # malformed value would otherwise reach `gcloud --project=`, match no
+          # instance, and silently skip the deletion this step exists to perform.
+          if ! printf '%s' "$PROJECT" | grep -Eq '^[a-z][a-z0-9-]{5,29}$'; then
+            echo "::error::vars.GCP_PROJECT_ID is unset or malformed ('$PROJECT'); refusing to run Cloud SQL cleanup"
+            exit 1
+          fi
           # Find and delete the staging Cloud SQL instance to avoid PostgreSQL dependency errors
           INSTANCE=$(gcloud sql instances list --project="$PROJECT" \
             --filter="name:cudly-staging" --format="value(name)" 2>/dev/null | head -1)

.github/workflows/destroy-fargate-dev.yml

@@ -12,20 +12,56 @@ on:
         description: 'Type "destroy" to confirm'
         required: true
 
+# Least privilege: `id-token: write` is granted per job, only to the job that
+# assumes the AWS deploy role, and that job is bound to the `dev` deployment
+# environment. `guard` authenticates to nothing and must not hold it.
+#
+# IMPORTANT — what the `environment:` binding does and does not buy. It scopes
+# secrets/vars and sets the OIDC subject to `repo:<org/repo>:environment:dev`,
+# which the AWS trust policy's sub allowlist accepts (role.tf:30).
+#
+# It is NOT a reviewer gate. GitHub only blocks a job once required-reviewer
+# protection rules are configured on the environment, and at the time of writing
+# no environment in this repo has any. Until that is configured out-of-band this
+# destroy job still runs unapproved. See #1660 for the live state — deliberately
+# not restated here, so this comment cannot rot into false reassurance.
+#
+# The environment subject is ref-agnostic, so the binding does not by itself keep
+# this workflow on `main`. The `guard` job below checks the ref, but that check is
+# defense-in-depth against ACCIDENTS ONLY and is NOT a security boundary:
+# `workflow_dispatch` runs the workflow file as it exists on the dispatched ref, so
+# anyone able to push a branch can delete the check and still present the
+# `environment:dev` subject. The only control that survives that is a deployment
+# branch policy, which GitHub evaluates before the job starts and before the token
+# is minted. See #1660.
 permissions:
-  id-token: write
   contents: read
 
 env:
-  AWS_REGION: ${{ vars.AWS_REGION || 'us-east-1' }}
   TF_VERSION: '1.10.0'
   FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true
 
 jobs:
   guard:
     name: Confirm Destruction
     runs-on: ubuntu-latest
+    permissions: {}
     steps:
+      # Catches the accidental "dispatched from the wrong branch" case. It does
+      # NOT stop a deliberate one: this file is attacker-controlled on the ref
+      # being dispatched, so the step can simply be deleted on that branch. The
+      # server-side equivalent is a deployment branch policy on `dev`
+      # (custom_branch_policies + a `main` pattern) — which does NOT require
+      # `main` to be a protected branch — tracked in #1660.
+      - name: Restrict to main
+        env:
+          REF: ${{ github.ref }}
+        run: |
+          if [ "$REF" != "refs/heads/main" ]; then
+            echo "::error::Refusing to destroy from '$REF'; this workflow may only be dispatched from refs/heads/main"
+            exit 1
+          fi
+
       - name: Check confirmation
         env:
           CONFIRM: ${{ inputs.confirm }}
@@ -39,6 +75,18 @@ jobs:
     name: Terraform Destroy (Fargate dev)
     runs-on: ubuntu-24.04-arm
     needs: guard
+    permissions:
+      id-token: write
+      contents: read
+    # Binds the OIDC subject to repo:<org/repo>:environment:dev, which the AWS
+    # trust policy matches on. Not a reviewer gate until protection rules exist
+    # on this environment -- see the note at the top of this file.
+    environment: dev
+    # Set at job level, not workflow level: workflow-level `env:` is resolved
+    # before this job's `dev` environment is in scope, so an environment-scoped
+    # AWS_REGION variable would be invisible there. Matches cleanup-staging.yml.
+    env:
+      AWS_REGION: ${{ vars.AWS_REGION || 'us-east-1' }}
 
     steps:
       - name: Checkout code

terraform/environments/azure/ci-cd-permissions/README.md

@@ -13,6 +13,7 @@ secrets ever need to be stored.
 | `azuread_service_principal.cudly_deploy` | Service principal (identity) |
 | `azuread_application_federated_identity_credential.github_main` | Federated credential for main-branch deployments |
 | `azuread_application_federated_identity_credential.github_pr` | Federated credential for pull-request plan checks |
+| `azuread_application_federated_identity_credential.github_environment` | Federated credential per deployment environment (`var.github_environments`) |
 | `azurerm_role_definition.cudly_deploy` | Custom role with minimum required permissions |
 | `azurerm_role_assignment.cudly_deploy` | Assigns the custom role to the SP at subscription scope |
 | `module.cudly_reservation_role` (`azurerm_role_definition`) | Host-side custom "CUDly Reservation Purchaser" role definition consumed by the runtime container-apps deploy |
@@ -159,12 +160,26 @@ the `az` CLI context for all subsequent steps.
 
 ### Federated credential subjects
 
-Two subjects are registered:
+Azure federated credentials allow no wildcards, so one resource is required per subject:
 
 | Credential | Subject | Use case |
 | --- | --- | --- |
 | `github-actions-main` | `repo:LeanerCloud/CUDly:ref:refs/heads/main` | Deployments from main |
 | `github-actions-pr` | `repo:LeanerCloud/CUDly:pull_request` | Plan runs on PRs |
-
-If you need to deploy from a different branch or repo, add additional
-`azuread_application_federated_identity_credential` resources to `sp.tf`.
+| `github-actions-env-<name>` | `repo:LeanerCloud/CUDly:environment:<name>` | Jobs bound to a deployment environment (one per `var.github_environments`) |
+
+A job declaring `environment: <name>` presents the **environment** subject, not the
+main-branch one, so it cannot authenticate unless `<name>` is in
+`var.github_environments`. Omitting this is what makes `azure/login` fail with
+`AADSTS70021` on an otherwise-correct workflow (see issue #1648).
+
+Note the environment subject is **ref-agnostic** — it does not encode the branch. Adding
+a name here therefore lets any branch that can reach a job bound to that environment
+obtain the deploy service principal. Restrict the branch via the environment's
+`deployment_branch_policy`; a ref check inside the workflow does **not** bind, because
+`workflow_dispatch` runs the workflow file as it exists on the dispatched ref, so the
+check can be removed on that branch.
+
+To allow a new deployment environment, add its name to `var.github_environments`. To
+deploy from a different branch or repo, add a further
+`azuread_application_federated_identity_credential` resource to `sp.tf`.

terraform/environments/azure/ci-cd-permissions/sp.tf

@@ -23,8 +23,18 @@ resource "azuread_service_principal" "cudly_deploy" {
 # GitHub Actions Federated Identity Credentials
 # =============================================================================
 # Azure federated credentials require one entry per allowed subject (no
-# wildcards). We create two: one for main-branch deployments and one for
-# pull-request plan checks.
+# wildcards), so each named deployment environment needs its own resource.
+#
+# The `github_environment` entries below are what let an environment-bound job
+# authenticate at all. A job carrying `environment: staging` presents the
+# subject `repo:<org/repo>:environment:staging`, NOT the main-branch subject,
+# so without a matching credential `azure/login` fails with AADSTS70021 even
+# though the workflow is running on main. Adding an `environment:` binding to
+# an Azure job and forgetting this is the exact breakage recorded in #1648.
+#
+# NOTE: this module is bootstrap-only (see CLAUDE.md) — it is applied manually
+# by a privileged human, not by the deploy workflow. The Azure destroy job in
+# cleanup-staging.yml cannot authenticate until that re-apply happens.
 
 resource "azuread_application_federated_identity_credential" "github_main" {
   count = var.github_repo != "" ? 1 : 0
@@ -47,3 +57,18 @@ resource "azuread_application_federated_identity_credential" "github_pr" {
   issuer         = "https://token.actions.githubusercontent.com"
   subject        = "repo:${var.github_repo}:pull_request"
 }
+
+# One credential per named deployment environment, so environment-bound jobs
+# can authenticate. Mirrors the `environment:{dev,staging,prod}` subjects the
+# AWS role's trust policy already allows
+# (terraform/environments/aws/ci-cd-permissions/role.tf).
+resource "azuread_application_federated_identity_credential" "github_environment" {
+  for_each = var.github_repo != "" ? toset(var.github_environments) : toset([])
+
+  application_id = azuread_application.cudly_deploy.id
+  display_name   = "github-actions-env-${each.value}"
+  description    = "GitHub Actions OIDC — ${var.github_repo} ${each.value} environment"
+  audiences      = ["api://AzureADTokenExchange"]
+  issuer         = "https://token.actions.githubusercontent.com"
+  subject        = "repo:${var.github_repo}:environment:${each.value}"
+}

terraform/environments/azure/ci-cd-permissions/variables.tf

@@ -8,3 +8,45 @@ variable "github_repo" {
   type        = string
   default     = "LeanerCloud/CUDly"
 }
+
+variable "github_environments" {
+  description = <<-EOT
+    GitHub deployment environment names whose jobs may authenticate via federated
+    identity credentials. A job carrying `environment: <name>` presents the OIDC
+    subject repo:<github_repo>:environment:<name>, so a name absent from this list
+    cannot authenticate (AADSTS70021).
+
+    NOTE the subject is ref-agnostic: it does not encode the branch, so a
+    credential here lets ANY branch that can reach a job bound to that environment
+    obtain the deploy service principal. Add a name only once a workflow actually
+    presents it, and restrict the branch via the environment's
+    deployment_branch_policy — an in-workflow ref check does not bind, because
+    workflow_dispatch runs the file as it exists on the dispatched ref.
+
+    This list is NOT a complete enumeration of the environment subjects this repo
+    presents to Azure. It covers exactly the two destroy jobs bound by
+    cleanup-staging.yml (`staging`) and destroy-fargate-dev.yml (`dev`).
+
+    Knowingly NOT covered, tracked in #1648 — these Azure jobs bind to compound
+    environment names and therefore still fail with AADSTS70021:
+      - rollback.yml           -> azure-{dev,staging,prod}-rollback
+      - database-migration.yml -> azure-db-{dev,staging,prod}
+
+    They are excluded here rather than fixed because each needs its environment
+    created and gated first; minting a credential for an ungated environment that
+    does not yet exist would widen access without adding a control. Do not read
+    the absence of a name as "no job uses it".
+  EOT
+  type        = list(string)
+  default     = ["dev", "staging"]
+
+  validation {
+    condition     = length(var.github_environments) == length(distinct(var.github_environments))
+    error_message = "github_environments must not contain duplicates; toset() would silently collapse them, so a duplicate indicates a typo."
+  }
+
+  validation {
+    condition     = alltrue([for e in var.github_environments : trimspace(e) != ""])
+    error_message = "github_environments entries must be non-empty; an empty name yields the unmatchable subject repo:<repo>:environment:."
+  }
+}