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

Add an additional rules for security hotspot #252

Merged
merged 1 commit into from
May 24, 2020
Merged

Conversation

Reamer
Copy link
Member

@Reamer Reamer commented Apr 17, 2020

Fixes #249

@Reamer Reamer force-pushed the security_hotspot branch 3 times, most recently from 57caa09 to 49cccb2 Compare April 20, 2020 09:55
@sonarcloud
Copy link

sonarcloud bot commented Apr 20, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@Reamer
Copy link
Member Author

Reamer commented Apr 20, 2020

@alixwar
Could you please test this feature and check, whether it meets your requirements. You must build a plugin version on your own.

@Reamer Reamer self-assigned this Apr 20, 2020
@alixwar
Copy link

alixwar commented Apr 20, 2020

@Reamer I built the plugin locally and downloaded SonarQube 8.1 community edition but I was surprised to see that the Security Hotspots feature is not there. So it seems it is a commercial feature only?

To deploy a hand-hacked plugin into our licensed server environment is unfortunately not feasible.
But I can confirm that the violations were reported as violations (e.g. status quo) so I don't think there is any huge risk merging this and then I could quickly verify the release in our licensed server environment.

Perhaps someone from SonarSource could help us out here?

@Reamer
Copy link
Member Author

Reamer commented Apr 20, 2020

Hi @alixwar,
Security-Hotspots are also available in the community edition. I've done some tests with the SonarQube 8.2 community edition. Can you please test with SonarQube 8.2?

You must also enable the function either via the global plug-in configuration or the sonar scanner configuration.

@alixwar
Copy link

alixwar commented Apr 20, 2020

@Reamer I've now installed SonarQube 8.2 and here I can actually see the Security Hotspots menu.
However, unfortunately all issues are reported as violations directly and not hotspots.

In other words the Security Hotspots list is completely empty but the vulnerable dependencies are listed as vulnerabilities.

An example is rule OWASP:UsingComponentWithKnownVulnerability

Have I missed something?

Update: Damnit. I think I built the wrong branch... Will try again...

Update 2: Nope... Same issue. I deleted the project before and made sure to delete any caches. The violations from the plugin are reported. But not as hotspots:

There are no Security Hotspots to review.
Next time you analyse a piece of code that contains a potential security risk, it will show up here.

@Reamer
Copy link
Member Author

Reamer commented Apr 21, 2020

Hi @alixwar
You should see the following configuration option once you've built and installed the correct branch.
Security-Hotspot

As I wrote you can enable the function either via the global plug-in configuration or the sonar scanner configuration. Check out this part of the official documentation.

@alixwar
Copy link

alixwar commented Apr 21, 2020

@Reamer I misunderstood you and had missed this option in my last verification. Will check now. Is there a reason to not have this option checked by default you think?

@Reamer
Copy link
Member Author

Reamer commented Apr 21, 2020

I doesn't want release a new major version. A minor release step should not change the logic.

@alixwar
Copy link

alixwar commented Apr 21, 2020

@Reamer Unfortunately I end up in this state:

sechotspots

I can confirm that it seems that the violations are reported as hotspots because the correct number of hot spots are listed in the dashboard. But as you can see I can't load the hotspots UI.

I can't find anything in the different server logs. Do you have any suggestions?

I'm guessing it's a javascript issue. When I refresh the page I can see for a millisecond a UI with one of the violations to review... But the rendering is breaking for some reason.

@Reamer
Copy link
Member Author

Reamer commented Apr 21, 2020

My user interface looks good. I'm using Firefox. Which browser did you use? The user interface comes from SonarQube. This plugin does not affect the user interface.

Maybe you can test with another project. Take a look at our example folder.

@alixwar
Copy link

alixwar commented Apr 21, 2020

@Reamer I tried Chrome and also Edge. Will check with Firefox... I don't see how changing project would help. There is something wrong that needs to be fixed. If it helps I can find out exactly which rule violations are causing issues. Maybe it's a character or similar in the rule name that is the problem here

@alixwar
Copy link

alixwar commented Apr 21, 2020

Same problem with Firefox:

firefox1
firefox2
firefox3

@Reamer
Copy link
Member Author

Reamer commented Apr 21, 2020

If it helps I can find out exactly which rule violations are causing issues.

This would be very helpful.

@alixwar
Copy link

alixwar commented Apr 22, 2020

@Reamer OK, I deleted the project in my local SonarQube instance and changed the setting to not use security hotspots and then reported again:

no-sec-hotspots
no-sec-hotspots2

@Reamer
Copy link
Member Author

Reamer commented Apr 24, 2020

@pethers Can you reproduce this issue?

@Reamer Reamer requested a review from pethers April 27, 2020 15:00
@Reamer
Copy link
Member Author

Reamer commented May 6, 2020

ping @pethers
I would like to merge this change. I have no problems displaying the page with the security hotspots. Can you please test this feature with a small sample project?

@pethers
Copy link
Contributor

pethers commented May 21, 2020

@Reamer sorry for the delay, just noticed this now but will test it.

@pethers
Copy link
Contributor

pethers commented May 22, 2020

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.

Report findings as Security Hotspots (instead of violations)
3 participants