Skip to content

fix(ci): verify pinned action version comments - #454

Merged
vladimirrott merged 2 commits into
lacs-project:mainfrom
QinXi-ai:fix/369-action-pin-comments
Sep 21, 2026
Merged

vladimirrott merged 2 commits into
lacs-project:mainfrom
QinXi-ai:fix/369-action-pin-comments

Conversation

@QinXi-ai

Copy link
Copy Markdown
Contributor

Fixes #369.

Pinned actions could carry stale version comments without any gate noticing. Add scripts/verify-action-pins.sh, adapted from maintainer-agent, to compare each pin against its claimed tag, peel annotated tags (including nested tags), and report explicitly marked branch references separately. API/ref resolution errors fail with a different verdict from SHA mismatches.

The YAML input helper preserves every occurrence, handles subpaths, spacing, continuation lines and flow mappings, and rejects empty or unreadable input. It does not change the release rehearsal extractor owned by #441/#442/#444. CI and the local hygiene group both require the verifier; the CI step receives a read-only workflow token. Local runs require authenticated gh, Python 3 and PyYAML.

Correct the comments and replace moving major-alias comments with the exact release tags matching the existing commits. All 54 action SHA pins and the local reusable-workflow reference are unchanged. Deliberate branches are marked stable (branch) / main (branch).

Validation on Windows/Git Bash:

  • Offline regression fixtures passed: mismatched/corrected comments, exact verdict counts, nested annotated tags, repository subpaths, explicit branches, missing comments/refs, empty input, malformed YAML and unreadable workflow inputs.
  • Live GitHub API verification passed: 54 verdicts (46 tag matches, 8 explicit branch references).
  • Local gate regression tests: 5 passed, including verifier failures becoming hard failures.
  • ShellCheck at warning severity, workflow yamllint, and git diff --check passed.
  • Full release rehearsal and Rust workspace validation were not run: this host has no Cargo. No Rust code or test baseline changed.

@QinXi-ai
QinXi-ai marked this pull request as ready for review September 19, 2026 02:25

@vladimirrott vladimirrott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed at 28958bbf.

Fifty-five uses: lines across nine workflow files, and the first thing I checked was whether any pin moved. None did:

python3 - <<'PY'
import re,pathlib
def pins(root):
    out=[]
    for p in sorted(pathlib.Path(root,'.github/workflows').iterdir()):
        if p.suffix not in ('.yml','.yaml'): continue
        for i,l in enumerate(p.read_text().splitlines(),1):
            m=re.search(r'uses:\s*([^\s#]+)', l)
            if m: out.append((p.name,m.group(1)))
    return out
a=pins('/tmp/wtmain'); b=pins('/tmp/wt454')
print("main uses: entries =",len(a),"  p454 =",len(b))
sa,sb=set(a),set(b)
print("refs only on main :",sorted(sa-sb))
print("refs only on p454 :",sorted(sb-sa))
PY
main uses: entries = 55   p454 = 55
refs only on main : []
refs only on p454 : []

Your counts are exact: 54 SHA pins, one local ./ reference, 8 marked (branch), 46 exact tags checked.

The comments were wrong, and one of them by a major version

I resolved every claimed tag with my own gh api calls rather than running your script with my token:

OK      actions/checkout                     v7.0.1 = 3d3c42e5
OK      actions/setup-node                   v7.0.0 = 82076278
OK      actions/setup-python                 v7.0.0 = 5fda3b95
BRANCH  dtolnay/rust-toolchain               stable (branch)
OK      Swatinem/rust-cache                  v2.9.2 = 6323deb1
OK      taiki-e/install-action               v2.87.4 = e67fa11c
OK      actions/dependency-review-action     v5.0.0 = a1d282b3
OK      github/codeql-action/init            v4.37.9 = cdf488f5
OK      github/codeql-action/analyze         v4.37.9 = cdf488f5
OK      actions/upload-pages-artifact        v5.0.0 = fc324d35
OK      actions/deploy-pages                 v5.0.1 = 368f8252
OK      actions/upload-artifact              v7.0.1 = 043fb46d
OK      anchore/sbom-action                  v0.24.2 = 3ad72834
OK      actions/attest                       v4.2.2 = 1e69f48a
OK      actions/download-artifact            v8.0.1 = 3e5f45b2
OK      ossf/scorecard-action                v2.4.4 = 2d114668
BRANCH  trufflesecurity/trufflehog           main (branch)
--- ok=15 mismatch=0 branch=2 ---

actions/dependency-review-action is the one that matters. main today reads # v4 beside a1d282b3, and that repository has no v4 tag at all:

gh api repos/actions/dependency-review-action/git/ref/tags/v4 --jq '.object.type+" "+.object.sha'
gh api repos/actions/dependency-review-action/tags --jq '.[].name'
{"message":"Not Found", ... "status":"404"}
v5.0.0
v4.9.0
v4.8.3

So anyone auditing our pins read "v4" and was wrong by a major version. That is the concrete thing this PR fixes, and it is why #369 was worth filing.

Mutations

Four, all offline in a container with no network, using your own gh stub:

=== M1-comparison-always-true (elif [[ "$real" == "$sha" ]] -> elif true) ===
rc=1   OK actions/checkout  aaaaaaaa is v9.9.9   (the test catches the false OK)

=== M2-missing-comment-skipped (raise ValueError -> continue) ===
rc=1   checker unexpectedly passed ... verify-action-pins: checked 2 pins

=== M3-unreadable-skipped (raise ValueError -> continue) ===
rc=1   verify-action-pins: no pinned actions found

=== M4-ci-local-wiring-removed (run_step line deleted from ci-local.sh) ===
rc=1   Ran 5 tests ... FAILED (failures=1)

Unmutated at your head: action pin comment fixtures passed rc=0, and ci-local.test.sh reports Ran 5 tests in 0.041s / OK rc=0. Shellcheck at --severity=warning over the four script trees returns 0.

The offline design is the part I would point other contributors at. A gh stub on PATH and PIN_API_LOG means the fixtures assert on which API calls were made, including that dtolnay/rust-toolchain is never looked up and that actions/checkout@v1.2.3 is resolved once rather than four times. That is a cache assertion most people would not have written.

I also pulled the real run: docs-and-hygiene on this head printed verify-action-pins: checked 54 pins with 46 OK, 8 BRANCH, 0 FAIL, 0 ERROR, and the step took about seven seconds wall clock.

One optional note

docs-and-hygiene is a required check, and it now fails when api.github.com does. Your script separates the verdicts already, ERROR ... cannot resolve tag ...; check ref, authentication and API availability against FAIL ... pinned X but Y is Z, so the message names the cause. What the exit code does not separate is "this tag does not exist", which is a finding, from "GitHub returned 502 for eight seconds", which is weather. #367 is the same complaint about npm audit and registry.npmjs.org, so this repo already has one required check that goes red for reasons unrelated to the pull request in front of it, and this makes two.

Not blocking, and I am not asking you to change it in this PR. It is a design question for the gate as a whole, and CONTRIBUTING.md in your diff already tells contributors that both docs-and-hygiene and ci-local.sh need a working GitHub API connection, which is the honest version.

Nothing blocking, and what stops the merge

The board is green on all eleven checks. What holds this is that it changes nine files under .github/workflows/, and I read every workflow diff myself before landing one rather than merging on a green board. I have read this one and the only changes are trailing comments plus two new steps in docs-and-hygiene; the read that remains is a second pair of eyes on a supply-chain-adjacent diff, not a doubt about the change.

Against #446, which rewrites the pin checker to parse YAML: git merge-tree --write-tree says both merge clean in either order. I built the combined tree and ran both suites on it, rehearsal rc=0 and pin fixtures rc=0, and confirmed the merged ci.yml carries your two steps and #446's explicit PyYAML==6.0.2. No rebase for you either way. One cosmetic consequence: your header says PyYAML is "provided by yamllint", and #446 stops relying on that. Worth a word change whenever you next touch the file, nothing more.

