Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2.7.5 release does not obey includeProvidedScope option #284

Closed
norrisjeremy opened this issue Feb 16, 2023 · 13 comments · Fixed by #302
Closed

2.7.5 release does not obey includeProvidedScope option #284

norrisjeremy opened this issue Feb 16, 2023 · 13 comments · Fixed by #302

Comments

@norrisjeremy
Copy link

With the 2.7.5. release, dependencies with <scope>provided</scope> are incorrectly include in the generated SBOM even though <includeProvidedScope>false</includeProvidedScope> is set.

@norrisjeremy
Copy link
Author

I would guess this was broken by 55b4bac.

@norrisjeremy
Copy link
Author

<includeCompileScope>false</includeCompileScope> appears to also be broken as well.

@norrisjeremy
Copy link
Author

To better illustrate the behavior change we're seeing with 2.7.5, consider the following:

In our project we have the following declared dependencies:

<dependency>
  <groupId>foo</groupId>
  <artifactId>foo</artifactId>
  <optional>true</optional>
</dependency>
<dependency>
  <groupId>bar</groupId>
  <artifactId>bar</artifactId>
  <scope>provided</scope>
  <optional>true</optional>
</dependency>

The foo project itself has the following declared dependencies:

<dependency>
  <groupId>bar</groupId>
  <artifactId>bar</artifactId>
  <scope>provided</scope>
</dependency>

In reality, the bar library is simply a set of Java annotation interfaces.
We only declare it as a direct dependency in our project because Maven won't pull it in transitively from our direct dependency on the foo library (since the foo library itself has it declared it as a provided dependency).
Unfortunately if it isn't present on the javac classpath for our project, it leads to compiler warnings being generated which causes our project to break since we keep the -Werror option turned on, hence the reason we declared it as a direct dependency in our project (we don't use any of these annotations from bar in our project).

With 2.7.4, we were able to prevent the SBOM from including this bar dependency by simply adding the <includeProvidedScope>false</includeProvidedScope> setting, which seemed to align with how we anticipated the <includeProvidedScope> setting to function.

But starting with 2.7.5, we're finding that this setting doesn't have any effect now for our project.
Instead to prevent the bar dependency from appearing in the SBOM, we have to set <includeCompileScope>false</includeCompileScope>, and it no longer matters whether or not we have <includeProvidedScope> enabled or disabled.

I'm not sure if the changes brought in by 55b4bac are intended to cause the bar dependency to appear now in SBOM generated for our project since it is still technically a transitive dependency (via the foo library) unless the <includeCompileScope> setting is disabled?

@norrisjeremy
Copy link
Author

Btw, you can see this behavior in mwiede/jsch#279, where I am attempting to add SBOM generation.
I'm attempting to prevent the com.kohlschutter:compiler-annotations dependency from appearing in the SBOM (it is a transitive provided scope dependency of com.kohlschutter.junixsocket:junixsocket-native-common).

@hboutemy
Copy link
Contributor

@knrc WDYT?

@knrc
Copy link
Contributor

knrc commented Feb 22, 2023

@hboutemy I can address this.

This worked previously because of the way the components were chosen, however this caused the problem with multiple roots in the dependency section when the maven resolution took place.

The current code now relies only on the maven filtering mechanism and provided is implied by compile, which would explain what @norrisjeremy has been seeing.

Fixing this would give me the opportunity to simplify the process, and even move over to using the underlying aether tree.

I'm on holiday this week, however I can likely get to this over the weekend or Monday.

@knrc
Copy link
Contributor

knrc commented Feb 22, 2023

@norrisjeremy I'll get this addressed as soon as possible

@norrisjeremy
Copy link
Author

Hi @knrc,

Great, thanks!

Something else that I noticed is that dependencies that marked as <optional>true</optional> in our pom.xml are marked with a required scope in the components list instead of an optional scope.

I don't think this is related to the the changes in 2.7.5 (as it happens with prior versions as well), but that seems to me as being incorrect. Wouldn't it be more correct to mark them with a optional scope (as well the transitive dependencies they pull in)?

Thanks,
Jeremy

@knrc
Copy link
Contributor

knrc commented Feb 22, 2023

That's definitely not related to my change, however I can look into that at the same time and see if I can discover what is happening.

Kev

knrc added a commit to knrc/cyclonedx-maven-plugin that referenced this issue Mar 1, 2023
knrc added a commit to knrc/cyclonedx-maven-plugin that referenced this issue Mar 1, 2023
knrc added a commit to knrc/cyclonedx-maven-plugin that referenced this issue Mar 1, 2023
knrc added a commit to knrc/cyclonedx-maven-plugin that referenced this issue Mar 1, 2023
@knrc
Copy link
Contributor

knrc commented Mar 1, 2023

@norrisjeremy I've submitted a PR to handle your original issue, but not the optional one. I'll take a look at that separately

@knrc
Copy link
Contributor

knrc commented Mar 1, 2023

With regards to the scope of the components this appears to be derived from the bytecode dependency analysis. If the artifact is used then it is marked as required, whereas if it is unused it is marked as optional. This is certainly different from defining it as optional in the pom, which appears to be ignored.

@norrisjeremy
Copy link
Author

Hi @knrc,

Regardless of the bytecode analysis, we would very much appreciate a way to flag <optional>true</optional> dependencies in our pom.xml as having an optional scope instead of required scope in the SBOM.

In our case with JSch, we have several non-mandatory dependencies that are not required for the overwhelming majority of consumers of our library. These dependencies are only needed by downstream consumers if they want to utilize these optional features.

It seems to me that that added an option similar to the includeProvidedScope would fit our use-case (named as includeOptional or something).

Thanks,
Jeremy

@knrc
Copy link
Contributor

knrc commented Mar 2, 2023

@norrisjeremy agreed, I'm not sure why it is done this way but I'll give it some thought and propose something

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 a pull request may close this issue.

3 participants