Skip to content

fix: [WIP]introduce Component Type Unknown in Cyclone DX for better conversion #3833

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

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

Conversation

VictorHuu
Copy link
Contributor

@VictorHuu VictorHuu commented Apr 27, 2025

Description

  1. In case of unexpected CycloneDX ComponentType, Unknown is used to prevent crash during conversion between Cyclone DX and other formats.
  2. Though Syft only support some of the ComponentType like container and file, other types defined in Cyclone DX Spec should also be included. Since most of these types share almost the same structure, a 'Unknown' type is enough to handle these types that Syft doesn't support now.

I'm not sure whether the solution is good or not,but I think a bit lossy information like 'application/library' to 'unknown' is acceptable.Otherwise,if we add types for every Component Type, quite a lot of even overwhelming test cases have to be updated.

Follow up: It seems that there are lots of issues related to the conversion, so this PR attempts to make the minimum changes in order not to introduce catastrophic results.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have added unit tests that cover changed behavior
  • I have tested my code in common scenarios and confirmed there are no regressions
  • I have added comments to my code, particularly in hard-to-understand sections

along with 'application' and 'library'

Signed-off-by: Yuntao Hu <[email protected]>
Signed-off-by: Yuntao Hu <[email protected]>
Signed-off-by: Yuntao Hu <[email protected]>
@VictorHuu VictorHuu changed the title fix: introduce Component Type Unknown in Cyclone DX for better compatibility with other formats fix: introduce Component Type Unknown in Cyclone DX for better conversion Apr 27, 2025
@VictorHuu VictorHuu changed the title fix: introduce Component Type Unknown in Cyclone DX for better conversion fix: [WIP]introduce Component Type Unknown in Cyclone DX for better conversion Apr 28, 2025
@VictorHuu VictorHuu marked this pull request as ready for review April 29, 2025 16:24
@VictorHuu VictorHuu marked this pull request as draft April 29, 2025 16:36
@VictorHuu
Copy link
Contributor Author

VictorHuu commented Apr 29, 2025

Troubleshoot: The so-called error line 193 and the test case TestSource_UnmarshalJSON/empty don't exist in the original file. And the CI/CD for same commit works well in my local PR.

FOLLOW UP: This is because some implicit test case for the empty string here: line. So I have to compromise a bit, but I think crashing for an SBOM with source type being empty is reasonable.

@VictorHuu VictorHuu marked this pull request as ready for review April 29, 2025 16:58
@VictorHuu VictorHuu closed this Apr 29, 2025
@VictorHuu VictorHuu reopened this Apr 29, 2025
Description string `json:"description,omitempty" yaml:"description,omitempty"`
PackageURL string `json:"purl" yaml:"purl"`
Licenses *cyclonedx.Licenses `json:"licenses,omitempty" xml:"licenses,omitempty"`
ExternalRef *[]cyclonedx.ExternalReference `json:"externalRef,omitempty" yaml:"externalRef,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

We won't be able to use CycloneDX data structures directly in public Syft APIs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll take it carefully. So these data structures needs to be tailored to store as much important infos as it can.

What about the following scheme?

type UnknownMetadata struct {
......
	Licenses    *[]LicenseChoice         `json:"licenses,omitempty" xml:"licenses,omitempty"`
	ExternalRef *[]ExternalReference     `json:"externalRef,omitempty" yaml:"externalRef,omitempty"`
	Authors     *[]OrganizationalContact `json:"authors,omitempty" xml:"authors>author,omitempty"`
}

type OrganizationalContact struct {
	Name string `json:"name,omitempty" xml:"name,omitempty"`
}

type ExternalReference struct {
	URL    string  `json:"url" xml:"url"`
	Hashes *[]Hash `json:"hashes,omitempty" xml:"hashes>hash,omitempty"`
	Type   string  `json:"type" xml:"type,attr"`
}

type Hash struct {
	Algorithm string `json:"alg" xml:"alg,attr"`
	Value     string `json:"content" xml:",chardata"`
}

type LicenseChoice struct {
	License    *License `json:"license,omitempty" xml:"-"`
}

type License struct {
	ID   string `json:"id,omitempty" xml:"id,omitempty"`
}

Copy link
Contributor

Choose a reason for hiding this comment

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

@VictorHuu @kzantow sorry to push this but could you take a look into this as it would help us with the syft convert issue a lot!

Copy link
Contributor

Choose a reason for hiding this comment

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

I just took some time and had a look and the initial concern that the cyclonedx types would have been used directly should be fixed. @kzantow could you take a look at this again? we currently use some workarounds to fix the linked issues and I would love to get rid of them 😅

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.

Syft convert from cdx.json -> syft.json -> cdx.json fails
3 participants