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

feat: Added support for severity in sarif report #1587

Merged
merged 4 commits into from
Feb 18, 2025

Conversation

mickem
Copy link
Contributor

@mickem mickem commented Feb 7, 2025

Adds severity to sarif reports.

This PR fixes #762

PS. I am not a proficient GO programmer so any and all feedback and ideas for improving this is appreciated.

The new fields are properties and severity in the example below, I added severity as a string to the bundle as that is how I interpret the GitHub docs A string representing a score that indicates the level of severity.
If no severity is available the field is left out.

          "rules": [
            {
              "id": "",
              "name": "",
              "shortDescription": {
                "text": ""
              },
              "fullDescription": {
                "text": "",
                "markdown": ""
              },
              "deprecatedIds": [
                "CVE-2022-24713",
                "RUSTSEC-2022-0013",
                "GHSA-m5pq-gvj9-9vr8"
              ],
              "help": {
                "text": "",
                "markdown": ""
              },
              "properties": {
                "security-severity": "7.5"
              }
            },

This should hopefully be according to GitHub specification here: https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#reportingdescriptor-object

@another-rex another-rex changed the title Added support for severity in sarif report feat: Added support for severity in sarif report Feb 11, 2025
Copy link
Collaborator

@another-rex another-rex left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! LGTM, just a minor nit.

@@ -2,10 +2,12 @@ package output

import (
"fmt"
"github.com/google/osv-scanner/v2/internal/utility/severity"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to go below the stdlib imports.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be fixed now.

@mickem
Copy link
Contributor Author

mickem commented Feb 12, 2025

Fixed some tests I had missed and since there were some conflicts with main I rebased and force pushed to keep the PR clean and green.

Verified

This commit was signed with the committer’s verified signature.
edgarrmondragon Edgar Ramírez Mondragón

Verified

This commit was signed with the committer’s verified signature.
edgarrmondragon Edgar Ramírez Mondragón

Verified

This commit was signed with the committer’s verified signature.
edgarrmondragon Edgar Ramírez Mondragón
@mickem mickem force-pushed the added_severity_to_sarif branch from c8c4460 to 4e37d8e Compare February 12, 2025 06:39
@mickem
Copy link
Contributor Author

mickem commented Feb 17, 2025

The golangci-lint I think fails as a side effect of the other jobs failing and GitHub pulls the plug on the run.

The other jobs fail as there is a new CVE since I updated the PR, so if I know when we will run the builds next I can make sure the PR is green before that time. As it seems to check live data there are new CVE:s popping up on and off.

Verified

This commit was signed with the committer’s verified signature.
edgarrmondragon Edgar Ramírez Mondragón
@codecov-commenter
Copy link

codecov-commenter commented Feb 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.79%. Comparing base (3bd1565) to head (a74c748).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1587      +/-   ##
==========================================
+ Coverage   68.77%   68.79%   +0.01%     
==========================================
  Files         199      199              
  Lines       18949    18961      +12     
==========================================
+ Hits        13032    13044      +12     
  Misses       5221     5221              
  Partials      696      696              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@another-rex another-rex merged commit de8293c into google:main Feb 18, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support recommended security-severity property in SARIF file export
3 participants