Next

Ten pull requests into this project and I have never pointed you at the thing I want your opinion on, so let me fix that. #367 is open and free: npm audit makes frontend fail whenever registry.npmjs.org does, which is the same class of problem as the note above, and you are now the person here who has thought hardest about a gate that depends on somebody else's uptime. It is yours if you want it and I will hold it.

And the thing I should have said several PRs ago: you have spent a lot of evenings on this project's CI without anyone asking whether you would want to use what it guards. If you run Linux machines anywhere that matters, I would like to know how SysKnife behaves on them. npx sysknife-setup is the whole setup, and sysknife doctor, sysknife history and sysknife audit verify touch nothing on the host. Say no freely.

@vladimirrott

Copy link
Copy Markdown
Member

Correction to my own review, and it is mine rather than yours.

I named #367 as free. It is not: I offered it to @Osheun on 2026-09-15 and they have not answered, so it sits with them and I should have checked before typing. Withdrawing that suggestion, with apologies to both of you.

Nothing else in the review changes. The approval stands and the merge is waiting on my read of the workflow diff, not on anything from you. I will come back with something in the same vein once it is genuinely free.

@vladimirrott
vladimirrott merged commit 35b3c1a into lacs-project:main Sep 21, 2026
12 checks passed
vladimirrott added a commit that referenced this pull request Sep 21, 2026
taiki-e/install-action v2.87.4 -> v2.87.10 (fa239534)
github/codeql-action init and analyze v4.37.9 -> v4.38.0 (b96794f0)

Rebuilt on top of #454, which landed exact version comments and a gate that
resolves each one against the tag it names. Dependabot's branch carried the
old `# v2` and `# v4` comments, which that gate now refuses, so the comments
move with the SHAs here.

Both SHAs were resolved against the upstream repositories rather than taken
from the pull request body: fa239534 carries the tag v2.87.10 and b96794f0
carries v4.38.0, and each is a release commit in its own repository.

  $ bash scripts/verify-action-pins.sh .
    OK taiki-e/install-action    fa239534 is v2.87.10 (ci.yml:296)
    OK taiki-e/install-action    fa239534 is v2.87.10 (ci.yml:351)
    OK github/codeql-action/init b96794f0 is v4.38.0 (codeql.yml:48)
    OK github/codeql-action/analyze b96794f0 is v4.38.0 (codeql.yml:55)
  verify-action-pins: checked 54 pins
vladimirrott added a commit that referenced this pull request Sep 22, 2026
taiki-e/install-action v2.87.4 -> v2.87.10 (fa239534)
github/codeql-action init and analyze v4.37.9 -> v4.38.0 (b96794f0)

Rebuilt on top of #454, which landed exact version comments and a gate that
resolves each one against the tag it names. Dependabot's branch carried the
old `# v2` and `# v4` comments, which that gate now refuses, so the comments
move with the SHAs here.

Both SHAs were resolved against the upstream repositories rather than taken
from the pull request body: fa239534 carries the tag v2.87.10 and b96794f0
carries v4.38.0, and each is a release commit in its own repository.

  $ bash scripts/verify-action-pins.sh .
    OK taiki-e/install-action    fa239534 is v2.87.10 (ci.yml:296)
    OK taiki-e/install-action    fa239534 is v2.87.10 (ci.yml:351)
    OK github/codeql-action/init b96794f0 is v4.38.0 (codeql.yml:48)
    OK github/codeql-action/analyze b96794f0 is v4.38.0 (codeql.yml:55)
  verify-action-pins: checked 54 pins

Co-authored-by: Vladimir Rotariu <sysknife-development@protonmail.com>
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.

Four action pins name a version they are not, and nothing checks

2 participants