chore: address PR feedback

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

Author: behnazh-w

docs/source/index.rst

@@ -46,7 +46,7 @@ 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:: Macaron checks descriptions
+.. list-table:: Macaron check descriptions
    :widths: 20 40 40
    :header-rows: 1
 

docs/source/pages/cli_usage/command_analyze.rst

@@ -1,4 +1,4 @@
-.. Copyright (c) 2023 - 2025, Oracle and/or its affiliates. All rights reserved.
+.. Copyright (c) 2023 - 2023, Oracle and/or its affiliates. All rights reserved.
 .. Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl/.
 
 .. _analyze-command-cli:

docs/source/pages/tutorials/detect_vulnerable_github_actions.rst

@@ -7,9 +7,9 @@
 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>`_.
+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 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 demonstrate how to enable and use this check to enhance the security of your development pipeline.
 
 For more information on other features of Macaron, please refer to the :ref:`documentation here <index>`.
 

src/macaron/slsa_analyzer/checks/github_actions_vulnerability_check.py

@@ -83,17 +83,16 @@ def run_check(self, ctx: AnalyzeContext) -> CheckResultData:
         CheckResultData
             The result of the check.
         """
-        result_tables: list[CheckFacts] = []
-
         ci_services = ctx.dynamic_data["ci_services"]
 
         external_workflows: dict[str, list] = {}
         for ci_info in ci_services:
             for callee in ci_info["callgraph"].bfs():
-                if isinstance(callee, GitHubWorkflowNode) and callee.node_type in [
+                if isinstance(callee, GitHubWorkflowNode) and callee.node_type in {
                     GitHubWorkflowType.EXTERNAL,
                     GitHubWorkflowType.REUSABLE,
-                ]:
+                }:
+                    workflow_name = workflow_version = ""
                     if "@" in callee.name:
                         workflow_name, workflow_version = callee.name.split("@")
                     else:
@@ -104,7 +103,10 @@ def run_check(self, ctx: AnalyzeContext) -> CheckResultData:
 
                     caller_path = callee.caller.source_path if callee.caller else None
 
-                    if not workflow_name:
+                    # Skip the workflow if `workflow_name` or `workflow_version` are missing,
+                    # or if `callee.name` lacks an '@' which can indicate an internal workflow
+                    # within the same repo .
+                    if not workflow_name or not workflow_version:
                         logger.debug("Workflow %s is not relevant. Skipping...", callee.name)
                         continue
 
@@ -144,6 +146,7 @@ def run_check(self, ctx: AnalyzeContext) -> CheckResultData:
         except APIAccessError as error:
             logger.debug(error)
 
+        result_tables: list[CheckFacts] = []
         for vuln_res in batch_vulns:
             vulns: list = []
             workflow_name = vuln_res["name"]

src/macaron/slsa_analyzer/git_url.py

@@ -909,7 +909,7 @@ def is_empty_repo(git_obj: Git) -> bool:
         return True
 
 
-def is_commit_hash(version_str: str) -> bool:
+def is_commit_hash(value: str) -> bool:
     """Check if a given string is a valid Git commit hash.
 
     A valid Git commit hash is a 40-character long hexadecimal string or
@@ -918,7 +918,8 @@ def is_commit_hash(version_str: str) -> bool:
 
     Parameters
     ----------
-    version_str (str): The string to be checked for validity as a commit hash.
+    value: str
+        The string value to be checked for validity as a commit hash.
 
     Returns
     -------
@@ -939,7 +940,7 @@ def is_commit_hash(version_str: str) -> bool:
     False
     """
     pattern = r"^[a-f0-9]{7,40}$"
-    return bool(re.match(pattern, version_str))
+    return bool(re.match(pattern, value))
 
 
 def get_tags_via_git_remote(repo: str) -> dict[str, str] | None:

src/macaron/slsa_analyzer/package_registry/osv_dev.py

@@ -118,32 +118,34 @@ def get_vulnerabilities_package_name_batch(packages: list) -> list:
         return results
 
     @staticmethod
-    def call_osv_query_api(query_data: dict) -> list:
-        """Query the OSV (Open Source Vulnerability) knowledge base API with the given data.
+    def get_osv_url(endpoint: str) -> str:
+        """Construct a full API URL for a given OSV endpoint using values from the .ini configuration.
 
-        This method sends a POST request to the OSV API and processes the response to extract
-        information about vulnerabilities based on the provided query data.
+        The configuration is expected to be in a section named `[osv_dev]` within the defaults object,
+        and must include the following keys:
+
+        - `url_netloc`: The base domain of the API.
+        - `url_scheme` (optional): The scheme (e.g., "https"). Defaults to "https" if not provided.
+        - A key matching the provided `endpoint` argument (e.g., "query_endpoint"), which defines the URL path.
 
         Parameters
         ----------
-        query_data : dict
-            A dictionary containing the query parameters to be sent to the OSV API.
-            The query data should conform to the format expected by the OSV API for querying vulnerabilities.
+        endpoint: str
+            The key name of the endpoint in the `[osv_dev]` section to construct the URL path.
 
         Returns
         -------
-        list
-            A list of vulnerabilities under the key "vulns" if the query is successful
-            and the response is valid.
+        str
+            The fully constructed API URL.
 
         Raises
         ------
         APIAccessError
-            If there are issues with the API URL construction, missing configuration values, or invalid responses.
+            If required keys are missing from the configuration or if the URL cannot be constructed.
         """
         section_name = "osv_dev"
         if not defaults.has_section(section_name):
-            return []
+            raise APIAccessError(f"The section [{section_name}] is missing in the .ini configuration file.")
         section = defaults[section_name]
 
         url_netloc = section.get("url_netloc")
@@ -152,13 +154,13 @@ def call_osv_query_api(query_data: dict) -> list:
                 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("query_endpoint")
+        query_endpoint = section.get(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(
+            return urllib.parse.urlunsplit(
                 urllib.parse.SplitResult(
                     scheme=url_scheme,
                     netloc=url_netloc,
@@ -170,6 +172,34 @@ def call_osv_query_api(query_data: dict) -> list:
         except ValueError as error:
             raise APIAccessError("Failed to construct the API URL.") from error
 
+    @staticmethod
+    def call_osv_query_api(query_data: dict) -> list:
+        """Query the OSV (Open Source Vulnerability) knowledge base API with the given data.
+
+        This method sends a POST request to the OSV API and processes the response to extract
+        information about vulnerabilities based on the provided query data.
+
+        Parameters
+        ----------
+        query_data : dict
+            A dictionary containing the query parameters to be sent to the OSV API.
+            The query data should conform to the format expected by the OSV API for querying vulnerabilities.
+
+        Returns
+        -------
+        list
+            A list of vulnerabilities under the key "vulns" if the query is successful
+            and the response is valid.
+
+        Raises
+        ------
+        APIAccessError
+            If there are issues with the API URL construction, missing configuration values, or invalid responses.
+        """
+        try:
+            url = OSVDevService.get_osv_url("query_endpoint")
+        except APIAccessError as error:
+            raise error
         response = send_post_http_raw(url, json_data=query_data, headers=None)
         res_obj = None
         if response:
@@ -209,8 +239,7 @@ def call_osv_querybatch_api(query_data: dict, expected_size: int | None = None)
         -------
         list
             A list of results from the OSV API containing the vulnerability data that matches
-            the query parameters. If no valid response is received or the results are
-            improperly formatted, an empty list is returned.
+            the query parameters.
 
         Raises
         ------
@@ -219,34 +248,10 @@ def call_osv_querybatch_api(query_data: dict, expected_size: int | None = None)
             fails, or if the response from the OSV API is invalid or the number of results
             does not match the expected size.
         """
-        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
+            url = OSVDevService.get_osv_url("querybatch_endpoint")
+        except APIAccessError as error:
+            raise error
 
         response = send_post_http_raw(url, json_data=query_data, headers=None)
         res_obj = None
@@ -261,11 +266,13 @@ def call_osv_querybatch_api(query_data: dict, expected_size: int | None = None)
         if isinstance(results, list):
             if expected_size:
                 if len(results) != expected_size:
-                    raise APIAccessError(f"Unable to get a valid result from {url}")
+                    raise APIAccessError(
+                        f"Failed to retrieve a valid result from {url}: result count does not match the expected count."
+                    )
 
             return results
 
-        return []
+        raise APIAccessError(f"The response from {url} does not contain a valid 'results' list.")
 
     @staticmethod
     def is_version_affected(
@@ -326,9 +333,13 @@ def is_version_affected(
                     pkg_version = tag
                     break
 
+            # If we were not able to find a tag for the commit hash, raise an exception.
+            if is_commit_hash(pkg_version):
+                raise APIAccessError(f"Failed to find a tag for {pkg_name}@{pkg_version}.")
+
         affected = json_extract(vuln, ["affected"], list)
         if not affected:
-            raise APIAccessError(f"Failed to extracted info for {pkg_name}@{pkg_version}.")
+            raise APIAccessError(f"Received invalid response for {pkg_name}@{pkg_version}.")
 
         affected_ranges: list | None = None
         for rec in affected:
@@ -342,12 +353,12 @@ def is_version_affected(
                 break
 
         if not affected_ranges:
-            raise APIAccessError(f"Failed to extracted affected versions for {pkg_name}@{pkg_version}.")
+            raise APIAccessError(f"Failed to extract affected versions for {pkg_name}@{pkg_version}.")
 
         for affected_range in affected_ranges:
             events = json_extract(affected_range, ["events"], list)
             if not events:
-                raise APIAccessError(f"Failed to extracted affected versions for {pkg_name}@{pkg_version}.")
+                raise APIAccessError(f"Failed to extract affected versions for {pkg_name}@{pkg_version}.")
 
             introduced = None
             fixed = None

tests/slsa_analyzer/checks/test_github_actions_vulnerability_check.py

@@ -35,12 +35,11 @@ def get_ci_info(ci_services: dict[str, BaseCIService], ci_name: str, workflow_pa
         provenances=[],
         build_info_results=InTotoV01Payload(statement=InferredProvenance().payload),
     )
-    match ci_name:
-        case "github_actions":
-            root_node: BaseNode = BaseNode()
-            workflow_node = build_call_graph_from_path(root_node, workflow_path=workflow_path, repo_path="")
-            root_node.add_callee(workflow_node)
-            ci_info["callgraph"] = CallGraph(root_node, "")
+    if ci_name == "github_actions":
+        root_node: BaseNode = BaseNode()
+        workflow_node = build_call_graph_from_path(root_node, workflow_path=workflow_path, repo_path="")
+        root_node.add_callee(workflow_node)
+        ci_info["callgraph"] = CallGraph(root_node, "")
 
     return ci_info
 

tests/slsa_analyzer/package_registry/test_osv_dev.py

@@ -54,15 +54,19 @@ def test_load_defaults_query_api(tmp_path: Path, user_config_input: str) -> None
 
 def test_is_affected_version_invalid_commit() -> None:
     """Test if the function can handle invalid commits"""
-    with pytest.raises(APIAccessError):
+    with pytest.raises(APIAccessError, match="^Failed to find a tag for"):
         OSVDevService.is_version_affected(
-            vuln={}, pkg_name="pkg", pkg_version="invalid_commit", ecosystem="GitHub Actions"
+            vuln={},
+            pkg_name="pkg",
+            pkg_version="c253e1f19ebfb98fe02a8354082cbbd282d446a0",
+            ecosystem="GitHub Actions",
+            source_repo="mock_repo",
         )
 
 
 def test_is_affected_version_invalid_response() -> None:
     """Test if the function can handle empty OSV response."""
-    with pytest.raises(APIAccessError):
+    with pytest.raises(APIAccessError, match="^Received invalid response for"):
         OSVDevService.is_version_affected(
             vuln={"vulns": []}, pkg_name="repo/workflow", pkg_version="1.0.0", ecosystem="GitHub Actions"
         )