fedora-infra / noggin

Self-service user portal for open-source communities to use over FreeIPA.
MIT License
111 stars 59 forks source link

As a developer I want a secure coding tool to analyse my code so that it adheres to best practices #101

Closed StephenCoady closed 4 years ago

StephenCoady commented 4 years ago

https://www.sonarqube.org/ is a code analyser which can identify bugs in code and also highlight best practices from a secure coding point of view. I think it would be good to have this tool as part of this project, especially considering it is an identity management solution.

In my opinion this user story could be broken into multiple parts.

1) Analise securitas using all 3 tools mentioned in this issue 2) Provide manual feedback to the team from this first report 3) Fix any and all issues the tools identify 4) Investigate the 3 tools and where/how we could host it or have it hosted 5) Investigate how these services could be integrated into our CI pipeline

I think 4 and 5 are stretch goals and will probably require input from someone with more operations knowledge than myself.

AC

  1. We have generated enough information to make an informed decision about which of these tools we will use.
  2. We have information about how the tool(s) we select could become integrated into our workflow.
  3. Any issues identified by these tools are fixed.

DoD

This issue can be considered done when this repo has had any and all security issues fixed and at least one of the tools investigated is part of the development workflow.

relrod commented 4 years ago

There's also Bandit that we should use. When I turned over the codebase to the fedora-infra org, it passed Bandit on the highest settings. Not sure if that's still the case, haven't checked recently.

abompard commented 4 years ago

Bandit is already checked in the tox.ini file.

I've never tried Sonarqube, but I have also heard of LGTM which I heard gives good results. I'm not against having as many analyzers as is relevant, given the sensitivity of the project. Thanks for opening this ticket Stephen.

StephenCoady commented 4 years ago

I think we should change the steps involved and add a spike to see which tool to use. I have no preference.

Maybe run all 3 and see which provides the most detailed/useful results. I think it would be good to evaluate.

relrod commented 4 years ago

I'm fine with using any and all static analyzers known to be good at tracking down security issues. I don't know if it needs to be a spike, imo just add them and if we run into issues with one of them we can disable it.

james02135 commented 4 years ago

I'm going to spend the rest of the week running Sonarqube, Bandit, and LGTM against Securitas and will have more info on Friday.

sfinn85 commented 4 years ago

Sounds good @james02135

sfinn85 commented 4 years ago

Sprint planning 3 meeting: @StephenCoady to add AC & DOD

sfinn85 commented 4 years ago

Daily stand up update: @james02135 working on this with @StephenCoady today regarding the report and will possibly add a report as a comment on this user story or just comment with progress.

Thanks @james02135

james02135 commented 4 years ago

TL;DR= Bandit is already installed and working - no need to change, LGTM does the same as Bandit just online, SonarQube(sonarcloud) does a wide range of checks, lets think about adding it

SONARQUBE

OVERVIEW

APPLICATION

BANDIT

OVERVIEW

APPLICATION

LGTM

OVERVIEW

APPLICATION

INTEGRATION

james02135 commented 4 years ago

After researching the 3 tools, it makes the most sense to keep Bandit as it is already integrated into the project and can be run when necessary. Also, add sonarqube to the CI checks via a webhook and use their free hosting site, sonarcloud.io

sfinn85 commented 4 years ago

Est S

sfinn85 commented 4 years ago

Waiting for Aurelien, to set up as he has permissions

abompard commented 4 years ago

I can do that in the next days when needed, @james02135 just send me the webhook urls when you have them.

james02135 commented 4 years ago

https://github.com/apps/sonarcloud @abompard that will have to be configured for "noggin", or "securitas", or whatever it will be named in the end.