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

Versions not printed out properly from maven pom.xml #1251

Closed
chtompki opened this issue Oct 6, 2022 · 8 comments · Fixed by #1278
Closed

Versions not printed out properly from maven pom.xml #1251

chtompki opened this issue Oct 6, 2022 · 8 comments · Fixed by #1278
Assignees
Labels
bug Something isn't working ecosystem:java relating to the java ecosystem

Comments

@chtompki
Copy link
Contributor

chtompki commented Oct 6, 2022

Note, I would love to help with submitting fixes here if possible. Though I don't know your codebase all that well, so I might need a little direction.

What happened:
When I run syft . on the Apache Project commons-text (note this is after having run mvn clean install), I get the following output from syft:

NAME                      VERSION                     TYPE         
assertj-core              3.23.1                      java-archive  
commons-io                2.11.0                      java-archive  
commons-lang3             3.12.0                      java-archive  
commons-rng-simple        ${commons.rng.version}      java-archive  
jmh-core                  ${jmh.version}              java-archive  
jmh-generator-annprocess  ${jmh.version}              java-archive  
js                        ${graalvm.version}          java-archive  
js-scriptengine           ${graalvm.version}          java-archive  
junit-jupiter                                         java-archive  
mockito-inline            ${commons.mockito.version}  java-archive 

What you expected to happen:

NAME                      VERSION                   TYPE         
assertj-core              3.23.1                    java-archive  
commons-io                2.11.0                    java-archive  
commons-lang3             3.12.0                    java-archive  
commons-rng-simple        1.4                       java-archive  
jmh-core                  1.35                      java-archive  
jmh-generator-annprocess  1.35                      java-archive  
js                        22.0.0.2                  java-archive  
js-scriptengine           22.0.0.2                  java-archive  
junit-jupiter             5.9.1                     java-archive  
mockito-inline            4.8.0                     java-archive 

How to reproduce it (as minimally and precisely as possible): Running syft on any maven java project where in the pom.xml, the versions are declared in the properties section as opposed to directly in <version></version> line of the dependency declaration.

Anything else we need to know?: I think this was working in version 48 of syft, and now isn't. Also worth noting is mvn dependency:tree generates a vastly different list of dependencies.

commons-text % mvn dependency:tree
[INFO] Scanning for projects...
[INFO] 
[INFO] ------------------< org.apache.commons:commons-text >-------------------
[INFO] Building Apache Commons Text 1.10.1-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-dependency-plugin:3.3.0:tree (default-cli) @ commons-text ---
[INFO] org.apache.commons:commons-text:jar:1.10.1-SNAPSHOT
[INFO] +- org.apache.commons:commons-lang3:jar:3.12.0:compile
[INFO] +- org.junit.jupiter:junit-jupiter:jar:5.9.1:test
[INFO] |  +- org.junit.jupiter:junit-jupiter-api:jar:5.9.1:test
[INFO] |  |  +- org.opentest4j:opentest4j:jar:1.2.0:test
[INFO] |  |  +- org.junit.platform:junit-platform-commons:jar:1.9.1:test
[INFO] |  |  \- org.apiguardian:apiguardian-api:jar:1.1.2:test
[INFO] |  +- org.junit.jupiter:junit-jupiter-params:jar:5.9.1:test
[INFO] |  \- org.junit.jupiter:junit-jupiter-engine:jar:5.9.1:test
[INFO] |     \- org.junit.platform:junit-platform-engine:jar:1.9.1:test
[INFO] +- org.assertj:assertj-core:jar:3.23.1:test
[INFO] |  \- net.bytebuddy:byte-buddy:jar:1.12.10:test
[INFO] +- commons-io:commons-io:jar:2.11.0:test
[INFO] +- org.mockito:mockito-inline:jar:4.8.0:test
[INFO] |  \- org.mockito:mockito-core:jar:4.8.0:test
[INFO] |     +- net.bytebuddy:byte-buddy-agent:jar:1.12.14:test
[INFO] |     \- org.objenesis:objenesis:jar:3.2:test
[INFO] +- org.graalvm.js:js:jar:22.0.0.2:test
[INFO] |  +- org.graalvm.regex:regex:jar:22.0.0.2:test
[INFO] |  +- org.graalvm.truffle:truffle-api:jar:22.0.0.2:test
[INFO] |  \- org.graalvm.sdk:graal-sdk:jar:22.0.0.2:test
[INFO] +- org.graalvm.js:js-scriptengine:jar:22.0.0.2:test
[INFO] +- org.apache.commons:commons-rng-simple:jar:1.4:test
[INFO] |  \- org.apache.commons:commons-rng-core:jar:1.4:test
[INFO] |     \- org.apache.commons:commons-rng-client-api:jar:1.4:test
[INFO] +- org.openjdk.jmh:jmh-core:jar:1.35:test
[INFO] |  +- net.sf.jopt-simple:jopt-simple:jar:5.0.4:test
[INFO] |  \- org.apache.commons:commons-math3:jar:3.2:test
[INFO] \- org.openjdk.jmh:jmh-generator-annprocess:jar:1.35:test
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  3.255 s
[INFO] Finished at: 2022-10-06T12:59:22-04:00
[INFO] ------------------------------------------------------------------------

