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: improved support for unknown version matches #3394

Closed
wants to merge 2 commits into from

Conversation

sw-sdiepold
Copy link
Contributor

The code from this pull request will add matched packages to the SBOM and for later CVE checking where no specific version could be found. This is important information for finding vulnerabilities since all contained packages even the ones with no easy to match version string can be vulnerable.

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

github's diff is making this annoying to read, but I think this is a good change.

Can you add something to the docs explaining what an "unknown" in the sbom will look like and what it means? I suspect it's the sort of thing people will find confusing when it comes up. Probably needs go to in MANUAL.md or in the SBOM how-to once it's written.

We could also use a test to make sure we don't accidentally revert this (a big risk, I suspect, since everyone wants different things out of their SBOMs). That could be a separate PR or I can just open a new issue after this is merged if you don't have time to work on that part.

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

Okay, it's been some months so clearly we're not going to get around to writing a test. I'll file a separate issue for that and get this merged as-is. Hopefully one of the GSo:C candidates will be interested in looking at improving the test situation.

@terriko terriko added this to the 3.3.1 milestone Apr 10, 2024
@terriko terriko modified the milestones: 3.4, future Aug 22, 2024
@terriko
Copy link
Contributor

terriko commented Aug 22, 2024

Okay, this has gotten stale and in revisiting the merge failures I'm noticing that we're wasting a lot of processing time on checks when it seems like the end result is "add all possible CVEs for a component into the list" so we could probably be more efficient in doing that.

Also, I think that might still be a thing that would be useful to people, but I'm not sure if I want it as a default rather than something that's chosen explicitly with a flag, because it would be very easy for people to get overwhelmed with (potentially incorrect) CVEs. That's maybe a benefit, since it'll encourage people to improve their SBOMs, but could be completely frustrating if they're using a pipeline for SBOM generation that isn't easy to edit. I'm currently leaning towards "this is a good idea but it needs to be an option rather than the default mode."

I'm going to go ahead and close this because I think we're going to need to rework more than just a merge fix, but I'll open a new issue so we can revisit the feature. Not going to happen for the 3.4 release but I think it's still good future work.

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.

2 participants