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

feat(sbom): Support license detection for SBOM scan #6072

Merged

Conversation

bedla
Copy link
Contributor

@bedla bedla commented Feb 6, 2024

Description

Hi, I find out that when scanning SBOM files I am not able to select License scanner, only Vulnerability scannes is available. I dig little bit in source code and it looks trivial - from high level.
What do you think about this change.
If you will guide me to tests I should write or other parts/use-cases I should test and fix, I can update it. I will also polish commits, PR title, and all the stuff required :)
Thx
Ivos

note: I have also started conversation here #6073

Issue

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@CLAassistant
Copy link

CLAassistant commented Feb 6, 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.

@knqyf263 knqyf263 requested a review from DmitriyLewen February 7, 2024 05:07
@knqyf263
Copy link
Collaborator

knqyf263 commented Feb 7, 2024

Looks good. Can you please test it @DmitriyLewen?

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 @bedla
Thanks for your work.

You can read about PR title here - https://aquasecurity.github.io/trivy/v0.49/community/contribute/pr/#title

I left some comments. Take a look, please.
Can you also update docs:

About tests - i think we can add 1 more testcase in integration sbom test:
add scanner to args, scan licenses for testdata/fixtures/sbom/centos-7-cyclonedx.json and create new golden file.

pkg/commands/app.go Show resolved Hide resolved
pkg/commands/app.go Show resolved Hide resolved
@bedla
Copy link
Contributor Author

bedla commented Feb 7, 2024

Sure, I will take a look and make changes.

@bedla bedla force-pushed the feature/support-license-scan-for-sbom branch from 5d7784b to 0ecb5d7 Compare February 18, 2024 19:50
@bedla bedla changed the title Support license detection for SBOM scan feat(sbom): Support license detection for SBOM scan Feb 18, 2024
@bedla bedla marked this pull request as ready for review February 18, 2024 19:52
@bedla bedla requested a review from knqyf263 as a code owner February 18, 2024 19:52
integration/testdata/fixtures/sbom/license-cyclonedx.json Outdated Show resolved Hide resolved
integration/testdata/fixtures/sbom/license-cyclonedx.json Outdated Show resolved Hide resolved
pkg/fanal/artifact/sbom/sbom.go Outdated Show resolved Hide resolved
integration/sbom_test.go Show resolved Hide resolved
integration/sbom_test.go Outdated Show resolved Hide resolved
@bedla bedla force-pushed the feature/support-license-scan-for-sbom branch from 0ecb5d7 to 5af436b Compare February 20, 2024 19:45
@bedla
Copy link
Contributor Author

bedla commented Feb 20, 2024

On my Maven test project I have generated integration/testdata/fixtures/sbom/license-cyclonedx.json file with command trivy fs --format cyclonedx --output result.json C:\Users\bedla.czech\IdeaProjects\sbom-demo.

Mind that golden file integration/testdata/license-cyclonedx.json.golden has all Licenses marked as UNKNOWN.
With deeper investigation I found out that it is because my Maven test project dependencies does not identify licenses using SPDX License ID.

I did more deeper investigation because Cyclone DX Maven plugin is identifying License ID at most cases correctly. What I found is that they use mapping file https://github.com/CycloneDX/cyclonedx-core-java/blob/master/src/main/resources/license-mapping.json to find those License IDs.

I think that new PR should be created to implement similar concept like they do. What do you think?

@DmitriyLewen
Copy link
Contributor

We already have mapping -

var mapping = map[string]string{
// GPL
"GPL-1": GPL10,
"GPL-1+": GPL10,
"GPL 1.0": GPL10,
"GPL 1": GPL10,
"GPL2": GPL20,
.
But it looks like we need to supplement our map with pairs from cycledx-core-java.

@christiankofler
Copy link

This is really a great improvement! Just to be sure: will license scanning on SBOM also work for SPDX json with this PR? If not, this would be another great improvement for trivy.

@DmitriyLewen
Copy link
Contributor

@christiankofler It should work for all supported SBOM formats

@knqyf263
Copy link
Collaborator

knqyf263 commented Mar 4, 2024

@bedla Do you have time to wrap it up? If not, we can take care of this PR.

@bedla
Copy link
Contributor Author

bedla commented Mar 4, 2024

sorry, I was on vacations last week. I will finish it this week.

@bedla bedla force-pushed the feature/support-license-scan-for-sbom branch from 5af436b to 9902696 Compare March 10, 2024 15:14
@bedla
Copy link
Contributor Author

bedla commented Mar 10, 2024

Hi, I have force pushed new changes to my branch (sorry for delay).
No thing to note is that it seems to https://github.com/CycloneDX/cyclonedx-core-java/blob/master/src/main/resources/license-mapping.json has more heuristic capabilities then Trivy has. So, I copied definition that I was sure that we can map to Trivy definition - other I left intact.

@bedla bedla force-pushed the feature/support-license-scan-for-sbom branch from 9902696 to 79ebdf9 Compare March 10, 2024 20:16
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 @bedla

Left comments.
Also pay attention to integration tests. integration/client_server_test.go is now broken.

@bedla
Copy link
Contributor Author

bedla commented Mar 11, 2024

ahh, sorry ... will fix it asap.
(note: it is hard to work with mage on windows, and also for some reason first compilation after some time is super-super slow => from time to time I am trying to find root causes on my local machine)

@bedla bedla force-pushed the feature/support-license-scan-for-sbom branch 2 times, most recently from 9137f7e to c09620f Compare March 13, 2024 05:03
@bedla
Copy link
Contributor Author

bedla commented Mar 14, 2024

all the stuff should be fixed

@bedla
Copy link
Contributor Author

bedla commented Mar 14, 2024

hmm, interesting that test is not failing on my local machine ... will check it.

@bedla bedla force-pushed the feature/support-license-scan-for-sbom branch from c09620f to a1fa7ad Compare March 16, 2024 13:03
@bedla
Copy link
Contributor Author

bedla commented Mar 16, 2024

I have force pushed fixes, now it should be ok.

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.

@bedla Thanks for your work!

@knqyf263 Take a look, when you have time, please.

@knqyf263 knqyf263 added this pull request to the merge queue Mar 18, 2024
@bedla
Copy link
Contributor Author

bedla commented Mar 18, 2024

@DmitriyLewen you are welcome, I am happy to help :)

Merged via the queue into aquasecurity:main with commit eb3ceb3 Mar 18, 2024
17 checks passed
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.

Support license detection for SBOM scan
5 participants