chore: address PR review comments

Signed-off-by: sophie-bates <[email protected]>

Author: sophie-bates

src/macaron/slsa_analyzer/build_tool/poetry.py

@@ -10,7 +10,6 @@
 import logging
 import os
 import tomllib
-from collections.abc import Iterable
 from pathlib import Path
 
 from macaron.config.defaults import defaults
@@ -124,36 +123,3 @@ def get_dep_analyzer(self, repo_path: str) -> DependencyAnalyzer:
         """
         # TODO: Implement this method.
         return NoneDependencyAnalyzer()
-
-    def get_build_dirs(self, repo_path: str) -> Iterable[Path]:
-        """Find directories in the repository that have their own build scripts.
-
-        This is especially important for applications that consist of multiple services.
-
-        Parameters
-        ----------
-        repo_path: str
-            The path to the target repo.
-
-        Yields
-        ------
-        Path
-            The relative paths from the repo path that contain build scripts.
-        """
-        config_paths: set[str] = set()
-        config_files = self.build_configs + self.package_lock
-        for build_cfg in config_files:
-            config_paths.update(glob.glob(os.path.join(repo_path, "**", build_cfg), recursive=True))
-
-        list_iter = iter(sorted(config_paths, key=lambda x: (str(Path(x).parent), len(Path(x).parts))))
-        try:
-            cfg_path = next(list_iter)
-            yield Path(cfg_path).parent.relative_to(repo_path)
-            while next_item := next(list_iter):
-                if str(Path(cfg_path).parent) in next_item:
-                    continue
-                cfg_path = next_item
-                yield Path(next_item).parent.relative_to(repo_path)
-
-        except StopIteration:
-            pass

tests/conftest.py

@@ -12,7 +12,6 @@
 from macaron.slsa_analyzer.build_tool.maven import Maven
 from macaron.slsa_analyzer.build_tool.pip import Pip
 from macaron.slsa_analyzer.build_tool.poetry import Poetry
-from macaron.slsa_analyzer.checks.check_result import CheckResult
 from macaron.slsa_analyzer.ci_service.circleci import CircleCI
 from macaron.slsa_analyzer.ci_service.github_actions import GitHubActions
 from macaron.slsa_analyzer.ci_service.gitlab_ci import GitLabCI
@@ -71,23 +70,6 @@ def setup_test(test_dir: Path, macaron_path: Path) -> NoReturn:  # type: ignore
     defaults.clear()
 
 
-@pytest.fixture()
-def check_result() -> CheckResult:  # pylint: disable=unused-argument
-    """Create a CheckResult instance.
-
-    Parameters
-    ----------
-    setup_test
-        Depends on setup_test fixture.
-
-    Returns
-    -------
-    CheckResult
-        The CheckResult instance.
-    """
-    return CheckResult(justification=[])  # type: ignore
-
-
 @pytest.fixture(autouse=True)
 def maven_tool(setup_test) -> Maven:  # type: ignore # pylint: disable=unused-argument
     """Create a Maven tool instance.

tests/slsa_analyzer/checks/test_build_as_code_check.py

@@ -7,8 +7,6 @@
 from pathlib import Path
 from unittest.mock import MagicMock
 
-import pytest
-
 import macaron
 from macaron.code_analyzer.call_graph import BaseNode, CallGraph
 from macaron.parsers.actionparser import parse as parse_action
@@ -28,26 +26,7 @@
 from macaron.slsa_analyzer.specs.ci_spec import CIInfo
 
 
