sec(ci): main is unprotected while ref:refs/heads/main is a trusted OIDC subject for the AWS deploy role

[cristim]

Summary

main has no required-review protection, while repo:LeanerCloud/CUDly:ref:refs/heads/main is a trusted OIDC subject for the cudly-terraform-deploy AWS role. Anyone with write access can therefore push a workflow edit directly to main and mint AWS deploy credentials, with no review and no human gate anywhere in the chain.

This is the exposure that makes LeanerCloud/cloud-commitments-cli#1649's release-tag injection collapse in severity: the tag path grants strictly less than an actor with write access already holds. Recording it separately because it is larger than the bug LeanerCloud/cloud-commitments-cli#1657 fixes, and it is not addressed by any open PR.

Evidence

1. main is not protected. Classic branch protection:

$ gh api repos/LeanerCloud/CUDly/branches/main/protection
{"message":"Branch not protected", "status":"404"}

Checked with a token reporting "admin": true on the repo, so this is a genuine 404 and not an insufficient-scope artifact.

2. The one active ruleset does not require review. protect-master-branch is enforcement: active, but the rules it actually contributes to main are:

$ gh api repos/LeanerCloud/CUDly/rules/branches/main --jq '[.[]|.type]'
["deletion","non_fast_forward"]

That blocks deleting main and force-pushing to it. It does not require a pull request, an approving review, or status checks. The ruleset's name suggests it may have been written for a master branch that no longer exists.

