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

fix: Add dependencyManagement exclusions to the child exclusions #6969

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

coheigea
Copy link

@coheigea coheigea commented Jun 19, 2024

I noticed that if I have a child pom with a dependency with some exclusions, and a parent pom with a dependencyManagement section with the same dependency with different exclusions, then Trivy only uses the child exclusions.

This is not the behaviour that the maven command line uses, the mvn dependency:tree combines the set of exclusions in this case.

To reproduce unzip "trivy.zip" attached and run:

 mvn dependency:tree | grep jettison

This will return nothing as jettison is excluded in the parent pom. Now run:

 trivy fs . | grep jettison

and see it returns findings in jettison even though it's not on the classpath

@CLAassistant
Copy link

CLAassistant commented Jun 19, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coheigea
Copy link
Author

trivy.zip

@coheigea coheigea changed the title Add dependencyManagement exclusions to the child exclusions fix: Add dependencyManagement exclusions to the child exclusions Jun 19, 2024
Copy link
Contributor

@DmitriyLewen DmitriyLewen left a comment

Choose a reason for hiding this comment

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

hello @coheigea
Thanks for your work!

Can you create new test for these changes?

Regards, Dmitriy

@coheigea coheigea force-pushed the coheigea/exclusions branch 2 times, most recently from 81492e4 to af516ff Compare June 20, 2024 08:24
@coheigea
Copy link
Author

@DmitriyLewen How do I run the tests...even on main, I'm getting failures with:

go test ./pkg/dependency/parser/java/pom

@DmitriyLewen
Copy link
Contributor

@coheigea
Copy link
Author

@DmitriyLewen I added a test, but I'm having trouble running the tests locally even with mage, can you enable the workflow please?

@DmitriyLewen
Copy link
Contributor

it should work

cd ./pkg/dependency/parser/java/pom 
➜  go test -run "TestPom_Parse"   
PASS
ok      github.com/aquasecurity/trivy/pkg/dependency/parser/java/pom    0.039s

@coheigea
Copy link
Author

Please re-run the tests

@coheigea
Copy link
Author

All checks passed 👍

@DmitriyLewen
Copy link
Contributor

DmitriyLewen commented Jun 24, 2024

@coheigea I changed your test to show that mvn merges exceptions from base pom and parent (your test showed that mvn takes exceptions from parent, but didn't show that exceptions from the base pom are also used).
Can you take a look?

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.

None yet

3 participants