Skip to content

Improve actions setup, add static analysis#416

Merged
simolus3 merged 2 commits into
mainfrom
actions-improvements
May 14, 2026
Merged

Improve actions setup, add static analysis#416
simolus3 merged 2 commits into
mainfrom
actions-improvements

Conversation

@simolus3
Copy link
Copy Markdown
Contributor

@simolus3 simolus3 commented May 12, 2026

This adopts zizmor as a static analysis tool for GitHub actions workflows in this repository, and fixes most lints. I'm mainly opening this to start a discussion - is this a tool we want to use or are the reports too noisy to be useful for us?

In this repository, the default configuration flagged:

  1. Using actions without pinned refs: Zizmor warns about using actions like actions/checkout@v6 since v6 is a mutable tag that could change between runs. GitHub allows us to forbid mutable references altogether, too. IMHO, this is a bit too strict by default:
    • Is someone managed to attack actions/checkout, we'd have bigger problems. So I think explicitly using the latest version makes sense.
    • It's a similar story for Homebrew and Dart. We use whatever they ship anyway, so pinning an action doesn't help us at all.
    • subosito/flutter-action is community-maintained and adding an explicit ref can make sense for those.
  2. To keep up with changes to ref-pinned actions dependencies, this configures dependabot. Zizmor also checks these configurations, and enforces a cooldown period for instance.
  3. Workflows without an explicit permissions block since the default value is too broad. This makes sense to fix, and we also get warnings from GitHub about this.
  4. actions/checkout uses without persist-credentials: false. That's a good point IMO, we don't need these credentials.
  5. ${{ ... }} expansions in scripts since it could "lead to running attacker-controlled code". This is a false-positive in publish.yml which was flagged here, but it can still be cleaned up with relatively little effort.

Zizmor can also be configured with a more strict ruleset (zizmor --persona pedantic .). In this repository, that additionally flags:

  • Jobs without a concurrency setting (only affects the publishing workflow in this repository which is harmless).
  • Jobs without an explicit name (this doesn't feel that useful to me).
  • Permissions without a YAML comment next to them explaining why they're necessary (this one seems helpful).

@github-advanced-security

This comment was marked as off-topic.

Copy link
Copy Markdown
Contributor

@rkistner rkistner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with these checks and changes.

Just to check - it looks like this also adds dependabot config to semi-automatically update github action versions?

@simolus3
Copy link
Copy Markdown
Contributor Author

it looks like this also adds dependabot config to semi-automatically update github action versions?

Yes, I should have mentioned that in the PR description. My thinking is that dependabot would create PRs to update refs since we no longer get updates for new minor releases (unlike with @v2 which resolves to the latest compatible version on each run). That still has some risk I suppose since these PRs would run with the context of the repository, but the empty permission set should cover that (and we get to check changes to actions before merging).

Copy link
Copy Markdown
Contributor

@stevensJourney stevensJourney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like these checks, I'd be happy if we added these to our other repos :)

@simolus3 simolus3 merged commit 7d65f70 into main May 14, 2026
13 checks passed
@simolus3 simolus3 deleted the actions-improvements branch May 14, 2026 14:34
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.

4 participants