Skip to content

chore(ci): no workflow linter runs, so Actions expression injection is undetectable #1646

Closed

Description

@cristim

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 actionlint and no zizmor in .github/workflows/ci.yml or in .pre-commit-config.yaml.
  • The Security Scanning job (ci.yml:282) runs govulncheck and gosec. Both are Go source scanners; neither opens a workflow file.
  • The trivy-config pre-commit hook targets Terraform / Dockerfile / Kubernetes misconfiguration, not Actions expression injection.
  • The check-yaml hook 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"
done

The backlog is almost entirely shellcheck SC2086 (unquoted expansion, mostly >> $GITHUB_STEP_SUMMARY) and SC2129 (consecutive redirects), both info/style level — mechanical to clear.

Proposed

  1. 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.
  2. Add actionlint to .pre-commit-config.yaml via the upstream rhysd/actionlint hook, pinned to an explicit version per the repo's no-@latest convention. shellcheck must 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.
  3. Add zizmor. This is the one that actually closes the detection gap: actionlint only catches this class indirectly, via shellcheck on the expanded script. zizmor has a dedicated template-injection rule that names the defect, plus dangerous-triggers and excessive-permissions — the latter two would also have flagged the workflow-level id-token: write in 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.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions