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(cyclonedx): Add initial support for loading external VEX files from SBOM references #8254

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

RingoDev
Copy link
Contributor

@RingoDev RingoDev commented Jan 17, 2025

Description

Related discussion

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 Jan 17, 2025

CLA assistant check
All committers have signed the CLA.

@knqyf263
Copy link
Collaborator

Thanks for your contribution. The next release is coming soon. We hope to add this feature in v0.60.0.

@RingoDev
Copy link
Contributor Author

RingoDev commented Feb 9, 2025

Thanks for your contribution. The next release is coming soon. We hope to add this feature in v0.60.0.

Sounds great, linting issues and co. should be gone by now 👍

@RingoDev
Copy link
Contributor Author

Any chance this still makes it into 0.60 @nikpivkin @knqyf263?

@knqyf263
Copy link
Collaborator

I'll take a look this week.

Copy link
Collaborator

@knqyf263 knqyf263 left a comment

Choose a reason for hiding this comment

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

I'm sorry to be late. It looks good to me. Thanks for your contribution! I left some small comments.

pkg/vex/vex.go Outdated
Comment on lines 118 to 123
v, err = NewSBOMReferenceSet(report)
if err != nil {
return nil, xerrors.Errorf("failed to create set of external VEX documents: %w", err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now, we should handle nil :)

Suggested change
v, err = NewSBOMReferenceSet(report)
if err != nil {
return nil, xerrors.Errorf("failed to create set of external VEX documents: %w", err)
}
v, err = NewSBOMReferenceSet(report)
if err != nil {
return nil, xerrors.Errorf("failed to create set of external VEX documents: %w", err)
} else if v == nil {
continue
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This now raises a question for me. If this is an experimental feature and somebody explicitly scans an SBOM with --vex sbom-ref shouldn't trivy throw an error if it finds no valid external references in the scanned SBOM?

Similar to how it is now throwing an error if e.g. a server responds with 404.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This now raises a question for me. If this is an experimental feature and somebody explicitly scans an SBOM with --vex sbom-ref shouldn't trivy throw an error if it finds no valid external references in the scanned SBOM?

Good question. The --vex sbom-ref flag tells Trivy that it should use any VEX reference found within the SBOM. Therefore, enabling it even if it's not yet clear whether external VEX references exist in the SBOM is expected behavior. Conversely, it would be unexpected if external references were silently ignored even when present.

Similarly, the -vex oci flag is designed to use any available VEX attestations. If no attestation is found, it doesn't trigger an error; however, if attestation is found but contains issues, an error will be returned.

That said, as you mentioned, if it is essential that external references are reliably loaded, returning an error might be more appropriate. This is something we can reconsider based on future community feedback. At the very least, it would be advisable to display a log message that no vex references are found for now.

@RingoDev RingoDev force-pushed the main branch 2 times, most recently from 77652c1 to c3ba95a Compare February 26, 2025 11:19
…fined in CycloneDX SBOMs

* by specifying option `--vex sbom-ref` the externalReferences of a CycloneDx SBOM are used to fetch external VEX documents referenced as type `exploitability-statement`
* trivy will error if one of the referenced VEX statements can not be fetched or parsed
* added documentation of feature
Copy link
Collaborator

@knqyf263 knqyf263 left a comment

Choose a reason for hiding this comment

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

LGTM. I want to add some tweaks, but it's my job. Thanks for your contribution.

@knqyf263 knqyf263 added this pull request to the merge queue Feb 27, 2025
Merged via the queue into aquasecurity:main with commit 4820eb7 Feb 27, 2025
17 checks passed
dstrelbytskyi pushed a commit to datarobot/trivy that referenced this pull request Mar 5, 2025
dstrelbytskyi pushed a commit to datarobot/trivy that referenced this pull request Mar 10, 2025
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.

3 participants