Repository navigation
sec(ci): main is unprotected while ref:refs/heads/main is a trusted OIDC subject for the AWS deploy role #154
Description
Activity
- addedtriagedItem has been triagedItem has been triagedpriority/p1Next up; this sprintNext up; this sprintseverity/highSignificant harmSignificant harmurgency/this-sprintWithin the current sprintWithin the current sprintimpact/internalTeam-internal onlyTeam-internal onlyeffort/sHoursHourstype/securitySecurity findingSecurity finding
on 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.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:GET /repos/LeanerCloud/CUDly/branches/main/protection -> 404 Branch not protectedOne 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:GET /repos/LeanerCloud/CUDly/rules/branches/main -> deletion, non_fast_forwardSo 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.Implemented. Ruleset
16369673updated —mainnow 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=0Also 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
- 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_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:
gh api -X PUT repos/LeanerCloud/CUDly/rulesets/16369673 --input <ruleset.json> # with count: 1bypass_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.- fix(scripts): add missing tfvars fields to generate-federation-iac.go cloud-commitments-cli#1710:
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-branchruleset 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 includesrepo:${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 leavemainunprotected. Once #2's bootstrap re-apply lands, this
issue's chain (write access -> push tomain-> 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,
againstLeanerCloud/CUDly).Suggested remediation (exact settings)
-
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_countis a genuine open policy question, not
a default to pick silently: 0 closes "push straight tomainwith 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. -
Add a
CODEOWNERSentry 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). -
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 who
can reach theref:refs/heads/mainOIDC 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/blockedsince the fix is a repo-settings change outside
what I can apply.-
- addedstatus/blockedBlocked on something externalBlocked on something external
on Sep 28, 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.$ 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.
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".- No CODEOWNERS file exists at the repo root,
.github/ordocs/, so.github/workflows/**andterraform/environments/*/ci-cd-permissions/**have no named reviewer. - No Environment has required reviewers: all 14 (
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.
Summary
mainhas no required-review protection, whilerepo:LeanerCloud/CUDly:ref:refs/heads/mainis a trusted OIDC subject for thecudly-terraform-deployAWS role. Anyone with write access can therefore push a workflow edit directly tomainand 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.
mainis not protected. Classic branch protection:Checked with a token reporting
"admin": trueon 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-branchisenforcement: active, but the rules it actually contributes tomainare:That blocks deleting
mainand 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 amasterbranch that no longer exists.3. No CODEOWNERS. No
CODEOWNERSfile at the repo root, in.github/, or indocs/. Nothing gives.github/workflows/**special review treatment.4.
mainis a trusted OIDC subject.terraform/environments/aws/ci-cd-permissions/role.tfonmain(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 theref:refs/heads/mainentry is relevant here: a workflow job running on apushtomainwithout anenvironment:binding presentssub = repo:LeanerCloud/CUDly:ref:refs/heads/mainand matches it.Reproduce the full list with:
Why it matters
The chain is: write access -> push a workflow to
main-> job runs withid-token: write-> OIDC subref:refs/heads/main-> assumecudly-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:
id-token: writeor repository secrets.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:
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.CODEOWNERSentry for.github/workflows/**andterraform/environments/*/ci-cd-permissions/**so credential-granting configuration needs a named reviewer.prodEnvironment at minimum (this is LeanerCloud/cloud-commitments-cli#1648).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
id-token: writefrom the two ungated jobs indeploy-aws-lambda.ymlVerification method
All four evidence points are reproducible read-only: