feat: verify whether the reported repository can be linked back to the artifact #873

mabdollahpour-ol · 2024-09-29T00:17:51Z

This version has initial support for maven and gradle build tools.

The core part is added under repo_verifier directory. analyzer calls the repo_verifier and adds the info to dynamic_data.

Also added a sample check (for maven) that shows how this data can be used.

src/macaron/repo_finder/repo_finder_java.py

behnazh-w · 2024-09-30T01:44:07Z

The core part is added as "repo_verifier" under "repo_finder". "analyzer" calls the "repo_verifier" and adds the info to "dynamic_data". I added a sample check (for maven) that shows how this data can be used.

Can you please add this information to the PR description?

mabdollahpour-ol · 2024-09-30T20:24:34Z

@behnazh-w thanks for the comments! I'll apply the changes by EOD.

src/macaron/repo_finder/repo_verifier.py

src/macaron/repo_finder/repo_finder_java.py

src/macaron/repo_finder/repo_verifier.py

src/macaron/slsa_analyzer/analyzer.py

src/macaron/slsa_analyzer/checks/maven_repo_verification_check.py

benmss · 2024-10-01T04:41:31Z

Is there a plan to add unit testing and integration testing as part of this PR? @behnazh-w

behnazh-w · 2024-10-01T05:18:25Z

Is there a plan to add unit testing and integration testing as part of this PR? @behnazh-w

Yes for sure. The PR needs unit tests and integration tests.

mabdollahpour-ol · 2024-10-01T20:41:41Z

Is there a plan to add unit testing and integration testing as part of this PR? @behnazh-w

Yes for sure. The PR needs unit tests and integration tests.

Yes tests are on the way.

src/macaron/repo_finder/repo_verifier.py

src/macaron/artifact/maven.py

… artifact Signed-off-by: Mohammad Abdollahpour <[email protected]>

Signed-off-by: Mohammad Abdollahpour <[email protected]>

…ctions Signed-off-by: Mohammad Abdollahpour <[email protected]>

Signed-off-by: Mohammad Abdollahpour <[email protected]>

src/macaron/repo_finder/repo_finder_deps_dev.py

src/macaron/repo_verifier/repo_verifier_base.py

Signed-off-by: Mohammad Abdollahpour <[email protected]>

benmss

Looks good to me.

Signed-off-by: Mohammad Abdollahpour <[email protected]>

behnazh-w · 2024-10-29T05:40:21Z

The core part is added as "repo_verifier" under "repo_finder". "analyzer" calls the "repo_verifier" and adds the info to "dynamic_data". I added a sample check (for maven) that shows how this data can be used.

Can you please add this information to the PR description?

Looks like you forgot to change the PR description. Could you please fix that?

behnazh-w · 2024-10-29T05:42:36Z

Please add the new check to the check table in the documentation at docs/source/index.rst.

behnazh-w · 2024-10-29T06:00:14Z

Please run make docs-full to generate the RST templates for the new modules on the documentation website.

behnazh-w · 2024-10-29T06:42:20Z

src/macaron/slsa_analyzer/checks/maven_repo_verification_check.py

+logger: logging.Logger = logging.getLogger(__name__)
+
+
+class MavenRepoVerificationFacts(CheckFacts):


This check should not be specific to Maven. Please clarify in the documentation that it currently supports only Maven, but avoid including Maven/maven in the names of the module, fact table, and check. In the implementation, you can verify that the artifact type is Maven and report UNKNOWN for all other ecosystems.

Where would be a good place to mention that it only supports Maven packages at this moment? The description field is the most visible one for the end users but technically under returns section of the docstring for run_check would be the proper place. What do you think? @behnazh-w

We can mention in in both places.

behnazh-w · 2024-10-29T06:43:48Z

src/macaron/slsa_analyzer/checks/maven_repo_verification_check.py

+    group: Mapped[str] = mapped_column(String, nullable=False)
+    artifact: Mapped[str] = mapped_column(String, nullable=False)
+    version: Mapped[str] = mapped_column(String, nullable=False)


Is there any particular reason for adding these columns? We can obtain them from the component table too.

Not really, just for the ease of analysis. I can remove them.

Yeah, specifically if we don't want it to be Maven-specific, they shouldn't be there.

behnazh-w · 2024-10-29T06:44:31Z

src/macaron/slsa_analyzer/checks/maven_repo_verification_check.py

+    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)


It would be good to make these visible in the HTML report. You just need to add the following property to the mapped_column.

Suggested change

repo_link: Mapped[str] = mapped_column(String, nullable=True)

repo_link: Mapped[str] = mapped_column(String, nullable=True, info={"justification": JustificationType.HREF})

behnazh-w · 2024-10-29T06:46:23Z

src/macaron/slsa_analyzer/checks/maven_repo_verification_check.py

+    repo_link: Mapped[str] = mapped_column(String, nullable=True)
+
+    # Repository link identified by deps.dev.
+    deps_dev_repo_link: Mapped[str | None] = mapped_column(String, nullable=True)


