Skip to content

Write NOASSERTION for artifacts resolved from a local repository - #193

Open
arpitjain099 wants to merge 2 commits into
spdx:mainfrom
arpitjain099:fix/file-repo-download-location
Open

Write NOASSERTION for artifacts resolved from a local repository#193
arpitjain099 wants to merge 2 commits into
spdx:mainfrom
arpitjain099:fix/file-repo-download-location

Conversation

@arpitjain099

Copy link
Copy Markdown

Fixes #192.

toDownloadLocation resolves the artifact path against whatever repository URI Gradle reports. For mavenLocal(), or any maven { url = uri("file:...") }, that is a file: URL, and SPDX 2 does not accept a local path as a download location. Before 0.12.0 the plugin wrote it anyway and produced an invalid SBOM; since 0.12.0 the same value reaches setDownloadLocation and the build fails with InvalidSPDXAnalysisException: Invalid download location, so no SBOM is written at all.

A file: repository now returns NOASSERTION, next to the existing NOASSERTION passthrough in the same method, so the caller in SpdxDocumentBuilder needs no change.

I left the package-manager external ref alone. It still carries repository_url=file:/... in the purl for these artifacts, which is odd but not what breaks the build, and dropping or rewriting it is a separate decision.

URIsTest.toDownloadLocation_localRepository covers it and fails on main with an AssertionFailedError. ./gradlew test and ./gradlew spotlessCheck both pass, run in an eclipse-temurin:21-jdk container.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
@loosebazooka

Copy link
Copy Markdown
Collaborator

I think this might need to happen at the SpdxDocumentBuilder level - after the task extension has mutated the object: https://github.com/spdx/spdx-gradle-plugin/blob/main/src/main/java/org/spdx/sbom/gradle/utils/SpdxDocumentBuilder.java#L365

Moves the check to after the task extension has had its chance to rewrite
the repository URI, and nulls the URI instead of special-casing the
download location. The existing null path already emits NOASSERTION and
skips the package-manager external reference, so the purl no longer gets
a file: repository_url either.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
@arpitjain099

Copy link
Copy Markdown
Author

Moved, thanks. Reading it again you're right, and for a stronger reason than the ordering: the repoUri == null branch just below already does the right thing, so nulling the URI reuses it instead of adding a parallel path.

That also fixes something my version missed. Special-casing toDownloadLocation left URIs.toPurl a few lines down still building a purl with repository_url=file:/home/user/.m2/repository. Nulling the URI at the builder means the existing branch emits NOASSERTION and skips the package-manager external reference entirely, so both sinks are covered. It applies to the container path too, since createContainerMavenPackage passes the same URI through.

On placement, it sits right after mapRepoUri, so an extension mapping a local mirror to a real remote still wins and only an unmapped file: URI is dropped.

Now 9 lines in one file. I removed the URIsTest case since the hook it tested is gone. There's no unit harness for SpdxDocumentBuilder, so the natural home for coverage is a functional test with a file: repository fixture alongside the existing ivy one. Happy to add that if you want it in this PR.

Comment on lines +366 to +370
// An artifact resolved from a local repository (mavenLocal(), or
// maven { url = uri("file:...") }) has no location a consumer can fetch it from.
// Its file: URL is neither a valid SPDX download location nor a usable purl
// repository_url, so treat the repository as unknown: the null path below already
// renders that as NOASSERTION with no package-manager external reference.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is too verbose.

@loosebazooka

Copy link
Copy Markdown
Collaborator

Please limit AI generated text as much as possible. Condense what you can where you can either through prompting or human involvement. I should not be exposed to the direct output of your conversation with an LLM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Local (file:) repositories give an invalid download location, and fail the build since 0.12.0

2 participants