-
Notifications
You must be signed in to change notification settings - Fork 745
Lint Github Actions workflows with zizmor #3188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: devel
Are you sure you want to change the base?
Changes from all commits
7830f43
c920ae8
7fd909e
05a1858
234868e
3623b3f
1656b3f
5f38280
116dce7
062e554
041e7a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,3 +9,5 @@ updates: | |
| directory: "/" | ||
| schedule: | ||
| interval: "weekly" | ||
| cooldown: | ||
| default-days: 4 | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -32,9 +32,6 @@ name: Build docs | |||||
| DOCS_BOT_TOKEN: | ||||||
| required: true | ||||||
|
|
||||||
| env: | ||||||
| PACKAGE_VERSION: ${{ inputs.ansible-package-version }} | ||||||
|
|
||||||
| jobs: | ||||||
| build-package-docs: | ||||||
| runs-on: ubuntu-latest | ||||||
|
|
@@ -50,26 +47,35 @@ jobs: | |||||
| }} | ||||||
| ref: ${{ inputs.repository-branch }} | ||||||
| path: build-directory | ||||||
| persist-credentials: false | ||||||
|
|
||||||
| - name: Setup nox | ||||||
| uses: wntrblm/nox@2025.10.16 | ||||||
|
|
||||||
| - name: Output Python info | ||||||
| run: python --version --version && which python | ||||||
|
|
||||||
| - name: Set the VERSION variable | ||||||
| run: echo VERSION="${PACKAGE_VERSION}" >> "${GITHUB_ENV}" | ||||||
| - name: Graft ansible-core | ||||||
| run: nox -s clone-core | ||||||
| working-directory: build-directory | ||||||
|
|
||||||
| - name: Build the Ansible community package docs | ||||||
| env: | ||||||
| PACKAGE_VERSION: >- | ||||||
| ${{ | ||||||
| inputs.ansible-package-version != 'devel' | ||||||
| && inputs.ansible-package-version | ||||||
| || '' | ||||||
| }} | ||||||
| run: >- | ||||||
| nox -e make -- webdocs ${{ | ||||||
| nox -e make -- webdocs ANSIBLE_VERSION="${PACKAGE_VERSION}" ${{ | ||||||
| inputs.generate-redirects && 'EXTRA_TAGS="-t redirects"' || '' | ||||||
| }} ANSIBLE_VERSION="${{ | ||||||
| env.PACKAGE_VERSION != 'devel' && env.PACKAGE_VERSION || '' | ||||||
| }}" | ||||||
| }} | ||||||
| working-directory: build-directory | ||||||
|
|
||||||
| - name: Create a tarball with the build contents | ||||||
| env: | ||||||
| PACKAGE_VERSION: "${{ inputs.ansible-package-version }}" | ||||||
| run: >- | ||||||
| tar -czvf | ||||||
| ansible-package-docs-html-"${PACKAGE_VERSION}"-"$(date '+%Y-%m-%d')"-${{ | ||||||
|
|
@@ -105,8 +111,10 @@ jobs: | |||||
| run: echo "TX_ID=$(date +%s)" >> "${GITHUB_ENV}" | ||||||
|
|
||||||
| - name: Notify the DaWGs in Matrix | ||||||
| # FAIL_MESSAGE is trusted input so okay to inject here. | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not still pass it through environment variables? That's usually the best way to pass things on. Also it will prevent future problems, for example if someone later adjusts the message so that it contains double quotes, backticks, ... |
||||||
| # zizmor: ignore[template-injection] | ||||||
| run: | | ||||||
| curl -X PUT "${{ env.ROOM_URL }}/${TX_ID}" \ | ||||||
| curl -X PUT "${ROOM_URL}/${TX_ID}" \ | ||||||
| -H "Authorization: Bearer ${{ secrets.DOCS_BOT_TOKEN }}" \ | ||||||
| -H "Content-Type: application/json" \ | ||||||
| -d '{"msgtype": "m.text", "body": "${{ env.FAIL_MESSAGE }}"}' | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honestly, I'd rather avoid the mixing shell and JSON quoting code smells entirely. What about something like this?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also fine for me. But please add quotes around |
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,10 +34,14 @@ jobs: | |
| - session: "pip-compile" | ||
| extra-args: "--check" | ||
| python-versions: "3.12" | ||
| - session: "zizmor" | ||
| python-versions: "3.12" | ||
|
Comment on lines
+37
to
+38
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The zizmor workflow not failing CI seems to be by design of the GHA author, keeping that data in the security section for action separate from success/failure of the CI itself. As for not being able to find the reported issues, that was a "I needed to learn how to use the tools together". I was running the scans on one branch, and the scan was showing data from another, and I just missed how I could adjust that to see the results (all of that was happening in my own fork). With all that being said, since a majority of the workflows for the project are setup using nox to make them portable between dev/CI environments, I can't think of any good objections to integrating it there. I initially went the GHA route because it appears to be the suggested method by the tool authors, with the intention of piping that data automatically into existing GH systems (which does work as intended, but doesn't sound like its exactly the use case we're targeting).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gotmax23 that workflow should be complementary as the nox session won't integrate with GH through SARIF. Just add another job separately. @x1101 I believe that once merged, the issues would show up in PRs inline. But it's a good idea to have both invocation mechanisms in place. The reusable workflow is for the GH Code Scanning integration while nox is for local use. However, it's a good idea to still run that nox session in CI to ensure that it works in dev env.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gotmax23 FYI the code scanning results don't normally show up until the workflow reporting them is in |
||
| name: "Run nox ${{ matrix.session }} session" | ||
| steps: | ||
| - name: Check out repo | ||
| uses: actions/checkout@v5 | ||
| with: | ||
| persist-credentials: false | ||
| - name: Setup nox | ||
| uses: wntrblm/nox@2025.10.16 | ||
| with: | ||
|
|
@@ -46,7 +50,8 @@ jobs: | |
| run: | | ||
| nox -e clone-core | ||
| - name: "Run nox -e ${{ matrix.session }}" | ||
| # Using GHA expression interpolation is fine here, | ||
| # as we control all the inputs. | ||
| # zizmor: ignore[template-injection] | ||
| run: | | ||
| # Using GHA expression interpolation is fine here, | ||
| # as we control all the inputs. | ||
| nox -e "${{ matrix.session }}" -- ${{ matrix.extra-args }} | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -35,23 +35,21 @@ name: "Refresh pinned dependencies" | |||||||||||||||||||||
| python-versions: | ||||||||||||||||||||||
| type: string | ||||||||||||||||||||||
| required: true | ||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| permissions: | ||||||||||||||||||||||
| contents: read | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||
| refresh: | ||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||
| environment: github-bot | ||||||||||||||||||||||
| steps: | ||||||||||||||||||||||
| - name: Generate temp GITHUB_TOKEN | ||||||||||||||||||||||
| id: create_token | ||||||||||||||||||||||
| uses: actions/create-github-app-token@v2 | ||||||||||||||||||||||
| with: | ||||||||||||||||||||||
| app-id: ${{ secrets.BOT_APP_ID }} | ||||||||||||||||||||||
| private-key: ${{ secrets.BOT_APP_KEY }} | ||||||||||||||||||||||
| - name: Check out repo | ||||||||||||||||||||||
| uses: actions/checkout@v5 | ||||||||||||||||||||||
| with: | ||||||||||||||||||||||
| fetch-depth: 0 | ||||||||||||||||||||||
| ref: "${{ inputs.base-branch }}" | ||||||||||||||||||||||
| token: "${{ steps.create_token.outputs.token }}" | ||||||||||||||||||||||
| persist-credentials: false | ||||||||||||||||||||||
| - name: Fetch required contents of ansible-core | ||||||||||||||||||||||
| run: | | ||||||||||||||||||||||
| python docs/bin/clone-core.py | ||||||||||||||||||||||
|
|
@@ -82,9 +80,23 @@ jobs: | |||||||||||||||||||||
| env: | ||||||||||||||||||||||
| # Ensure the latest pip version is used | ||||||||||||||||||||||
| VIRTUALENV_DOWNLOAD: '1' | ||||||||||||||||||||||
| # | ||||||||||||||||||||||
| # nox-args is defined statically in the calling workflows and passing it | ||||||||||||||||||||||
| # as a shell variable presents quoting issues, so ignore zizmor error | ||||||||||||||||||||||
| # for now. | ||||||||||||||||||||||
| # zizmor: ignore[template-injection] | ||||||||||||||||||||||
| run: | | ||||||||||||||||||||||
| nox ${{ inputs.nox-args }} | ||||||||||||||||||||||
| - name: Generate temp GITHUB_TOKEN | ||||||||||||||||||||||
| id: create_token | ||||||||||||||||||||||
| uses: actions/create-github-app-token@v2 | ||||||||||||||||||||||
| with: | ||||||||||||||||||||||
| app-id: ${{ secrets.BOT_APP_ID }} | ||||||||||||||||||||||
| private-key: ${{ secrets.BOT_APP_KEY }} | ||||||||||||||||||||||
| # We could rely on the checkout action to persist the token in the | ||||||||||||||||||||||
| # repository config, but this way, we can prevent the previous steps | ||||||||||||||||||||||
| # from having unnecessary access. | ||||||||||||||||||||||
| - name: "Set up token authentication" | ||||||||||||||||||||||
| run: gh auth setup-git --force --hostname github.com | ||||||||||||||||||||||
| - name: Push new dependency versions and create a PR | ||||||||||||||||||||||
| env: | ||||||||||||||||||||||
| GITHUB_TOKEN: ${{ steps.create_token.outputs.token }} | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| rules: | ||
| unpinned-uses: | ||
| config: | ||
| policies: | ||
| "wntrblm/nox": ref-pin | ||
| "re-actors/alls-green": ref-pin | ||
| "actions/*": ref-pin |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| zizmor |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| # This file was autogenerated by uv via the following command: | ||
| # uv pip compile --universal --output-file tests/zizmor.txt tests/zizmor.in | ||
| zizmor==1.16.0 | ||
| # via -r tests/zizmor.in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That workflow should declare an explicit
secrets:section. Then, passing secrets will work. Just likeinputs:.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, but the secrets are part of the
github-botenvironment, and I don't thinkenvironmentcan be set in the calling workflow.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, so https://docs.github.com/en/actions/how-tos/reuse-automations/reuse-workflows#using-inputs-and-secrets-in-a-reusable-workflow implies that the environment should be set in the reusable workflow but doesn't explicitly talk about how that interacts with the secrets key (beyond the inability to pass them explicitly).