Summary
LeanerCloud/cloud-commitments-cli#1641 established the rule that untrusted values must reach a run: block through env:, never through ${{ }} interpolation, because interpolation splices the value into the shell script as source text before bash ever sees it. The rule has been applied file by file. 159 interpolations remain inside run: blocks across 7 workflow files on main.
This issue is the census, so whoever finishes the sweep can see the whole surface and reproduce the count.
Current state on main
| count |
workflow |
notes |
| 44 |
database-migration.yml |
|
| 26 |
deploy-all.yml |
release-triggered |
| 24 |
deploy-azure.yml |
|
| 23 |
deploy-aws-lambda.yml |
fixed by LeanerCloud/cloud-commitments-cli#1657 (open at time of writing) |
| 19 |
deploy-aws-fargate.yml |
|
| 18 |
deploy-gcp.yml |
|
| 5 |
ci.yml |
|
| 159 |
7 of 16 workflow files |
|
Already clean, for reference: rollback.yml and cleanup-staging.yml were fixed by LeanerCloud/cloud-commitments-cli#1641 itself (5f850f214), and destroy-fargate-dev.yml, aws_sanity.yml, azure_sanity.yml and the remaining files have none.
Once LeanerCloud/cloud-commitments-cli#1657 merges this drops to 136 across 6 files.
How the count was produced
Deliberately not grep '\${{'. A grep cannot distinguish a run: body from an env: value, a with: input, an if: condition, or a comment, and most ${{ }} in these files are in positions where it is correct and expected. The count comes from parsing the YAML and walking only jobs.*.steps[].run scalars:
import yaml, re
INTERP = re.compile(r"\$\{\{")
doc = yaml.safe_load(open(path))
for job_name, job in (doc.get("jobs") or {}).items():
for step in (job.get("steps") or []):
run = step.get("run")
if isinstance(run, str):
for lineno, line in enumerate(run.splitlines(), 1):
if INTERP.search(line):
print(job_name, step.get("name"), lineno, line.strip())
Negative control (worth keeping when re-running this): point the same parser at a known-bad file before trusting a 0. Run against deploy-aws-lambda.yml at LeanerCloud/cloud-commitments-cli#1657's merge-base it reports 23 hits; against LeanerCloud/cloud-commitments-cli#1657's head it reports 0. A parser that reports 0 everywhere because it silently failed to find jobs looks identical to a clean repo.
Why this is worth finishing
Not every one of the 159 is exploitable. Many interpolate values that are not attacker-influenced (github.sha, needs.*.result). The reason to finish the sweep anyway is that the safe ones and the dangerous ones are indistinguishable at a glance, so the invariant "no interpolation in run:" is checkable while "no interpolation of untrusted values in run:" requires re-deriving the trust of every value on every read. LeanerCloud/cloud-commitments-cli#1657's own diff makes this point in a comment: relying on "an upstream job validated this" makes the rule "raw interpolation is fine when someone else checked", which breaks silently the moment the upstream guard moves.
Two properties make the difference concrete:
- A value interpolated into a
run: block is shell source, so a $(...), backtick, quote, or newline in it is executed or breaks parsing. Via env: it is inert data.
- Several of these jobs hold
id-token: write and can assume the AWS deploy role, so code execution in them is credential access, not just a broken build.
Useful context for urgency
The release path has never fired. The repository has zero remote tags and zero GitHub releases (git ls-remote --tags origin is empty; gh release list is empty). The four tags that exist are local-only backup tags that were never pushed. So release-triggered interpolation paths are latent rather than exercised, which argues for doing this properly rather than urgently.
Suggested approach
One PR per workflow file, matching how LeanerCloud/cloud-commitments-cli#1641 and LeanerCloud/cloud-commitments-cli#1657 have done it. Each is mechanical and independently reviewable: move each interpolated value to a job- or step-level env: entry and reference it quoted ("$VAR"). Nothing else.
In particular, do not add set -euo pipefail to blocks that lack it as part of the same change. It is worth doing, but it alters whether a step fails on a non-zero intermediate command, so it turns a diff a reviewer can check by inspection into one that needs the workflow exercised, on workflows that deploy. Separate pass, separate PR.
Remedy simplified (2026-08-03): the set -euo pipefail addition was split out of the sweep. It is a behaviour change riding along with a mechanical one, and it is the part that would stop each PR from being reviewable by inspection.
A lint step that runs the parser above over .github/workflows/ and fails on any hit would keep the count at zero once the sweep lands. That is probably worth a follow-up issue rather than part of this one.
Related
Summary
LeanerCloud/cloud-commitments-cli#1641 established the rule that untrusted values must reach a
run:block throughenv:, never through${{ }}interpolation, because interpolation splices the value into the shell script as source text before bash ever sees it. The rule has been applied file by file. 159 interpolations remain insiderun:blocks across 7 workflow files onmain.This issue is the census, so whoever finishes the sweep can see the whole surface and reproduce the count.
Current state on
maindatabase-migration.ymldeploy-all.ymldeploy-azure.ymldeploy-aws-lambda.ymldeploy-aws-fargate.ymldeploy-gcp.ymlci.ymlAlready clean, for reference:
rollback.ymlandcleanup-staging.ymlwere fixed by LeanerCloud/cloud-commitments-cli#1641 itself (5f850f214), anddestroy-fargate-dev.yml,aws_sanity.yml,azure_sanity.ymland the remaining files have none.Once LeanerCloud/cloud-commitments-cli#1657 merges this drops to 136 across 6 files.
How the count was produced
Deliberately not
grep '\${{'. A grep cannot distinguish arun:body from anenv:value, awith:input, anif:condition, or a comment, and most${{ }}in these files are in positions where it is correct and expected. The count comes from parsing the YAML and walking onlyjobs.*.steps[].runscalars:Negative control (worth keeping when re-running this): point the same parser at a known-bad file before trusting a
0. Run againstdeploy-aws-lambda.ymlat LeanerCloud/cloud-commitments-cli#1657's merge-base it reports 23 hits; against LeanerCloud/cloud-commitments-cli#1657's head it reports 0. A parser that reports 0 everywhere because it silently failed to findjobslooks identical to a clean repo.Why this is worth finishing
Not every one of the 159 is exploitable. Many interpolate values that are not attacker-influenced (
github.sha,needs.*.result). The reason to finish the sweep anyway is that the safe ones and the dangerous ones are indistinguishable at a glance, so the invariant "no interpolation inrun:" is checkable while "no interpolation of untrusted values inrun:" requires re-deriving the trust of every value on every read. LeanerCloud/cloud-commitments-cli#1657's own diff makes this point in a comment: relying on "an upstream job validated this" makes the rule "raw interpolation is fine when someone else checked", which breaks silently the moment the upstream guard moves.Two properties make the difference concrete:
run:block is shell source, so a$(...), backtick, quote, or newline in it is executed or breaks parsing. Viaenv:it is inert data.id-token: writeand can assume the AWS deploy role, so code execution in them is credential access, not just a broken build.Useful context for urgency
The release path has never fired. The repository has zero remote tags and zero GitHub releases (
git ls-remote --tags originis empty;gh release listis empty). The four tags that exist are local-only backup tags that were never pushed. So release-triggered interpolation paths are latent rather than exercised, which argues for doing this properly rather than urgently.Suggested approach
One PR per workflow file, matching how LeanerCloud/cloud-commitments-cli#1641 and LeanerCloud/cloud-commitments-cli#1657 have done it. Each is mechanical and independently reviewable: move each interpolated value to a job- or step-level
env:entry and reference it quoted ("$VAR"). Nothing else.In particular, do not add
set -euo pipefailto blocks that lack it as part of the same change. It is worth doing, but it alters whether a step fails on a non-zero intermediate command, so it turns a diff a reviewer can check by inspection into one that needs the workflow exercised, on workflows that deploy. Separate pass, separate PR.Remedy simplified (2026-08-03): the
set -euo pipefailaddition was split out of the sweep. It is a behaviour change riding along with a mechanical one, and it is the part that would stop each PR from being reviewable by inspection.A lint step that runs the parser above over
.github/workflows/and fails on any hit would keep the count at zero once the sweep lands. That is probably worth a follow-up issue rather than part of this one.Related
rollback.ymlandcleanup-staging.ymldeploy-aws-lambda.yml(23 of the 159)mainunprotected whileref:refs/heads/mainis a trusted OIDC subject; the reason code execution in these jobs matters