[IAmJSD]

Root cause

terraform/environments/azure/ci-cd-permissions/variables.tf: github_environments has defaulted to ["dev", "staging", "prod"] since #358. Its description said a credential must not be minted "for an ungated environment that does not yet exist". When this issue was filed (2026-09-27) no prod environment existed, so the default broke the variable's own rule.

That has since changed. On 2026-10-04 a maintainer created dev, staging and prod (and the aws-*, gcp-db-*, azure-db-* families), each with a deployment branch policy of exactly main (see the #141 and #2 comments). I checked the live state for this PR:

$ gh api repos/LeanerCloud/cloud-commitments-platform/environments/prod --jq '{protection_rules,deployment_branch_policy}'
{"deployment_branch_policy":{"custom_branch_policies":true,"protected_branches":false},"protection_rules":[{"id":67658840,...,"type":"branch_policy"}]}
$ gh api repos/LeanerCloud/cloud-commitments-platform/environments/prod/deployment-branch-policies --jq '.branch_policies[]|{name,type}'
{"name":"main","type":"branch"}

(dev and staging return the same. None of them has required reviewers.)

So prod now meets the rule, but the description still read as if it did not, and it never stated the rule precisely.

Fix

I took the issue's second option and amended the description. Removing prod would cause Azure prod deploys (deploy-azure.yml) and rollback.yml's rollback-azure to fail with AADSTS70021 after the next bootstrap re-apply.

• Gating rule, stated explicitly: add a name only once its environment exists with a deployment branch policy that admits only main. The service principal already trusts repo:<repo>:ref:refs/heads/main (sp.tf, github_main), and only a run on main can enter such an environment, so its credential adds no reach. A credential for an environment that does not exist does widen access, because GitHub creates a referenced environment on first use with no protection. The description also gives the gh api command to check.
• prod is not an exception: it has the same main-only policy and the same service principal as dev/staging. The description also notes that no environment has required reviewers, and that widening a policy past main (e.g. a v* tag policy for release-triggered prod deploys) widens this credential's reach too.
• azure-db-* note: the old reason for leaving them out ("needs its environment created and gated first") is no longer true. Those environments exist with main-only policies. The note now says that, and leaves adding them to #1648 because that mints new credentials and needs a re-apply.
• README: the "To allow a new deployment environment" step now says to create the environment with a main-only policy first, and points at the rule.
• New test tests/github_environments.tftest.hcl (mock azurerm/azuread providers, command = plan). It pins the default credential set to exactly {dev, staging, prod}, the prod subject, and the github_main subject the "no added reach" argument depends on. It also checks that github_repo = "" creates no environment credentials. CI's existing "Test CI/CD trust (immutable ids)" step runs it, since that step runs terraform test for any terraform/environments/<cloud>/ci-cd-permissions/tests directory.

Bootstrap re-apply

None needed. ci-cd-permissions is applied manually. This PR changes only a variable's description, a README and a test. The default value and every resource are unchanged, so a plan against live state would show no changes.

Verification

All of this is local and offline: terraform with -backend=false and mock providers. No real Azure or GitHub writes, no plan against a real backend, no apply. The only live calls were read-only gh api GETs on the repository's environments.

cd terraform/environments/azure/ci-cd-permissions
TF_DATA_DIR=<private tmp> terraform init -backend=false -input=false -lockfile=readonly   # OK, existing lock file unchanged
terraform fmt -check -recursive                                                         # clean
terraform validate                                                                      # Success! The configuration is valid.
terraform test                                                                          # Success! 2 passed, 0 failed.

About fail-before/pass-after: this issue is a contradiction in documentation, not a behavior bug. The fix changes no plan, so no test can fail before it and pass after it. The new test guards the decision the description records. Mutation probes (default edited temporarily, then reverted):

Mutation of default	Result
["dev", "staging"] (drop prod)	default_environments_are_the_gated_plain_names fail (set assertion and prod subject assertion)
["dev", "staging", "prod", "azure-db-prod"] (add a name)	default_environments_are_the_gated_plain_names fail (set assertion)
unchanged	2 passed, 0 failed

Other checks on the touched files:

• markdownlint-cli@0.47.0 --config .markdownlint.yaml on the README: clean.
• trivy config --severity HIGH,CRITICAL --exit-code 1 on the module: exit 0.
• git-secrets 1.3.0 (--register-aws, repo .gitallowed) over the three changed files, in a fresh git init repo with an isolated HOME: clean. scripts/test-git-secrets-allowlist.sh: 42 PASS, 0 FAIL.
• git diff --check: clean.
• Not run locally: tflint, which isn't installed here. CI's pre-commit terraform_tflint hook covers it.

No Go code changed, so I didn't run Go build, vet, golangci-lint, gocyclo or internal/mocks (they don't apply).

