sonatype-nexus-community / scan-gradle-plugin

Gradle plugin that scans the dependencies of a Gradle project using Sonatype platforms: OSS Index and Nexus IQ Server.
Apache License 2.0
77 stars 21 forks source link

[FEATURE] For dependency graph report, a non-vulnerable dependency should list number of transitive vulnerabilities detected. #55

Closed shaikhu closed 3 years ago

shaikhu commented 3 years ago
+--- org.springframework.boot:spring-boot-starter-web:1.0.0.RELEASE: 0 vulnerabilities detected
|    +--- com.fasterxml.jackson.core:jackson-databind:2.9.10.4 (*): 6 vulnerabilities detected
|         ...
|    +--- org.springframework:spring-web:4.0.3.RELEASE: 3 vulnerabilities detected
|         ...
|    +--- org.springframework:spring-webmvc:4.0.3.RELEASE: 3 vulnerabilities detected
|         ...

to

+--- org.springframework.boot:spring-boot-starter-web:1.0.0.RELEASE: 12 transitive vulnerabilities detected
|    +--- com.fasterxml.jackson.core:jackson-databind:2.9.10.4 (*): 6 vulnerabilities detected
|        ...
|    +--- org.springframework:spring-web:4.0.3.RELEASE: 3 vulnerabilities detected
|        ...
|    +--- org.springframework:spring-webmvc:4.0.3.RELEASE: 3 vulnerabilities detected
|        ...

Or if we exclude the number and just print "transitive dependencies detected". I think both makes it clearer why the dependency was included in the report.

cc @bhamail / @DarthHater / @guillermo-varela

guillermo-varela commented 3 years ago

@shaikhu If the direct dependency has vulnerabilities how would it be shown?

2 vulnerabilities and 12 transitive vulnerabilities detected ? 2 direct vulnerabilities and 12 transitive vulnerabilities detected ? 2 own vulnerabilities and 12 transitive vulnerabilities detected ?

DarthHater commented 3 years ago

Maybe "has 2 vulnerabilities and 12 additional vulnerabilities in transitive dependencies" if that's not too wordy?

shaikhu commented 3 years ago

@shaikhu If the direct dependency has vulnerabilities how would it be shown?

Yeah I didn't think about that possibility. I was only concerned with getting rid of the "0 vulnerabilities detected" text.

2 direct vulnerabilities and 12 transitive vulnerabilities detected ?

I like this one the best but wondering if it's not too wordy?

pctF commented 3 years ago

I believe it can be misleading. Having vulnerable dependencies not, making component itself vulnerable. But statement about "transitive vulnerabilities" can lead to this state of mind. It is common practice for vulnerabilities to be created BASED on transitive ones. And if there is no such vulnerabilities in core component it is good chance that it is not affected.

There is nothing wrong with this enrichment but I really question if it is worth it.

shaikhu commented 3 years ago

I believe it can be misleading. Having vulnerable dependencies not, making component itself vulnerable. But statement about "transitive vulnerabilities" can lead to this state of mind. It is common practice for vulnerabilities to be created BASED on transitive ones. And if there is no such vulnerabilities in core component it is good chance that it is not affected.

There is nothing wrong with this enrichment but I really question if it is worth it.

Thank you for the feedback. It's very much appreciated. 👍 Agree with your thoughts, and feel the end result isn't worth the effort to implement. Considering more urgent issues (#58) I'm going to close this.