Repository navigation
chore(ci): finish the #1641 sweep - 159 ${{ }} interpolations remain in run: blocks across 7 workflows #155
Description
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. Viaenv:it is inert data. - Several of these jobs hold
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 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
- fix(ci): stop interpolating dispatch inputs into rollback run blocks cloud-commitments-cli#1641 -- established the rule; fixed
rollback.ymlandcleanup-staging.yml - fix(ci): stop interpolating the release tag into a run block cloud-commitments-cli#1657 -- fixes
deploy-aws-lambda.yml(23 of the 159) - 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 -- the release-tag injection that motivated fix(ci): stop interpolating the release tag into a run block cloud-commitments-cli#1657
- sec(ci): main is unprotected while ref:refs/heads/main is a trusted OIDC subject for the AWS deploy role #154 --
mainunprotected whileref:refs/heads/mainis a trusted OIDC subject; the reason code execution in these jobs matters
Activity
fix(deps): bump cloud-commitments-go providers/gcp to a32fd1a (#671)