From 7d3c63ec447460f6567d83cf959d8517e0bb6bb5 Mon Sep 17 00:00:00 2001 From: Mohammad Abdollahpour Date: Sun, 3 Nov 2024 23:33:34 -0500 Subject: [PATCH] chore: address comments from PR#873 (#913) This PR aims to address the remaining comments from PR#873. - Generate API documents for modified and new code. - Make the repository verification check generic. - Fix repo verification fact parameter docs. Signed-off-by: Mohammad Abdollahpour Co-authored-by: Behnaz Hassanshahi --- docs/source/index.rst | 3 + .../apidoc/macaron.parsers.rst | 8 ++ .../apidoc/macaron.repo_verifier.rst | 42 ++++++++++ .../pages/developers_guide/apidoc/macaron.rst | 1 + .../apidoc/macaron.slsa_analyzer.checks.rst | 8 ++ ...ion_check.py => scm_authenticity_check.py} | 83 +++++++++---------- .../config.ini | 2 +- .../policy_fail_1.dl | 2 +- .../policy_pass_1.dl | 2 +- .../policy_pass_2.dl | 2 +- .../test.yaml | 2 +- ...eck.py => test_repo_verification_check.py} | 16 ++-- 12 files changed, 115 insertions(+), 56 deletions(-) create mode 100644 docs/source/pages/developers_guide/apidoc/macaron.repo_verifier.rst rename src/macaron/slsa_analyzer/checks/{maven_repo_verification_check.py => scm_authenticity_check.py} (56%) rename tests/integration/cases/{maven_repo_verification => scm_authenticity}/config.ini (84%) rename tests/integration/cases/{maven_repo_verification => scm_authenticity}/policy_fail_1.dl (86%) rename tests/integration/cases/{maven_repo_verification => scm_authenticity}/policy_pass_1.dl (85%) rename tests/integration/cases/{maven_repo_verification => scm_authenticity}/policy_pass_2.dl (85%) rename tests/integration/cases/{maven_repo_verification => scm_authenticity}/test.yaml (94%) rename tests/slsa_analyzer/checks/{test_maven_repo_verification_check.py => test_repo_verification_check.py} (85%) diff --git a/docs/source/index.rst b/docs/source/index.rst index 1336efb9f..0b7342ac6 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -89,6 +89,9 @@ the requirements that are currently supported by Macaron. * - ``mcn_provenance_derived_commit_1`` - **Provenance derived commit** - Check if the analysis target's commit matches the commit in the provenance. - If there is no commit, this check will fail. + * - ``mcn_scm_authenticity_check_1`` + - **Source repo authenticity** - Check whether the claims of a source code repository made by a package can be corroborated. + - If the source code repository contains conflicting evidence regarding its claim of the source code repository, this check will fail. If no source code repository or corroborating evidence is found, or if the build system is unsupported, the check will return ``UNKNOWN`` as the result. This check currently supports only Maven artifacts. **************************************************************************************** Macaron checks that report integrity issues but do not map to SLSA requirements directly diff --git a/docs/source/pages/developers_guide/apidoc/macaron.parsers.rst b/docs/source/pages/developers_guide/apidoc/macaron.parsers.rst index 47423e1af..833df895a 100644 --- a/docs/source/pages/developers_guide/apidoc/macaron.parsers.rst +++ b/docs/source/pages/developers_guide/apidoc/macaron.parsers.rst @@ -40,3 +40,11 @@ macaron.parsers.github\_workflow\_model module :members: :undoc-members: :show-inheritance: + +macaron.parsers.pomparser module +-------------------------------- + +.. automodule:: macaron.parsers.pomparser + :members: + :undoc-members: + :show-inheritance: diff --git a/docs/source/pages/developers_guide/apidoc/macaron.repo_verifier.rst b/docs/source/pages/developers_guide/apidoc/macaron.repo_verifier.rst new file mode 100644 index 000000000..54c4f346d --- /dev/null +++ b/docs/source/pages/developers_guide/apidoc/macaron.repo_verifier.rst @@ -0,0 +1,42 @@ +macaron.repo\_verifier package +============================== + +.. automodule:: macaron.repo_verifier + :members: + :undoc-members: + :show-inheritance: + +Submodules +---------- + +macaron.repo\_verifier.repo\_verifier module +-------------------------------------------- + +.. automodule:: macaron.repo_verifier.repo_verifier + :members: + :undoc-members: + :show-inheritance: + +macaron.repo\_verifier.repo\_verifier\_base module +-------------------------------------------------- + +.. automodule:: macaron.repo_verifier.repo_verifier_base + :members: + :undoc-members: + :show-inheritance: + +macaron.repo\_verifier.repo\_verifier\_gradle module +---------------------------------------------------- + +.. automodule:: macaron.repo_verifier.repo_verifier_gradle + :members: + :undoc-members: + :show-inheritance: + +macaron.repo\_verifier.repo\_verifier\_maven module +--------------------------------------------------- + +.. automodule:: macaron.repo_verifier.repo_verifier_maven + :members: + :undoc-members: + :show-inheritance: diff --git a/docs/source/pages/developers_guide/apidoc/macaron.rst b/docs/source/pages/developers_guide/apidoc/macaron.rst index 3dffa65fc..d8fdf3e64 100644 --- a/docs/source/pages/developers_guide/apidoc/macaron.rst +++ b/docs/source/pages/developers_guide/apidoc/macaron.rst @@ -21,6 +21,7 @@ Subpackages macaron.parsers macaron.policy_engine macaron.repo_finder + macaron.repo_verifier macaron.slsa_analyzer macaron.vsa diff --git a/docs/source/pages/developers_guide/apidoc/macaron.slsa_analyzer.checks.rst b/docs/source/pages/developers_guide/apidoc/macaron.slsa_analyzer.checks.rst index a10c38247..8592e1d5f 100644 --- a/docs/source/pages/developers_guide/apidoc/macaron.slsa_analyzer.checks.rst +++ b/docs/source/pages/developers_guide/apidoc/macaron.slsa_analyzer.checks.rst @@ -121,6 +121,14 @@ macaron.slsa\_analyzer.checks.provenance\_witness\_l1\_check module :undoc-members: :show-inheritance: +macaron.slsa\_analyzer.checks.scm\_authenticity\_check module +------------------------------------------------------------- + +.. automodule:: macaron.slsa_analyzer.checks.scm_authenticity_check + :members: + :undoc-members: + :show-inheritance: + macaron.slsa\_analyzer.checks.trusted\_builder\_l3\_check module ---------------------------------------------------------------- diff --git a/src/macaron/slsa_analyzer/checks/maven_repo_verification_check.py b/src/macaron/slsa_analyzer/checks/scm_authenticity_check.py similarity index 56% rename from src/macaron/slsa_analyzer/checks/maven_repo_verification_check.py rename to src/macaron/slsa_analyzer/checks/scm_authenticity_check.py index 006ca3e69..b53ed851f 100644 --- a/src/macaron/slsa_analyzer/checks/maven_repo_verification_check.py +++ b/src/macaron/slsa_analyzer/checks/scm_authenticity_check.py @@ -1,11 +1,10 @@ # Copyright (c) 2024 - 2024, 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/. -"""A check to determine whether the source repository of a maven package can be independently verified.""" +"""A check to determine whether the source repository of a package can be independently verified.""" import logging -from packageurl import PackageURL from sqlalchemy import ForeignKey, Integer, String from sqlalchemy.orm import Mapped, mapped_column @@ -14,59 +13,58 @@ from macaron.repo_verifier.repo_verifier_base import RepositoryVerificationStatus from macaron.slsa_analyzer.analyze_context import AnalyzeContext from macaron.slsa_analyzer.checks.base_check import BaseCheck -from macaron.slsa_analyzer.checks.check_result import CheckResultData, CheckResultType, Confidence +from macaron.slsa_analyzer.checks.check_result import CheckResultData, CheckResultType, Confidence, JustificationType from macaron.slsa_analyzer.registry import registry logger: logging.Logger = logging.getLogger(__name__) -class MavenRepoVerificationFacts(CheckFacts): - """The ORM mapping for justifications in maven source repo check.""" +class ScmAuthenticityFacts(CheckFacts): + """The ORM mapping for justifications in scm authenticity check.""" - __tablename__ = "_maven_repo_verification_check" + __tablename__ = "_scm_authenticity_check" #: The primary key. id: Mapped[int] = mapped_column(ForeignKey("_check_facts.id"), primary_key=True) # noqa: A003 - group: Mapped[str] = mapped_column(String, nullable=False) - artifact: Mapped[str] = mapped_column(String, nullable=False) - version: Mapped[str] = mapped_column(String, nullable=False) + #: Repository link identified by Macaron's repo finder. + repo_link: Mapped[str] = mapped_column(String, nullable=True, info={"justification": JustificationType.HREF}) - # Repository link identified by Macaron's repo finder. - repo_link: Mapped[str] = mapped_column(String, nullable=True) + #: Number of stars on the repository. + stars_count: Mapped[int | None] = mapped_column( + Integer, nullable=True, info={"justification": JustificationType.TEXT} + ) - # Repository link identified by deps.dev. - deps_dev_repo_link: Mapped[str | None] = mapped_column(String, nullable=True) + #: Number of forks on the repository. + fork_count: Mapped[int | None] = mapped_column( + Integer, nullable=True, info={"justification": JustificationType.TEXT} + ) - # Number of stars on the repository identified by deps.dev. - deps_dev_stars_count: Mapped[int | None] = mapped_column(Integer, nullable=True) + #: The status of repo verification: passed, failed, or unknown. + status: Mapped[str] = mapped_column(String, nullable=False, info={"justification": JustificationType.TEXT}) - # Number of forks on the repository identified by deps.dev. - deps_dev_fork_count: Mapped[int | None] = mapped_column(Integer, nullable=True) + #: The reason for the status. + reason: Mapped[str] = mapped_column(String, nullable=False, info={"justification": JustificationType.TEXT}) - # The status of the check: passed, failed, or unknown. - status: Mapped[str] = mapped_column(String, nullable=False) - - # The reason for the status. - reason: Mapped[str] = mapped_column(String, nullable=False) - - # The build tool used to build the package. - build_tool: Mapped[str] = mapped_column(String, nullable=False) + #: The build tool used to build the package. + build_tool: Mapped[str] = mapped_column(String, nullable=False, info={"justification": JustificationType.TEXT}) __mapper_args__ = { - "polymorphic_identity": "_maven_repo_verification_check", + "polymorphic_identity": __tablename__, } -class MavenRepoVerificationCheck(BaseCheck): - """Check whether the claims of a source repository provenance made by a maven package can be independently verified.""" +class ScmAuthenticityCheck(BaseCheck): + """Check whether the claims of a source repository provenance made by a package can be corroborated.""" def __init__(self) -> None: """Initialize a check instance.""" - check_id = "mcn_maven_repo_verification_1" + check_id = "mcn_scm_authenticity_1" description = ( "Check whether the claims of a source repository provenance" - " made by a maven package can be independently verified." + " made by a package can be corroborated." + " At this moment, this check only supports Maven packages" + " and returns UNKNOWN for others." ) super().__init__( @@ -87,15 +85,18 @@ def run_check(self, ctx: AnalyzeContext) -> CheckResultData: CheckResultData The result of the check. """ + # Only support Maven at the moment. + # TODO: Add support for other systems. if ctx.component.type != "maven": return CheckResultData(result_tables=[], result_type=CheckResultType.UNKNOWN) - deps_dev_repo_finder = DepsDevRepoFinder() - deps_dev_repo_link = deps_dev_repo_finder.find_repo(PackageURL.from_string(ctx.component.purl)) - deps_dev_repo_info = deps_dev_repo_finder.get_project_info(deps_dev_repo_link) - stars_count: int | None = None fork_count: int | None = None + deps_dev_repo_info: dict | None = None + + repo_link = ctx.component.repository.remote_path if ctx.component.repository else None + if repo_link: + deps_dev_repo_info = DepsDevRepoFinder.get_project_info(repo_link) if deps_dev_repo_info: stars_count = deps_dev_repo_info.get("starsCount") @@ -105,18 +106,14 @@ def run_check(self, ctx: AnalyzeContext) -> CheckResultData: result_tables: list[CheckFacts] = [] for verification_result in ctx.dynamic_data.get("repo_verification", []): result_tables.append( - MavenRepoVerificationFacts( - group=ctx.component.namespace, - artifact=ctx.component.name, - version=ctx.component.version, - repo_link=ctx.component.repository.remote_path if ctx.component.repository else None, + ScmAuthenticityFacts( + repo_link=repo_link, reason=verification_result.reason, status=verification_result.status.value, build_tool=verification_result.build_tool.name, confidence=Confidence.MEDIUM, - deps_dev_repo_link=deps_dev_repo_link, - deps_dev_stars_count=stars_count, - deps_dev_fork_count=fork_count, + stars_count=stars_count, + fork_count=fork_count, ) ) @@ -129,4 +126,4 @@ def run_check(self, ctx: AnalyzeContext) -> CheckResultData: return CheckResultData(result_tables=result_tables, result_type=result_type) -registry.register(MavenRepoVerificationCheck()) +registry.register(ScmAuthenticityCheck()) diff --git a/tests/integration/cases/maven_repo_verification/config.ini b/tests/integration/cases/scm_authenticity/config.ini similarity index 84% rename from tests/integration/cases/maven_repo_verification/config.ini rename to tests/integration/cases/scm_authenticity/config.ini index 90d59ea35..de14988d8 100644 --- a/tests/integration/cases/maven_repo_verification/config.ini +++ b/tests/integration/cases/scm_authenticity/config.ini @@ -3,4 +3,4 @@ [analysis.checks] exclude = -include = mcn_maven_repo_verification_1 +include = mcn_scm_authenticity_1 diff --git a/tests/integration/cases/maven_repo_verification/policy_fail_1.dl b/tests/integration/cases/scm_authenticity/policy_fail_1.dl similarity index 86% rename from tests/integration/cases/maven_repo_verification/policy_fail_1.dl rename to tests/integration/cases/scm_authenticity/policy_fail_1.dl index 0e36f0005..e594c7a2d 100644 --- a/tests/integration/cases/maven_repo_verification/policy_fail_1.dl +++ b/tests/integration/cases/scm_authenticity/policy_fail_1.dl @@ -4,7 +4,7 @@ #include "prelude.dl" Policy("test_policy", component_id, "") :- - check_failed(component_id, "mcn_maven_repo_verification_1"). + check_failed(component_id, "mcn_scm_authenticity_1"). apply_policy_to("test_policy", component_id) :- is_component(component_id, "pkg:maven/com.alibaba.ververica/flink-cep@1.17-vvr-8.0.8"). diff --git a/tests/integration/cases/maven_repo_verification/policy_pass_1.dl b/tests/integration/cases/scm_authenticity/policy_pass_1.dl similarity index 85% rename from tests/integration/cases/maven_repo_verification/policy_pass_1.dl rename to tests/integration/cases/scm_authenticity/policy_pass_1.dl index d43fd1f0f..2e0bbce57 100644 --- a/tests/integration/cases/maven_repo_verification/policy_pass_1.dl +++ b/tests/integration/cases/scm_authenticity/policy_pass_1.dl @@ -4,7 +4,7 @@ #include "prelude.dl" Policy("test_policy", component_id, "") :- - check_passed(component_id, "mcn_maven_repo_verification_1"). + check_passed(component_id, "mcn_scm_authenticity_1"). apply_policy_to("test_policy", component_id) :- is_component(component_id, "pkg:maven/org.antlr/antlr4-maven-plugin@4.13.2"). diff --git a/tests/integration/cases/maven_repo_verification/policy_pass_2.dl b/tests/integration/cases/scm_authenticity/policy_pass_2.dl similarity index 85% rename from tests/integration/cases/maven_repo_verification/policy_pass_2.dl rename to tests/integration/cases/scm_authenticity/policy_pass_2.dl index 49cd44d1e..8a9bb928a 100644 --- a/tests/integration/cases/maven_repo_verification/policy_pass_2.dl +++ b/tests/integration/cases/scm_authenticity/policy_pass_2.dl @@ -4,7 +4,7 @@ #include "prelude.dl" Policy("test_policy", component_id, "") :- - check_passed(component_id, "mcn_maven_repo_verification_1"). + check_passed(component_id, "mcn_scm_authenticity_1"). apply_policy_to("test_policy", component_id) :- is_component(component_id, "pkg:maven/org.neo4j/cypher-parser-common@5.21.2"). diff --git a/tests/integration/cases/maven_repo_verification/test.yaml b/tests/integration/cases/scm_authenticity/test.yaml similarity index 94% rename from tests/integration/cases/maven_repo_verification/test.yaml rename to tests/integration/cases/scm_authenticity/test.yaml index 2d084c954..9868f0980 100644 --- a/tests/integration/cases/maven_repo_verification/test.yaml +++ b/tests/integration/cases/scm_authenticity/test.yaml @@ -2,7 +2,7 @@ # Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl/. description: | - Integration tests for mcn_maven_repo_verification_1 check. + Integration tests for mcn_scm_authenticity_1 check. tags: - macaron-python-package diff --git a/tests/slsa_analyzer/checks/test_maven_repo_verification_check.py b/tests/slsa_analyzer/checks/test_repo_verification_check.py similarity index 85% rename from tests/slsa_analyzer/checks/test_maven_repo_verification_check.py rename to tests/slsa_analyzer/checks/test_repo_verification_check.py index 9d12c751c..f0f3dd923 100644 --- a/tests/slsa_analyzer/checks/test_maven_repo_verification_check.py +++ b/tests/slsa_analyzer/checks/test_repo_verification_check.py @@ -1,14 +1,14 @@ # Copyright (c) 2024 - 2024, 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/. -"""Module to test the maven repository verification check.""" +"""Module to test the repository verification check.""" from pathlib import Path from macaron.repo_verifier.repo_verifier_base import RepositoryVerificationResult, RepositoryVerificationStatus from macaron.slsa_analyzer.build_tool.base_build_tool import BaseBuildTool from macaron.slsa_analyzer.checks.check_result import CheckResultType -from macaron.slsa_analyzer.checks.maven_repo_verification_check import MavenRepoVerificationCheck +from macaron.slsa_analyzer.checks.scm_authenticity_check import ScmAuthenticityCheck from macaron.slsa_analyzer.package_registry import PyPIRegistry from macaron.slsa_analyzer.package_registry.maven_central_registry import MavenCentralRegistry from macaron.slsa_analyzer.specs.package_registry_spec import PackageRegistryInfo @@ -19,7 +19,7 @@ def test_repo_verification_pass(maven_tool: BaseBuildTool, macaron_path: Path) -> None: """Test that the check passes when the repository is verified.""" - check = MavenRepoVerificationCheck() + check = ScmAuthenticityCheck() ctx = MockAnalyzeContext(macaron_path=macaron_path, output_dir="", purl="pkg:maven/test/test") maven_registry = MavenCentralRegistry() @@ -37,7 +37,7 @@ def test_repo_verification_pass(maven_tool: BaseBuildTool, macaron_path: Path) - def test_repo_verification_fail(maven_tool: BaseBuildTool, macaron_path: Path) -> None: """Test that the check fails when the repository verification is failed.""" - check = MavenRepoVerificationCheck() + check = ScmAuthenticityCheck() ctx = MockAnalyzeContext(macaron_path=macaron_path, output_dir="", purl="pkg:maven/test/test") maven_registry = MavenCentralRegistry() @@ -53,9 +53,9 @@ def test_repo_verification_fail(maven_tool: BaseBuildTool, macaron_path: Path) - assert check.run_check(ctx).result_type == CheckResultType.FAILED -def test_repo_verification_unknown_for_unknown_repo_verification(maven_tool: BaseBuildTool, macaron_path: Path) -> None: +def test_check_unknown_for_unknown_repo_verification(maven_tool: BaseBuildTool, macaron_path: Path) -> None: """Test that the check returns unknown when the repository verification is unknown.""" - check = MavenRepoVerificationCheck() + check = ScmAuthenticityCheck() ctx = MockAnalyzeContext(macaron_path=macaron_path, output_dir="", purl="pkg:maven/test/test") maven_registry = MavenCentralRegistry() @@ -71,9 +71,9 @@ def test_repo_verification_unknown_for_unknown_repo_verification(maven_tool: Bas assert check.run_check(ctx).result_type == CheckResultType.UNKNOWN -def test_repo_verification_unknown_for_unsupported_build_tools(pip_tool: BaseBuildTool, macaron_path: Path) -> None: +def test_check_unknown_for_unsupported_build_tools(pip_tool: BaseBuildTool, macaron_path: Path) -> None: """Test that the check returns unknown for unsupported build tools.""" - check = MavenRepoVerificationCheck() + check = ScmAuthenticityCheck() ctx = MockAnalyzeContext(macaron_path=macaron_path, output_dir="", purl="pkg:pypi/test/test") pypi_registry = PyPIRegistry()