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.

Sign up for GitHub

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

Jump to bottom

Ensure that all cyclonedx components have bom-refs #914

Merged
merged 4 commits into from
Apr 1, 2022
Merged

Ensure that all cyclonedx components have bom-refs #914

merged 4 commits into from
Apr 1, 2022

Conversation

sambhav
Copy link
Contributor

@sambhav sambhav commented Mar 23, 2022 •
edited
Loading

Signed-off-by: Sambhav Kothari skothari44@bloomberg.net

BOM-Refs are important for VEX to refer to components uniquely. This allows us to create an independent vex document and refer to components within the SBOM through a BOM Link (see https://cyclonedx.org/capabilities/bomlink/)

The current logic for setting the bom ref is -

  • PURL + syft id as a qualifier if PURL exists
  • syft artifact id otherwise.

cc: @stevespringett @coderpatros

This is needed for independent VEX documents in grype.

@sambhav
Copy link
Contributor Author

sambhav commented Mar 23, 2022

Notes -

  • At some point we will need to add fields to sbom.SBOM to add optional 'decode' source information. i.e. if syft decoded an sbom into its format model then it should have some metadata from the original sbom preserved. The main things we would need are the bom uuid.
  • We would need to make bom-ref a part of the syft model at some point so that we can use it in grype for referring to components. At this point, we can move the current bom ref logic to a method in pkg.Package but I wanted to get something out in the meantime as these changes mean that the syft model may have to be slightly aware of the output presentation format.

@kzantow
Copy link
Contributor

kzantow commented Mar 28, 2022

@samj1912 what if we always include the ID, something like bom-ref = fmt.Sprintf("%s-%s", p.PURL, p.ID())? This way we can have a more readable bom-ref and guarantee uniqueness?

@sambhav
Ensure that all cyclonedx components have bom-refs
e56abe9 
Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>
@sambhav sambhav requested a review from kzantow March 31, 2022 21:38
@sambhav sambhav mentioned this pull request Mar 31, 2022
Merged
kzantow
kzantow reviewed Mar 31, 2022
View reviewed changes
internal/formats/cyclonedxjson/test-fixtures/snapshot/TestCycloneDxDirectoryEncoder.golden
@@ -20,6 +20,7 @@
},
"components": [
{
"bom-ref": "b85dbb4e6ece5082",
Copy link
Contributor

Choose a reason for hiding this comment

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

An odd thing: none of these golden examples actually include a PURL-based bom-ref; maybe something is wrong about the logic or none of them have populated PURLs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

None of them have populated purls. if you run make validate-schema, you can see some actual examples.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, since the examples in the test suite aren't valid pURLs.

wagoodman
wagoodman reviewed Apr 1, 2022
View reviewed changes
internal/formats/common/cyclonedxhelpers/component.go Outdated
// TODO: In the future we may want to dedupe by PURL and combine components with
// the same PURL while preserving their unique metadata.
if parsedPURL, err := packageurl.FromString(p.PURL); err == nil {
parsedPURL.Qualifiers = append(parsedPURL.Qualifiers, packageurl.Qualifier{Key: "syft-id", Value: string(p.ID())})
Copy link
Contributor

Choose a reason for hiding this comment

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

nice touch making this a pURL qualifier

@wagoodman
put cyclonedx bom-ref derivation under test
2c12d90 
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
@wagoodman
update common formats fixture to have valid pURL
adaafaa 
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
@wagoodman
update all format snapshots with valid pURL examples
17a764a 
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
wagoodman
wagoodman approved these changes Apr 1, 2022
View reviewed changes
Copy link
Contributor

@wagoodman wagoodman left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice job @samj1912 ! I only made a couple of changes:

  1. Updated the common format test fixtures to have an example of a valid pURL
  2. Added a few tests for the bom-ref creation logic

@wagoodman wagoodman merged commit 8bc5d84 into anchore:main Apr 1, 2022
@sambhav sambhav deleted the bom-refs branch April 1, 2022 17:01
@sambhav
Copy link
Contributor Author

sambhav commented Apr 1, 2022

Thanks!

spiffcs added a commit that referenced this pull request May 2, 2022
@spiffcs
Merge branch 'main' into 835-keyless-attestation-upgrade
cb25858 
* main: (31 commits)
  reduce noise of log output (#976)
  add version info and remove double config call (#977)
  Rename syft-id to package-id (#970)
  update to cyclonedx-go 0.5.2 (#971)
  refactor command package to remove globals and add dependency injection
  fix: #953 Derive language from pURL - https://github.com/anchore/syft… (#957)
  Fix typo in CPE-parsing error (#966)
  Preserve syft IDs on SBOM decode (#963)
  Update GitHub format package_url and correlator (#961)
  Ensure SPDXIDs are valid (#955)
  Auto-PR needs to run go mod tidy (#958)
  Add workflow for automatic PR for new stereoscope updates (#954)
  Minor readme update to correct format information (#948)
  Update spdx22json to only take uppercase checksum algorithm (#946)
  add additional vendors for springframework (#945)
  Add digest property to parent and nested java package metadata (#941)
  Update write permissions and log into ghcr.io for release (#942)
  Retry auth URL lookup without docker credentialhelper workaround (#939)
  Ensure that all cyclonedx components have bom-refs (#914)
  Additionally publish docker images to GHCR (#934)
  ...

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
@sambhav @wagoodman
Ensure that all cyclonedx components have bom-refs (anchore#914)
411ef7b 
Co-authored-by: Alex Goodman <alex.goodman@anchore.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kzantow kzantow kzantow left review comments

@wagoodman wagoodman wagoodman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sambhav @kzantow @wagoodman