From 0976211cd441ce7d6cfbb266185949b0317ab484 Mon Sep 17 00:00:00 2001 From: rhythmrx9 <33546975+rhythmrx9@users.noreply.github.com> Date: Wed, 23 Feb 2022 06:37:21 +0530 Subject: [PATCH] ci: run bandit on test code (fixes #1528) (#1579) * ci: skip assert on tests * fix: issue B108 in test_scanner.py * fix: issue B310 in utils.py and test_json.py * fix: issue B307 in test_input_engine.py and test_merge.py * doc: update bandit in contributing.md --- CONTRIBUTING.md | 6 +++--- bandit.conf | 8 +++----- test/test_input_engine.py | 3 ++- test/test_merge.py | 3 ++- test/test_scanner.py | 5 ++--- 5 files changed, 12 insertions(+), 13 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fc9b22d89b..98bbaba743 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -211,12 +211,12 @@ specify a whole folder using ```./``` ### Running bandit by itself -We have a configuration file for bandit called `bandit.conf` that you should use. This disables a few of the checkers and disables scanning of the test directory. +We have a configuration file for bandit called `bandit.conf` that you should use. This disables a few of the checkers. To run it on all the code we scan, use the following: ```bash -bandit -c bandit.conf -r cve_bin_tool/ +bandit -c bandit.conf -r cve_bin_tool/ test/ ``` You can also run it on individual files: @@ -225,7 +225,7 @@ You can also run it on individual files: bandit -c bandit.conf filename.py ``` -If you run it without the config file, it will run a few extra checkers and will run on test code, so you'll get additional warnings. +If you run it without the config file, it will run a few extra checkers, so you'll get additional warnings. Bandit helps you target manual code review, but bandit issues aren't always things that need to be fixed, just reviewed. If you have a bandit finding that doesn't actually need a fix, you can mark it as reviewed using a `# nosec` comment. If possible, include details as to why the bandit results are ok for future reviewers. For example, we have comments like `#nosec uses static https url above` in cases where bandit prompted us to review the variable being passed to urlopen(). diff --git a/bandit.conf b/bandit.conf index 959c40db4c..b34fd6bb0e 100644 --- a/bandit.conf +++ b/bandit.conf @@ -92,11 +92,9 @@ skips: ['B603', 'B607', 'B404', "B608"] # Explantion: cve-bin-tool is at heart a shell script that calls other processes. # Switching to pure python has significant performance impacts. -exclude_dirs: - - "test/" - - "/test/" - - "./test/" - - "./build/lib/test/" +# skips assert rule on tests +assert_used: + skips: ['*/test_*.py'] ### (optional) plugin settings - some test plugins require configuration data ### that may be given here, per-plugin. All bandit test plugins have a built in diff --git a/test/test_input_engine.py b/test/test_input_engine.py index 0a80484483..2b70e9490c 100644 --- a/test/test_input_engine.py +++ b/test/test_input_engine.py @@ -3,6 +3,7 @@ import os import re +from ast import literal_eval import pytest @@ -109,7 +110,7 @@ def test_missing_fields(self, filepath, missing_fields): match = self.MISSING_FIELD_REGEX.search(exc.value.args[0]) raised_fields = match.group(1) - assert missing_fields - eval(raised_fields) == set() + assert missing_fields - literal_eval(raised_fields) == set() @pytest.mark.parametrize( "filepath, parsed_data", diff --git a/test/test_merge.py b/test/test_merge.py index 896eba919d..b6e499879c 100644 --- a/test/test_merge.py +++ b/test/test_merge.py @@ -3,6 +3,7 @@ import os import re +from ast import literal_eval import pytest @@ -80,7 +81,7 @@ def test_missing_fields(self, filepaths, missing_fields): match = self.MISSING_FIELD_REGEX.search(exc.value.args[0]) raised_fields = match.group(1) - assert missing_fields - eval(raised_fields) == set() + assert missing_fields - literal_eval(raised_fields) == set() @pytest.mark.parametrize( "filepaths, merged_data", diff --git a/test/test_scanner.py b/test/test_scanner.py index 51422b885a..ddf0a64082 100644 --- a/test/test_scanner.py +++ b/test/test_scanner.py @@ -284,9 +284,8 @@ def test_cannot_open_file(self, caplog): assert str.find("Invalid file", caplog.text) def test_clean_file_path(self): - filepath = ( - "/tmp/cve-bin-tool/dhtei34fd/file_name.extracted/usr/bin/vulnerable_file" - ) + filepath = "/tmp/cve-bin-tool/dhtei34fd/file_name.extracted/usr/bin/vulnerable_file" # nosec + # temp path is hardcoded for testing, not for usage expected_path = "/usr/bin/vulnerable_file" cleaned_path = self.scanner.clean_file_path(filepath)