Skip to content

Fix findings of the workflow linters - #341

Open
ondrejmirtes wants to merge 1 commit into
2.4.xfrom
fix-workflow-lint-findings
Open

ondrejmirtes wants to merge 1 commit into
2.4.xfrom
fix-workflow-lint-findings

Conversation

@ondrejmirtes

Copy link
Copy Markdown
Member

Fixes what the linters in lint-workflows.yml report. I ran each one locally the way CI runs it: actionlint 1.7.12 with shellcheck, octoscan through the action's own entrypoint.sh with the same inputs, poutine 1.1.4, and zizmor 1.31.1 with --persona=auditor.

Linter Before After
actionlint 0 0
octoscan 6 0
poutine 25 0
zizmor (auditor) 68 0 (2 ignored inline, with reasons)

What changed

  • Credentials: checkouts set persist-credentials: false, except in create-tag.yml. There, rickstaa/action-create-tag pushes the tag with the persisted PHPSTAN_BOT_TOKEN, and only a tag pushed by that token, not by GITHUB_TOKEN, triggers release.yml. This is now explicit and commented. Every other step that writes uses its own token through the API or an input.
  • Permissions: every workflow defaults to permissions: {}, and each job is granted what it uses, with a comment. The default token was already read-only (contents, metadata, packages: read), so in practice only the unused packages: read is dropped.
  • Concurrency: in PR and push workflows, a new push to a pull request cancels that PR's previous run. Each push to 2.4.x still gets its own run. Release workflows (tags, releases, dispatch, schedule, merge of 1.22.x) queue new runs with cancel-in-progress: false instead of cancelling.
  • Template injection: matrix.branch in apiref.yml and github.ref_name in send-pr.yml reach the shell through env.
  • Job names: added to the jobs that had none. No required status checks depend on job names.
  • Release: softprops/action-gh-release is replaced by gh release create "$TAG" --verify-tag --title "$TAG" --notes "$CHANGELOG", using the same bot token. peter-evans/create-pull-request stays, because it commits, pushes a branch and opens the PR in one step. Its zizmor superfluous-actions finding is ignored inline.
  • actionlint download: a pinned release tarball whose checksum is verified, instead of bash <(curl …/main/…).

Suppressions (please review)

  • .github/poutine.yml skips github_action_from_unverified_creator_used for an explicit list of the pinned actions we use, such as setup-php, lock-threads, and the release/tag actions.
  • .github/zizmor.yml allows the repository secrets used outside environments (secrets-outside-env). Fixing this properly means moving the secrets into dedicated environments in the repository settings first.
  • octoscan's dangerous-checkout flags every ref that isn't made of letters and dashes only, so 2.3.x, 2.x, a pinned SHA and matrix.branch all count. octoscan can only ignore findings by message, not by file, so this message is now ignored globally. zizmor and poutine still check for checkouts of untrusted code.

The release workflows (release, send-pr, create-tag, toot/tweet) can't run from a PR, so the release changes are verified only by reading the actions' sources.

actionlint, octoscan, poutine and zizmor (auditor persona) now report nothing.

- Check out without persisting credentials, except in create-tag, where
  rickstaa/action-create-tag pushes the tag with the bot token so that the
  push triggers release.yml
- Default every workflow to no permissions and grant each job what it uses
- Add concurrency: superseded pull request runs are cancelled, pushes to
  2.4.x each keep their own run, and release workflows are queued, not cancelled
- Pass matrix.branch and github.ref_name to scripts through env
- Name the remaining anonymous jobs
- Create the release with gh release create instead of softprops/action-gh-release
- Download a pinned actionlint release and verify its checksum instead of
  piping a script from main into bash
- Allow the reviewed actions from unverified creators in .github/poutine.yml
  and the repository secrets used outside environments in .github/zizmor.yml
- Ignore octoscan's dangerous-checkout message: every flagged ref is a fixed
  branch or commit, and octoscan cannot ignore findings per file

Amp-Thread-ID: https://ampcode.com/threads/T-01a197a6-ea3e-77f5-80d6-1bebc3252d55

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant