Security Hotspots are not Issues to fix immediately: they are highlighting security-sensitive piece of code that should be carefully reviewed to determine if a code change is required. Security Hotspots have been made to help developers make their code safer and raise the overall security awareness.
In order to avoid developers thinking that a Security Hotspot is a problem to fix, we decided to use a blue background, a dedicated icon and that's it. Nothing else distinguish a Security Hotspot from an Issue (Bug, Vulnerability or Code Smell). The UX is not different (same "Comment", same drop-down menu to change the status, ...) while we know what we expect a developer to do while looking at a Security Hotspot: start a review process following a well defined workflow help by rule's description.
As a developer coming from the SQ Project Home Page, selecting the overall Security Hotspot metric, I would like to see all the Security Hotspots sorted by priority (aka: Review priority) because I don't want to loose time to think about what to tackle first (first ones to review are on top of the screen). The same should happen when selecting the "New Security Hotspots" metric or when coming from a PR analysis.
A developer reviewing a Security Hotspot is expected to follow this process:
- understand why this Security Hotspot is raised
- read the "Ask Yourself Whether" questions to determine if his code is used in a security-sensitive context
- read the "Recommended Secure Coding Practices" and "See" section of the rule's description to learn things from SQ in case he needs to fix his code to make it safe
- fix the code if required to mitigate the potential vulnerability and close the Security Hotspot as "Fixed"
- add comments to share his findings about this particular Security Hotspot and close the Security Hotspot as "Safe"
Everything can be done as of now with the current UI but the process is not guided. The goal of this MMF is to make things much more easy to understand.
As a developer, I have a dedicated space in SQ, to review Security Hotspots raised during the New Code period but also old ones.
Management of Security Hotspots is done using a dedicated workflow, they are not Issues, so they are handled differently in SQ.
***High-Fidelity mockup of the page: https://invis.io/RBUQQ09H4TN#/395600622_SecurityHotspots_1-1–overview
Filters status for different workflows
Security Hotspots are presented following a specific order:
- by SonarSource Category sorted by Likelihood of Vulnerability (see https://docs.google.com/spreadsheets/d/16EKZdULUrH-iOGfUFPBWOe0nZbtjcsGCVH_bzDtyhEc/edit#gid=0 to get the "Likelihood of Vulnerability" associated to each SonarSource Category"
- by number of Security Hotspots for each rule belonging to the category
In each SonarSource Category, Security Hotspots are grouped by Rules, then for each rule, the newer Security Hotspots are showed first.
SonarSource Category (from High to Low, then number of Security Hotspot in Category) > Rules > Security Hotspots from New to Old
|Reviewed As||Messages||SQ Status||SQ Resolution|
|Fixed||The code has been modified to match recommended secured coding practices.||Reviewed||Fixed|
|Safe||The code is not at risk and doesn't need to be modified.||Reviewed||Safe|
|Needs additional review||Someone else needs to review this Security Hotspot.||To Review||Unresolved|
The "In Review" status shall be dropped because this feature is not used at SonarSource nor on SonarCloud.io (665,596 security hotspots opened vs 19 "in review")
The possibility to transform a Security Hotspot to a Vulnerability is dropped without any deprecation period because this is a useless step. Developers are expected to review and mitigate directly the problem if any. Transforming into a Vulnerability a Security Hotspot is bringing no additional value. It was done in SQ 7.9 LTS to force the Quality Gate to be FAILED but it's possible to enforce that using a criteria on Security Hotspots.
TODO Matt: the UX should be adjusted to show to which Project a given Security Hotspot belongs to.
Security Hotspots should be dropped from the Global Issues page because no review will happen at this level.
"Security Hotspot type" and "Security Hotspots status" should be dropped because Hotspots will be managed in their own space.
The "Security Reports" page should be adjusted and the column "In Review" should be dropped.
The Severity associated with a Security Hotspot rule was only there to be the default severity of a Manual Vulnerability created from a Security Hotspot. Because there is no longer a way to create Manual Vulnerabilities, the Security Hotspot's Severity should be hidden on the Rule page.
Security Hotspots should not be counted in the Issues measure (same as of today).
The Review History tab should show the comments added by developer and the status changes.
For example if a developer is marking a Security Hotspot as "Safe" without any comment, this status change should appear in the "Review History" tab with the date and author of the status change.
When a comment is added at the same time status is changed, it should displayed in the list as if they were a single item.
If the content of any tab is not returned by the WS, the tab should not be displayed.
Because the "In Review" status is dropped, existing Security Hotspots being in that status will be migrated to "To Review".
Existing Manual Vulnerability should be migrated to Security Hotspots with the status "To Review". A comment should be added "Migrated from Manual Vulnerability". It is accepted that a FAILED Quality Gate because of this Manual Vulnerability will be back to SUCCESS status once the project will be re-analyzed.
The "Browse" permission allows to:
- change the assignee
The permission "Administer Security Hotspots" is required to be able to change the status of a Security Hotspot. By default, "Administer Security Hotspots" is part of the default Permission Template.
The description should be adjusted to:
"Resolved a Security Hotspot as reviewed (fixed or safe), reset it as to review (users also need Browse permission)."
When a user doesn't have the permission "Administer Security Hotspots", the options to change the status should be visible but disable and greyed out.
The same notifications as today should be sent. The notifications related to "Manual Vulnerabilities" should be removed because the concept is dropped.
The only expected changes in the notification email are the title + link.
In the UI, the titles related to notifications in the Account > Notifications menu should be changed as:
- "Changes in issues assigned to me" => "Changes in issues/hotspots assigned to me"
Out of scope:
- there is no notification sent today for "New hotspots" or "My new hotspots" and that won't change with this MMF.
- there is no notification sent today for hotspots close/removed by the analyser when the hostposts no longer exist: that won't change with this MMF and we don't want to cover this use case - there is no way to see in the UI such hotspots
Security Hotpots are not purged today when they are "resolved". This should not change and they should never be purged to keep track of the decision taken in the past about them (comment + status).
The same way mapping from CWE to SonarQube security categories are hardcoded into SonarQube's code (see SecurityStandardHelper.java), the Risk exposure for each security category will be hardcoded into SonarQube's code.
- api/hotspots/search (see
- api/hotspots/show (see
- api/hotspots/change_status (see
- api/hotspots/assign (see
- api/hotspots/add_comment (see
- manual vulnerabilities must be dropped (see
- issue status "IN_REVIEW" must be dropped (see