Suggested change

deps_dev_repo_link: Mapped[str | None] = mapped_column(String, nullable=True)

deps_dev_repo_link: Mapped[str | None] = mapped_column(String, nullable=True), info={"justification": JustificationType.HREF}

behnazh-w · 2024-10-29T06:47:48Z

src/macaron/slsa_analyzer/checks/maven_repo_verification_check.py

+    artifact: Mapped[str] = mapped_column(String, nullable=False)
+    version: Mapped[str] = mapped_column(String, nullable=False)
+
+    # Repository link identified by Macaron's repo finder.


The comments should be in #: format to ensure the docstring renders correctly.

This comment applies for all the properties.

behnazh-w · 2024-10-29T06:49:36Z

src/macaron/slsa_analyzer/checks/maven_repo_verification_check.py

+    deps_dev_repo_link: Mapped[str | None] = mapped_column(String, nullable=True)
+
+    # Number of stars on the repository identified by deps.dev.
+    deps_dev_stars_count: Mapped[int | None] = mapped_column(Integer, nullable=True)


This property should be rendered as TEXT so, it's slightly different:

Suggested change

deps_dev_stars_count: Mapped[int | None] = mapped_column(Integer, nullable=True)

deps_dev_stars_count: Mapped[int | None] = mapped_column(Integer, nullable=True, info={"justification": JustificationType.TEXT})

behnazh-w · 2024-10-29T06:50:03Z

src/macaron/slsa_analyzer/checks/maven_repo_verification_check.py

+    deps_dev_stars_count: Mapped[int | None] = mapped_column(Integer, nullable=True)
+
+    # Number of forks on the repository identified by deps.dev.
+    deps_dev_fork_count: Mapped[int | None] = mapped_column(Integer, nullable=True)


Same comment as above regarding adding info={"justification": JustificationType.TEXT}.

behnazh-w · 2024-10-29T06:50:10Z

src/macaron/slsa_analyzer/checks/maven_repo_verification_check.py

+    deps_dev_fork_count: Mapped[int | None] = mapped_column(Integer, nullable=True)
+
+    # The status of the check: passed, failed, or unknown.
+    status: Mapped[str] = mapped_column(String, nullable=False)


Same comment as above regarding adding info={"justification": JustificationType.TEXT}.

behnazh-w · 2024-10-29T06:50:15Z

src/macaron/slsa_analyzer/checks/maven_repo_verification_check.py

+    status: Mapped[str] = mapped_column(String, nullable=False)
+
+    # The reason for the status.
+    reason: Mapped[str] = mapped_column(String, nullable=False)


Same comment as above regarding adding info={"justification": JustificationType.TEXT}.

behnazh-w · 2024-10-29T06:50:22Z

src/macaron/slsa_analyzer/checks/maven_repo_verification_check.py

+    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)


Same comment as above regarding adding info={"justification": JustificationType.TEXT}.

mabdollahpour-ol · 2024-10-30T01:51:05Z

The core part is added as "repo_verifier" under "repo_finder". "analyzer" calls the "repo_verifier" and adds the info to "dynamic_data". I added a sample check (for maven) that shows how this data can be used.

Can you please add this information to the PR description?

Looks like you forgot to change the PR description. Could you please fix that?

I moved repo_verifier to its own directory. I changed the PR description (the first comment in this page if that's what you mean) to reflect the change. I didn't add this info the body of the squashed commit but I will in the next merge. Does that sound good?

mabdollahpour-ol · 2024-10-30T02:56:43Z

Please add the new check to the check table in the documentation at docs/source/index.rst.

The SLSA requirement is "Source - Version controlled", right?

behnazh-w · 2024-10-30T09:10:36Z

Please add the new check to the check table in the documentation at docs/source/index.rst.

The SLSA requirement is "Source - Version controlled", right?

I've added my suggestion to the PR. This check is related to Source - Version controlled, but it also includes a validation aspect. Generally, while the SLSA requirement column may not use the exact terminology from slsa.dev, there is a relationship.

mabdollahpour-ol self-assigned this Sep 29, 2024

mabdollahpour-ol requested review from behnazh-w and tromai as code owners September 29, 2024 00:17

oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Sep 29, 2024

behnazh-w requested a review from benmss September 30, 2024 01:33

behnazh-w reviewed Sep 30, 2024

View reviewed changes

src/macaron/repo_finder/repo_finder_java.py Outdated Show resolved Hide resolved

src/macaron/repo_finder/repo_finder_java.py Outdated Show resolved Hide resolved

behnazh-w reviewed Sep 30, 2024

View reviewed changes

src/macaron/repo_finder/repo_verifier.py Outdated Show resolved Hide resolved

src/macaron/repo_finder/repo_verifier.py Outdated Show resolved Hide resolved

behnazh-w reviewed Sep 30, 2024

View reviewed changes

src/macaron/repo_finder/repo_verifier.py Outdated Show resolved Hide resolved