Lint Github Actions workflows with zizmor #3188

gotmax23 · 2025-10-27T22:15:15Z

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.

Inspired-by: Lyle McKarns x1101@users.noreply.github.com

gotmax23 · 2025-10-27T22:36:26Z

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.

gotmax23 · 2025-10-27T22:39:46Z

Test labeler run: https://github.com/ansible/ansible-documentation/actions/runs/18857970482/job/53810211332

gotmax23 · 2025-10-27T22:58:35Z

.github/workflows/reusable-nox.yml

+          - session: "zizmor"
+            python-versions: "3.12"


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.

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).

@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.

@gotmax23 FYI the code scanning results don't normally show up until the workflow reporting them is in devel. That might be the reason.

gotmax23 · 2025-10-27T23:03:58Z

Test docs deploy to the staging deployment environment: https://github.com/ansible/ansible-documentation/actions/runs/18858431601

oraNod

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.

noxfile.py

gotmax23 · 2025-10-31T20:34:47Z

I applied the feedback and added an Inspired-by trailer to the PR description.

oraNod

+1 to this approach. Thanks @gotmax23

I can also see some additional issues when running zizmor with either the pedantic or auditor personas. Maybe those will be better handled as follows up as the default persona is fine.

felixfontein · 2025-11-06T21:08:21Z

The PR currently has a conflict.

webknjaz · 2025-11-07T01:22:38Z

.github/workflows/pip-compile-dev.yml

      labels: "${{ inputs.labels || 'no_backport,tooling' }}"
-    secrets: inherit
+    # Pass using inherit, as this seems to be the only possible way to access
+    # secrets defined in an enviornment when using nested workflows.


That workflow should declare an explicit secrets: section. Then, passing secrets will work. Just like inputs:.

Right, but the secrets are part of the github-bot environment, and I don't think environment can be set in the calling workflow.

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).

webknjaz · 2025-11-07T01:26:26Z

.github/workflows/reusable-pip-compile.yml

        default: ""
      python-versions:
        type: string
        required: true


Suggested change

required: true

required: true

secrets:

BOT_APP_ID:

description: An ID for the GitHub App to authenticate as.

required: true

BOT_APP_KEY:

description: A private key for the GitHub App authentication.

required: true

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. Also, when rebasing this commit, I added back the manual `nox -s clone-core` step to keep the outputs separate.

- 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.

Co-authored-by: Don Naro <dnaro@redhat.com>

gotmax23 · 2025-11-07T01:58:11Z

.github/workflows/reusable-build-docs.yaml

-      run: >-
-        nox -e make -- webdocs ${{
+      env:
+        PACKAGE_VERSION: "${{ inputs.ansible-package-version }}"


@oraNod, do we need to also export VERSION like it was before in the Set the VERSION variable step? Is $VERSION used in the Makefile?

Suggested change

PACKAGE_VERSION: "${{ inputs.ansible-package-version }}"

PACKAGE_VERSION: "${{ inputs.ansible-package-version }}"

VERSION: "${{ inputs.ansible-package-version }}"

Hi @gotmax23 ANSIBLE_VERSION gets passed to the Makefile but not VERSION.

I think VERSION is a relic from the Jenkins job that is used in a couple of places including the step to add a symlink on the web server: ln -s \$VERSION _build/latest. Maybe we needed it when we first started with the workflow but it should be fine to drop it as part of this PR.

I'm pretty sure I built this branch before I approved but will do it again now. Thanks, as always, for the thoughtful questions.

Just did a run with this branch from the devel branch and see this in the logs: nox > make -C docs/docsite PYTHON=/home/runner/work/ansible-documentation/ansible-documentation/build-directory/.nox/make/bin/python webdocs ANSIBLE_VERSION= 'EXTRA_TAGS=-t redirects'

Everything in the tarball looks good too. I think we can safely remove the Set the VERSION variable step.

gotmax23 · 2025-11-07T02:00:00Z

.github/workflows/pip-compile-dev.yml

      labels: "${{ inputs.labels || 'no_backport,tooling' }}"
-    secrets: inherit
+    # Pass using inherit, as this seems to be the only possible way to access
+    # secrets defined in an enviornment when using nested workflows.


Right, but the secrets are part of the github-bot environment, and I don't think environment can be set in the calling workflow.

.github/workflows/reusable-build-docs.yaml

felixfontein · 2025-10-28T20:31:56Z

.github/workflows/reusable-build-docs.yaml

      run: echo "TX_ID=$(date +%s)" >> "${GITHUB_ENV}"

    - name: Notify the DaWGs in Matrix
+      # FAIL_MESSAGE is trusted input so okay to inject here.


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, ...

felixfontein · 2025-10-28T20:33:42Z

.github/workflows/reusable-build-docs.yaml

+        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 }}"}'


Suggested change

-d '{"msgtype": "m.text", "body": "${{ env.FAIL_MESSAGE }}"}'

-d '{"msgtype": "m.text", "body": "'"${FAIL_MESSAGE}"'"}'

Honestly, I'd rather avoid the mixing shell and JSON quoting code smells entirely. What about something like this?

