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

Use new severity levels (high, medium, low) #895

Merged
merged 5 commits into from
Dec 20, 2023

Conversation

NIGCH
Copy link
Contributor

@NIGCH NIGCH commented Dec 19, 2023

A tentative fix for #870.

A change to DependencyCheckUtils.severityToScore() also appears to be needed to always use the CVSS scores in the specification to map a severity string to a CVSS score, rather than the configurable thresholds which determine how CVSS scores map to SonarQube severities. However, this will be addressed in a later pull request.

Copy link
Member

@Reamer Reamer left a comment

Choose a reason for hiding this comment

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

Thank you very much for the pull request. This is quite a complicated pull request. Thank you for splitting it up. I haven't looked at these lines of code for a long time.

I have added a few comments where we need to take a look.

btw. Table entry in "Plugin version compatibility" (Readme.md) is missing.

@Reamer
Copy link
Member

Reamer commented Dec 19, 2023

Perhaps I was too hasty with the request to split the pull request.

I think it is necessary that the constants are also required in this pull request.
https://github.com/dependency-check/dependency-check-sonar-plugin/pull/894/files#diff-ce8ce310fb93461c861183d57d10967a8bf3d7a2de59aa4053436ef5ef661817R38-R41

It seems that in the current code the score of CVSS and SonarQube has been mixed.

@Reamer
Copy link
Member

Reamer commented Dec 19, 2023

I think that makes sense, doesn't it?

@NIGCH
Copy link
Contributor Author

NIGCH commented Dec 20, 2023

I think that makes sense, doesn't it?

Yes, thanks, it looks good to me. Is the compatibility entry I've added correct?

Copy link
Member

@Reamer Reamer left a comment

Choose a reason for hiding this comment

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

Only minor changes required to make it easier for other users to update.

@Reamer
Copy link
Member

Reamer commented Dec 20, 2023

I think that makes sense, doesn't it?

Yes, thanks, it looks good to me. Is the compatibility entry I've added correct?

Looks good.

Copy link
Member

@Reamer Reamer left a comment

Choose a reason for hiding this comment

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

LGTM

@Reamer Reamer merged commit ada5d87 into dependency-check:master Dec 20, 2023
3 checks passed
@NIGCH NIGCH deleted the feature/use-new-severity-levels branch December 20, 2023 12:20
@NIGCH
Copy link
Contributor Author

NIGCH commented Dec 20, 2023

Thank you @Reamer - much appreciated.

@Reamer
Copy link
Member

Reamer commented Dec 20, 2023

Thank you very much for your work.
Is there any more code from the first pull request #894?

@NIGCH
Copy link
Contributor Author

NIGCH commented Dec 20, 2023

No, that's everything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants