Repository navigation
sec(ci): main is unprotected while ref:refs/heads/main is a trusted OIDC subject for the AWS deploy role #154
Description
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: writeor repository secrets. - Severity is
highrather thancriticalon 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:
- Require pull requests plus at least one approving review on
main(either as a ruleset rule or classic protection). The existingprotect-master-branchruleset appears to be the intended home and is currently near-empty formain. - Add a
CODEOWNERSentry for.github/workflows/**andterraform/environments/*/ci-cd-permissions/**so credential-granting configuration needs a named reviewer. - Configure required reviewers on the
prodEnvironment at minimum (this is LeanerCloud/cloud-commitments-cli#1648). - Consider whether
ref:refs/heads/mainneeds to remain a trusted subject at all, or whether every AWS-assuming job could bind anenvironment: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: writefrom the two ungated jobs indeploy-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
Activity
cristim commentedon Aug 3, 2026
Correction to evidence point 4: the original text quoted a 4-entry subject allowlist. That was read from
role.tfat a stale commit (before LeanerCloud/cloud-commitments-cli#1683 landed). Onmain(cd4ee031f) the list has 16 entries. Body updated.This does not change the issue:
repo:${var.github_repo}:ref:refs/heads/mainis still present, and the finding is aboutmainbeing an unreviewed and trusted subject, not about the size of the list.cristim commentedon Aug 3, 2026
Verified your premise against live repo settings, since the rest of this issue depends on it.
mainis unprotected in the sense that matters. There is no classic branch protection at all:One ruleset is active and it does cover
main(its name isprotect-master-branch, but its condition is~DEFAULT_BRANCH, and the default branch ismain, so the misleading name is not itself the bug). Its complete rule set is: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
mainwith failing CI.Combined with
ref:refs/heads/mainbeing allowlist entry 1 in thecudly-terraform-deploytrust 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
AdministratorAccessto 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-branchon amainrepo invites exactly the misreading I had to rule out here.cristim commentedon Aug 6, 2026
Implemented. Ruleset
16369673updated —mainnow requires a pull request and passing status checks.Before
Force-push and deletion blocked; nothing else. Direct pushes with red CI were permitted.
After
Also renamed
protect-master-branch->protect-default-branch. The old name suggested it targeted amasterbranch 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 Successis a pure aggregator:ci-successinci.yml:594-608declaresneeds:on all ten jobs (lint, unit, integration, docker-build, terraform-validate, security-scan, e2e, azure-role-parity, aws-iam-parity, gcp-secret-scope) withif: 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.ymlandpre-commit.ymleach trigger onpull_request: branches: [main]with nopaths:filter, so both run on every PR. This mattered: a required check that does not run on some PRs blocks those PRs permanently.CodeRabbitis deliberately NOT required. Its check entry has been observed with bothstatusandconclusionempty indefinitely, and separately readingsuccesswhile its description said "Review rate limited". Requiring it would have blocked every merge. Review coverage is verified out-of-band by comparing the review objectscommit_id` to the PR head, which is a SHA identity rather than a check state.strict_required_status_checks_policyis 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
CI Success=SUCCESS,Run pre-commit hooks=SUCCESS,CLEANSUCCESS,CLEANBLOCKED— 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_countis 0. A pull request is now mandatory and CI must pass, which closes "push a workflow edit straight tomainwith 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:
bypass_actorsis 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 commentedon Sep 28, 2026
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.
So the issue is confirmed, and actually worse than the pre-split state this
issue originally described: the old monorepo (
LeanerCloud/CUDly) had aprotect-master-branch->protect-default-branchruleset requiring a PRand 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 onmain(current HEAD) still includesrepo:${var.github_repo}:ref:refs/heads/mainas 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
mainunprotected. Once #2's bootstrap re-apply lands, thisissue'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, environmentprotection 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)
Require a pull request and passing status checks on
main. Theprecedent from the old repo's fix:
required_approving_review_countis a genuine open policy question, nota default to pick silently: 0 closes "push straight to
mainwith redCI" 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.
Add a
CODEOWNERSentry for.github/workflows/**andterraform/environments/*/ci-cd-permissions/**so credential-grantingconfiguration 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).
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
mainnarrows whocan reach the
ref:refs/heads/mainOIDC subject, but that subject andthe 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/blockedsince the fix is a repo-settings change outsidewhat I can apply.
cristim commentedon Oct 4, 2026
Applied on 2026-10-04.
Created ruleset
24469860protect-default-branchon this repo as a copy ofcloud-commitments-cli's:mainnow requires a pull request (0 approvals) and the checksCI SuccessandRun pre-commit hooks, blocks deletion and force-push, and has no bypass actors.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.
cristim commentedon Oct 7, 2026
Re-verified the residual gap against live repo settings on 2026-10-07 (worker cc-platform-w4, claim).
Remediation item 1 (PR + checks on
main) is live: ruleset24469860protect-default-branch,enforcement: active, contributing[deletion,non_fast_forward,pull_request,required_status_checks]tomain,bypass_actors: [].What still keeps this open, all of it an owner decision rather than a code change:
required_approving_review_count: 0andrequire_code_owner_review: false. So the current posture is "PR plus green CI", not "reviewed"..github/ordocs/, so.github/workflows/**andterraform/environments/*/ci-cd-permissions/**have no named reviewer.prod,staging,dev,{aws,azure,gcp}-{db,fargate}-{dev,staging,prod}) report 0required_reviewersand 0wait_timerprotection rules. Tracked as LeanerCloud/cloud-commitments-cli#1648.Parked, not closed: the owner already declined to close this on 2026-10-04 with the same limits noted. Item 1's approval count and item 3 are owner decisions, so a worker cannot land them. The CODEOWNERS file (item 2) is the only repo-local piece and is safe to add without changing merge behaviour while
require_code_owner_reviewstays false.