Coverage gap: I did not dispatch a run from a feature branch to prove that the main-only policy refuses it. That would start a real deploy. The "no added reach" argument relies on GitHub enforcing deployment branch policies before it issues the environment-subject token.

Out of scope (noted, not changed)

• azure-db-* credentials: azure-db-{dev,staging,prod} now meet the gating rule. Adding them to the default (#1648) mints three credentials and needs a bootstrap re-apply.
• Release-triggered prod deploys: these run from a tag ref, which the main-only policy refuses (maintainer note on sec(ci): deployment environments have no protection rules, so environment-bound credentialed jobs run unapproved #141). Adding a tag policy would widen this credential's reach. The new description says so, but the decision is left to the maintainers.
• GCP (from the issue body): GCP's WIF condition keys on the ref, not sub, so environment bindings change nothing there. That belongs to sec(iac): rename-proof OIDC deploy trust + branch-restricted environments for the split platform repo #2.
• Admin bypass: prod has can_admins_bypass: true. Repository admins can change these settings anyway, so I didn't treat it as reach this credential adds.

Labels

A maintainer please mirror the issue's triage labels onto this PR: impact/internal, type/chore, triaged, priority/p3, severity/low, urgency/this-quarter, effort/xs.

Closes #364

🤖 Generated with Claude Code

Summary by CodeRabbit

• Documentation
  • Clarified requirements for configuring GitHub deployment environments, including a main-only deployment branch policy.
  • Documented the status of the default dev, staging, and prod environments and deferred database-migration environments.
• Tests
  • Added validation for default environment credentials, production federation settings, main-branch credentials, and empty repository configurations.

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: ab232e3a-739b-4bbf-a615-d80b13ae0213

📥 Commits

Reviewing files that changed from the base of the PR and between ba38120 and be6a6dd.

📒 Files selected for processing: 3

• terraform/environments/azure/ci-cd-permissions/README.md
• terraform/environments/azure/ci-cd-permissions/tests/github_environments.tftest.hcl
• terraform/environments/azure/ci-cd-permissions/variables.tf

+-----------------------------------------------------------------------------------+
| The best way to predict the future is to implement it. - David Heinemeier Hansson |
+-----------------------------------------------------------------------------------+
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Independent review (claude-cc-platform reviewer, cold brief): APPROVE be6a6dd6d3ac97f4433d17bb47420a3e3127ffbb

No blocking issues.

Live gating, checked read-only with gh api: prod, dev and staging each have exactly one deployment branch policy, main, plus the branch_policy protection rule. None has required reviewers or a wait timer. The description's claim is accurate.

Credential reach: sp.tf is unchanged. The subjects stay ref:refs/heads/main plus environment:<dev|staging|prod>, and since each environment admits only main, they add no reach beyond the existing main subject. The PR changes only the description, the README and the tftest; the default is identical. Merging has no applied-state effect and needs no re-apply.

Base drift: clean merge onto current main (tree a6e7be9b). Main has not touched ci-cd-permissions since the merge base.

Nits:

• The description's "applied 2026-10-04" date will go stale.
• chore(iac/azure): reconcile the prod entry in github_environments with the variable's gating rule (follow-up to #358) #364 offered two options; this PR takes option 2 (keep prod, amend the description) rather than adding required reviewers. Closing chore(iac/azure): reconcile the prod entry in github_environments with the variable's gating rule (follow-up to #358) #364 with that choice should be deliberate.
• chore(iac/azure): reconcile the prod entry in github_environments with the variable's gating rule (follow-up to #358) #364's note about the sec(ci): bind deploy-gcp/deploy-azure authenticating jobs to a GitHub Environment #358 body (GCP binding) is not addressed; it is informational only.

Attacked: terraform init -backend=false, validate and test (mock providers, no credentials) pass on an export of the head. Mutation probes, each failing a tftest run:

• prod removed from the default;
• an extra qa environment;
• the environment: subject mutated;
• the github_repo != "" guard dropped.

The Azure-side state was not checked (needs credentials).

[cristim]

Independent review (claude-fleet): APPROVE be6a6dd

Findings (none blocking):

• terraform/environments/azure/ci-cd-permissions/variables.tf:26 [non-blocking] - Terraform does not enforce the gating rule; it exists only in prose, because the module has no github provider. A future edit to the default that adds an ungated name would pass validate. The only guard is the new test's exact-set pin, which catches that edit in CI. That is the right design at this scope: adding a github provider to a bootstrap module just for this check would be over-engineering.
• terraform/environments/azure/ci-cd-permissions/variables.tf:39 [non-blocking] - "the branch policy (plus main's ruleset) is the only gate" leaves out that every environment, prod included, has can_admins_bypass: true on the live repo. The PR body mentions it; the description does not. Failure scenario: a reader assumes nobody can deploy to prod from outside main. Repository admins can rewrite the policy anyway, so this credential adds no reach. One clause would make the doc complete.
• terraform/environments/azure/ci-cd-permissions/variables.tf:37,56 [non-blocking] - The "2026-10-04" snapshot dates go stale if the environments change later. The gh api check command at :33 is the durable part.

Attacked:

• Issue chore(iac/azure): reconcile the prod entry in github_environments with the variable's gating rule (follow-up to #358) #364: the PR takes the issue's option 2 (keep prod, amend the description).
  • The default list and sp.tf are byte-identical to base, so there is no plan change and no re-apply.
  • Option 1's required reviewers are not adopted. The description says so openly at :39, so this is a disclosed maintainer choice, not a silent weakening.
• Consumers:
  • The github_environment for_each in sp.tf is the only consumer of the variable.
  • deploy-azure.yml allowlists dev|staging|prod and binds the prepared target environment.
  • rollback.yml rollback-azure binds inputs.environment, cleanup-staging.yml binds staging, and destroy-fargate-dev.yml binds dev.
  • database-migration.yml binds azure-db-*, which is documented as out of scope (#1648).
  • The AWS trust already covers environment dev/staging/prod, so nothing needs mirroring.
• Live state, via read-only gh api GETs:
  • All 15 environments exist.
  • dev, staging, prod and azure-db-{dev,staging,prod} each have exactly one deployment branch policy, main (branch type).
  • Each has protected_branches false, 0 required reviewers, no wait timer, and admin bypass on.
  • This matches every claim in the description, so there is no drift.
• Validation semantics: the existing duplicate and non-empty validations are unchanged. An empty list makes alltrue([]) true and yields zero credentials, which is harmless. The test's try(..., "") wraps only the prod subject lookup, so a missing key fails the assertion instead of erroring.
• CI: the ci.yml "Test CI/CD trust (immutable ids)" matrix includes azure and runs terraform test once tests/ exists. Base had no azure tests dir, so the new file is picked up.
• The new test passes on base (2/2). That is expected for a doc-only fix: it pins existing behaviour rather than reproducing a bug, as the PR body says.
• Mutation probes on a scratch copy of HEAD, each reverted with an identical checksum afterwards:
  1. The default drops prod: default_environments_are_the_gated_plain_names FAILS.
  2. The default adds qa: the same test FAILS.
  3. A suffix is added to the sp.tf environment subject: the same test FAILS.
  4. The github_repo != "" guard is removed from sp.tf: no_environment_credentials_without_repo FAILS.
  5. The main subject is widened to refs/heads/*: default_environments_are_the_gated_plain_names FAILS.

Verified locally on a git archive export of HEAD, in terraform/environments/azure/ci-cd-permissions, with no cloud credentials (AWS config pointed at /dev/null, ARM/AZURE/GOOGLE vars unset). No plan or apply was run.

• terraform fmt -check -recursive: exit 0.
• terraform init -backend=false -input=false -lockfile=readonly, under the heavy lock: OK.
• terraform validate: Success.
• terraform test: 2 passed, 0 failed, both before and after the probes.
• Base export with the new test copied in: terraform test gives 2 passed.

Reviewed by an independent agent with no author context. Review only: nothing was pushed, and the GitHub review state is unchanged.

[cristim]

Local verification (macOS, exact head be6a6dd6d3ac97f4433d17bb47420a3e3127ffbb, export of terraform/): in terraform/environments/azure/ci-cd-permissions, terraform fmt -check -recursive, init -backend=false, validate and test pass (tests/github_environments.tftest.hcl: 2 passed). No plan or apply, and no credentials.

Gate status:

• Two independent reviews approve this SHA (posted above).
• Local verification passes.
• Merge waits on @IAmJSD marking the PR ready (see the question on fix(api): add opt-in in-memory cursor pagination to GET /api/recommendations #974) and on green CI at the head.

[cristim]

Local verification at be6a6dd6d3ac97f4433d17bb47420a3e3127ffbb merged onto current main (1ce5430c; merge-tree 63c835af; macOS):

• Terraform fmt -check, init -backend=false, validate and test pass for: terraform/environments/azure/ci-cd-permissions
• Nothing was applied to any cloud.

[cristim]

Marked ready and merged by the owner's agent after 2 independent reviews and local verification (see the review and verification comments above, all at be6a6dd6d3ac97f4433d17bb47420a3e3127ffbb).