Environment:

  • Output of syft version: 0.58.0
  • OS (e.g: cat /etc/os-release or similar): MacOS 12.6
@chtompki chtompki added the bug Something isn't working label Oct 6, 2022
@tgerla
Copy link
Contributor

tgerla commented Oct 6, 2022

Thanks for the report, @chtompki. We will update this when we are able. I believe this is related to #1129 as well.

@chtompki
Copy link
Contributor Author

chtompki commented Oct 6, 2022

I'm happy to close the issue as redundant. Also, will try to get in and see if I can get a PR that gets it working as a fix to #1129

@tgerla
Copy link
Contributor

tgerla commented Oct 6, 2022

A PR would be amazing! Let us know if we can help with anything, and feel free to drop by the Slack to discuss with the team.

@benken-parasoft
Copy link

Related to this, Syft generates a malformed "purl" which does not parse as a URI. I believe the dollar sign in these version strings are not being uri/percent-encoded when generating the "purl" string.

@chtompki
Copy link
Contributor Author

chtompki commented Oct 7, 2022

Yeah that would make sense. Minimally if we see a ${ at the beginning of the version string we can assume that the version is in the properties of the pom.xml.

Also, I've been thinking about how we get transitive dependencies for a project because currently we're only getting what's declared in the pom, and that's only a portion of the dependencies of the project.

@kzantow
Copy link
Contributor

kzantow commented Oct 7, 2022

The within-the-same-pom property resolution should not be especially hard to do, if we're not doing that already. I've implemented something that did this for a project many years ago. The problem is if the properties come from a parent pom.

There are a few ways to get transitive and parent dependencies I can think of:

  1. Run mvn dependency:tree
    This option is something we'd like to avoid, as it takes Syft outside of a static analysis tool, mvn could have been compromised somehow to alter the results, etc.

  2. Implement our own maven dependency resolution
    This is a nontrivial effort, obviously, and requires accessing network resources to resolve dependency/parent POM references, which might need secrets for private maven repos or proxies or other unknowns. I feel this is a little bit better but also requires implementing the exact maven logic, which can be complicated, especially when version ranges are used

  3. Follow on-disk metadata (e.g. ~/.m2/repository) to make a best-guess resolution
    This would require having run a mvn build or something of the sort before running Syft and of course could also have been tampered with. However, I think this would be a simpler compromise between option 1 and 2. It also has the side effect of not requiring network access or secrets for private maven repos, etc.

It should also be noted we take the 3rd option approach to find additional information, if it's on disk for Node projects, which also requires running an install before Syft to get richer data.

Any thoughts here?

@chtompki
Copy link
Contributor Author

chtompki commented Oct 7, 2022

The only other option I can think of is to make a maven contribution that leaves a dependency:tree or dependency:build-classpath output in the target directory. I'm not opposed to trying this path if need be. Otherwise I would think we'd likely go for option 2 from your list.

@wagoodman wagoodman added the ecosystem:java relating to the java ecosystem label Oct 20, 2022
chtompki added a commit to chtompki/syft that referenced this issue Oct 20, 2022
chtompki added a commit to chtompki/syft that referenced this issue Oct 20, 2022
Signed-off-by: Rob Tompkins <chtompki@apache.org>
chtompki added a commit to chtompki/syft that referenced this issue Oct 24, 2022
Signed-off-by: Rob Tompkins <chtompki@apache.org>
chtompki added a commit to chtompki/syft that referenced this issue Oct 24, 2022
Signed-off-by: Rob Tompkins <chtompki@apache.org>
chtompki added a commit to chtompki/syft that referenced this issue Oct 24, 2022
Signed-off-by: Rob Tompkins <chtompki@apache.org>
@kzantow kzantow self-assigned this Oct 25, 2022
@kzantow
Copy link
Contributor

kzantow commented Oct 25, 2022

With the current PR (#1278) for this the output is:

NAME                      VERSION   TYPE         
assertj-core              3.23.1    java-archive  
commons-io                2.11.0    java-archive  
commons-lang3             3.12.0    java-archive  
commons-rng-simple        1.5       java-archive  
jmh-core                  1.35      java-archive  
jmh-generator-annprocess  1.35      java-archive  
js                        22.0.0.2  java-archive  
js-scriptengine           22.0.0.2  java-archive  
junit-jupiter                       java-archive  
mockito-inline            4.8.1     java-archive  

Note: junit-jupiter still does not have a version number because it is coming from a parent POM. Would this be an acceptable solution for this issue at present? (Otherwise, we're going to have to implement parent and possibly dependency POM resolution, which is a much larger issue that we should figure out separately)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ecosystem:java relating to the java ecosystem
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants