Uploaded image for project: 'Product Roadmaps'
  1. Product Roadmaps
  2. MMF-1868

Security Hotspots should live their life separated from Issues

    XMLWordPrintable

    Details

    • Type: MMF
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Labels:

      Description

      WHY

      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.

      WHAT

      Use Cases

      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.

      Solution

      New Space

      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.

      ***Wireframes: https://invis.io/RBUQQ09H4TN

      ***High-Fidelity mockup of the page:  https://invis.io/RBUQQ09H4TN#/395600622_SecurityHotspots_1-1–overview

       

      Filters status for different workflows

         

       

      Order of Security Hotspots

      Security Hotspots are presented following a specific order:

      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

      Status/Resolution/Messages

      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

      Security Hotspot "In Review" Status

      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")

      Security Hotspot to Vulnerability

      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.

      Security Hotspots vs Application

      TODO Matt: the UX should be adjusted to show to which Project a given Security Hotspot belongs to.

      Impact Global Issues Page

      Security Hotspots should be dropped from the Global Issues page because no review will happen at this level.

      Impact on Project Issues Page

      "Security Hotspot type" and "Security Hotspots status" should be dropped because Hotspots will be managed in their own space.

      Impact on Security Reports (EE)

      The "Security Reports" page should be adjusted and the column "In Review" should be dropped.

      Impact on Rules Page

      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.

      Impact on Measures

      Security Hotspots should not be counted in the Issues measure (same as of today).

      Review History tab

      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.

      Security Hotspots Details tabs

      If the content of any tab is not returned by the WS, the tab should not be displayed.

      Migration

      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.

      Permissions

      The "Browse" permission allows to:

      • change the assignee
      • comment

      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.

      Notifications

      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

      Purge 

      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).

      HOW

      Risk exposure values

      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.

      New WebServices 

      DB migrations

      • manual vulnerabilities must be dropped (see SONAR-12725)
      • issue status "IN_REVIEW" must be dropped (see SONAR-12722)

       

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              alexandre.gigleux Alexandre Gigleux
              Reporter:
              alexandre.gigleux Alexandre Gigleux
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: