Feature/added filter by severity option

[ghost]

What does this PR do?

• Adds an option to the scan, scan_policy_file and scan_multi_account features to filter results by severity: using -f or --filter-severity. This option can be used multiple times and its can be "critical", "high", "medium", "low" or "none". It is case insensitive.
• Adds severity field to json output for every risk type - this changed required some changes along the source code when getting the findings from the json results because the json output format changed. The risks fields have now the following format:

{
    "severity": "",
    "description": "",
    "findings": []
}

instead of only the findings list.
The severity for each risk type is now defined on the constants.py file.

• Adds a Dockerfile to build an image of cloudsplaining, making it available to run in a container.

What gif best describes this PR or how it makes you feel?

Completion checklist

• Additions and changes have unit tests
• The pull request has been appropriately labeled using the provided PR labels
• GitHub actions automation is passing (make test, make lint, make security-test, make test-js)
• If the UI contents or JavaScript files have been modified, generate a new example report:

# Generate the updated Javascript bundle
make build-js

# Generate the example report
make generate-report

[ghost]

Please add 'CRITICAL' and 'NONE' options to the choices list, to bring the possible severity settings in line with CVSS v3 (CVSS v3.1 Specification Document (first.org))

[ghost]

@melaniap-codsec This looks like a useful feature, thank you for adding it!

I have two requests based on my organization's use of Cloudsplaining that would make this feature more broadly useful:

1. Make the -f flag available on the other two scanning commands, scan_policy_file and scan_multi_account
2. Add "CRITICAL" and possibly "NONE" options to the list of choices for the flag, to support all the severity labels defined by the CVSS v3 standard (CVSS v3.1 Specification Document (first.org))

Our organization, for example, uses Cloudsplaining via its scan_multi_accounts command, and we consider Privilege Escalation findings to be Critical severity, so they align properly with our other vulnerability-reporting tools.

Thank you for your consideration, and for your contribution to the project. :)

[ghost]

@danjacobson-orbis i added what you requested, hope the changes fit your requirements. happy to contribute :)

[tiagopinto-codsec]

@kmcquade is there any chance this can PR can be merged into master? It is a cool functionality that gives value and from CI/CD we ideally would like to use the master branch.

[kmcquade]

@melaniap-codsec wow, what a great contribution! I love what you've done. The improvements with more actions is helpful, and the updated data in the example report is so much better than the stuff that I had there before. And @danjacobson-orbis thanks for your comments and review. I left some comments in a review.

I feel like it is important to set some context for my requests for changes.

First of all, I am no longer working in the IAM space, and I kind of check this repository occasionally out of curiosity, and because it was a really impactful and popular project. I don't consider myself an active maintainer of this anymore. I have my own startup now (www.nightvision.net) and all of my energy is currently focused on that. However - this PR looked really cool, and I couldn't help but look at it.

If I do make any changes, I have the responsibility that any changes that I or others make must be in the best interest of the security community. This package has been downloaded half a million times in the last 30 days, with over 6 million downloads. This includes several security vendors/CSPM/CNAPP tools, and it is built into IAC tools like Checkov which runs in CI/CD pipelines of many many customers. I also personally worked on the Checkov integration.

1. They do not introduce any new dependencies.
2. If they do make significant changes to methods that perform policy doc evaluation, I have to make sure that they do not break Checkov especially.

So while I would like to just accept requests immediately, I have a responsibility to not do this blindly - and right now, I don't really have capacity to test it out myself.

As such, I have a few requests before this can be merged:

1. Please remove the packages that would be introduced and modify the code so it does not rely on those packages. I noted those in my review comments.
2. Please try running Checkov's unit tests with this version of the package installed. I believe you can do this by modifying the dependencies in Checkov to point to your fork. If you can paste a screenshot showing that you tried this, with the result and the package modification, that would be sufficient.

After the comments are addressed and the above is done, I can merge the PR.

And thanks again - this looks amazing, great contribution.

[ghost]

@kmcquade i have removed the dependencies as requested.
Checkov's scan runs with no security issues as seen in the image below

Also, i ran Checkov's unit tests twice, first with their code unmodified, using cloudsplaining >=0.4.3 version and then with this version, pointing to my fork. As you can see in the images below, there are two tests that fail, but they fail using both versions, so i guess this is not a problem from my modifications to cloudsplaining. Is fixing these failed tests the goal, even if they fail without my changes?

Checkov's unit tests results with cloudsplaining version >=0.4.3:

Checkov's unit tests results with cloudsplaining pointing to my fork:

[kmcquade]

@melaniap-codsec thanks for removing the python dependencies, and for running the test with Checkov.

It looks like the Javascript dependencies were not reverted though. Can you revert package.json and package-lock.json to the previous state? Then we will be good to go, and I can merge.

Thanks again!

[kmcquade]

Oh, and it looks like the unit tests are failing for Python and for NodeJS. Please fix those too. Thanks!

[ghost]

@kmcquade i have removed the JavaScript dependencies and fixed the unit tests.
I ran again the checkov unit tests too, and again only those 2 failed, as you can see below, that are the ones that fail already without my changes

thank you :)

[kmcquade]

@melaniap-codsec looks like there are still some failures. The unit tests work, but there is a mypy (static type check) error.
https://github.com/salesforce/cloudsplaining/actions/runs/5163913858/jobs/9363810878

and the package-lock.json has not been reverted to the previous state yet.

Sorry, I thought this would be more straightforward but I hadn't seen the static type checks failing.

[ghost]

@kmcquade i fixed the mypy errors and reverted package-lock.json to its previous state. hope now all the checks will pass, let me now if there's anything else to fix.
thank you :)

[kmcquade]

@melaniap-codsec this is now part of the 0.6.0 release 🚀 thanks for the contribution!

I've also published a new version of Policy Sentry with the latest IAM definition updates.

[ghost]

@kmcquade thank you! 😄