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