Lint Github Actions workflows with zizmor #3188
Conversation
This fixes issues identified by the zizmor linter which checks for Github Actions security best practicies. Summary of changes: - Remove possibilities for shell injection. These can all only be activated by workflow_dispatch input provided by people who already have access to the repository but still a good idea to tidy this up. Many of these occur in the build-package-docs actions. We should test everything to make sure nothing is broken by these changes. - Explicitly set permissions. This is not strictly required, because we already enforce a limited set of default permissions in the repo's GHA settings, but zizmor wants us to be explicit. - Use `persist-credentials: false` with the checkout action.
- Adds lockfile - Adds nox session - Adds nox session to CI matrix
- Add default permissions to new workflows - Add cooldown to dependabot
We could configure dependabot to pin shared workflow commit SHA hashes, but for now, let's relax the unpinned-uses relax
See comment for more details.
|
I tested the pip-compile workflow with #3189 and https://github.com/ansible/ansible-documentation/actions/runs/18857876754/job/53809921493. It seems to have worked. |
| - session: "zizmor" | ||
| python-versions: "3.12" |
There was a problem hiding this comment.
We could potentially remove this in favor of the workflow added in #3141 and just keep the zizmor in the noxfile for contributors to run locally.
Although, I'm concerned that:
- The zizmor workflow in Add Zizmor Scanning on push/pull request #3141 doesn't make CI red even though there are failures.
- If we have to manage the zizmor version pinned in the zizmor workflow and the version used by the nox job, they may get out of sync, which could be confusing for developers, as they'll get different results when running locally vs. CI.
- I'm not sure how much the integration with Github Code Scanning is helpful to our community. Case in point, @x1101 spent time contributing the workflow (thank you!), but they can't even see the failures it reported. I personally think it's better to integrate zizmor with our existing nox setup but am happy to be convinced otherwise.
|
Test docs deploy to the staging deployment environment: https://github.com/ansible/ansible-documentation/actions/runs/18858431601 |
There was a problem hiding this comment.
Thanks for doing all this work @gotmax23 Honestly I do prefer the approach to using a nox session over a workflow because it's consistent with the pattern we've taken with tooling.
I also agree with point 3 and the integration with GitHub Code Scanning. I think this actually caused a bit of uncertainty when @x1101 started on #3141
On a related note I'd like to ask that we give attribution in a commit message somewhere if this PR is accepted over #3141. @x1101 put in a solid effort and ultimately led us to a change that is going to improve the toolchain.
| @nox.session | ||
| def zizmor(session: nox.Session) -> None: | ||
| """ | ||
| Ren zizmor, a Github Actions security checker |
There was a problem hiding this comment.
| Ren zizmor, a Github Actions security checker | |
| Run zizmor, a Github Actions security checker |
| Ren zizmor, a Github Actions security checker | ||
| """ | ||
| install(session, req="zizmor") | ||
| session.run("zizmor", "--persona=regular", ".github/workflows") |
There was a problem hiding this comment.
Would it make sense to allow passing the persona as an arg?
It could be persona by default but perhaps there could be cases where I'd want to use this session to conduct an audit and set --persona=auditor. I think this flexibility would give the nox session approach more of an advantage over the workflow.
2 participants
Fixes: #2681
See commit messages for more details. I pushed this to the zizmor2 branch on the main repository, so we can test the updated workflows with workflow_dispatch.