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

SBOM ingestion #81

Merged
merged 38 commits into from
Jul 16, 2024
Merged

SBOM ingestion #81

merged 38 commits into from
Jul 16, 2024

Conversation

barv-jfrog
Copy link
Contributor

@barv-jfrog barv-jfrog commented Jun 9, 2024

  • The pull request is targeting the dev branch.
  • The code has been validated to compile successfully by running go vet ./....
  • The code has been formatted properly using go fmt ./....
  • All static analysis checks passed.
  • All tests have passed. If this feature is not already covered by the tests, new tests have been added.
  • All changes are detailed at the description. if not already covered at JFrog Documentation, new documentation have been added.

Depends on:
jfrog/jfrog-client-go#961
jfrog/jfrog-cli-core#1193


Description: new command jf-sbom enrich, the point is to take an XML/JSON file which contains an SBOM of a package in CycloneDX format and enrich it with vulnerabilities which xray finds.

Usage:
jf sbom-enrich <file.xml/file.json>
Output:
the file.xml/file.json with an additional vulnerabilities section.

For example:
image

And the output is the file with an additional section (provided here is an image with the extra section)
image

This is only part of the output, the output is the file that was put as input but with an additional section.

Copy link

github-actions bot commented Jun 9, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@barv-jfrog barv-jfrog marked this pull request as draft June 9, 2024 11:04
@barv-jfrog
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@attiasas attiasas added the safe to test Approve running integration tests on a pull request label Jul 15, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Jul 15, 2024
Copy link
Contributor

@attiasas attiasas left a comment

Choose a reason for hiding this comment

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

Check out my comments. Make sure the Clinet PR is approved by ecosystem team

cli/docs/flags.go Outdated Show resolved Hide resolved
cli/docs/enrich/help.go Outdated Show resolved Hide resolved
commands/enrich/enrichgraph/enrichgraph.go Outdated Show resolved Hide resolved
tests/utils/test_utils.go Outdated Show resolved Hide resolved
Comment on lines 36 to 41
type EnrichJson struct {
Vulnerability []struct {
BomRef string `json:"bom-ref,"`
Id string `json:"id"`
} `json:"vulnerabilities"`
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
type EnrichJson struct {
Vulnerability []struct {
BomRef string `json:"bom-ref,"`
Id string `json:"id"`
} `json:"vulnerabilities"`
}
type EnrichBom struct {
Vulnerabilities []Vulnerability `json:"vulnerabilities xml:"Vulnerabilities"`
}

Use the struc already defined. you can combine and define xml and json tags at the same object...
Also move it to format

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know I can combine but XML is a bit different in structure than JSON, must separate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to format

Comment on lines 43 to 50
type Bom struct {
Vulnerabilities struct {
Vulnerability []struct {
BomRef string `xml:"bom-ref,attr"`
Id string `xml:"id"`
} `xml:"vulnerability"`
} `xml:"vulnerabilities"`
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
type Bom struct {
Vulnerabilities struct {
Vulnerability []struct {
BomRef string `xml:"bom-ref,attr"`
Id string `xml:"id"`
} `xml:"vulnerability"`
} `xml:"vulnerabilities"`
}

remove duplicated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its not duplicated

Comment on lines +12 to +20
func testVulns(t *testing.T, vulns []struct {
BomRef string
Id string
}) {
for _, vuln := range vulns {
assert.NotEqual(t, vuln.BomRef, nil)
assert.NotEqual(t, vuln.Id, nil)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func testVulns(t *testing.T, vulns []struct {
BomRef string
Id string
}) {
for _, vuln := range vulns {
assert.NotEqual(t, vuln.BomRef, nil)
assert.NotEqual(t, vuln.Id, nil)
}
}
func testVulns(t *testing.T, vulns Vulnerabilities) {
for _, vuln := range vulns {
assert.NotEqual(t, vuln.BomRef, nil)
assert.NotEqual(t, vuln.Id, nil)
}
}

Use the structs from the formats that you added. you dont need anonymus structs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want the testVulns function to receive both xml and json structs. I only care about specific fields in this so I used annonymous structs to make this function generic. whats wrong with that ?

utils/resultwriter.go Outdated Show resolved Hide resolved
@attiasas attiasas added the safe to test Approve running integration tests on a pull request label Jul 15, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Jul 15, 2024
@attiasas attiasas added the safe to test Approve running integration tests on a pull request label Jul 15, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Jul 15, 2024
Copy link

👍 Frogbot scanned this pull request and did not find any new security issues.


@attiasas attiasas merged commit a25d0d3 into jfrog:dev Jul 16, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature Automatically generated release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants