feat: detect vulnerable GitHub Actions #1021

behnazh-w · 2025-03-21T07:39:42Z

Summary

This pull request introduces a new check mcn_githubactions_vulnerabilities_1 to detect vulnerable GitHub Actions, enhancing the security of workflows and automating the identification of potential risks in CI/CD pipelines. The key changes include:

Changes

A check for specific versions of third-party GitHub Actions that are known to be affected by vulnerabilities. The version can be a commit SHA associated with a tag (version).
A new module has been added to implement interactions with OSV (Open Source Vulnerability) API.

Documentation

The documentation website has been updated to reflect this new check.
A tutorial has been added to guide users through the process of setting up and using the check in their workflows.

Tests

Integration and unit tests have been added.

Signed-off-by: behnazh-w <[email protected]>

benmss · 2025-04-08T02:40:47Z

docs/source/index.rst

@@ -46,12 +46,12 @@ Current checks in Macaron
 The table below shows the current set of actionable checks derived from
 the requirements that are currently supported by Macaron.

-.. list-table:: Mapping SLSA requirements to Macaron checks
+.. list-table:: Macaron checks descriptions


Suggested change

.. list-table:: Macaron checks descriptions

.. list-table:: Macaron check descriptions

benmss · 2025-04-08T02:44:12Z

docs/source/pages/tutorials/detect_vulnerable_github_actions.rst

+How to detect vulnerable GitHub Actions
+=======================================
+
+This tutorial explains how to use a check in Macaron that detects vulnerable third-party GitHub Actions. This check is important for preventing security issues in your CI/CD pipeline, especially in light of recent incidents, such as vulnerabilities discovered in popular GitHub Actions like `tj-actions/changed-files <https://www.cve.org/CVERecord?id=CVE-2025-30066>`_, and `reviewdog/action-setup <https://www.cve.org/CVERecord?id=CVE-2025-30154>`_.


Suggested change

This tutorial explains how to use a check in Macaron that detects vulnerable third-party GitHub Actions. This check is important for preventing security issues in your CI/CD pipeline, especially in light of recent incidents, such as vulnerabilities discovered in popular GitHub Actions like `tj-actions/changed-files <https://www.cve.org/CVERecord?id=CVE-2025-30066>`_, and `reviewdog/action-setup <https://www.cve.org/CVERecord?id=CVE-2025-30154>`_.

This tutorial explains how to use a check in Macaron to detect vulnerable third-party GitHub Actions. This check is important for preventing security issues in your CI/CD pipeline, especially in light of recent incidents, such as vulnerabilities discovered in popular GitHub Actions: `tj-actions/changed-files <https://www.cve.org/CVERecord?id=CVE-2025-30066>`_, and `reviewdog/action-setup <https://www.cve.org/CVERecord?id=CVE-2025-30154>`_.

benmss · 2025-04-08T02:44:31Z

docs/source/pages/tutorials/detect_vulnerable_github_actions.rst

+
+This tutorial explains how to use a check in Macaron that detects vulnerable third-party GitHub Actions. This check is important for preventing security issues in your CI/CD pipeline, especially in light of recent incidents, such as vulnerabilities discovered in popular GitHub Actions like `tj-actions/changed-files <https://www.cve.org/CVERecord?id=CVE-2025-30066>`_, and `reviewdog/action-setup <https://www.cve.org/CVERecord?id=CVE-2025-30154>`_.
+
+We will guide you on how to enable and use this check to enhance the security of your development pipeline.


Suggested change

We will guide you on how to enable and use this check to enhance the security of your development pipeline.

We will demonstrate how to enable and use this check to enhance the security of your development pipeline.

benmss · 2025-04-08T02:56:39Z

src/macaron/slsa_analyzer/checks/github_actions_vulnerability_check.py

+                if isinstance(callee, GitHubWorkflowNode) and callee.node_type in [
+                    GitHubWorkflowType.EXTERNAL,
+                    GitHubWorkflowType.REUSABLE,
+                ]:


Suggested change

if isinstance(callee, GitHubWorkflowNode) and callee.node_type in [

GitHubWorkflowType.EXTERNAL,

GitHubWorkflowType.REUSABLE,

]:

if isinstance(callee, GitHubWorkflowNode) and callee.node_type in {

GitHubWorkflowType.EXTERNAL,

GitHubWorkflowType.REUSABLE,

}:

benmss · 2025-04-08T02:59:32Z

src/macaron/slsa_analyzer/checks/github_actions_vulnerability_check.py

+                    if not workflow_name:
+                        logger.debug("Workflow %s is not relevant. Skipping...", callee.name)
+                        continue


This will only trigger if callee.name has nothing before the @ symbol it contains. Is that expected in some cases?

benmss · 2025-04-08T03:16:13Z

src/macaron/slsa_analyzer/checks/github_actions_vulnerability_check.py

