Skip to content

ci: pin validation.yaml actions so zizmor stops failing every PR - #652

Closed
kalenkevich wants to merge 1 commit into
mainfrom
ci/pin-validation-actions
Closed

ci: pin validation.yaml actions so zizmor stops failing every PR#652
kalenkevich wants to merge 1 commit into
mainfrom
ci/pin-validation-actions

Conversation

@kalenkevich

Copy link
Copy Markdown
Collaborator

Link to Issue or Description of Change

2. Or, if no issue exists, describe the change:

Problem:

The zizmor-output gate fails on every pull request, including ones that
touch no workflow file at all:

CRITICAL: The following mandatory checks failed:
- zizmor/unpinned-uses at line 21 of .github/workflows/validation.yaml
- zizmor/unpinned-uses at line 24 of .github/workflows/validation.yaml
- zizmor/unpinned-uses at line 27 of .github/workflows/validation.yaml
Failing due to zizmor finding severity of Medium/High (exit code: 14).

validation.yaml on main still references three actions by tag, and
unpinned-uses is one of zizmor's mandatory checks. Because the finding lives
in the base branch rather than in the diff, an author gets a red required check
for something they did not write and cannot fix from their own branch.

Solution:

Pin actions/checkout, actions/setup-node and actions/setup-python to the
commit hash of the tag each one already referenced, add the read-only
contents permission to the job, and set persist-credentials: false on
checkout.

The hashes were verified against the upstream tags:

Action Tag SHA
actions/checkout v6.1.0 d23441a48e516b6c34aea4fa41551a30e30af803
actions/setup-node v6.5.0 249970729cb0ef3589644e2896645e5dc5ba9c38
actions/setup-python v5.6.0 a26af69be951a213d495a4c3e4e4022e16d87065

This is the same hunk #634 currently carries. Splitting it out so it can land on
its own and unblock the other open PRs; #634 can drop its copy once this merges.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

No unit tests: this changes CI configuration only, with no runtime code in the
diff. The proof is this PR's own zizmor-output check going green while the
same check is red on #650 and #651, which branch from the same base and touch
no workflow file.

Manual End-to-End (E2E) Tests:

zizmor could not be run locally — the pinned build resolves through an internal
index that needs credentials:

$ uvx --from zizmor==1.25.2 zizmor --no-ignores --offline .github/workflows/validation.yaml
× No solution found when resolving tool dependencies
  hint: An index URL (...) could not be queried due to a lack of valid
  authentication credentials (401 Unauthorized)

So verification is CI-side. The change is byte-identical to the hunk already
passing zizmor on #634, and the version-to-hash mapping was checked against the
GitHub API for all three actions.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Additional context

Noticed while checking CI on #650 and #651, two workflow fixes from a bug bash
of the graph-workflow docs samples. Both are red on zizmor-output purely
because of this base-branch finding.

Generated with CloudCode, session ses_00c664b88ffeuSsmLI4zfs0a3q.

validation.yaml still references actions/checkout, setup-node and
setup-python by tag. zizmor treats unpinned-uses as a mandatory check, so
the zizmor-output gate fails on every pull request that does not itself
touch this file — the finding is in the base branch, not in the PR.

Pin all three to the commit hash for the tag they already used, add the
read-only `contents` permission for the job, and turn off credential
persistence on checkout.

The same hunk is currently carried by #634; splitting it out so it can
land on its own and unblock the other open PRs.

@AmaadMartin AmaadMartin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This PR is fully redundant: #648 landed the identical validation.yaml hunk as 82c7b600, 12 minutes before you opened this one. I checked all three pinned SHAs against the upstream tag refs and they match, but they already sit on main at lines 23, 28 and 31. A simulated merge into main returns main's tree unchanged, so please close this PR.

Comment on lines +15 to +31
permissions:
contents: read
strategy:
matrix:
os: [ubuntu-latest, windows-latest, macos-latest]

steps:
- name: Checkout code
uses: actions/checkout@v6
uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0
with:
persist-credentials: false

- name: Use Node.js
uses: actions/setup-node@v6
uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6.5.0

- name: Setup Python
uses: actions/setup-python@v5
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not a nit. Main already carries this exact change, so this PR is a no-op.

PR #648 merged as 82c7b600 12 minutes before you opened this one. Its validation.yaml hunk is byte-identical to yours: the same three SHAs, the same permissions: contents: read, and the same persist-credentials: false. Main today, at lines 23, 28 and 31:

uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0
uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6.5.0
uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0

Your branch point 5742875d predates that merge, so the diff still displays. git merge-tree 82c7b600 19aa8ee returns main's tree unchanged. Please close this PR.

@ScottMansfield

Copy link
Copy Markdown
Member

Based on Amaad's comments, I checked the current validation.yaml and saw they are already pinned. I'm closing this PR without merging.

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