Fixing/Suppressing Hotspots doesn't improve Security Review Rating

  • SonarQube Enterprise Edition
  • Version 8.6.1 (build 40680)

We are working to resolve overall code security hotspots to improve our security review grade rating. The security review rating is based upon the percentage of hotspots reviewed (marked as fixed or safe).

One of the challenges we have encountered is that marking a security hotspot as fixed/safe on one branch doesn’t mark it fixed/safe on other branches. We end up having to indicate that in multiple places.

As a solution to that, if we review code and see that it is safe, we instead use the Suppresswarning directive along with squid:rule# to suppress it everywhere, for just that one rule. However, that has the effect of just taking it out of the count of the number of security hotspots and there is no increase in the count of the number reviewed. Thus, we have addressed over 50% of our issues in this way, but it has had no effect on our Security Review rating. We are still at “E” because Sonarqube doesn’t count these as reviewed.

Is there a different flow we should be using?
Is there a different way we should be using Supprewarning such that the change is counted as reviewed?

Thanks.

1 Like

I imagine you should be doing your marking of issues on the “main/master” branch, after the changes made to fix the issue have been merged to that branch and properly scanned.

@David_Karr the problem is that after adding the Suppreswarning directive in the code, the Hotspot is no longer reported. There’s nothing to mark as fixed. That’s the problem. It’s just gone. Say I had 10 Hotspots discovered. After suppressing one in the source, I now have only 9, all in the “To Review” state. Thus I’ve fixed one and I should be at 10% reviewed, but instead I’m still at 0% of 9 reviewed.

In contrast, for “Issues” such as Bugs, the equivalent method of using Suppresswarning shows the issue as existing but is reported as Closed (Removed). Hence, there is “credit” for the recognition of the problem and suppressing it. Why don’t Hotspots work the same way?

Hello,

First, thanks for the feedback, I appreciate to get insights about how Security Hotspots are used for real and to compare this with what we imagine.
I think we should not try to fix the @SuppressWarning impact on the “Security Review Rating” before we understand what could be the ideal workflow and what’s the sync problem between Branches if there is one.

The way we designed the workflow is this one:

  • the Security Hotspots review created in the past are reviewed in the branch that will be delivered in PROD, so the “main/develop” branch generally
  • the Security Hotspots introduced while you are developing a new feature / new code are reviewed as part of the PR review and then when you merge that PR into a branch, the status are synchronized.

Your problem today is when you review an old Security Hotspot in a Branch X, there is nothing asking you if you want to replicate your review decision in all branches under analysis in SonarQube.
I see a problem. It’s not possible to replicate automatically without notice in all the branches because the branches could have diverged a lot. In the case, the context of the app can be different and a Security Hotspot that is Safe in the Branch X, could be something to fix in the Branch Y.

Alex

@Alexandre_Gigleux

Thank you for the explanation. I will consider whether we can make use of that recommended flow.

Alternatively, if we know we are going to suppress an issue, if we mark it as safe or fixed before in Sonarqube on the main branch before we add the Suppresswarning and commit the code, will the state change of the hotspot persist even after the next activity is posted on that code change?

@Alexandre_Gigleux we have checked to see if marking a hotspot as safe or fixed before suppressing the warning will persists after suppressing it. Unfortunately it does not:
" The requested hotspots no longer exist. They have been closed because the code involved has been changed or removed."

@Alexandre_Gigleux this is what we are doing, i.e. fixing hotspots on main. Are you saying that if we mark a hotspot as fixed on the main branch with the UI, and a develop branch is created after the commit on the main branch, that SonarQube will not flag that hotspot on the develop branch?

HI, on my project we have issues with hotspots as well, which are preventing us from merging feature branch to develop
Even after marking new hotspot as safe on sonarqube ui for our feature branch it is still detected after pipeline step for “sonarqube scan” is re-run

So how should this work in regards to Quality Gates? Let’s say my QG requires an A rating for Security Review and my branch fails because of this. I go to Sonar, mark the hotspot as Safe in the PR branch, after which the QG passes and the branch is merged. Now the develop branch will have the same hotspot still unreviewed, do I have to mark it again as Safe? This is duplicate work.

FYI we use Sonarcloud.