-
Notifications
You must be signed in to change notification settings - Fork 41
feat(materials): Support Gitlab security report materials #1339
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
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
migmartri
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
| @@ -0,0 +1,68 @@ | |||
| // | |||
| // Copyright 2023 The Chainloop Authors. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2024 :)
| } | ||
|
|
||
| var glReport report.Report | ||
| if err = json.Unmarshal(data, &glReport); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in a follow up patch we could validate the document against their json schema similarly to what we have done for cyclonedx? Do you mind creating an issue for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided doing it this way because they don't have a "base" schema, so we'd need to take one and remove the specifics from all the variants. Those differences are well located, though, we can do it anyways.
| return fmt.Errorf("error unmarshalling report file: %w", err) | ||
| } | ||
|
|
||
| if glReport.Scan.Type == "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you might want to also make sure that Scan is not nil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In his case Scan is not a pointer, so always has a value.
|
|
||
| var glReport report.Report | ||
| if err = json.Unmarshal(data, &glReport); err != nil { | ||
| return fmt.Errorf("error unmarshalling report file: %w", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what error do you get if you try to pass a random file as --kind GITLAB_SECURITY_REPORT` ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, nothing. json.Unmarshal by default ignores all unknown fields. We'd need to create a custom decoder to fail on ignored fields. I'll try that before merging.
javirln
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once validating against the json schemas we might need to rethink if the golang library is still needed.
Awesome Jose thanks 😬
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
This PR adds support for Gitlab security reports.
The implementation uses the Gitlab report Go bindings, so that all schemas are supported:
Closes #1335