Skip to content

feat: bom.vulnerabilities JSON normalization #548

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

xmasoracle
Copy link
Contributor

@xmasoracle xmasoracle commented Mar 16, 2023

Introduce normalization of vulnerabilities for JSON for CDX>=1.4 .
This is part of #164 .

⚠️ Note that:
* It only adds support for a subset of properties of Models.Vulnerability.
* I made some Models.Vulnerability.* comparable, to be able to copy the implementation of normalizeIterable, but I am not really confident in what I did, and was unsure how to test it.

@xmasoracle xmasoracle requested a review from a team as a code owner March 16, 2023 11:01
Copy link
Member

@jkowalleck jkowalleck left a comment

Choose a reason for hiding this comment

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

quick review.
marked with ❌ are wrong and need to be fixed.

@jkowalleck
Copy link
Member

jkowalleck commented Mar 16, 2023

Thank you for your work, @xmasoracle .

Your implementation appears promising, please continue.
Your test method is sufficient, just some cases are missing.

Please be informed: I will not merge this into master, unless

  • all properties are normalized as expected
  • normalization for XML and JSON is in place

I'll set this PR to "draft" until it is finished.

@jkowalleck jkowalleck changed the title feat: bom.vulnerabilities JSON normalization/serialization (#164) feat: bom.vulnerabilities JSON normalization Mar 16, 2023
@jkowalleck jkowalleck marked this pull request as draft March 16, 2023 17:44
@jkowalleck jkowalleck changed the title feat: bom.vulnerabilities JSON normalization [WIP] feat: bom.vulnerabilities JSON normalization Mar 16, 2023
@jkowalleck
Copy link
Member

@jkowalleck
Copy link
Member

implementation detail: as new rendering for BomRefs is added, they need to be discriminated.
add them to the existing collector:

#getAllBomRefs (bom: Bom): Iterable<BomRef> {
const bomRefs = new Set<BomRef>()
function iterComponents (cs: Iterable<Component>): void {
for (const { bomRef, components } of cs) {
bomRefs.add(bomRef)
iterComponents(components)
}
}
if (bom.metadata.component !== undefined) {
bomRefs.add(bom.metadata.component.bomRef)
iterComponents(bom.metadata.component.components)
}
iterComponents(bom.components)
return bomRefs.values()
}

@xmasoracle xmasoracle force-pushed the feat/vulnerabilities_json branch from 04b963d to 3b491b7 Compare March 20, 2023 09:22
@xmasoracle xmasoracle force-pushed the feat/vulnerabilities_json branch from 3b491b7 to ab6809f Compare March 20, 2023 09:31
@xmasoracle xmasoracle force-pushed the feat/vulnerabilities_json branch from 8a7f9cc to fad9573 Compare March 20, 2023 13:47
@xmasoracle xmasoracle force-pushed the feat/vulnerabilities_json branch from fad9573 to 3ca1aed Compare March 20, 2023 15:35
@xmasoracle
Copy link
Contributor Author

Hi @jkowalleck, I will not have time to work on the XML serialization part.
Could this be merged? I'm afraid it will linger here for months, and drift away from the main branch...

@jkowalleck
Copy link
Member

jkowalleck commented Apr 4, 2023

nope, will not be merged any soon.

It contains breaking changes, and I do not want to release a set of breaking changes that is not complete.
Leave it open, and I might consider it for the next major release, if it was complete until then.
Time is not an issue.

PS: this is open source. Others might take it from here and complete the missing part.
Or even try it out and improve it in other ways.

@jkowalleck
Copy link
Member

FYI: i will be working on #620 soon.

@jkowalleck
Copy link
Member

jkowalleck commented Apr 23, 2023

BOM validators are now in place.
and they are part of existing integration tests. see #652

@jkowalleck
Copy link
Member

jkowalleck commented Apr 26, 2023

JSON validation is now part of this library and all its test suites.

@xmasoracle please rebase your feature on latest main normalize-vulnerability branch and run the tests, to see if your feature passes the JSON schema. Your efforts are appreciated.

@jkowalleck jkowalleck changed the base branch from main to normalize-vulnerability May 8, 2023 10:11
@jkowalleck
Copy link
Member

adjusted PR target branch to be (normalize-vulnerability)[https://github.com/CycloneDX/cyclonedx-javascript-library/tree/normalize-vulnerability]

reason: multiple smaller PRs will come together to solve #164
see #722

@jkowalleck
Copy link
Member

jkowalleck commented May 9, 2023

re: #issuecomment-1495568466
FYI: i will work on the XML serialization soon.

@jkowalleck jkowalleck marked this pull request as ready for review May 9, 2023 10:26
@jkowalleck jkowalleck self-requested a review May 9, 2023 10:26
@jkowalleck jkowalleck changed the title [WIP] feat: bom.vulnerabilities JSON normalization feat: bom.vulnerabilities JSON normalization May 9, 2023
@jkowalleck jkowalleck merged commit 37e88ad into CycloneDX:normalize-vulnerability May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants