Merge pull request #743 from apache/verify-action-binary-download-check

verify-action-build: add binary download verification check

Author: potiuk

utils/tests/verify_action_build/test_security.py

@@ -19,6 +19,8 @@
 from unittest import mock
 
 from verify_action_build.security import (
+    analyze_binary_downloads,
+    analyze_binary_downloads_recursive,
     analyze_dockerfile,
     analyze_scripts,
     analyze_action_metadata,
@@ -184,6 +186,236 @@ def test_secret_default_warns(self):
         assert any("secret" in w for w in warnings)
 
 
+class TestAnalyzeBinaryDownloads:
+    def _mock_fetch(self, files: dict):
+        def fetch(org, repo, commit, path):
+            return files.get(path)
+        return fetch
+
+    def test_no_files_no_results(self):
+        with mock.patch("verify_action_build.security.fetch_file_from_github", return_value=None):
+            with mock.patch("verify_action_build.security.fetch_action_yml", return_value=None):
+                warnings, failures = analyze_binary_downloads("org", "repo", "a" * 40)
+        assert warnings == []
+        assert failures == []
+
+    def test_pipe_to_shell_fails(self):
+        files = {
+            "Dockerfile": (
+                "FROM alpine@sha256:abc\n"
+                "RUN curl -fsSL https://example.com/install.sh | sh\n"
+            ),
+        }
+        with mock.patch("verify_action_build.security.fetch_file_from_github", side_effect=self._mock_fetch(files)):
+            with mock.patch("verify_action_build.security.fetch_action_yml", return_value=None):
+                warnings, failures = analyze_binary_downloads("org", "repo", "a" * 40)
+        assert len(failures) >= 1
+        assert any("install.sh" in f for f in failures)
+
+    def test_curl_binary_without_verification_fails(self):
+        files = {
+            "Dockerfile": (
+                "FROM alpine@sha256:abc\n"
+                "RUN curl -fsSLO https://example.com/tool.tar.gz && tar xf tool.tar.gz\n"
+            ),
+        }
+        with mock.patch("verify_action_build.security.fetch_file_from_github", side_effect=self._mock_fetch(files)):
+            with mock.patch("verify_action_build.security.fetch_action_yml", return_value=None):
+                warnings, failures = analyze_binary_downloads("org", "repo", "a" * 40)
+        assert len(failures) >= 1
+        assert any("tool.tar.gz" in f for f in failures)
+
+    def test_sha256sum_verification_passes(self):
+        files = {
+            "Dockerfile": (
+                "FROM alpine@sha256:abc\n"
+                "RUN curl -fsSLO https://example.com/tool.tar.gz \\\n"
+                " && echo 'abc123deadbeefcafef00d  tool.tar.gz' | sha256sum -c\n"
+            ),
+        }
+        with mock.patch("verify_action_build.security.fetch_file_from_github", side_effect=self._mock_fetch(files)):
+            with mock.patch("verify_action_build.security.fetch_action_yml", return_value=None):
+                warnings, failures = analyze_binary_downloads("org", "repo", "a" * 40)
+        assert failures == []
+
+    def test_gpg_verify_passes(self):
+        files = {
+            "Dockerfile": (
+                "FROM alpine@sha256:abc\n"
+                "RUN curl -fsSLO https://example.com/tool.tar.gz \\\n"
+                " && curl -fsSLO https://example.com/tool.tar.gz.sig \\\n"
+                " && gpg --verify tool.tar.gz.sig tool.tar.gz\n"
+            ),
+        }
+        with mock.patch("verify_action_build.security.fetch_file_from_github", side_effect=self._mock_fetch(files)):
+            with mock.patch("verify_action_build.security.fetch_action_yml", return_value=None):
+                warnings, failures = analyze_binary_downloads("org", "repo", "a" * 40)
+        assert failures == []
+
+    def test_cosign_verify_passes(self):
+        files = {
+            "Dockerfile": (
+                "FROM alpine@sha256:abc\n"
+                "RUN curl -fsSLO https://example.com/tool.tar.gz && cosign verify-blob tool.tar.gz\n"
+            ),
+        }
+        with mock.patch("verify_action_build.security.fetch_file_from_github", side_effect=self._mock_fetch(files)):
+            with mock.patch("verify_action_build.security.fetch_action_yml", return_value=None):
+                warnings, failures = analyze_binary_downloads("org", "repo", "a" * 40)
+        assert failures == []
+
+    def test_apk_add_not_flagged(self):
+        files = {
+            "Dockerfile": (
+                "FROM alpine@sha256:abc\n"
+                "RUN apk add --no-cache curl wget bash\n"
+            ),
+        }
+        with mock.patch("verify_action_build.security.fetch_file_from_github", side_effect=self._mock_fetch(files)):
+            with mock.patch("verify_action_build.security.fetch_action_yml", return_value=None):
+                warnings, failures = analyze_binary_downloads("org", "repo", "a" * 40)
+        assert failures == []
+
+    def test_pip_install_not_flagged(self):
+        files = {
+            "Dockerfile": (
+                "FROM python:3.12@sha256:abc\n"
+                "RUN pip install requests==2.31.0\n"
+            ),
+        }
+        with mock.patch("verify_action_build.security.fetch_file_from_github", side_effect=self._mock_fetch(files)):
+            with mock.patch("verify_action_build.security.fetch_action_yml", return_value=None):
+                warnings, failures = analyze_binary_downloads("org", "repo", "a" * 40)
+        assert failures == []
+
+    def test_dockerfile_add_url_fails(self):
+        files = {
+            "Dockerfile": (
+                "FROM alpine@sha256:abc\n"
+                "ADD https://example.com/app.tar.gz /app.tar.gz\n"
+            ),
+        }
+        with mock.patch("verify_action_build.security.fetch_file_from_github", side_effect=self._mock_fetch(files)):
+            with mock.patch("verify_action_build.security.fetch_action_yml", return_value=None):
+                warnings, failures = analyze_binary_downloads("org", "repo", "a" * 40)
+        assert len(failures) >= 1
+
+    def test_action_yml_run_block_unverified_fails(self):
+        action_yml = """\
+name: Test
+runs:
+  using: composite
+  steps:
+    - name: Download tool
+      shell: bash
+      run: |
+        curl -fsSLO https://example.com/tool.tar.gz
+        tar xf tool.tar.gz
+"""
+        with mock.patch("verify_action_build.security.fetch_action_yml", return_value=action_yml):
+            with mock.patch("verify_action_build.security.fetch_file_from_github", return_value=None):
+                warnings, failures = analyze_binary_downloads("org", "repo", "a" * 40)
+        assert len(failures) >= 1
+
+    def test_action_yml_run_block_with_verification_passes(self):
+        action_yml = """\
+name: Test
+runs:
+  using: composite
+  steps:
+    - name: Download and verify
+      shell: bash
+      run: |
+        curl -fsSLO https://example.com/tool.tar.gz
+        echo "abc123deadbeef  tool.tar.gz" | sha256sum -c
+        tar xf tool.tar.gz
+"""
+        with mock.patch("verify_action_build.security.fetch_action_yml", return_value=action_yml):
+            with mock.patch("verify_action_build.security.fetch_file_from_github", return_value=None):
+                warnings, failures = analyze_binary_downloads("org", "repo", "a" * 40)
+        assert failures == []
+
+    def test_releases_download_path_flagged(self):
+        # URL without a binary extension but under /releases/download/ still
+        # looks like a binary artefact — flag it.
+        files = {
+            "Dockerfile": (
+                "FROM alpine@sha256:abc\n"
+                "RUN curl -fsSLo /usr/local/bin/mytool "
+                "https://github.com/x/y/releases/download/v1/mytool-linux-amd64 "
+                "&& chmod +x /usr/local/bin/mytool\n"
+            ),
+        }
+        with mock.patch("verify_action_build.security.fetch_file_from_github", side_effect=self._mock_fetch(files)):
+            with mock.patch("verify_action_build.security.fetch_action_yml", return_value=None):
+                warnings, failures = analyze_binary_downloads("org", "repo", "a" * 40)
+        assert len(failures) >= 1
+
+
+class TestAnalyzeBinaryDownloadsRecursive:
+    def test_recurses_through_composite(self):
+        # Root is a composite action that uses an unpinned-looking hash-pinned
+        # nested action; nested action has an unverified download.
+        root_yml = """\
+name: Root
+runs:
+  using: composite
+  steps:
+    - uses: other/helper@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+"""
+        nested_yml = """\
+name: Helper
+runs:
+  using: composite
+  steps:
+    - name: Download
+      shell: bash
+      run: |
+        curl -fsSLO https://example.com/tool.tar.gz
+"""
+
+        def fake_action_yml(org, repo, commit, sub_path=""):
+            if org == "myorg":
+                return root_yml
+            if org == "other":
+                return nested_yml
+            return None
+
+        def fake_file(org, repo, commit, path):
+            return None
+
+        with mock.patch("verify_action_build.security.fetch_action_yml", side_effect=fake_action_yml):
+            with mock.patch("verify_action_build.security.fetch_file_from_github", side_effect=fake_file):
+                warnings, failures = analyze_binary_downloads_recursive(
+                    "myorg", "rootrepo", "b" * 40,
+                )
+        assert len(failures) >= 1
+        assert any("tool.tar.gz" in f for f in failures)
+
+    def test_skips_trusted_org(self):
+        # Nested uses: actions/checkout — should be skipped (trusted), so no
+        # recursion into it even if it had downloads.
+        root_yml = """\
+name: Root
+runs:
+  using: composite
+  steps:
+    - uses: actions/checkout@cccccccccccccccccccccccccccccccccccccccc
+"""
+
+        def fake_action_yml(org, repo, commit, sub_path=""):
+            if org == "myorg":
+                return root_yml
+            raise AssertionError(f"should not fetch nested yml for {org}/{repo}")
+
+        with mock.patch("verify_action_build.security.fetch_action_yml", side_effect=fake_action_yml):
+            with mock.patch("verify_action_build.security.fetch_file_from_github", return_value=None):
+                warnings, failures = analyze_binary_downloads_recursive(
+                    "myorg", "rootrepo", "b" * 40,
+                )
+        assert failures == []
+
+
 class TestAnalyzeRepoMetadata:
     def test_mit_license_detected(self):
         def fetch(org, repo, commit, path):

utils/verify_action_build/cli.py

@@ -82,11 +82,22 @@ def main() -> None:
         action="store_true",
         help="Show Docker build step summary on successful builds (always shown on failure)",
     )
+    parser.add_argument(
+        "--no-binary-download-check",
+        action="store_true",
+        help=(
+            "Disable the binary download verification check (enabled by default). "
+            "The check scans Dockerfiles, run: blocks and referenced scripts for "
+            "curl/wget/ADD downloads of binaries or scripts and fails the run if "
+            "the file has no detectable checksum/signature verification."
+        ),
+    )
     args = parser.parse_args()
 
     ci_mode = args.ci
     cache = not args.no_cache
     show_build_steps = args.show_build_steps
+    check_binary_downloads = not args.no_binary_download_check
 
     if not shutil.which("docker"):
         console.print("[red]Error:[/red] docker is required but not found in PATH")
@@ -117,12 +128,26 @@ def main() -> None:
             _exit(1)
         for ref in action_refs:
             console.print(f"  Extracted action reference from PR #{args.from_pr}: [bold]{ref}[/bold]")
-        passed = all(verify_single_action(ref, gh=gh, ci_mode=ci_mode, cache=cache, show_build_steps=show_build_steps) for ref in action_refs)
+        passed = all(
+            verify_single_action(
+                ref, gh=gh, ci_mode=ci_mode, cache=cache,
+                show_build_steps=show_build_steps,
+                check_binary_downloads=check_binary_downloads,
+            )
+            for ref in action_refs
+        )
         _exit(0 if passed else 1)
     elif args.check_dependabot_prs:
-        check_dependabot_prs(gh=gh, cache=cache, show_build_steps=show_build_steps)
+        check_dependabot_prs(
+            gh=gh, cache=cache, show_build_steps=show_build_steps,
+            check_binary_downloads=check_binary_downloads,
+        )
     elif args.action_ref:
-        passed = verify_single_action(args.action_ref, gh=gh, ci_mode=ci_mode, cache=cache, show_build_steps=show_build_steps)
+        passed = verify_single_action(
+            args.action_ref, gh=gh, ci_mode=ci_mode, cache=cache,
+            show_build_steps=show_build_steps,
+            check_binary_downloads=check_binary_downloads,
+        )
         _exit(0 if passed else 1)
     else:
         parser.print_help()

utils/verify_action_build/dependabot.py

@@ -35,7 +35,10 @@ def get_gh_user(gh: GitHubClient | None = None) -> str:
     return gh.get_authenticated_user()
 
 
-def check_dependabot_prs(gh: GitHubClient, cache: bool = True, show_build_steps: bool = False) -> None:
+def check_dependabot_prs(
+    gh: GitHubClient, cache: bool = True, show_build_steps: bool = False,
+    check_binary_downloads: bool = True,
+) -> None:
     """List open dependabot PRs, verify each, and optionally merge."""
     console.print()
     console.rule("[bold]Dependabot PR Review[/bold]")
@@ -158,10 +161,17 @@ def check_dependabot_prs(gh: GitHubClient, cache: bool = True, show_build_steps:
                         sub_ref = f"{org_repo}/{sp}@{commit_hash}"
                     else:
                         sub_ref = f"{org_repo}@{commit_hash}"
-                    if not verify_single_action(sub_ref, gh=gh, cache=cache, show_build_steps=show_build_steps):
+                    if not verify_single_action(
+                        sub_ref, gh=gh, cache=cache, show_build_steps=show_build_steps,
+                        check_binary_downloads=check_binary_downloads,
+                    ):
                         passed = False
             else:
-                if not verify_single_action(f"{org_repo}@{commit_hash}", gh=gh, cache=cache, show_build_steps=show_build_steps):
+                if not verify_single_action(
+                    f"{org_repo}@{commit_hash}", gh=gh, cache=cache,
+                    show_build_steps=show_build_steps,
+                    check_binary_downloads=check_binary_downloads,
+                ):
                     passed = False
 
         if not passed: