verify-action-build: reject actions that ship pre-compiled binaries#819
Open
potiuk wants to merge 4 commits intoapache:mainfrom
Open
verify-action-build: reject actions that ship pre-compiled binaries#819potiuk wants to merge 4 commits intoapache:mainfrom
potiuk wants to merge 4 commits intoapache:mainfrom
Conversation
Add a repo-scoped Claude Code skill at .claude/skills/analyze-action-pr/ that triages allowlist PRs end-to-end: extract action refs via verify-action-build --from-pr, classify each finding (pipe-to-shell, unverified-download, nested-action-issue, verify-script-bug, all-clean), look up what verification material upstream already publishes, and draft the right next moves — recommend approval, open an upstream issue + ping the PR author, or fix verify-action-build itself with a regression test. Includes an "Improve this skill" step so future runs that hit a new shape (false positive, novel verification mechanism, asset-naming convention) leave the skill better than they found it: the SKILL.md grows by a row in the case table or a line in the precedents table, not by re-deriving the analysis. The skill encodes shapes recently triaged: PR apache#795 / apache#798 (TS-generic regex hole), PR apache#802 (carabiner-dev nested install actions), PR apache#803 / apache#804 (multi-action extraction), PR apache#806 (jbangdev pipe-to-shell). Replace the broad .claude/ rule in .gitignore with specific entries for personal state (settings*.json, worktrees/, debug/) so .claude/ itself can host shared assets without leaking per-user files.
Pre-commit on PR apache#807 failed because .claude/skills/analyze-action-pr/ SKILL.md was missing the Apache license header. The insert-license hook auto-fixes locally but aborts CI because there's no follow-up commit to land the change. Add the header so CI passes. Document the local pre-commit workflow in AGENTS.md so future agent runs install prek and run ``prek run --all-files`` before pushing, catching the same class of bounce locally instead of in CI.
Case E previously read as "the script has a bug" — too narrow. Real gaps over the life of verify-action-build include whole-action-type support (Deno, Dart), new build flows (npm run start, multi-step Docker), new verification heuristics (sibling sha256sum, JSON data fetches, *Json helpers), false positives on legitimate inputs (multi-stage FROM, vendored node_modules), and extraction-shape gaps (hashes under an existing actions.yml key). Reword the case E row to cover all of these, and add a sub-table under step 5/E that lists 15 historical extensions with the PR or commit that closed each. The table is the template a future agent reaches for when "the verdict is wrong but not for a security reason" — instead of waving the finding off, they can match it to the closest precedent and propose a similar extension. Also tighten the steps: name each module under utils/verify_action_build/ and what it owns (pr_extraction, security, verification, docker_build, action_ref/release_lookup/github_client), so picking the right place for the fix isn't a treasure hunt. Add an explicit "run prek before push" step pointing at AGENTS.md.
…n-tree Add `analyze_in_tree_binaries` to flag actions that commit native binaries (Go cross-compile, .exe/.dll/.so/.dylib/.jar/.wasm, etc.) directly in their repo and exec them from a small launcher. The JS-rebuild check verifies the launcher; the binaries are opaque executable code that no part of our pipeline can reconcile with source. runs-on/[email protected] (apache#809) is the case in point — ~10 MB of UPX-packed Go binaries run as root. The check is path-only (cheap, no fetch): known binary extensions plus the <name>-<os>-<arch>(.exe)? cross-compile suffix. Add the new failure path to the verification summary and overall result. Also document the criterion in the security review checklist and add case F (in-tree native binaries) + a runs-on/action precedent to the analyze-action-pr skill.
Member
|
My approval is contingent on @gmcdonald confirming that If it still needs to be used then I think we will need an allow-list of actions that pass verify. Maybe these are ones we can wildcard. |
Member
Author
Note that failing the test does not mean rejection. It just means failing the test. It can still be approved. |
Member
Author
Also - if you look there runs-on/action#36 The maintainer had been very responsive and implemented attestation for building those binaries (cc: @ppkarwasz ) - so we still might allow the actions where attestations are present - they do not give us 100% reproducibility - but I think the trust in this case moves from the author to GitHub + review of the build steps. Am I right Piotr? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
verify-action-build. Detects actions that ship pre-compiled native binaries directly in the repo (Go cross-compiled likemain-linux-amd64, plus*.exe/*.dll/*.so/*.dylib/*.jar/*.wasm, etc.) and exec them from a small launcher.<name>-<os>-<arch>(.exe)?cross-compile suffix. No fetch, no magic-byte sniffing.runs-on/[email protected]precedent to theanalyze-action-prskill.runs-on/[email protected]); upstream issue at Add build provenance attestations + signed release artifacts so downstream consumers can verify binaries runs-on/action#36.Test plan
uv run pytest utils/tests/— 224/224 (15 new).prek run --all-filesclean.verify-action-build runs-on/action@e46a3c6d…flags 3 binaries and fails.verify-action-build docker/bake-action@a66e1c87…(clean bundled JS action) passes.Stacked on #807 — this PR edits
.claude/skills/analyze-action-pr/SKILL.mdwhich is introduced by #807. Until #807 merges, this PR's diff againstmainwill include #807's commits; once #807 merges, only this PR's content remains.Generated-by Claude Code.