Skip to content

[WIP] feat: Add support for Component Identity Evidence #657

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

Conversation

wahl-sec
Copy link

@wahl-sec wahl-sec commented Aug 4, 2024

This pull request aims to implement the componentIdentityEvidence type from 1.5 and 1.6 as part of fulfilling #578

@wahl-sec wahl-sec requested a review from a team as a code owner August 4, 2024 13:49
@wahl-sec
Copy link
Author

wahl-sec commented Aug 4, 2024

This PR is a work in progress and does not currently pass the tests and does not propertly handle the 1.5 schema that only allows the componentIdentityEvidence to be an object and not an array of object as in 1.6. If anyone can help out with this please do as I have limited time (and knowledge of how the tests are setup 😄 )

@jkowalleck jkowalleck marked this pull request as draft August 7, 2024 14:32
@jkowalleck jkowalleck added enhancement New feature or request schema 1.6 labels Aug 7, 2024
@wahl-sec wahl-sec force-pushed the 578_support_component_evidence_identity branch from a30629c to 27d4470 Compare August 26, 2024 12:51
@wahl-sec
Copy link
Author

Identified some issues with some invalid tests, updated those and now it works for schema 1.6 with arrays. However, the only remaining issue is the object support for componentIdentityEvidence in schema 1.5 and 1.6.

@wahl-sec wahl-sec changed the base branch from main to 8.0.0-dev August 26, 2024 12:55
@wahl-sec wahl-sec changed the title [WIP] feat!: Add support for Componment Identity Evidence [WIP] feat!: Add support for Component Identity Evidence Aug 26, 2024
@jkowalleck
Copy link
Member

@wahl-sec is this work-in-progress, or ready for review?

does this change include any breaking changes?
If not, then the target branch is main instead of 8.0.0-dev, and then the title would be feat: ...instead offeat!: ...`.

@wahl-sec wahl-sec changed the base branch from 8.0.0-dev to main August 29, 2024 16:22
@wahl-sec wahl-sec changed the title [WIP] feat!: Add support for Component Identity Evidence [WIP] feat: Add support for Component Identity Evidence Aug 29, 2024
@wahl-sec
Copy link
Author

@jkowalleck No breaking changes, only added the optional identity field to the evidence section in components. But as I noted above this PR currently only implements the array option and not the object type that was supported in 1.5. Not sure how it would be implemented to support both in this case. Updated the title and branch, thanks!

@jloehel
Copy link

jloehel commented Apr 23, 2025

@wahl-sec @jkowalleck How can I help to get this merged?

I guess to support both a single identity object and an array, the setter needs to accept both an Iterable of ComponentIdentityEvidence and a single instance of ComponentIdentityEvidence?

    @identity.setter
    def identity(self, identity: Iterable[ComponentIdentityEvidence] | ComponentIdentityEvidence) -> None:

The getter returns always an Iterable and serialize the xml/json with a helper which differes between the schema versions. Like BomMetaData.tools is doing it.

@jkowalleck
Copy link
Member

jkowalleck commented Apr 23, 2025

this PR is outdated, and it has conflicts.
It requires lo a lot of refactorings, fixings, and most importantly, the implementation is not in accordance to the current architecture.

@jloehel, If I were you, I would start fresh and implement the needed Enums and class properties, then write fresh tests, and make them pass by properly decorating the properties according to the spec.

I agree, the setters need to be adjusted to accept a single element as well as an iterable; and the getter should always return an SortedSet or something.

I will not have the time to assist in the implementation for the next month; so if you want to give it a try, feel free to pull request a draft and ask the community for reviews. 👍

PS: since getters might change return type, the result might introduce breaking changes. this should not be an issue, just change what is needed, so that the results match the CycloneDX spec. Let's embrace the change.

@wahl-sec
Copy link
Author

Yes that sounds good, I dont currently have time to work on this so I'll close this one out.

@wahl-sec wahl-sec closed this Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request schema 1.6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants