From ca6100eff269f385dc7a524b723cfa22421d8f55 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Thu, 27 Aug 2026 20:38:34 +0200 Subject: [PATCH] verify-action-build: credit in-tree binaries the rebuild reproduces The in-tree binary check only knew how to verify a binary against GitHub release provenance - a SLSA attestation or a SHA256SUMS asset. A bundler- copied dependency asset has neither and never will: 1Password/load-secrets- action's dist/core_bg.wasm is ncc output copied from @1password/sdk-core, pinned by sha512 integrity in package-lock.json, so PR #1195 hard-failed on a binary whose provenance chain is npm's rather than the action's. Delete those binaries from the output directory before the rebuild, as the minified JS already is, and credit any that come back byte-identical. That is the same guarantee the JS rebuild check gives. A fully-reproduced action now skips the release lookups entirely; everything else is unchanged. Generated-by: Claude Code (Opus 5) --- README.md | 2 +- .../verify_action_build/test_security.py | 119 ++++++++++++++++++ .../dockerfiles/build_action.Dockerfile | 19 +++ utils/verify_action_build/security.py | 77 ++++++++++-- utils/verify_action_build/verification.py | 13 +- 5 files changed, 219 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index b03f70dfc..c06123fa9 100644 --- a/README.md +++ b/README.md @@ -270,7 +270,7 @@ When reviewing an action (new or updated), watch for these potential issues in t - **Obfuscated code**: hex-encoded strings, base64 blobs, or intentionally unreadable code in source files (not in compiled `dist/`). - **File-system tampering**: writing to locations outside the workspace (`$GITHUB_WORKSPACE`), modifying `$GITHUB_ENV`, `$GITHUB_PATH`, or `$GITHUB_OUTPUT` in unexpected ways to influence subsequent workflow steps. - **Compiled JS mismatch**: any unexplained diff between the published `dist/` and a clean rebuild — this is the primary check the verification script performs. -- **Pre-compiled native binaries shipped in-tree**: actions that commit Go/Rust/C-style binaries (`main-linux-amd64`, `*.exe`, `*.dll`, `*.so`, `*.dylib`, `*.jar`, `*.wasm`, etc.) directly in the repo and exec them from a small launcher are running opaque executable code on the runner. The JS-rebuild check verifies the launcher but **cannot** reconcile the binaries with source on its own. `verify-action-build`'s **In-tree binary check** tries to close the gap automatically: each detected binary is verified first via `gh attestation verify --owner ` (the SLSA attestation transparency log populated by [`actions/attest-build-provenance`](https://github.com/actions/attest-build-provenance)), then by SHA256-comparing each binary against the release's `SHA256SUMS` asset. Binaries that pass either check are ✓; binaries that pass neither are a hard reject. Push back on actions in this shape until upstream adds attestation or `SHA256SUMS` so the chain from release to artifact can be verified. +- **Pre-compiled native binaries shipped in-tree**: actions that commit Go/Rust/C-style binaries (`main-linux-amd64`, `*.exe`, `*.dll`, `*.so`, `*.dylib`, `*.jar`, `*.wasm`, etc.) directly in the repo and exec them from a small launcher are running opaque executable code on the runner. The JS-rebuild check verifies the launcher but **cannot** reconcile the binaries with source on its own. `verify-action-build`'s **In-tree binary check** tries to close the gap automatically: each detected binary is verified first by the clean rebuild (binaries a bundler copies into the output directory — `.wasm`, `.node`, native libraries — are deleted before the rebuild along with the minified JS, so one that comes back byte-identical was regenerated from the lockfile-pinned dependency tree and needs no release provenance of its own; `1Password/load-secrets-action` ships `dist/core_bg.wasm` this way, copied by `ncc` out of `@1password/sdk-core`), then via `gh attestation verify --owner ` (the SLSA attestation transparency log populated by [`actions/attest-build-provenance`](https://github.com/actions/attest-build-provenance)), then by SHA256-comparing each binary against the release's `SHA256SUMS` asset. Binaries that pass any of the three are ✓; binaries that pass none are a hard reject. Push back on actions in this shape until upstream adds attestation or `SHA256SUMS` so the chain from release to artifact can be verified. - **Runtime binary downloads without an in-source checksum**: some actions pull their tool binary at runtime via `tc.downloadTool` / `curl` / `fetch` and rely on the publishing pipeline (GitHub release immutability + Sigstore attestation) for integrity rather than an inline `sha256sum -c` / `cosign verify-blob`. The **Binary Download Verification** check fails these by default. A per-action escape hatch lives in `utils/verify_action_build/security.py` as the `TRUSTED_DOWNLOAD_PROVENANCE` dict — an entry asserts that the configured `release_repo` publishes immutable releases AND emits Sigstore attestations via `actions/attest-build-provenance`. Adding an entry is a security review decision and the rationale must link the upstream confirmation (e.g. a maintainer comment). The config alone is not enough: at scan time the verify pipeline GETs `releases/latest` of the configured `release_repo`, confirms `release.immutable` is true, downloads one small attested asset (`.sbom.json` preferred), and runs `gh attestation verify` against it. Only when both halves pass are the action's unverified-download findings reclassified as warnings; if the runtime check fails, failures stay failures and the reason is printed. Note the scope: the spot-check proves the *release repo's pipeline* attests and that its latest release is immutable — it does not machine-verify that the action downloads from that `release_repo`, nor that the *specific version* it fetches is itself immutable (only `releases/latest` is checked). That binding remains the reviewer's call, backed by the entry's `rationale`. For the full approval policy and requirements, see the [ASF GitHub Actions Policy](https://infra.apache.org/github-actions-policy.html). diff --git a/utils/tests/verify_action_build/test_security.py b/utils/tests/verify_action_build/test_security.py index 0816af03f..353a8d720 100644 --- a/utils/tests/verify_action_build/test_security.py +++ b/utils/tests/verify_action_build/test_security.py @@ -23,6 +23,7 @@ analyze_binary_downloads_recursive, analyze_dockerfile, analyze_in_tree_binaries, + find_rebuild_reproduced_binaries, analyze_lock_files, analyze_scripts, analyze_action_metadata, @@ -1602,6 +1603,124 @@ def test_platform_dir_requires_parent(self): assert not _looks_like_in_tree_binary("docs/glnxa64.md") +class TestFindRebuildReproducedBinaries: + """1Password/load-secrets-action ships dist/core_bg.wasm, which ncc copies + out of the lockfile-pinned @1password/sdk-core package. It has no GitHub + release provenance of its own and never will, so the in-tree check's + attestation/SHA256SUMS cascade rejected it. The rebuild deletes it and + puts it back, which is the guarantee that actually applies.""" + + def _tree(self, root, files): + for rel, data in files.items(): + path = root / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(data) + + def test_identical_binary_is_credited(self, tmp_path): + original, rebuilt = tmp_path / "orig", tmp_path / "new" + self._tree(original, {"core_bg.wasm": b"\x00asm\x01wasm-bytes"}) + self._tree(rebuilt, {"core_bg.wasm": b"\x00asm\x01wasm-bytes"}) + assert find_rebuild_reproduced_binaries(original, rebuilt, "dist") == { + "dist/core_bg.wasm" + } + + def test_differing_binary_is_not_credited(self, tmp_path): + original, rebuilt = tmp_path / "orig", tmp_path / "new" + self._tree(original, {"core_bg.wasm": b"\x00asm\x01committed"}) + self._tree(rebuilt, {"core_bg.wasm": b"\x00asm\x01rebuilt"}) + assert find_rebuild_reproduced_binaries(original, rebuilt, "dist") == set() + + def test_binary_missing_from_rebuild_is_not_credited(self, tmp_path): + # The deletion step removed it and the build never put it back — that + # is shipped code the rebuild cannot account for. + original, rebuilt = tmp_path / "orig", tmp_path / "new" + self._tree(original, {"core_bg.wasm": b"\x00asm\x01bytes"}) + rebuilt.mkdir(parents=True) + assert find_rebuild_reproduced_binaries(original, rebuilt, "dist") == set() + + def test_nested_path_and_out_dir_prefix(self, tmp_path): + original, rebuilt = tmp_path / "orig", tmp_path / "new" + self._tree(original, {"vendor/native.node": b"native"}) + self._tree(rebuilt, {"vendor/native.node": b"native"}) + assert find_rebuild_reproduced_binaries(original, rebuilt, "lib") == { + "lib/vendor/native.node" + } + + def test_non_binary_files_ignored(self, tmp_path): + original, rebuilt = tmp_path / "orig", tmp_path / "new" + self._tree(original, {"index.js": b"console.log(1)"}) + self._tree(rebuilt, {"index.js": b"console.log(1)"}) + assert find_rebuild_reproduced_binaries(original, rebuilt, "dist") == set() + + def test_missing_directories(self, tmp_path): + assert find_rebuild_reproduced_binaries( + tmp_path / "nope", tmp_path / "also-nope", "dist" + ) == set() + + +class TestInTreeBinaryRebuildCredit: + def _patch_tree(self, paths): + return mock.patch( + "verify_action_build.security._list_repo_files", return_value=list(paths) + ) + + def test_reproduced_binary_passes_without_release_provenance(self): + # No attestation, no SHA256SUMS — the old cascade's only outcome was a + # hard error. The rebuild credit has to short-circuit before either + # network path is consulted. + with self._patch_tree(["dist/index.js", "dist/core_bg.wasm"]), \ + mock.patch("verify_action_build.security._resolve_tag_for_commit") as tag, \ + mock.patch("verify_action_build.security._fetch_blob_bytes") as blob: + errors = analyze_in_tree_binaries( + "org", "repo", "a" * 40, + reproduced_by_rebuild={"dist/core_bg.wasm"}, + ) + assert errors == [] + tag.assert_not_called() + blob.assert_not_called() + + def test_unreproduced_binary_still_fails(self): + with self._patch_tree(["dist/core_bg.wasm"]), \ + mock.patch( + "verify_action_build.security._resolve_tag_for_commit", + return_value=None, + ), \ + mock.patch( + "verify_action_build.security._fetch_blob_bytes", + return_value=b"opaque", + ), \ + mock.patch( + "verify_action_build.security._verify_via_gh_attestation", + return_value=False, + ): + errors = analyze_in_tree_binaries("org", "repo", "a" * 40) + assert len(errors) == 1 + assert "core_bg.wasm" in errors[0] + + def test_credit_does_not_leak_to_other_binaries(self): + # A committed launcher binary next to a reproduced asset must still be + # rejected on its own merits. + with self._patch_tree(["dist/core_bg.wasm", "bin/main-linux-amd64"]), \ + mock.patch( + "verify_action_build.security._resolve_tag_for_commit", + return_value=None, + ), \ + mock.patch( + "verify_action_build.security._fetch_blob_bytes", + return_value=b"opaque", + ), \ + mock.patch( + "verify_action_build.security._verify_via_gh_attestation", + return_value=False, + ): + errors = analyze_in_tree_binaries( + "org", "repo", "a" * 40, + reproduced_by_rebuild={"dist/core_bg.wasm"}, + ) + assert len(errors) == 1 + assert "main-linux-amd64" in errors[0] + + class TestParseSha256sums: """Parse the standard `` `` format used by ``sha256sum`` and emitted by GitHub's ``actions/attest-build-provenance`` example diff --git a/utils/verify_action_build/dockerfiles/build_action.Dockerfile b/utils/verify_action_build/dockerfiles/build_action.Dockerfile index 79746adfa..22ec07730 100644 --- a/utils/verify_action_build/dockerfiles/build_action.Dockerfile +++ b/utils/verify_action_build/dockerfiles/build_action.Dockerfile @@ -169,6 +169,25 @@ RUN OUT_DIR=$(cat /out-dir.txt); \ echo "no $OUT_DIR/ directory" > /deleted-js.log; \ fi +# Bundlers copy dependency assets into the output directory alongside the JS +# they emit — @vercel/ncc does this for the .wasm that backs a Rust-built SDK, +# for instance. Those land in the tree as pre-compiled binaries with no +# GitHub-release provenance of their own, so the in-tree binary check has +# nothing to verify them against. Delete them here for the same reason the +# minified JS is deleted: if the rebuild puts an identical file back, it is +# build output reproduced from the lockfile-pinned dependencies rather than +# opaque committed code, and the in-tree check can credit it on that basis. +RUN OUT_DIR=$(cat /out-dir.txt); \ + : > /deleted-binaries.log; \ + if [ -d "$OUT_DIR" ]; then \ + find "$OUT_DIR" \( -name '*.wasm' -o -name '*.node' -o -name '*.so' \ + -o -name '*.dll' -o -name '*.dylib' -o -name '*.exe' \) -type f \ + | while IFS= read -r f; do \ + echo "$f" >> /deleted-binaries.log; \ + rm -f "$f"; \ + done; \ + fi + # If an approved (previous) commit hash is provided, restore the dev-dependency # lock files from that commit so the rebuild uses the same toolchain (e.g. same # rollup/ncc/webpack version) that produced the original dist/. diff --git a/utils/verify_action_build/security.py b/utils/verify_action_build/security.py index 1936f1f73..5435b6267 100644 --- a/utils/verify_action_build/security.py +++ b/utils/verify_action_build/security.py @@ -1770,6 +1770,14 @@ def analyze_repo_metadata( ".wasm", ) +# Binaries a JS bundler copies into the output directory as dependency assets +# rather than the action committing them by hand. These are deleted before +# the rebuild (see build_action.Dockerfile) so that a byte-identical file +# coming back proves the rebuild produced it. +_REBUILDABLE_BINARY_EXTENSIONS = ( + ".wasm", ".node", ".so", ".dll", ".dylib", ".exe", +) + # Cross-compiled binary naming convention used by Go, Rust, and similar # toolchains: ``--`` with an optional ``.exe``. Catches # the runs-on/action shape (main-linux-amd64, main-windows-amd64.exe). @@ -2184,8 +2192,42 @@ def verify_trusted_download_provenance( ) +def find_rebuild_reproduced_binaries( + original_dir: Path, rebuilt_dir: Path, out_dir_name: str, +) -> set[str]: + """Repo-relative paths of output-dir binaries the rebuild reproduced. + + The rebuild deletes bundler-copied binaries (``*.wasm``, ``*.node``, + native libraries) from the output directory before building, exactly as + it deletes minified JS. A file that comes back byte-identical was + therefore regenerated from the lockfile-pinned dependency tree rather + than read back off the committed tree — the same guarantee the JS + rebuild check provides, so the in-tree binary check can credit it. + + Returns paths prefixed with *out_dir_name* (e.g. ``dist/core_bg.wasm``) + so they line up with the repo-relative paths the in-tree check uses. + """ + reproduced: set[str] = set() + if not original_dir.is_dir() or not rebuilt_dir.is_dir(): + return reproduced + + for original in original_dir.rglob("*"): + if not original.is_file(): + continue + rel = original.relative_to(original_dir) + if not rel.name.lower().endswith(_REBUILDABLE_BINARY_EXTENSIONS): + continue + rebuilt = rebuilt_dir / rel + if not rebuilt.is_file(): + continue + if original.read_bytes() == rebuilt.read_bytes(): + reproduced.add(f"{out_dir_name}/{rel.as_posix()}") + return reproduced + + def analyze_in_tree_binaries( org: str, repo: str, commit_hash: str, sub_path: str = "", + reproduced_by_rebuild: set[str] | None = None, ) -> list[str]: """Flag pre-compiled native binaries shipped in the action's tree that lack verifiable upstream provenance. @@ -2237,20 +2279,31 @@ def analyze_in_tree_binaries( console.print() console.rule("[bold]In-tree Binary Check[/bold]") - # Look up the tag once — both verification paths key off the release - # for this commit. - tag_name = _resolve_tag_for_commit(org, repo, commit_hash) + # Binaries the clean rebuild regenerated byte-for-byte are build output + # from the lockfile-pinned dependency tree, not opaque code the action + # committed — upstream release provenance is the wrong thing to ask of + # them. Settle these first so a fully-reproduced action skips the + # release lookups entirely. + reproduced = reproduced_by_rebuild or set() + verified_rebuild = [b for b in binaries if b in reproduced] + pending = [b for b in binaries if b not in reproduced] + + tag_name: str | None = None sha256sums: dict[str, str] | None = None - if tag_name: - text = _fetch_release_asset_text(org, repo, tag_name, "SHA256SUMS") - if text: - sha256sums = _parse_sha256sums(text) + if pending: + # Look up the tag once — both remaining verification paths key off + # the release for this commit. + tag_name = _resolve_tag_for_commit(org, repo, commit_hash) + if tag_name: + text = _fetch_release_asset_text(org, repo, tag_name, "SHA256SUMS") + if text: + sha256sums = _parse_sha256sums(text) verified_attestation: list[str] = [] verified_sha256sums: list[str] = [] unverified: list[tuple[str, str]] = [] # (path, reason) - for binary in binaries: + for binary in pending: full_path = f"{prefix}{binary}" content = _fetch_blob_bytes(org, repo, commit_hash, full_path) if content is None: @@ -2294,6 +2347,14 @@ def analyze_in_tree_binaries( unverified.append((binary, reason)) # Summary block. + if verified_rebuild: + console.print( + f" [green]✓[/green] {len(verified_rebuild)} binary(ies) " + f"reproduced byte-for-byte by the clean rebuild (bundler-copied " + f"dependency assets):" + ) + for path in verified_rebuild: + console.print(f" [green]✓[/green] {path}") if verified_attestation: console.print( f" [green]✓[/green] {len(verified_attestation)} binary(ies) " diff --git a/utils/verify_action_build/verification.py b/utils/verify_action_build/verification.py index 1291e57b7..2be2c438c 100644 --- a/utils/verify_action_build/verification.py +++ b/utils/verify_action_build/verification.py @@ -50,6 +50,7 @@ analyze_nested_actions, analyze_repo_metadata, analyze_scripts, + find_rebuild_reproduced_binaries, ) SECURITY_CHECKLIST_URL = "https://github.com/apache/infrastructure-actions#security-review-checklist" @@ -328,18 +329,26 @@ def verify_single_action( # that the JS-rebuild check cannot reconcile with source — the # launcher matches, the binary doesn't. See runs-on/action@v2.1.x # for the canonical case. + # Binaries a bundler copied into the output directory are deleted + # before the rebuild, so any that come back byte-identical are build + # output from the lockfile-pinned dependency tree and need no + # upstream release provenance of their own. + rebuild_reproduced_binaries = find_rebuild_reproduced_binaries( + original_dir, rebuilt_dir, out_dir_name, + ) in_tree_binary_errors = analyze_in_tree_binaries( org, repo, commit_hash, sub_path, + reproduced_by_rebuild=rebuild_reproduced_binaries, ) if in_tree_binary_errors: checks_performed.append(( "In-tree binary check", "fail", - "unverified binaries in repo (no SLSA attestation / SHA256SUMS)", + "unverified binaries in repo (no rebuild match / SLSA attestation / SHA256SUMS)", )) else: checks_performed.append(( "In-tree binary check", "pass", - "no in-tree binaries (or all verified via attestation / SHA256SUMS)", + "no in-tree binaries (or all verified via rebuild / attestation / SHA256SUMS)", )) # Vendored npm dependency check: when an action commits its