3. No CODEOWNERS. No CODEOWNERS file at the repo root, in .github/, or in docs/. Nothing gives .github/workflows/** special review treatment.

4. main is a trusted OIDC subject. terraform/environments/aws/ci-cd-permissions/role.tf on main (cd4ee031f, after LeanerCloud/cloud-commitments-cli#1683) lists 16 subjects. The first is:

"repo:${var.github_repo}:ref:refs/heads/main",

The remaining 15 are environment:* subjects covering the deploy, DB-migration, fargate and rollback environment families. Only the ref:refs/heads/main entry is relevant here: a workflow job running on a push to main without an environment: binding presents sub = repo:LeanerCloud/CUDly:ref:refs/heads/main and matches it.

Reproduce the full list with:

git show origin/main:terraform/environments/aws/ci-cd-permissions/role.tf \
  | grep -n '"repo:${var.github_repo}'

Why it matters

The chain is: write access -> push a workflow to main -> job runs with id-token: write -> OIDC sub ref:refs/heads/main -> assume cudly-terraform-deploy -> full deploy permissions.

No step requires review, approval, or any second party.

Compounding factor (LeanerCloud/cloud-commitments-cli#1648): no GitHub Environment in this repo has required-reviewer protection configured. So the environment: bindings that scope the OIDC subject provide subject scoping but not a human gate. A release auto-deploys to prod with AWS credentials and nothing pauses it.

What this is NOT

Stating the limits explicitly, because several recent p0s in this backlog have had threat models that did not survive scrutiny:

• This requires write access to the repository. It is an insider or compromised-credential exposure, not an anonymous or external one.
• It is not remote code execution by an unauthenticated party, and not a data-exfiltration path on its own.
• A fork pull request does not reach it: fork PRs do not receive id-token: write or repository secrets.
• Severity is high rather than critical on that basis: real, unreviewed, and privilege-granting, but gated behind an already-trusted principal.

The honest framing is that this removes the second line of defence rather than the first. It means a single compromised contributor account, or a single bad merge, reaches production AWS with nothing in the way.

Suggested remediation

Not prescribing the exact policy, but the shape:

1. Require pull requests plus at least one approving review on main (either as a ruleset rule or classic protection). The existing protect-master-branch ruleset appears to be the intended home and is currently near-empty for main.
2. Add a CODEOWNERS entry for .github/workflows/** and terraform/environments/*/ci-cd-permissions/** so credential-granting configuration needs a named reviewer.
3. Configure required reviewers on the prod Environment at minimum (this is LeanerCloud/cloud-commitments-cli#1648).
4. Consider whether ref:refs/heads/main needs to remain a trusted subject at all, or whether every AWS-assuming job could bind an environment: instead, leaving only environment subjects in the trust policy. See the caveat in fix(iac/aws): extend OIDC trust allowlist to cover every job assuming cudly_deploy cloud-commitments-cli#1683 before narrowing further.

Related

• LeanerCloud/cloud-commitments-cli#1648 -- no Environment has required reviewers (the compounding factor above)
• sec(ci): deploy-aws-lambda.yml interpolates a release tag name into a run block in an ungated job holding id-token write cloud-commitments-cli#1649 -- release-tag injection; collapses in severity because of this issue
• fix(ci): stop interpolating the release tag into a run block cloud-commitments-cli#1657 -- fixes sec(ci): deploy-aws-lambda.yml interpolates a release tag name into a run block in an ungated job holding id-token write cloud-commitments-cli#1649's interpolation and removes id-token: write from the two ungated jobs in deploy-aws-lambda.yml
• LeanerCloud/cloud-commitments-cli#1660 -- related CI/OIDC hardening
• fix(iac/aws): extend OIDC trust allowlist to cover every job assuming cudly_deploy cloud-commitments-cli#1683 -- the merged trust allowlist referenced above (note: merged but not yet applied to live AWS at time of writing)

Verification method

All four evidence points are reproducible read-only:

gh api repos/LeanerCloud/CUDly/branches/main/protection
gh api repos/LeanerCloud/CUDly/rulesets --jq '.[]|{name,target,enforcement}'
gh api repos/LeanerCloud/CUDly/rules/branches/main --jq '[.[]|.type]'
ls CODEOWNERS .github/CODEOWNERS docs/CODEOWNERS
sed -n '20,40p' terraform/environments/aws/ci-cd-permissions/role.tf

[cristim]

Correction to evidence point 4: the original text quoted a 4-entry subject allowlist. That was read from role.tf at a stale commit (before LeanerCloud/cloud-commitments-cli#1683 landed). On main (cd4ee031f) the list has 16 entries. Body updated.

This does not change the issue: repo:${var.github_repo}:ref:refs/heads/main is still present, and the finding is about main being an unreviewed and trusted subject, not about the size of the list.

[cristim]

Verified your premise against live repo settings, since the rest of this issue depends on it.

main is unprotected in the sense that matters. There is no classic branch protection at all:

GET /repos/LeanerCloud/CUDly/branches/main/protection  ->  404 Branch not protected

One ruleset is active and it does cover main (its name is protect-master-branch, but its condition is ~DEFAULT_BRANCH, and the default branch is main, so the misleading name is not itself the bug). Its complete rule set is:

GET /repos/LeanerCloud/CUDly/rules/branches/main  ->  deletion, non_fast_forward

So force-pushes and deletion are blocked, and that is all. No required pull request, no required status checks, no required reviews, no bypass restrictions. Anyone with write access can push directly to main with failing CI.

Combined with ref:refs/heads/main being allowlist entry 1 in the cudly-terraform-deploy trust policy, that makes repo write access sufficient to obtain the deploy role, without review and without green checks.

I have separately verified that the deploy role can then create a new role and attach AdministratorAccess to it, with no permissions boundary and no Deny covering that path, and re-triaged #1705 to p0 with the evidence. These two issues are one chain: this issue is the cheap entry, LeanerCloud/cloud-commitments-cli#1705 is the escalation. Fixing either one breaks it, but they should be assessed together rather than independently.

One note on scope for whoever fixes this: adding a required-status-checks rule changes the merge path for every open PR in the repo, so it wants sequencing rather than a drive-by edit. Also worth confirming whether the ruleset should simply be renamed while it is being edited, since protect-master-branch on a main repo invites exactly the misreading I had to rule out here.

[cristim]

Implemented. Ruleset 16369673 updated — main now requires a pull request and passing status checks.

Before

$ gh api repos/LeanerCloud/CUDly/rules/branches/main --jq "[.[]|.type]"
["deletion","non_fast_forward"]

Force-push and deletion blocked; nothing else. Direct pushes with red CI were permitted.

After

rules on main:   deletion, non_fast_forward, pull_request, required_status_checks
required checks: CI Success, Run pre-commit hooks
PR required, approvals=0

Also renamed protect-master-branch -> protect-default-branch. The old name suggested it targeted a master branch that does not exist; its condition was always ~DEFAULT_BRANCH, so the name was misleading rather than wrong, and this issue called that out.

Why these two required checks specifically

CI Success is a pure aggregator: ci-success in ci.yml:594-608 declares needs: on all ten jobs (lint, unit, integration, docker-build, terraform-validate, security-scan, e2e, azure-role-parity, aws-iam-parity, gcp-secret-scope) with if: always(), and fails if any of them failed. Requiring the single aggregate covers all ten without pinning ten names that can be renamed independently.

Both are unconditional. ci.yml and pre-commit.yml each trigger on pull_request: branches: [main] with no paths: filter, so both run on every PR. This mattered: a required check that does not run on some PRs blocks those PRs permanently.

CodeRabbit is deliberately NOT required. Its check entry has been observed with both status and conclusion empty indefinitely, and separately reading success while its description said "Review rate limited". Requiring it would have blocked every merge. Review coverage is verified out-of-band by comparing the review objects commit_id` to the PR head, which is a SHA identity rather than a check state.

strict_required_status_checks_policy is false: branches need not be rebased onto the tip before merging. With a multi-PR queue, strict mode forces a rebase-and-rerun after every merge.

Verified after applying

• fix(scripts): add missing tfvars fields to generate-federation-iac.go cloud-commitments-cli#1710: CI Success=SUCCESS, Run pre-commit hooks=SUCCESS, CLEAN
• fix(purchase): refuse to execute armed pre-#1668 retry successors (#1718) cloud-commitments-cli#1721: both SUCCESS, CLEAN
• sec(iac/aws): gate deploy-role IAM writes on a permissions boundary cloud-commitments-cli#1722: BLOCKED — correctly, because its head had just moved and CI was still running. That is the rule working, not a regression.

What this does NOT do — the open policy question

required_approving_review_count is 0. A pull request is now mandatory and CI must pass, which closes "push a workflow edit straight to main with red CI". It does not add a human gate: one actor can still open a PR and merge it themselves.

I left it at 0 deliberately rather than deciding for you, because raising it to 1 stops the automated merge flow currently landing this queue — there is no second reviewer to approve. Raise it with:

gh api -X PUT repos/LeanerCloud/CUDly/rulesets/16369673 --input <ruleset.json>   # with count: 1

bypass_actors is empty, so the rules apply to admins too. An admin can still edit the ruleset, so this is a speed bump rather than a lockout.

Leaving this issue open pending your call on the approval count, since that is the part of the original concern ("no review, no human gate anywhere in the chain") that a count of 0 does not close.

Relevant to LeanerCloud/cloud-commitments-cli#1705: this was the cheap half of that escalation chain. The precondition was repo write access plus an unprotected main; the second half is now materially harder.

[cristim]

Verified against the split repo (LeanerCloud/cloud-commitments-platform)

Re-checked this issue's premise against the live repo this issue now lives
in, since the split created a new repository that does not inherit the old
monorepo's branch protection or rulesets.

$ gh api repos/LeanerCloud/cloud-commitments-platform/branches/main/protection
{"message":"Branch not protected","status":"404"}

$ gh api repos/LeanerCloud/cloud-commitments-platform/rulesets
[]

$ gh api repos/LeanerCloud/cloud-commitments-platform/rules/branches/main
[]

So the issue is confirmed, and actually worse than the pre-split state this
issue originally described: the old monorepo (LeanerCloud/CUDly) had a
protect-master-branch -> protect-default-branch ruleset requiring a PR
and passing status checks (see this issue's own history). The new split repo
has zero rulesets and zero branch protection -- not even the
"blocks force-push and deletion" floor the old repo started from before this
issue was fixed there.

terraform/environments/aws/ci-cd-permissions/role.tf's trust policy on
main (current HEAD) still includes repo:${var.github_repo}:ref:refs/heads/main
as a trusted OIDC subject, and (per #2) the live AWS/Azure/GCP trusts still
name the old monorepo, so no branch in this repo can currently assume that
role at all -- but that is a separate, orthogonal gap tracked in #2, not a
reason to leave main unprotected. Once #2's bootstrap re-apply lands, this
issue's chain (write access -> push to main -> job runs unreviewed ->
mints deploy credentials) becomes live again immediately unless this is
fixed first or in the same window.

This is a repo-admin action, not something I can fix via API

Per this repo's CLAUDE.md: "Repo settings (branch rulesets, environment
protection rules, secrets) are admin actions. Don't try to change them via
the API." Branch protection / rulesets fall squarely in that category, so I
am not applying the fix myself -- flagging it here with the exact commands a
privileged human should run, mirroring what was already done successfully on
the old repo (see the "Implemented" comment above on this same issue,
against LeanerCloud/CUDly).

Suggested remediation (exact settings)

1. Require a pull request and passing status checks on main. The
   precedent from the old repo's fix:

   # Create (or update) a ruleset targeting the default branch with:
   #   rules: deletion, non_fast_forward, pull_request, required_status_checks
   #   required checks: "CI Success", "Run pre-commit hooks"
   #   (both are unconditional on every PR to main -- no `paths:` filter --
   #   so neither can permanently block a PR the way a filtered check would)
   #   strict_required_status_checks_policy: false (no forced rebase-before-merge)
   #   bypass_actors: [] (rules apply to admins too)
   gh api -X POST repos/LeanerCloud/cloud-commitments-platform/rulesets \
     --input ruleset.json

   required_approving_review_count is a genuine open policy question, not
   a default to pick silently: 0 closes "push straight to main with red
   CI" but does not add a second-party human gate ("one actor opens a PR and
   merges it themselves" stays possible). The old repo's fix deliberately
   left it at 0 and left the issue open pending that call -- worth the same
   explicit decision here rather than copying a number.

2. Add a CODEOWNERS entry for .github/workflows/** and
   terraform/environments/*/ci-cd-permissions/** so credential-granting
   configuration needs a named reviewer. I am not adding this file myself:
   it needs a real GitHub username or team as the owner, and guessing one
   risks assigning review responsibility to the wrong person silently. Once
   review is required (step 1), a human should add this with the correct
   owner(s).

3. This is sequenced with sec(iac): rename-proof OIDC deploy trust + branch-restricted environments for the split platform repo #2 and sec(ci): deployment environments have no protection rules, so environment-bound credentialed jobs run unapproved #141: protecting main narrows who
   can reach the ref:refs/heads/main OIDC subject, but that subject and
   the environments it's paired with also need sec(iac): rename-proof OIDC deploy trust + branch-restricted environments for the split platform repo #2's bootstrap re-apply
   (rename-proof trust, this repo's name) and sec(ci): deployment environments have no protection rules, so environment-bound credentialed jobs run unapproved #141's environment protection
   rules before the full chain this issue describes is actually closed.

Labeling status/blocked since the fix is a repo-settings change outside
what I can apply.

[cristim]

Applied on 2026-10-04.

Created ruleset 24469860 protect-default-branch on this repo as a copy of cloud-commitments-cli's: main now requires a pull request (0 approvals) and the checks CI Success and Run pre-commit hooks, blocks deletion and force-push, and has no bypass actors.

$ gh api repos/LeanerCloud/cloud-commitments-platform/rules/branches/main --jq '[.[].type]|sort'
["deletion","non_fast_forward","pull_request","required_status_checks"]

Both required checks report on PRs here, so merging is not deadlocked.

Limits: 0 required approvals and one admin, so this means "PR plus green CI", not "reviewed". Same trade-off as the cli repo. Not closing; leaving that to the owner.