diff --git a/terraform/environments/aws/ci-cd-permissions/README.md b/terraform/environments/aws/ci-cd-permissions/README.md index 056c69b8d..6cb9c528c 100644 --- a/terraform/environments/aws/ci-cd-permissions/README.md +++ b/terraform/environments/aws/ci-cd-permissions/README.md @@ -126,5 +126,76 @@ for temporary AWS credentials via `sts:AssumeRoleWithWebIdentity`, and injects ` ### Trust policy conditions -The trust policy allows only workflows from `repo:LeanerCloud/CUDly:*`. If you fork CUDly or use a -different repo, set `github_repo` in `terraform.tfvars` to the new `owner/repo` value and re-apply. +The trust policy does **not** allow all workflows from the repo: `repo:LeanerCloud/CUDly:*` would +let any branch, including an unprotected feature branch, mint valid deploy credentials. Instead the +OIDC `sub` claim is checked against an explicit, enumerated allowlist (`role.tf`'s +`token.actions.githubusercontent.com:sub` condition): + +- `repo:LeanerCloud/CUDly:ref:refs/heads/main`, for workflows dispatched on `main` with no + `environment:` binding. +- `repo:LeanerCloud/CUDly:environment:`, one entry per exact environment name a job that + assumes this role binds to. A job's `environment:` **replaces** the ref-based subject with an + environment-scoped one (never both), so every such value must be listed explicitly or that job + cannot authenticate. See the comment above the `sub` list in `role.tf` for the full derivation: + which workflow/job each entry covers, and what was deliberately left out and why (`pull_request` + jobs, and an unreachable `workflow_call` input path), kept in one place so it cannot drift out of + sync with the policy itself. + +If you fork CUDly or use a different repo, set `github_repo` in `terraform.tfvars` to the new +`owner/repo` value and re-apply; the prefix changes, the enumerated suffixes do not. If you add a +new workflow job that assumes this role and binds to a not-yet-listed `environment:`, add its exact +subject to `role.tf` and re-apply *before* that job runs, or `configure-aws-credentials` fails with +`AssumeRoleWithWebIdentity`/`Not authorized`. + +### This allowlist alone does not restrict deploys to `main` + +The owner constraint on this repo is: only `main` may deploy. Three **different** GitHub-side +controls keep getting conflated across this repo's issues, and this module (and this allowlist) +implements only the first of them: + +1. **Allowlist membership** (`role.tf`'s `sub` list, this module): can the job authenticate to AWS + at all? Nothing below matters if this says no. +2. **Deployment branch policy** (a GitHub environment setting, not configured by this module): which + *branch* may trigger a deploy to that environment. This is what "only `main` may deploy" actually + requires, and nothing in this module sets it. +3. **Required reviewers / protection rules** (a GitHub environment setting, not configured by this + module): *who* must approve before a job bound to that environment proceeds. The subject of + #1591/#1674/#1660, not this module. + +`ref:refs/heads/main` in the allowlist enforces (2) on its own for a job with no `environment:` +binding: no environment, no policy needed, the ref check *is* the restriction. `environment:` +subjects enforce **neither (2) nor (3)** on their own, because the OIDC `sub` for those is +`repo::environment:`, which is ref-agnostic: it says which environment the job bound to, +nothing about which branch triggered the run. A `workflow_dispatch` fired from any branch against an +environment-bound job presents the exact same subject a `main` run would, so this allowlist admits +it. The only control that reattaches the branch requirement is a deployment branch policy +(`custom_branch_policies` restricted to `main`) on that environment, configured on the GitHub side. + +**Live state, checked against the GitHub API directly** (`gh api repos/LeanerCloud/CUDly/environments`), +not assumed from an earlier issue's snapshot: + +| Environment | Exists today? | Protection rules | Deployment branch policy | +| --- | --- | --- | --- | +| `dev` | Yes | none | none | +| `staging` | **No** | n/a | n/a | +| `prod` | **No** | n/a | n/a | +| `aws-fargate-dev` | Yes | none | none | +| `aws-fargate-staging` | Yes | none | none | +| `aws-fargate-prod` | **No** | n/a | n/a | +| `aws-db-dev` / `aws-db-staging` / `aws-db-prod` | **No** (all three) | n/a | n/a | +| `aws-lambda-{dev,staging,prod}-rollback` | **No** (all three) | n/a | n/a | +| `aws-fargate-{dev,staging,prod}-rollback` | **No** (all three) | n/a | n/a | + +Only 3 of the 15 environments this allowlist names exist yet, and none of the 3, including `dev` +which multiple deploy jobs already use in production, has a branch policy or protection rules of any +kind. Every environment above needs (2) configured (and the 12 that don't exist yet also need to be +created) before this allowlist actually delivers "only `main` deploys" rather than "only these +environments deploy, from any branch". This module has no GitHub provider configured (no +`provider "github"` or `github_repository_*` resource anywhere under `terraform/` or `iac/`), so +none of this, creation, branch policy, or reviewers, can be done declaratively today; it is a +manual, per-environment step in **Settings -> Environments** on the repo. Adding a GitHub Terraform +provider so this becomes code is #1660's scope, not this module's. + +Related: #1648 (this allowlist gap, control 1), #1660 (controls 2 and 3: environments need creating, +a branch policy, and protection rules), #1674 (binds the destroy workflows' jobs to environments +covered by this list). diff --git a/terraform/environments/aws/ci-cd-permissions/role.tf b/terraform/environments/aws/ci-cd-permissions/role.tf index 967a9c86d..4a2133551 100644 --- a/terraform/environments/aws/ci-cd-permissions/role.tf +++ b/terraform/environments/aws/ci-cd-permissions/role.tf @@ -25,11 +25,123 @@ resource "aws_iam_role" "cudly_deploy" { # Restrict to protected branches and named deployment environments. # Any-branch wildcard (repo:org/repo:*) would let a developer push to # an unprotected feature branch and mint valid deploy credentials. + # + # An OIDC `sub` is EITHER `...:ref:refs/heads/` OR + # `...:environment:` -- never both -- so every job in every + # AWS-assuming workflow that binds to an `environment:` needs its + # exact subject listed here, or it cannot authenticate at all. This + # list is derived from grepping every `role-to-assume: ${{ + # vars.AWS_ROLE_TO_ASSUME }}` step across .github/workflows/ and + # tracing each job's `environment:` value to its source (see #1648 + # and the PR that added this comment for the full per-job table). + # `StringEquals` (exact match, not `StringLike`) on purpose: every + # value below is enumerable from a `type: choice` input, so a + # pattern would only widen what this policy admits, never narrow + # it usefully. + # + # THIS LIST DOES NOT BY ITSELF RESTRICT DEPLOYS TO `main`. Owner + # constraint is "only main may deploy". Three DIFFERENT controls + # keep getting conflated in this repo's issues; this list is only + # the first of them: + # 1. Allowlist membership (THIS list): can the job authenticate + # to AWS at all? Nothing else below matters if this says no. + # 2. Deployment branch policy (GitHub environment setting, + # manual, NOT configured by this module): which BRANCH may + # trigger a deploy to that environment. This is what "only + # main may deploy" actually requires, and nothing here sets + # it. + # 3. Required reviewers / protection rules (GitHub environment + # setting, manual, NOT configured by this module): WHO must + # approve before a job bound to that environment proceeds. + # The subject of #1591/#1674/#1660, not this file. + # + # `ref:refs/heads/main` enforces (2) on its own, for an unbound + # job: no environment, no policy needed, the ref check IS the + # restriction. `environment:` subjects enforce NEITHER (2) + # nor (3) on their own: they are ref-agnostic (the subject encodes + # which environment the job bound to, not which branch triggered + # the run), so a `workflow_dispatch` from ANY branch against an + # environment-bound job presents the identical subject a `main` + # run would. Until a deployment branch policy exists on that + # environment, this allowlist delivers "only these environments + # deploy, from any branch", not "only main deploys" -- and per a + # live check against the GitHub API, none of the environments + # below have one, or any protection rules, today. See the PR that + # added this comment for the full list of environments needing + # that policy, and which of them exist yet at all. + # + # Deliberately NOT listed, and why: + # - `pull_request` subjects: no job that assumes THIS role + # (cudly_deploy) runs on `pull_request`. `aws_sanity.yml` is the + # only pull_request-triggered workflow that calls + # configure-aws-credentials, and it assumes a separate, + # already-read-only role via `secrets.AWS_CICD_READONLY_ROLE_ARN`, + # not this one. Also note GitHub does not mint OIDC tokens for + # `pull_request` runs from forked repos by default, independent + # of this policy. + # - `environment:aws-db-` from database-migration.yml's + # `workflow_call` trigger: that trigger declares `environment` + # as an unconstrained `type: string`, but nothing in this repo + # currently calls this workflow via `workflow_call` (grepped + # `.github/workflows/` for `uses:.*database-migration.yml`: no + # hits). Only the `workflow_dispatch` path, constrained to + # dev/staging/prod, is reachable today -- covered below. If a + # caller is ever added, this must be revisited before that + # caller can authenticate. "token.actions.githubusercontent.com:sub" = [ "repo:${var.github_repo}:ref:refs/heads/main", + + # Bare deployment environments. Bound directly by + # deploy-aws-lambda.yml's build-and-deploy / test-deployment jobs + # (environment: ${{ needs.prepare.outputs.environment }}, always + # dev/staging/prod per that job's own resolution logic), and -- + # once #1674 merges -- by cleanup-staging.yml's two AWS destroy + # jobs (environment: staging) and destroy-fargate-dev.yml's + # destroy job (environment: dev). + # Live check (`gh api repos/.../environments`): `dev` exists, + # `staging`/`prod` do not. None has a deployment branch policy or + # protection rules -- see the top-of-list note. "repo:${var.github_repo}:environment:dev", "repo:${var.github_repo}:environment:staging", "repo:${var.github_repo}:environment:prod", + + # deploy-aws-fargate.yml's `deploy` job binds to + # `aws-fargate-${{ needs.prepare.outputs.environment }}`, and that + # output is always dev/staging/prod (workflow_dispatch choice + # input, workflow_call from deploy-all.yml which is itself + # choice-constrained, or the untriggered-input "dev" fallback). + # Live check: `aws-fargate-dev` and `aws-fargate-staging` exist, + # `aws-fargate-prod` does not. Same "no branch policy, no + # protection rules" caveat as above applies to all three. + "repo:${var.github_repo}:environment:aws-fargate-dev", + "repo:${var.github_repo}:environment:aws-fargate-staging", + "repo:${var.github_repo}:environment:aws-fargate-prod", + + # database-migration.yml's `migrate-aws` job binds to + # `aws-db-${{ inputs.environment }}` on its `workflow_dispatch` + # trigger, where `inputs.environment` is a choice input + # constrained to dev/staging/prod. See the workflow_call caveat + # above for what is deliberately excluded. Live check: none of + # the three exist yet. + "repo:${var.github_repo}:environment:aws-db-dev", + "repo:${var.github_repo}:environment:aws-db-staging", + "repo:${var.github_repo}:environment:aws-db-prod", + + # rollback.yml's rollback-aws-lambda / rollback-aws-fargate jobs + # bind to `aws-lambda-${{ inputs.environment }}-rollback` / + # `aws-fargate-${{ inputs.environment }}-rollback`; inputs.environment + # is a workflow_dispatch choice input constrained to + # dev/staging/prod. Listing the subject here only fixes control + # (1) above (this issue, #1648); it does nothing for controls + # (2) or (3). Live check: none of these six exist yet -- first + # dispatch auto-creates them bare, with neither a branch policy + # nor reviewers, unless #1660's provisioning work lands first. + "repo:${var.github_repo}:environment:aws-lambda-dev-rollback", + "repo:${var.github_repo}:environment:aws-lambda-staging-rollback", + "repo:${var.github_repo}:environment:aws-lambda-prod-rollback", + "repo:${var.github_repo}:environment:aws-fargate-dev-rollback", + "repo:${var.github_repo}:environment:aws-fargate-staging-rollback", + "repo:${var.github_repo}:environment:aws-fargate-prod-rollback", ] } }