-@pytest.fixture()
-def build_as_code_check(setup_test) -> BuildAsCodeCheck:  # type: ignore # pylint: disable=unused-argument
-    """Create a BuildAsCodeCheck instance.
-
-    Parameters
-    ----------
-    setup_test
-        Depends on setup_test fixture.
-
-    Returns
-    -------
-    BuildAsCodeCheck
-        The BuildAsCodeCheck instance.
-    """
-    return BuildAsCodeCheck()
-
-
 def test_build_as_code_check(
-    build_as_code_check: BuildAsCodeCheck,  # pylint: disable=redefined-outer-name
-    check_result: CheckResult,
     maven_tool: Maven,
     gradle_tool: Gradle,
     poetry_tool: Poetry,
@@ -59,6 +38,8 @@ def test_build_as_code_check(
     gitlab_ci_service: GitLabCI,
 ) -> None:
     """Test the Build As Code Check."""
+    check = BuildAsCodeCheck()
+    check_result = CheckResult(justification=[])  # type: ignore
     bash_commands = BashCommands(caller_path="source_file", CI_path="ci_file", CI_type="github_actions", commands=[[]])
     ci_info = CIInfo(
         service=github_actions_service,
@@ -72,131 +53,131 @@ def test_build_as_code_check(
     # The target repo uses Maven build tool but does not deploy artifacts.
     use_build_tool = AnalyzeContext("use_build_tool", os.path.abspath("./"), MagicMock())
     use_build_tool.dynamic_data["build_spec"]["tool"] = maven_tool
-    assert build_as_code_check.run_check(use_build_tool, check_result) == CheckResultType.FAILED
+    assert check.run_check(use_build_tool, check_result) == CheckResultType.FAILED
 
     # The target repo uses Gradle build tool but does not deploy artifacts.
     use_build_tool = AnalyzeContext("use_build_tool", os.path.abspath("./"), MagicMock())
     use_build_tool.dynamic_data["build_spec"]["tool"] = gradle_tool
-    assert build_as_code_check.run_check(use_build_tool, check_result) == CheckResultType.FAILED
+    assert check.run_check(use_build_tool, check_result) == CheckResultType.FAILED
 
     # The target repo uses Poetry build tool but does not deploy artifacts.
     use_build_tool = AnalyzeContext("use_build_tool", os.path.abspath("./"), MagicMock())
     use_build_tool.dynamic_data["build_spec"]["tool"] = poetry_tool
-    assert build_as_code_check.run_check(use_build_tool, check_result) == CheckResultType.FAILED
+    assert check.run_check(use_build_tool, check_result) == CheckResultType.FAILED
 
     # The target repo uses Pip build tool but does not deploy artifacts.
     use_build_tool = AnalyzeContext("use_build_tool", os.path.abspath("./"), MagicMock())
     use_build_tool.dynamic_data["build_spec"]["tool"] = pip_tool
-    assert build_as_code_check.run_check(use_build_tool, check_result) == CheckResultType.FAILED
+    assert check.run_check(use_build_tool, check_result) == CheckResultType.FAILED
 
     # The target repo does not use a build tool.
     no_build_tool = AnalyzeContext("no_build_tool", os.path.abspath("./"), MagicMock())
-    assert build_as_code_check.run_check(no_build_tool, check_result) == CheckResultType.FAILED
+    assert check.run_check(no_build_tool, check_result) == CheckResultType.FAILED
 
     # Use mvn deploy to deploy the artifact.
     maven_deploy = AnalyzeContext("use_build_tool", os.path.abspath("./"), MagicMock())
     maven_deploy.dynamic_data["build_spec"]["tool"] = maven_tool
     bash_commands["commands"] = [["mvn", "deploy"]]
     maven_deploy.dynamic_data["ci_services"] = [ci_info]
-    assert build_as_code_check.run_check(maven_deploy, check_result) == CheckResultType.PASSED
+    assert check.run_check(maven_deploy, check_result) == CheckResultType.PASSED
 
     # Use the mvn in the local directory to deploy the artifact.
     bash_commands["commands"] = [["./mvn", "deploy"]]
     maven_deploy.dynamic_data["ci_services"] = [ci_info]
-    assert build_as_code_check.run_check(maven_deploy, check_result) == CheckResultType.PASSED
+    assert check.run_check(maven_deploy, check_result) == CheckResultType.PASSED
 
     # Use an invalid build command that has mvn.
     bash_commands["commands"] = [["mvnblah", "deploy"]]
     maven_deploy.dynamic_data["ci_services"] = [ci_info]
-    assert build_as_code_check.run_check(maven_deploy, check_result) == CheckResultType.FAILED
+    assert check.run_check(maven_deploy, check_result) == CheckResultType.FAILED
 
     # Use mvn but do not deploy artifacts.
     no_maven_deploy = AnalyzeContext("use_build_tool", os.path.abspath("./"), MagicMock())
     no_maven_deploy.dynamic_data["build_spec"]["tool"] = maven_tool
     bash_commands["commands"] = [["mvn", "verify"]]
     no_maven_deploy.dynamic_data["ci_services"] = [ci_info]
-    assert build_as_code_check.run_check(no_maven_deploy, check_result) == CheckResultType.FAILED
+    assert check.run_check(no_maven_deploy, check_result) == CheckResultType.FAILED
 
     # Use an invalid goal that has deploy keyword.
     bash_commands["commands"] = [["mvnb", "deployblah"]]
     no_maven_deploy.dynamic_data["ci_services"] = [ci_info]
-    assert build_as_code_check.run_check(no_maven_deploy, check_result) == CheckResultType.FAILED
+    assert check.run_check(no_maven_deploy, check_result) == CheckResultType.FAILED
 
     # Use gradle to deploy the artifact.
     gradle_deploy = AnalyzeContext("use_build_tool", os.path.abspath("./"), MagicMock())
     gradle_deploy.dynamic_data["build_spec"]["tool"] = gradle_tool
     bash_commands["commands"] = [["./gradlew", "publishToSonatype"]]
     gradle_deploy.dynamic_data["ci_services"] = [ci_info]
-    assert build_as_code_check.run_check(gradle_deploy, check_result) == CheckResultType.PASSED
+    assert check.run_check(gradle_deploy, check_result) == CheckResultType.PASSED
 
     # Use poetry publish to publish the artifact.
     poetry_publish = AnalyzeContext("use_build_tool", os.path.abspath("./"), MagicMock())
     poetry_publish.dynamic_data["build_spec"]["tool"] = poetry_tool
     bash_commands["commands"] = [["poetry", "publish"]]
     poetry_publish.dynamic_data["ci_services"] = [ci_info]
-    assert build_as_code_check.run_check(poetry_publish, check_result) == CheckResultType.PASSED
+    assert check.run_check(poetry_publish, check_result) == CheckResultType.PASSED
 
     # Use Poetry but do not deploy artifacts.
     no_poetry_deploy = AnalyzeContext("use_build_tool", os.path.abspath("./"), MagicMock())
     no_poetry_deploy.dynamic_data["build_spec"]["tool"] = poetry_tool
     bash_commands["commands"] = [["poetry", "upload"]]
     no_poetry_deploy.dynamic_data["ci_services"] = [ci_info]
-    assert build_as_code_check.run_check(no_maven_deploy, check_result) == CheckResultType.FAILED
+    assert check.run_check(no_maven_deploy, check_result) == CheckResultType.FAILED
 
     # Use twine upload to deploy the artifact.
     twine_upload = AnalyzeContext("use_build_tool", os.path.abspath("./"), MagicMock())
     twine_upload.dynamic_data["build_spec"]["tool"] = pip_tool
     bash_commands["commands"] = [["twine", "upload", "dist/*"]]
     twine_upload.dynamic_data["ci_services"] = [ci_info]
-    assert build_as_code_check.run_check(twine_upload, check_result) == CheckResultType.PASSED
+    assert check.run_check(twine_upload, check_result) == CheckResultType.PASSED
 
     # Use flit publish to deploy the artifact.
     flit_publish = AnalyzeContext("use_build_tool", os.path.abspath("./"), MagicMock())
     flit_publish.dynamic_data["build_spec"]["tool"] = pip_tool
     bash_commands["commands"] = [["flit", "publish"]]
     flit_publish.dynamic_data["ci_services"] = [ci_info]
-    assert build_as_code_check.run_check(flit_publish, check_result) == CheckResultType.PASSED
+    assert check.run_check(flit_publish, check_result) == CheckResultType.PASSED
 
     # Test Jenkins.
     maven_deploy = AnalyzeContext("use_build_tool", os.path.abspath("./"), MagicMock())
     maven_deploy.dynamic_data["build_spec"]["tool"] = maven_tool
     ci_info["service"] = jenkins_service
     bash_commands["commands"] = []
     maven_deploy.dynamic_data["ci_services"] = [ci_info]
-    assert build_as_code_check.run_check(maven_deploy, check_result) == CheckResultType.FAILED
+    assert check.run_check(maven_deploy, check_result) == CheckResultType.FAILED
 
     # Test Travis.
     maven_deploy = AnalyzeContext("use_build_tool", os.path.abspath("./"), MagicMock())
     maven_deploy.dynamic_data["build_spec"]["tool"] = maven_tool
     ci_info["service"] = travis_service
     bash_commands["commands"] = []
     maven_deploy.dynamic_data["ci_services"] = [ci_info]
-    assert build_as_code_check.run_check(maven_deploy, check_result) == CheckResultType.FAILED
+    assert check.run_check(maven_deploy, check_result) == CheckResultType.FAILED
 
     # Test Circle CI.
     maven_deploy = AnalyzeContext("use_build_tool", os.path.abspath("./"), MagicMock())
     maven_deploy.dynamic_data["build_spec"]["tool"] = maven_tool
     ci_info["service"] = circle_ci_service
     bash_commands["commands"] = []
     maven_deploy.dynamic_data["ci_services"] = [ci_info]
-    assert build_as_code_check.run_check(maven_deploy, check_result) == CheckResultType.FAILED
+    assert check.run_check(maven_deploy, check_result) == CheckResultType.FAILED
 
     # Test GitLab CI.
     maven_deploy = AnalyzeContext("use_build_tool", os.path.abspath("./"), MagicMock())
     maven_deploy.dynamic_data["build_spec"]["tool"] = maven_tool
     ci_info["service"] = gitlab_ci_service
     bash_commands["commands"] = []
     maven_deploy.dynamic_data["ci_services"] = [ci_info]
-    assert build_as_code_check.run_check(maven_deploy, check_result) == CheckResultType.FAILED
+    assert check.run_check(maven_deploy, check_result) == CheckResultType.FAILED
 
 
 def test_gha_workflow_deployment(
-    build_as_code_check: BuildAsCodeCheck,  # pylint: disable=redefined-outer-name
-    check_result: CheckResult,
     pip_tool: Pip,
     github_actions_service: GitHubActions,
 ) -> None:
     """Test the use of verified GitHub Actions to deploy."""
+    check = BuildAsCodeCheck()
+    check_result = CheckResult(justification=[])  # type: ignore
     ci_info = CIInfo(
         service=github_actions_service,
         bash_commands=[],
@@ -227,4 +208,21 @@ def test_gha_workflow_deployment(
     root.add_callee(callee)
     github_actions_service.build_call_graph_from_node(callee)
     ci_info["callgraph"] = gh_cg
-    assert build_as_code_check.run_check(gha_deploy, check_result) == CheckResultType.PASSED
+    assert check.run_check(gha_deploy, check_result) == CheckResultType.PASSED
+
+    # This Github Actions workflow is not using a trusted action to publish the artifact.
+    root = GitHubNode(name="root", node_type=GHWorkflowType.NONE, source_path="", parsed_obj={}, caller_path="")
+    gh_cg = CallGraph(root, "")
+    workflow_path = os.path.join(workflows_dir, "pypi_publish_blah.yaml")
+    parsed_obj = parse_action(workflow_path, macaron_path=str(Path(macaron.MACARON_PATH)))
+    callee = GitHubNode(
+        name=os.path.basename(workflow_path),
+        node_type=GHWorkflowType.INTERNAL,
+        source_path=workflow_path,
+        parsed_obj=parsed_obj,
+        caller_path="",
+    )
+    root.add_callee(callee)
+    github_actions_service.build_call_graph_from_node(callee)
+    ci_info["callgraph"] = gh_cg
+    assert check.run_check(gha_deploy, check_result) == CheckResultType.FAILED