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

Improve CodeScanning top 10 report to use better severities #79

Merged
merged 8 commits into from
Jan 20, 2025

Conversation

felickz
Copy link
Contributor

@felickz felickz commented Jan 16, 2025

This pull request includes changes to improve the handling and comparison of alert severities in the CodeScanning feature. The most important changes include modifying the severity level handling in the summaryTop10 method and refactoring the severity comparison logic in AlertMetrics.

Enhancements to CodeScanning class:

  • Added Weakness to the attributes list in the CodeScanning class to include CWE information in the output.
  • Implemented the cweFromTags method to extract CWE information from alert tags, improving the clarity and detail of the alert summaries.
  • Updated the summaryTop10 method to include CWE information and improve the formatting of file paths in the output.

Improvements to AlertMetrics utility:

  • Added the getAlertSeverity function to centralize and simplify the logic for determining the severity of different types of alerts.
  • Refactored the compareAlertSeverity function to use the new getAlertSeverity function, improving code readability and maintainability.

Before:

  • prioritizing note/warning severity items
    image

After:

  • uses security severity and prioritizes based on highest severity order
  • Adds CWE information
  • Adds start line to the location

image

@theztefan theztefan merged commit e1771fd into theztefan:main Jan 20, 2025
1 check passed
@theztefan
Copy link
Owner

Thanks @felickz!

The resulting severity values make a lot more sense as they are the generally accepted severity levels. The refactoring has been spot on as well... the code could still use a lot more love but we'll get there 😅

This is now Release v1.2.3

@felickz
Copy link
Contributor Author

felickz commented Jan 20, 2025

Thanks @felickz!

The resulting severity values make a lot more sense as they are the generally accepted severity levels. The refactoring has been spot on as well... the code could still use a lot more love but we'll get there 😅

This is now Release v1.2.3

Thanks for maintaining this project!

Having to choose a severity (ex: is medium > error) is a side effect of Code Scanning favoring SARIF from disparate sources over enforcing a strict standard ( favors passing CVSS score in the SARIF security.severity to get security levels crit/high/med/low). I put the severities in the most sensible order - which was just standing on your shoulders to get it done.

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.

2 participants