Do not fail upon SARIF 'toolExecutionNotifications' of level 'error'
What does this MR do?
This MR fixes a Semgrep analyzer failure reported by a customer/prospect in this Slack thread: https://gitlab.slack.com/archives/CLA54H7PY/p1669087664596579
Our Semgrep analyzer already tests for fatal errors based on exit codes: https://gitlab.com/gitlab-org/security-products/analyzers/semgrep/-/blob/43ec59b7aaaf347b62927b7f925b6965fffe75ed/analyze.go#L41
In the code above, we determined that exit code 2 is not fatal, so we proceed with converting the report. That however immediately fails due to the presence of an toolExecutionNotifications
at level error
.
Quoting SARIF specification Appendix I. (Informative) Detecting incomplete result sets:
- If any
invocation
object (§3.20) intheRun.invocations
(§3.14.11) has a value offalse
for itsexecutionSuccessful
property (§3.20.14), the tool either failed to start, terminated with an exit code that denotes failure, or terminated with an unhandled exception or signal.. - If any
notification
object (§3.58) ininvocation.toolExecutionNotifications
(§3.20.21) ortoolConfigurationNotifications
(§3.20.22) has a value of "error" for its level property (§3.58.6), it is possible that the tool was unable to execute every analysis rule on every analysis target. Therefore, the results cannot be assumed to be complete.
This prospect/customer is interested in seeing partial results as only a single file within their project is causing the error: https://github.com/espressif/esp-idf/blob/release/v4.4/components/driver/gpio.c
The SARIF specification also state:
At a minimum, viewers SHOULD make users aware of tool notifications whose level property is "error".
This MR therefore logs such notifications instead of considering them as fatal.
Note that we currently do not test executionSuccessful
, but could do so. This MR removes it from the SARIF structure as it is presently unused.
Unit tests covering this scenario all relate to Semgrep. I do not know how this change could impact other analyzers.
What are the relevant issue numbers?
Does this MR meet the acceptance criteria?
-
Changelog entry added -
Documentation created/updated for GitLab EE, if necessary -
Documentation created/updated for this project, if necessary -
Documentation reviewed by technical writer or follow-up review issue created -
Tests added for this feature/bug -
Job definition updated, if necessary -
Ensure the report version matches the equivalent schema version -
Conforms to the code review guidelines -
Conforms to the Go guidelines -
Security reports checked/validated by reviewer