+        CheckResultData
+            The result of the check.
+        """
+        result_tables: list[CheckFacts] = []


I suggest moving this further down the function to just before where it's actually used. E.g. for vuln_res in batch_vulns:

benmss · 2025-04-08T06:52:20Z

src/macaron/slsa_analyzer/package_registry/osv_dev.py

+        section_name = "osv_dev"
+        if not defaults.has_section(section_name):
+            return []
+        section = defaults[section_name]
+
+        url_netloc = section.get("url_netloc")
+        if not url_netloc:
+            raise APIAccessError(
+                f'The "url_netloc" key is missing in section [{section_name}] of the .ini configuration file.'
+            )
+        url_scheme = section.get("url_scheme", "https")
+        query_endpoint = section.get("querybatch_endpoint")
+        if not query_endpoint:
+            raise APIAccessError(
+                f'The "query_endpoint" key is missing in section [{section_name}] of the .ini configuration file.'
+            )
+        try:
+            url = urllib.parse.urlunsplit(
+                urllib.parse.SplitResult(
+                    scheme=url_scheme,
+                    netloc=url_netloc,
+                    path=query_endpoint,
+                    query="",
+                    fragment="",
+                )
+            )
+        except ValueError as error:
+            raise APIAccessError("Failed to construct the API URL.") from error
+
+        response = send_post_http_raw(url, json_data=query_data, headers=None)
+        res_obj = None
+        if response:
+            try:
+                res_obj = response.json()
+            except requests.exceptions.JSONDecodeError as error:
+                raise APIAccessError(f"Unable to get a valid response from {url}: {error}") from error
+
+        results = res_obj.get("results") if res_obj else None


Most of this code is identical to call_osv_query_api. I think we should be re-using it instead of duplicating it.

benmss · 2025-04-08T07:01:35Z

tests/integration/cases/oracle-macaron/test.yaml

 # Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl/.

 description: |
-  Analyzing with PURL and repository path without dependency resolution.
+  Analyzing the staging branch of the Macaron repo to detect vulnerable GitHub Actions.


This test should be updated to use branch main.

benmss · 2025-04-08T23:59:33Z

tests/slsa_analyzer/checks/test_github_actions_vulnerability_check.py

+        provenances=[],
+        build_info_results=InTotoV01Payload(statement=InferredProvenance().payload),
+    )
+    match ci_name:


What is the reason for using a match statement here?

benmss · 2025-04-09T00:11:43Z

tests/slsa_analyzer/package_registry/test_osv_dev.py

+def test_is_affected_version_invalid_commit() -> None:
+    """Test if the function can handle invalid commits"""
+    with pytest.raises(APIAccessError):
+        OSVDevService.is_version_affected(
+            vuln={}, pkg_name="pkg", pkg_version="invalid_commit", ecosystem="GitHub Actions"
+        )
+
+
+def test_is_affected_version_invalid_response() -> None:
+    """Test if the function can handle empty OSV response."""
+    with pytest.raises(APIAccessError):
+        OSVDevService.is_version_affected(
+            vuln={"vulns": []}, pkg_name="repo/workflow", pkg_version="1.0.0", ecosystem="GitHub Actions"
+        )


To me it seems that both of these functions fail because vuln is not correctly populated. In is_version_affected we have the following block:

affected = json_extract(vuln, ["affected"], list) if not affected: raise APIAccessError(f"Failed to extracted info for {pkg_name}@{pkg_version}.")

Can we be sure that the error is being raised from the expected part of the function?

tromai · 2025-04-10T03:43:38Z

docs/source/pages/cli_usage/command_analyze.rst

@@ -1,4 +1,4 @@
-.. Copyright (c) 2023 - 2023, Oracle and/or its affiliates. All rights reserved.
+.. Copyright (c) 2023 - 2025, Oracle and/or its affiliates. All rights reserved.


Hmmm, I don't think this file's copyright notice should be updated.

tromai · 2025-04-10T03:46:08Z

src/macaron/slsa_analyzer/git_url.py

@@ -907,3 +907,79 @@ def is_empty_repo(git_obj: Git) -> bool:
        return False
    except GitCommandError:
        return True
+
+
+def is_commit_hash(version_str: str) -> bool:


Could we rename this variable to make it more generic? From the doc string, I don't think it must be a version string.

tromai · 2025-04-10T04:32:30Z

src/macaron/slsa_analyzer/package_registry/osv_dev.py

+            except requests.exceptions.JSONDecodeError as error:
+                raise APIAccessError(f"Unable to get a valid response from {url}: {error}") from error
+
+        vulns = res_obj.get("vulns") if res_obj else None


Do we assume that the key vulns always presents in res_obj dictionary ?

tromai · 2025-04-10T05:05:48Z

src/macaron/slsa_analyzer/package_registry/osv_dev.py

+        if isinstance(results, list):
+            if expected_size:
+                if len(results) != expected_size:
+                    raise APIAccessError(f"Unable to get a valid result from {url}")


nit: can we add the reason why this result is invalid (for example, saying that the results doesn't have the expected number of elements).

tromai · 2025-04-10T05:09:08Z

src/macaron/slsa_analyzer/package_registry/osv_dev.py

+
+        results = res_obj.get("results") if res_obj else None
+
+        if isinstance(results, list):


If results is not of type list, should we raise an exception instead ? Or we treat that as an "empty" result 🤔 .

tromai · 2025-04-10T06:23:02Z

src/macaron/slsa_analyzer/package_registry/osv_dev.py

+                    pkg_version = tag
+                    break
+
+        affected = json_extract(vuln, ["affected"], list)


I believe we are assuming that affected, if available, will never be an empty list right ?

tromai · 2025-04-10T06:47:10Z

src/macaron/slsa_analyzer/checks/github_actions_vulnerability_check.py

+                                workflow_name,
+                                workflow_inv["version"],
+                                "GitHub Actions",
+                                source_repo=f"https://github.com/{workflow_name}",


Should we use urllib to construct the URL in this case (i.e. using urllib.parse.urlunsplit) ? In this case, github.com is a trusted domain so it wouldn't be a big problem, so both ways are okay to me.

oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Mar 21, 2025

behnazh-w force-pushed the behnaz/add-gh-vuln-gha-check branch 2 times, most recently from 3f5b70c to e056b7c Compare March 26, 2025 00:30

behnazh-w force-pushed the behnaz/add-gh-vuln-gha-check branch 4 times, most recently from 6dd85e0 to 6e429f7 Compare April 4, 2025 05:36

feat: detect vulnerable GitHub Actions

9776ec8

Signed-off-by: behnazh-w <[email protected]>

behnazh-w force-pushed the behnaz/add-gh-vuln-gha-check branch from 6e429f7 to 9776ec8 Compare April 4, 2025 05:38

behnazh-w marked this pull request as ready for review April 7, 2025 04:48

behnazh-w requested a review from tromai as a code owner April 7, 2025 04:48

benmss self-requested a review April 7, 2025 04:53

benmss reviewed Apr 8, 2025

View reviewed changes

behnazh-w changed the base branch from staging to main April 8, 2025 04:41

benmss reviewed Apr 8, 2025

View reviewed changes

benmss reviewed Apr 9, 2025

View reviewed changes

tromai reviewed Apr 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: detect vulnerable GitHub Actions #1021

feat: detect vulnerable GitHub Actions #1021

behnazh-w commented Mar 21, 2025 •

edited

Loading

benmss Apr 8, 2025

benmss Apr 8, 2025

benmss Apr 8, 2025

benmss Apr 8, 2025

benmss Apr 8, 2025

benmss Apr 8, 2025

benmss Apr 8, 2025

benmss Apr 8, 2025

benmss Apr 8, 2025

benmss Apr 9, 2025

tromai Apr 10, 2025

tromai Apr 10, 2025

tromai Apr 10, 2025

tromai Apr 10, 2025

tromai Apr 10, 2025

tromai Apr 10, 2025

tromai Apr 10, 2025 •

edited

Loading

	.. list-table:: Macaron checks descriptions
	.. list-table:: Macaron check descriptions


		This tutorial explains how to use a check in Macaron that detects vulnerable third-party GitHub Actions. This check is important for preventing security issues in your CI/CD pipeline, especially in light of recent incidents, such as vulnerabilities discovered in popular GitHub Actions like `tj-actions/changed-files <https://www.cve.org/CVERecord?id=CVE-2025-30066>`_, and `reviewdog/action-setup <https://www.cve.org/CVERecord?id=CVE-2025-30154>`_.

		We will guide you on how to enable and use this check to enhance the security of your development pipeline.

	We will guide you on how to enable and use this check to enhance the security of your development pipeline.
	We will demonstrate how to enable and use this check to enhance the security of your development pipeline.

		@@ -1,4 +1,4 @@
		.. Copyright (c) 2023 - 2023, Oracle and/or its affiliates. All rights reserved.
		.. Copyright (c) 2023 - 2025, Oracle and/or its affiliates. All rights reserved.


		results = res_obj.get("results") if res_obj else None

		if isinstance(results, list):

feat: detect vulnerable GitHub Actions #1021

Are you sure you want to change the base?

feat: detect vulnerable GitHub Actions #1021

Conversation

behnazh-w commented Mar 21, 2025 • edited Loading

Summary

Changes

Documentation

Tests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tromai Apr 10, 2025 • edited Loading

Choose a reason for hiding this comment

behnazh-w commented Mar 21, 2025 •

edited

Loading

tromai Apr 10, 2025 •

edited

Loading