Repository navigation
chore(ci): no workflow linter runs, so Actions expression injection is undetectable #1646
Description
This is the systemic finding behind #1542, #1647 and #1649. Those three are instances; this is the reason all of them were invisible. Fixing them individually removes three bugs — adding a workflow linter prevents the entire class, including the next one nobody has found yet. Prioritise accordingly.
What
Nothing in this repo inspects .github/workflows/*.yml for GitHub Actions expression injection, or for any other workflow-level defect.
- No
actionlintand nozizmorin.github/workflows/ci.ymlor in.pre-commit-config.yaml. - The
Security Scanningjob (ci.yml:282) runsgovulncheckandgosec. Both are Go source scanners; neither opens a workflow file. - The
trivy-configpre-commit hook targets Terraform / Dockerfile / Kubernetes misconfiguration, not Actions expression injection. - The
check-yamlhook only proves the YAML parses.
That is why #1542 (critical, shell injection reaching production cloud credentials) and #1649 (critical, same shape via a release tag) both sat unflagged through every CI run on every push.
Evidence it would have been caught
actionlint with shellcheck on PATH, run against the pre-fix rollback.yml: exit 1. Against the fixed version (PR #1641): exit 0.
Current scope — 8 of 16 files fail today
Measured against origin/main @ 887d51fd6. A CI gate added without clearing these first would land red and get reverted, so the batch fix is part of this issue, not a follow-up:
| file | actionlint | note |
|---|---|---|
ci.yml |
exit 1 | |
database-migration.yml |
exit 1 | also #1647 |
deploy-all.yml |
exit 1 | |
deploy-aws-fargate.yml |
exit 1 | |
deploy-aws-lambda.yml |
exit 1 | 32 findings; PR #1657 takes it to 26, all pre-existing |
deploy-azure.yml |
exit 1 | |
deploy-gcp.yml |
exit 1 | |
rollback.yml |
exit 1 | → exit 0 once PR #1641 merges |
aws_sanity.yml, azure_sanity.yml, cleanup-staging.yml, destroy-fargate-dev.yml, frontend-build.yml, frontend-build-sentinel.yml, frontend-e2e.yml, pre-commit.yml |
exit 0 | already clean |
Reproduce (note: capture rc=$? on its own statement — a $(basename …) in the same line overwrites $? and reports every file as passing):
for f in .github/workflows/*.yml; do
actionlint "$f" >/dev/null 2>&1; rc=$?
b=$(basename "$f"); [ $rc -ne 0 ] && echo "FAIL: $b"
doneThe backlog is almost entirely shellcheck SC2086 (unquoted expansion, mostly >> $GITHUB_STEP_SUMMARY) and SC2129 (consecutive redirects), both info/style level — mechanical to clear.
Proposed
- Clear the findings in the 8 files above in one batch commit. Do not gate-suppress them (
only-new-issues, blanket disables) — that is the masking-CI-debt anti-pattern. - Add
actionlintto.pre-commit-config.yamlvia the upstreamrhysd/actionlinthook, pinned to an explicit version per the repo's no-@latestconvention.shellcheckmust be available or actionlint silently skips shell linting — which would have missed sec(ci): rollback.yml interpolates the free-text reason input into run blocks in a job holding id-token write #1542. - Add
zizmor. This is the one that actually closes the detection gap:actionlintonly catches this class indirectly, via shellcheck on the expanded script.zizmorhas a dedicatedtemplate-injectionrule that names the defect, plusdangerous-triggersandexcessive-permissions— the latter two would also have flagged the workflow-levelid-token: writein sec(ci): deploy-aws-lambda.yml interpolates a release tag name into a run block in an ungated job holding id-token write #1649 and sec(ci): database-migration.yml interpolates inputs into run blocks behind a post-interpolation guard #1647.
Instances of the class found so far: #1542 (fixed, PR #1641), #1649 (fixed, PR #1657), #1647 (open). Related: #1591, LeanerCloud/cloud-commitments-platform#139.
Activity
fix(ci): stop interpolating the release tag into a run block
fix(ci): stop interpolating the release tag into a run block (#1657)
fix(ci): stop interpolating the release tag into a run block (#1657)