bandit icon indicating copy to clipboard operation
bandit copied to clipboard

Option to output violation information of skipped tests/lines

Open syl-ms opened this issue 4 years ago • 2 comments

Problem to Solve Without the complete violation information, we cannot know whether certain tests or lines are being skipped reasonably. This option allows us to make more informed judgements.

Proposed Solution

  1. Our team works with the SARIF output format. Therefore, it's desirable if Bandit could output to SARIF directly when used with the flag -f sarif.
  2. When Bandit is executed with the combination of flags -f sarif and --output-suppression, it should run all lines in a file against not only the tests to include specified via -t (command line) / tests (external command line argument & external configuration file), but also the tests specified to be skipped via -s (command line) / skips (external command line argument & external configuration file).
  3. In the SARIF output of Bandit, aside from the original content it normally contains, there should also be violations caused by "skipped" tests and lines, along with the kind and justification of a skipped test (a.k.a. suppression in SARIF terms). Here is an example: image
  4. Ideally every skipped test/line should have an accompanying explanation (justification), as my colleague André stated in #478. We are also willing to contribute to this.

Alternatives As said in #646, if the SARIF output is planned to be included in the near future, our team would be happy to contribute. If not, we are okay with modifying bandit-sarif-formatter together as well.

Please let us know what you think. Thank you.

syl-ms avatar Sep 16 '21 09:09 syl-ms

Hi @ericwb , we are good to contribute to this feature😊. And we could provide more details about our goals and designs if needed. Please let us know if there are any concerns or feedbacks.

Yiwei-Ding avatar Nov 10 '21 03:11 Yiwei-Ding

Hi @ericwb. Please find below our design document for the change. We will carry on with this design if no objection. Thank you.

Problem

Bandit

Bandit is a tool designed to find common security issues in Python code. To do this, Bandit processes each .py file, builds an AST from it, and runs appropriate plugins against the AST nodes. Once Bandit has finished scanning all the files it generates a report which can be exported into the following formats: csv, custom, html, json, screen, txt, xml, and yaml. Users can choose which tests to run or skip on their project, and the result of those skipped tests is not included in the report. However, such result could contain valuable information, especially when certain tests would cause violations. Therefore, we would like to include result of these skipped tests in the output, including whether there were violations in the project, how they were skipped (kind of suppression), and why they were skipped (justification of suppression), which could help generate corresponding signals for auditing purposes.

Suppression in Bandit

Running Bandit tests to output Sarif is done via command: bandit {path-to-file} or {-r path-to-directory} -f sarif -o {path-to-output-file}. Suppression of a test is referred to as an exclusion in Bandit, and it can be performed in five ways:

  1. inSource: Mark lines that should be ignored using # nosec. Example:
self.process = subprocess.Popen('/bin/echo', shell=True)  # nosec

In this case, it doesn’t disable any specific test, but rather runs all but the excluded tests, if any, on all lines of a file and doesn’t write the results of marked lines to the output file.

  • Such suppressions can be overridden by executing with the --ignore-nosec flag, which runs Bandit without skipping lines marked with # nosec. Example:
bandit {path-to-file} -f sarif -o {path-to-output-file} –ignore-nosec
  1. Baseline: Provide a baseline to ignore violations. Example:
bandit {path-to-file} {path-to-file} -b {baseline-file} -f sarif -o {path-to-output-file}
  1. Command line to skip or include rules
  • Execute with a -s or –skip flag to skip rules. Example:
bandit {path-to-file} -f sarif -o {path-to-output-file} -s TestID1,TestID2
  • Execute with a -t or -tests flag to add rules. Example:
bandit {path-to-file} -f sarif -o {path-to-output-file} -t TestID1,TestID2
  1. External command line argument: Disable rules in an optional .bandit configuration file, which specifies command line arguments that should be supplied for the project. Example content of the file:
[bandit]
skips: TestID1,TestID2

Usage:

bandit --ini {path-to-.bandit-file}
  1. External configuration file: Disable rules in an optional YAML config file, which is used for selecting plugins and overriding defaults. Example content of the file:
skips: [‘TestID1’, ‘TestID2’]

Usage:

bandit -c {config-file}

Cases 3, 4 and 5 remove the specified tests to skip from those against which Bandit runs a target file. No result is produced for these excluded tests and therefore nothing gets written to the output file. The image below shows how Bandit works currently: image

What We Want

We propose to add a new option --output-suppression and add suppressions in the output of Bandit when using the option. For Case 1, we will not ignore violations marked with # nosec. For Case 2, we will not filter out violations in baseline. For Case 3, 4 and 5, we would like to export to the output file the results of running all lines in a file against not only the tests to include specified via -t (Case 3. Command line) / tests (Case 4. External command line argument & Case 5. External configuration file), but also the tests specified to be skipped via -s (Case 3) / skips (Cases 4 and 5).

Suppression category What we want
1 Inline suppressions with # nosec Not ignore violations marked with # nosec.
2 Baseline suppressions Not filter out violations in baseline.
3 Command line to skip or add rules Run all rules and tag violations from skipped rules as suppressed.
4 External command line argument Same as above
5 External configuration file Same as above

image image The images above show an example of the desired output and its format. Each piece of suppression information contains two parts:

  • kind: It specifies whether a test is disabled inSource (case 1) or in an external way(cases 2, 3, 4 and 5).
  • justification: It describes the reason for which a test is disabled if given and set to an empty string "" otherwise.

There is not a justification terminology in Bandit. We could add it in the following ways:

  1. inSource: Bandit only catches #nosec or # nosec at the beginning of a comment. We could put the justification after two adjacent dashes, like #nosec -- justification.
  2. Baseline: Put the justification in baseline like BinSkim does.
  3. Command line to skip or add rules & external command line argument & external configuration file: There is no good place to put justification yet so we will need to revisit this.

Goals

  1. Add a new option --output-suppression for Bandit to add suppression info (including inline suppressions, baseline suppressions, and skip/add rules externally) in its output.
  2. Keep the current behavior of Bandit for all other cases.

Design

image

syl-ms avatar Dec 07 '21 00:12 syl-ms