- name: Notify the DaWGs in Matrix run: | body=$(python3 -c ' import json import os import sys json.dump({ "msgtype": "m.text", "body": os.environ["FAIL_MESSAGE"], }, sys.stdout)' ) curl -X PUT "${ROOM_URL}/${TX_ID}" \ -H "Authorization: Bearer ${{ secrets.DOCS_BOT_TOKEN }}" \ -H "Content-Type: application/json" \ -d "${body}"

.github/workflows/reusable-build-docs.yaml

Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <wk@sydorenko.org.ua>

gotmax23 temporarily deployed to github-bot October 27, 2025 22:15 — with GitHub Actions Inactive

gotmax23 temporarily deployed to github-bot October 27, 2025 22:25 — with GitHub Actions Inactive

gotmax23 had a problem deploying to github-bot October 27, 2025 22:25 — with GitHub Actions Failure

gotmax23 temporarily deployed to github-bot October 27, 2025 22:28 — with GitHub Actions Inactive

gotmax23 temporarily deployed to github-bot October 27, 2025 22:30 — with GitHub Actions Inactive

gotmax23 force-pushed the zizmor2 branch from 180ede5 to 5214e73 Compare October 27, 2025 22:31

gotmax23 temporarily deployed to github-bot October 27, 2025 22:31 — with GitHub Actions Inactive

gotmax23 temporarily deployed to github-bot October 27, 2025 22:33 — with GitHub Actions Inactive

gotmax23 temporarily deployed to github-bot October 27, 2025 22:38 — with GitHub Actions Inactive

gotmax23 mentioned this pull request Oct 27, 2025

Harden GHA with Zizmor #2681

Open

gotmax23 commented Oct 27, 2025

View reviewed changes

gotmax23 mentioned this pull request Oct 27, 2025

Add Zizmor Scanning on push/pull request #3141

Closed

gotmax23 requested review from a team and webknjaz October 27, 2025 23:02

gotmax23 temporarily deployed to github-bot October 27, 2025 23:07 — with GitHub Actions Inactive

oraNod reviewed Oct 28, 2025

View reviewed changes

noxfile.py Outdated Show resolved Hide resolved

noxfile.py Outdated Show resolved Hide resolved

gotmax23 temporarily deployed to github-bot October 31, 2025 20:31 — with GitHub Actions Inactive

gotmax23 requested a review from oraNod October 31, 2025 20:34

gotmax23 temporarily deployed to github-bot October 31, 2025 20:35 — with GitHub Actions Inactive

oraNod approved these changes Nov 3, 2025

View reviewed changes

oraNod requested a review from x1101 November 6, 2025 08:01

oraNod requested review from felixfontein and removed request for x1101 November 6, 2025 08:01

webknjaz reviewed Nov 7, 2025

View reviewed changes

gotmax23 and others added 10 commits November 6, 2025 19:52

ci: run zizmor in CI and noxfile

c920ae8

- Adds lockfile - Adds nox session - Adds nox session to CI matrix

ci: fix additional issues identified by zizmor

7fd909e

- Add default permissions to new workflows - Add cooldown to dependabot

ci: add zizmor configuration for unpinned-uses

05a1858

We could configure dependabot to pin shared workflow commit SHA hashes, but for now, let's relax the unpinned-uses relax

ci: restore secrets: inheirt for pip-compile workflows

234868e

See comment for more details.

ci: fix token auth for pip-compile workflow

3623b3f

README: mention that lint session runs GHA checks

1656b3f

Update noxfile.py

5f38280

Co-authored-by: Don Naro <dnaro@redhat.com>

nox zizmor: allow overriding persona

116dce7

nox: actually run zizmor as part of lint session

062e554

gotmax23 force-pushed the zizmor2 branch from 53d523d to 062e554 Compare November 7, 2025 01:53

gotmax23 temporarily deployed to github-bot November 7, 2025 01:53 — with GitHub Actions Inactive

gotmax23 commented Nov 7, 2025

View reviewed changes

oraNod self-requested a review November 7, 2025 08:42

webknjaz reviewed Nov 8, 2025

View reviewed changes

.github/workflows/reusable-build-docs.yaml Outdated Show resolved Hide resolved

felixfontein reviewed Nov 8, 2025

View reviewed changes

ci: use GHA expression instead of shell test

041e7a0

Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <wk@sydorenko.org.ua>

gotmax23 temporarily deployed to github-bot November 10, 2025 18:13 — with GitHub Actions Inactive

	PACKAGE_VERSION: "${{ inputs.ansible-package-version }}"
	PACKAGE_VERSION: "${{ inputs.ansible-package-version }}"
	VERSION: "${{ inputs.ansible-package-version }}"

	-d '{"msgtype": "m.text", "body": "${{ env.FAIL_MESSAGE }}"}'
	-d '{"msgtype": "m.text", "body": "'"${FAIL_MESSAGE}"'"}'

Lint Github Actions workflows with zizmor #3188

Are you sure you want to change the base?

Lint Github Actions workflows with zizmor #3188

Uh oh!

Conversation

gotmax23 commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gotmax23 commented Oct 27, 2025

Uh oh!

gotmax23 commented Oct 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gotmax23 commented Oct 27, 2025

Uh oh!

oraNod left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gotmax23 commented Oct 31, 2025

Uh oh!

oraNod left a comment

Choose a reason for hiding this comment

Uh oh!

felixfontein commented Nov 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gotmax23 commented Oct 27, 2025 •

edited

Loading