Repository navigation
Determine some more criteria for actions review #686
Description
Following the discussion in #674 (comment)
Possible criteria:
- Reproducibility — implemented in Add action build verification script #561, Add automated build verification for dependabot PRs #614, Enhance verify-action-build.py with deep composite/docker verification #629, verify-action-build: add node_modules verification for vendored deps #652, Retry JS verification with approved version's lock files on mismatch #674, Refactor verify-action-build into modular package with tests #675, verify-action-build: support .cjs and .mjs compiled bundles #734
- Code review of differences vs. past approved version — implemented in Improve verify-action-build.py CI output and add security review checklist #647, Add interactive open-in-browser and approve flow #671
- Passing cooldown (now 0 days) — implemented in Add cooldown to dependabot and expand .gitignore #560, dependabot: apply cooldown except for action reviews #712, Update README with current 0 day cooldown #737
- Correctness of the action (for example errors in build pipelines) — ongoing fixes in Fix verify-action-build.py for monorepo and non-dist output dirs #617, Fix verify-action-build.py Node.js version detection #628, Fix verify-action-build to try 'npm run start' for actions that use it #664, Fix Docker build for actions with multi-step builds #685, verify-action-build: skip pin warning for multi-stage FROM references #733, verify-action-build: support .cjs and .mjs compiled bundles #734
- Hash-pinning of composite actions — implemented in Enhance verify-action-build.py with deep composite/docker verification #629 (supporting fixes: Fix
pypa/gh-action-pypi-publish: pin to commit SHA instead of branch pattern #619, Fixdtolnay/rust-toolchain: pin to commit SHA instead of branch name #620) - Checking if downloaded binaries are verified — pending in verify-action-build: add binary download verification check #743
- Reject actions that ship pre-compiled native binaries (Go, Rust, C, etc.) in-tree without verifiable build provenance.
- Compatible licencing
Goal not criteria:
- Reject actions with suspicious builds, e.g. those that use different dependencies than what the source implies (proposed by @dave2wave) — partially addressed by verify-action-build warnings
Rejected:
- [ ] Attestations
Maybe others?
I would love to hear what others think.
Activity
potiuk commentedon Apr 9, 2026
I'd personally add [x] hash pinning and [x] Compatible licencing (even if optional and default is not-compatible). Possibly we should add some checks for the latter if the actions are used properly.
potiuk commentedon Apr 9, 2026
Also I think good idea when we reject some action we should follow up with the 3rd party - to at least let them know they should fix things (or even create PR). That should be IMHO default behaviour when we reject things.
dave2wave commentedon Apr 9, 2026
#683 is for discussion of what cooldown periods should be recommended.
dave2wave commentedon Apr 9, 2026
What is the point of enforcing "compatible licensing"? Policy allows any build tool as long as it is unmodified.
dave2wave commentedon Apr 9, 2026
potiuk commentedon Apr 9, 2026
I don't think this was "suspicious" - there is clear explanation on why it happened (and why it could have happened for others). The problem was (see my PR SonarSource/sonarqube-scan-action#228 ) was that their dependabot PRs did not have a check if dist was rebuilt and release was made after that without rebuiliding it with the new tools.
This likely common pattern that might happen to others. Problem with rebuilding in dependabot PRs is that such dependabot PRs will always fail and will have to be manually updated - when such compiled javascript is committed to the repo. Or there must be another mechanism implemented to prevent the release if rebuild is not done (let's see what SonarQube will respond to my PR). So I think there is nothing "suspicious" about it - this is an innocent build/release pipeline bug.
dave2wave commentedon Apr 13, 2026
One criteria would be to list which repositories are impacted by the approval. We have capability to survey periodically all repositories and provide an inventory. This inventory could be inverted to list out every use of an action along with its current pin. The users of the action should be able to help judge the correctness of any upgrade.
ppkarwasz commentedon Apr 17, 2026
I was experimenting with using Ampel to automatically check how actions have been released during a Dependabot PR.
For example
aquasecurity/trivy-actionnow releases attestation that can be verified.potiuk commentedon Apr 17, 2026
Do you know how well it stacks against the actions we currently have in our "approved" list ?
ppkarwasz commentedon Apr 17, 2026
I am working on it.
ppkarwasz commentedon Apr 19, 2026
I perused an LLM to check which actions have additional authentication factors we could use. As it turns out
aquasecurity/trivy-actionseems the only one to publish an attestation.Thinking about it a little bit more, attestations don't seem to be very relevant here, since they can only attest that the action's distribution was build from the source. Unless I am mistaken, actions are always built locally and committed to the same commit that does source code changes.
Actions with attestations (non-empty store)
Only
pypa/cibuildwheelhas actual attestations — 44 entries covering wheel releasesfrom v2.23.4 through v3.4.1. All other repos with an attestation store are empty.
Note: attestations cover the published wheel packages, not the action source itself.
Actions with attestations attached to the release
Only
aquasecurity/trivy-actionseems to publish an attestation, which is no longer allowed.Actions with GPG-signed tags (verified=valid)
1Password/load-secrets-actionbrowser-actions/setup-firefoxdorny/test-reportererisu/license-checker-actionilammy/setup-nasmsbt/setup-sbtNotable findings
helm/chart-releaser-actionhas an annotated tag withreason=bad_cert— signedbut the certificate is no longer trusted.
bazel-contrib/setup-bazel,biomejs/setup-biome,JetBrains/qodana-action,JustinBeckwith/linkinator-action,matlab-actions/run-tests,matlab-actions/setup-matlab,mozilla-actions/sccache-action,nwtgck/actions-netlify,pypa/cibuildwheel,runs-on/action.browser-actions/setup-geckodriver,damccorm/tag-ur-it) have no tagcomment in action.yml and were not checked.
potiuk commentedon Apr 19, 2026
Quite agree.
potiuk commentedon Apr 19, 2026
I updated the issue to reflect the current state of proposed checks:
Also @dave2wave
I think that really depends on the type of "issue" - but I would agree in general. I am continuously updating the "verify" action whenever we find an action that needs it - so this rule might also be pretty conditional. My idea is that if:
a) we are able to explain what's going on
b) there is no security risk
Warning and action of addressing it (for example opening a PR or issue in action repository) might be better choice than blocking. In the verify script those things might cause "error" (this is what I did with lack of dist rebuilding after dependencies updated in #662 - is a good "middle ground" for those - they will be flagged with failing check, but from the output it will be clear if the only issue is because old dependencies were used to build it (which is a relatively common issue for those who implemented trivial dependabot upgrade check without running rebuild after.
potiuk commentedon Apr 19, 2026
I also updated the issue to link to the appropriate PRs that implemented those checks already.
potiuk commentedon Apr 19, 2026
Opened #743 to implement the binary download verification check.
It scans Dockerfiles,
action.ymlrun: blocks and referenced scripts for binary/script downloads (curl, wget, Invoke-WebRequest, DockerfileADD <url>, pipe-to-shell) and requires a detectable verification step nearby —sha*sum,shasum,openssl dgst,gpg --verify,cosign verify,slsa-verifier,gh attestation verify,minisign,ssh-keygen -Y verify,CertUtil -hashfile,Get-FileHash. Package-manager invocations (apt/apk/pip/npm/brew/cargo/…) are skipped since they bring their own integrity checks.Files that contain downloads but no verification patterns fail the run. The check recurses through composite nested actions (MAX_DEPTH=3, skipping trusted orgs
actions/github) and is toggleable via--no-binary-download-check(on by default).dave2wave commentedon Apr 19, 2026
@potiuk
I think this is an overall goal and we will always find cases we decide to address by adding to verify-action-build. I would just remove it from the rule list.
potiuk commentedon Apr 19, 2026
Good question. I think in this case it really refers to "gradle" case - where at some point in time they added "mandatory" licence approval.
potiuk commentedon Apr 19, 2026
I saw you modified - it - I moved it to a separate section.
verify-action-build: unified walker over the composite action graph
potiuk commentedon May 7, 2026
Adding one more proposed criterion based on #809:
runs-on/action@v2.1.1: ~10 MB of UPX-packed Go binaries committed directly to the repo, run as root via a tinyindex.jslauncher. verify-action-build's JS-rebuild check only verifies the launcher, not the opaque binaries — so the action's actual code can't be reconciled with its source. An action in this shape should fail review unless upstream provides SLSA attestations (actions/attest-build-provenance) or a signedSHA256SUMSfile we can verify against.Concrete follow-ups, with task estimates:
verify-action-build(security.py).analyze-action-prskill (Ship reusable analyze-action-pr skill under .claude/skills/ #807) so future agents catch it.potiuk commentedon May 7, 2026
cc: @ppkarwasz @dave2wave @raboof WDYT? This
runs-onaction sounds fishydave2wave commentedon May 7, 2026
@gmcdonald was testing
runs-onas it is a commercial product that may allow Cheaper GH Runners that can have consistent IP addresses.