search

jfrog / jfrog-client-go

All go clients for JFrog products
Apache License 2.0
265 stars 141 forks source link

Unable to marshal Xray Scan Report #468

Open romelusw opened 3 years ago

romelusw commented 3 years ago

Describe the bug CVSV2 and CVSV3 Max score properties appear to be float types in the API response of https://www.jfrog.com/confluence/display/JFROG/Xray+REST+API#XrayRESTAPI-GetVulnerabilitiesReportContent. My initial PR for the new reports endpoint attempted to correct that data type in the scan.go resource:https://github.com/jfrog/jfrog-client-go/pull/428#discussion_r716095933, however, @eyalbe4 suggested reverting the fix and is now causing marshaling issues when using the go library.

To Reproduce

Expected behavior API response should be translatable to the models defined in https://github.com/jfrog/jfrog-client-go/tree/master/xray/services.

Versions

Additional context Sample vulnerability CVE payload: "cves": [ { "cve": "CVE-2021-21285", "cvss_v2_score": 4.3, "cvss_v2_vector": "CVSS:2.0/AV:N/AC:M/Au:N/C:N/I:N/A:P", "cvss_v3_score": 6.5, "cvss_v3_vector": "CVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:N/I:N/A:H" } ],

eyalbe4 commented 3 years ago

Thanks for pointing this out @romelusw. Feel free to issue a follow-up PR to correct this. We'll take care of making this change compatible with usages of struct you'll be modifying outside of jfrog-client-go.

romelusw commented 3 years ago

Thanks for pointing this out @romelusw. Feel free to issue a follow-up PR to correct this. We'll take care of making this change compatible with usages of struct you'll be modifying outside of jfrog-client-go.

@eyalbe4 please see: https://github.com/jfrog/jfrog-client-go/pull/470

eyalbe4 commented 3 years ago

The PR is merged @romelusw. Thanks!

romelusw commented 3 years ago

Unit test failure fix: https://github.com/jfrog/jfrog-client-go/pull/476

asafgabai commented 2 years ago

Hi @romelusw, In Xray's documentation (that you referred to above) it seems like the CVSS fields ("cvss_v2" and "cvss_v3") are strings, and the CVSS scores are at the beginning of these strings. I found another API there ("/api/v1/reports/violations") where the CVSS score field is a float, but this API is not used in jfrog-client-go.

romelusw commented 2 years ago

Hi @asafgabai the documentation is also incorrect as those properties and their values do not match with what is returned by the API. Please see my sample payload. You can cross verify with an X-ray instance matching the versions I described in the issue.

asafgabai commented 2 years ago

@romelusw Thanks for your answer. I took a look at the PR you opened. Unfortunately, it causes compilation errors in jfrog-cli-core, so it can't be merged right now. I'll think of a solution for this and I'll update here on the progress.

asafgabai commented 2 years ago

Hi @romelusw, I opened a PR (#497) for fixing this issue. I'll update here when it's released.

asafgabai commented 2 years ago

Hi @romelusw, the PR was merged and released in v1.7.1 We'd appreciate your feedback on it.

romelusw commented 2 years ago

@asafgabai it looks good, can you please update https://github.com/jfrog/jfrog-client-go/pull/476 to account for the new ReportCve type you introduced

asafgabai commented 2 years ago

@romelusw I'm glad to hear that. It looks like your PR (#476) doesn't need an update. The Cve struct you changed there was replaced by a new struct in my PR so there's no need to change it anymore. I guess you can close your PR.