From b72c0ddd49c5b4b6d212ac2804a890e575b0fb0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Mare=C5=A1?= Date: Wed, 13 Jan 2021 12:31:41 +0100 Subject: [PATCH 1/5] Fix confusing function name Renamse `_log_option_source` to `_decide_option_source`. The old name suggests the function only logs something but the function decides which value will be used. --- bandit/cli/main.py | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/bandit/cli/main.py b/bandit/cli/main.py index 6752d9fda..b6fb7e418 100644 --- a/bandit/cli/main.py +++ b/bandit/cli/main.py @@ -80,8 +80,8 @@ def _init_extensions(): return ext_loader.MANAGER -def _log_option_source(arg_val, ini_val, option_name): - """It's useful to show the source of each option.""" +def _decide_option_source(arg_val, ini_val, option_name): + """Decide witch option source should be used. And log source to log.""" if arg_val: LOG.info("Using command line arg for %s", option_name) return arg_val @@ -312,17 +312,17 @@ def main(): ini_options = _get_options_from_ini(args.ini_path, args.targets) if ini_options: # prefer command line, then ini file - args.excluded_paths = _log_option_source( + args.excluded_paths = _decide_option_source( args.excluded_paths, ini_options.get('exclude'), 'excluded paths') - args.skips = _log_option_source( + args.skips = _decide_option_source( args.skips, ini_options.get('skips'), 'skipped tests') - args.tests = _log_option_source( + args.tests = _decide_option_source( args.tests, ini_options.get('tests'), 'selected tests') @@ -331,79 +331,79 @@ def main(): if ini_targets: ini_targets = ini_targets.split(',') - args.targets = _log_option_source( + args.targets = _decide_option_source( args.targets, ini_targets, 'selected targets') # TODO(tmcpeak): any other useful options to pass from .bandit? - args.recursive = _log_option_source( + args.recursive = _decide_option_source( args.recursive, ini_options.get('recursive'), 'recursive scan') - args.agg_type = _log_option_source( + args.agg_type = _decide_option_source( args.agg_type, ini_options.get('aggregate'), 'aggregate output type') - args.context_lines = _log_option_source( + args.context_lines = _decide_option_source( args.context_lines, ini_options.get('number'), 'max code lines output for issue') - args.profile = _log_option_source( + args.profile = _decide_option_source( args.profile, ini_options.get('profile'), 'profile') - args.severity = _log_option_source( + args.severity = _decide_option_source( args.severity, ini_options.get('level'), 'severity level') - args.confidence = _log_option_source( + args.confidence = _decide_option_source( args.confidence, ini_options.get('confidence'), 'confidence level') - args.output_format = _log_option_source( + args.output_format = _decide_option_source( args.output_format, ini_options.get('format'), 'output format') - args.msg_template = _log_option_source( + args.msg_template = _decide_option_source( args.msg_template, ini_options.get('msg-template'), 'output message template') - args.output_file = _log_option_source( + args.output_file = _decide_option_source( args.output_file, ini_options.get('output'), 'output file') - args.verbose = _log_option_source( + args.verbose = _decide_option_source( args.verbose, ini_options.get('verbose'), 'output extra information') - args.debug = _log_option_source( + args.debug = _decide_option_source( args.debug, ini_options.get('debug'), 'debug mode') - args.quiet = _log_option_source( + args.quiet = _decide_option_source( args.quiet, ini_options.get('quiet'), 'silent mode') - args.ignore_nosec = _log_option_source( + args.ignore_nosec = _decide_option_source( args.ignore_nosec, ini_options.get('ignore-nosec'), 'do not skip lines with # nosec') - args.baseline = _log_option_source( + args.baseline = _decide_option_source( args.baseline, ini_options.get('baseline'), 'path of a baseline report') From 2560176a6de94651f5d815c7ce1d2c869e4a366f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Mare=C5=A1?= Date: Wed, 13 Jan 2021 13:45:17 +0100 Subject: [PATCH 2/5] Fix ini configuration was overriden by default values Fix #657 --- bandit/cli/main.py | 281 +++++++++++++++++++++++++-------------------- 1 file changed, 157 insertions(+), 124 deletions(-) diff --git a/bandit/cli/main.py b/bandit/cli/main.py index b6fb7e418..be55a36ed 100644 --- a/bandit/cli/main.py +++ b/bandit/cli/main.py @@ -72,7 +72,7 @@ def _get_options_from_ini(ini_path, target): if ini_file: return utils.parse_ini_file(ini_file) else: - return None + return {} def _init_extensions(): @@ -80,7 +80,7 @@ def _init_extensions(): return ext_loader.MANAGER -def _decide_option_source(arg_val, ini_val, option_name): +def _decide_option_source(arg_val, ini_val, default_val, option_name): """Decide witch option source should be used. And log source to log.""" if arg_val: LOG.info("Using command line arg for %s", option_name) @@ -88,6 +88,9 @@ def _decide_option_source(arg_val, ini_val, option_name): elif ini_val: LOG.info("Using ini file for %s", option_name) return ini_val + elif default_val is not None: + print("Using default value for %s" % option_name) + return default_val else: return None @@ -136,6 +139,7 @@ def main(): extension_mgr.formatters)] # now do normal startup + # command line arguments (don't store defaults here) parser = argparse.ArgumentParser( description='Bandit - a Python source code security analyzer', formatter_class=argparse.RawDescriptionHelpFormatter @@ -143,21 +147,25 @@ def main(): parser.add_argument( 'targets', metavar='targets', type=str, nargs='*', help='source file(s) or directory(s) to be tested' + # don't place default here, use _decide_option_source bellow ) parser.add_argument( '-r', '--recursive', dest='recursive', action='store_true', help='find and process files in subdirectories' + # don't place default here, use _decide_option_source bellow ) parser.add_argument( '-a', '--aggregate', dest='agg_type', action='store', default='file', type=str, choices=['file', 'vuln'], help='aggregate output by vulnerability (default) or by filename' + # don't place default here, use _decide_option_source bellow ) parser.add_argument( '-n', '--number', dest='context_lines', action='store', default=3, type=int, help='maximum number of code lines to output for each issue' + # don't place default here, use _decide_option_source bellow ) parser.add_argument( '-c', '--configfile', dest='config_file', @@ -167,77 +175,89 @@ def main(): ) parser.add_argument( '-p', '--profile', dest='profile', - action='store', default=None, type=str, + action='store', type=str, help='profile to use (defaults to executing all tests)' + # don't place default here, use _decide_option_source bellow ) parser.add_argument( '-t', '--tests', dest='tests', - action='store', default=None, type=str, + action='store', type=str, help='comma-separated list of test IDs to run' + # don't place default here, use _decide_option_source bellow ) parser.add_argument( '-s', '--skip', dest='skips', - action='store', default=None, type=str, + action='store', type=str, help='comma-separated list of test IDs to skip' + # don't place default here, use _decide_option_source bellow ) parser.add_argument( '-l', '--level', dest='severity', action='count', - default=1, help='report only issues of a given severity level or ' - 'higher (-l for LOW, -ll for MEDIUM, -lll for HIGH)' + help='report only issues of a given severity level or ' + 'higher (-l for LOW, -ll for MEDIUM, -lll for HIGH)' + # don't place default here, use _decide_option_source bellow ) parser.add_argument( '-i', '--confidence', dest='confidence', action='count', - default=1, help='report only issues of a given confidence level or ' - 'higher (-i for LOW, -ii for MEDIUM, -iii for HIGH)' + help='report only issues of a given confidence level or ' + 'higher (-i for LOW, -ii for MEDIUM, -iii for HIGH)' + # don't place default here, use _decide_option_source bellow ) - output_format = 'screen' if sys.stdout.isatty() else 'txt' parser.add_argument( '-f', '--format', dest='output_format', action='store', - default=output_format, help='specify output format', + help='specify output format', choices=sorted(extension_mgr.formatter_names) + # don't place default here, use _decide_option_source bellow ) parser.add_argument( '--msg-template', action='store', - default=None, help='specify output message template' - ' (only usable with --format custom),' - ' see CUSTOM FORMAT section' - ' for list of available values', + help='specify output message template' + ' (only usable with --format custom),' + ' see CUSTOM FORMAT section' + ' for list of available values', + # don't place default here, use _decide_option_source bellow ) parser.add_argument( '-o', '--output', dest='output_file', action='store', nargs='?', - type=argparse.FileType('w', encoding='utf-8'), default=sys.stdout, + type=argparse.FileType('w', encoding='utf-8'), help='write report to filename' + # don't place default here, use _decide_option_source bellow ) group = parser.add_mutually_exclusive_group(required=False) group.add_argument( '-v', '--verbose', dest='verbose', action='store_true', help='output extra information like excluded and included files' + # don't place default here, use _decide_option_source bellow ) parser.add_argument( '-d', '--debug', dest='debug', action='store_true', help='turn on debug mode' + # don't place default here, use _decide_option_source bellow ) group.add_argument( '-q', '--quiet', '--silent', dest='quiet', action='store_true', help='only show output in the case of an error' + # don't place default here, use _decide_option_source bellow ) parser.add_argument( '--ignore-nosec', dest='ignore_nosec', action='store_true', help='do not skip lines with # nosec comments' + # don't place default here, use _decide_option_source bellow ) parser.add_argument( '-x', '--exclude', dest='excluded_paths', action='store', - default=','.join(constants.EXCLUDE), help='comma-separated list of paths (glob patterns ' 'supported) to exclude from scan ' '(note that these are in addition to the excluded ' 'paths provided in the config file) (default: ' + ','.join(constants.EXCLUDE) + ')' + # don't place default here, use _decide_option_source bellow ) parser.add_argument( '-b', '--baseline', dest='baseline', action='store', - default=None, help='path of a baseline report to compare against ' - '(only JSON-formatted files are accepted)' + help='path of a baseline report to compare against ' + '(only JSON-formatted files are accepted)' + # don't place default here, use _decide_option_source bellow ) parser.add_argument( '--ini', dest='ini_path', action='store', default=None, @@ -253,11 +273,6 @@ def main(): version=bandit.__version__, python=python_ver) ) - parser.set_defaults(debug=False) - parser.set_defaults(verbose=False) - parser.set_defaults(quiet=False) - parser.set_defaults(ignore_nosec=False) - plugin_info = ["%s\t%s" % (a[0], a[1].name) for a in extension_mgr.plugins_by_id.items()] blacklist_info = [] @@ -298,9 +313,6 @@ def main(): # setup work - parse arguments, and initialize BanditManager args = parser.parse_args() - # Check if `--msg-template` is not present without custom formatter - if args.output_format != 'custom' and args.msg_template is not None: - parser.error("--msg-template can only be used with --format=custom") try: b_conf = b_config.BanditConfig(config_file=args.config_file) @@ -310,103 +322,124 @@ def main(): # Handle .bandit files in projects to pass cmdline args from file ini_options = _get_options_from_ini(args.ini_path, args.targets) - if ini_options: - # prefer command line, then ini file - args.excluded_paths = _decide_option_source( - args.excluded_paths, - ini_options.get('exclude'), - 'excluded paths') - - args.skips = _decide_option_source( - args.skips, - ini_options.get('skips'), - 'skipped tests') - - args.tests = _decide_option_source( - args.tests, - ini_options.get('tests'), - 'selected tests') - - ini_targets = ini_options.get('targets') - if ini_targets: - ini_targets = ini_targets.split(',') - - args.targets = _decide_option_source( - args.targets, - ini_targets, - 'selected targets') - - # TODO(tmcpeak): any other useful options to pass from .bandit? - - args.recursive = _decide_option_source( - args.recursive, - ini_options.get('recursive'), - 'recursive scan') - - args.agg_type = _decide_option_source( - args.agg_type, - ini_options.get('aggregate'), - 'aggregate output type') - - args.context_lines = _decide_option_source( - args.context_lines, - ini_options.get('number'), - 'max code lines output for issue') - - args.profile = _decide_option_source( - args.profile, - ini_options.get('profile'), - 'profile') - - args.severity = _decide_option_source( - args.severity, - ini_options.get('level'), - 'severity level') - - args.confidence = _decide_option_source( - args.confidence, - ini_options.get('confidence'), - 'confidence level') - - args.output_format = _decide_option_source( - args.output_format, - ini_options.get('format'), - 'output format') - - args.msg_template = _decide_option_source( - args.msg_template, - ini_options.get('msg-template'), - 'output message template') - - args.output_file = _decide_option_source( - args.output_file, - ini_options.get('output'), - 'output file') - - args.verbose = _decide_option_source( - args.verbose, - ini_options.get('verbose'), - 'output extra information') - - args.debug = _decide_option_source( - args.debug, - ini_options.get('debug'), - 'debug mode') - - args.quiet = _decide_option_source( - args.quiet, - ini_options.get('quiet'), - 'silent mode') - - args.ignore_nosec = _decide_option_source( - args.ignore_nosec, - ini_options.get('ignore-nosec'), - 'do not skip lines with # nosec') - - args.baseline = _decide_option_source( - args.baseline, - ini_options.get('baseline'), - 'path of a baseline report') + # prefer command line, then ini file, than default + args.excluded_paths = _decide_option_source( + args.excluded_paths, + ini_options.get('exclude'), + ','.join(constants.EXCLUDE), + 'excluded paths') + + args.skips = _decide_option_source( + args.skips, + ini_options.get('skips'), + None, + 'skipped tests') + + args.tests = _decide_option_source( + args.tests, + ini_options.get('tests'), + None, + 'selected tests') + + ini_targets = ini_options.get('targets') + if ini_targets: + ini_targets = ini_targets.split(',') + + args.targets = _decide_option_source( + args.targets, + ini_targets, + None, + 'selected targets') + + # TODO(tmcpeak): any other useful options to pass from .bandit? + + args.recursive = _decide_option_source( + args.recursive, + ini_options.get('recursive'), + None, + 'recursive scan') + + args.agg_type = _decide_option_source( + args.agg_type, + ini_options.get('aggregate'), + None, + 'aggregate output type') + + args.context_lines = _decide_option_source( + args.context_lines, + ini_options.get('number'), + None, + 'max code lines output for issue') + + args.profile = _decide_option_source( + args.profile, + ini_options.get('profile'), + None, + 'profile') + + args.severity = _decide_option_source( + args.severity, + ini_options.get('level'), + 1, + 'severity level') + + args.confidence = _decide_option_source( + args.confidence, + ini_options.get('confidence'), + 1, + 'confidence level') + + args.output_format = _decide_option_source( + args.output_format, + ini_options.get('format'), + 'screen' if sys.stdout.isatty() else 'txt', + 'output format') + + args.msg_template = _decide_option_source( + args.msg_template, + ini_options.get('msg-template'), + None, + 'output message template') + + args.output_file = _decide_option_source( + args.output_file, + ini_options.get('output'), + sys.stdout, + 'output file') + + args.verbose = _decide_option_source( + args.verbose, + ini_options.get('verbose'), + False, + 'output extra information') + + args.debug = _decide_option_source( + args.debug, + ini_options.get('debug'), + False, + 'debug mode') + + args.quiet = _decide_option_source( + args.quiet, + ini_options.get('quiet'), + False, + 'silent mode') + + args.ignore_nosec = _decide_option_source( + args.ignore_nosec, + ini_options.get('ignore-nosec'), + False, + 'do not skip lines with # nosec') + + args.baseline = _decide_option_source( + args.baseline, + ini_options.get('baseline'), + None, + 'path of a baseline report') + + # Check if `--msg-template` is not present without custom formatter + if args.output_format != 'custom' and args.msg_template is not None: + parser.error("--msg-template can only be used with --format=custom") if not args.targets: LOG.error("No targets found in CLI or ini files, exiting.") From e1e5ff201e46b03324fc89f1ecbb64ed958cefbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Mare=C5=A1?= Date: Wed, 13 Jan 2021 14:40:24 +0100 Subject: [PATCH 3/5] Fix logging --- bandit/cli/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bandit/cli/main.py b/bandit/cli/main.py index be55a36ed..3d620839d 100644 --- a/bandit/cli/main.py +++ b/bandit/cli/main.py @@ -89,7 +89,7 @@ def _decide_option_source(arg_val, ini_val, default_val, option_name): LOG.info("Using ini file for %s", option_name) return ini_val elif default_val is not None: - print("Using default value for %s" % option_name) + LOG.info("Using default value for %s", option_name) return default_val else: return None From 9ca63bfc0528232ed28fa9915a379c023854a704 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Mare=C5=A1?= Date: Wed, 13 Jan 2021 14:44:13 +0100 Subject: [PATCH 4/5] Fix tests --- bandit/core/utils.py | 4 +++- tests/unit/cli/test_main.py | 39 ++++++++++++++++++++++++------------ tests/unit/core/test_util.py | 3 +-- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/bandit/core/utils.py b/bandit/core/utils.py index 7a0a3f275..531a9771e 100644 --- a/bandit/core/utils.py +++ b/bandit/core/utils.py @@ -95,6 +95,7 @@ class InvalidModulePath(Exception): class ConfigError(Exception): """Raised when the config file fails validation.""" + def __init__(self, message, config_file): self.config_file = config_file self.message = "{0} : {1}".format(config_file, message) @@ -103,6 +104,7 @@ def __init__(self, message, config_file): class ProfileNotFound(Exception): """Raised when chosen profile cannot be found.""" + def __init__(self, config_file, profile): self.config_file = config_file self.profile = profile @@ -312,7 +314,7 @@ def parse_ini_file(f_loc): LOG.warning("Unable to parse config file %s or missing [bandit] " "section", f_loc) - return None + return {} def check_ast_node(name): diff --git a/tests/unit/cli/test_main.py b/tests/unit/cli/test_main.py index 815e3c118..2a099ac28 100644 --- a/tests/unit/cli/test_main.py +++ b/tests/unit/cli/test_main.py @@ -8,7 +8,6 @@ import fixtures import mock import testtools - from bandit.cli import main as bandit from bandit.core import extension_loader as ext_loader from bandit.core import utils @@ -89,20 +88,20 @@ def tearDown(self): def test_get_options_from_ini_no_ini_path_no_target(self): # Test that no config options are loaded when no ini path or target # directory are provided - self.assertIsNone(bandit._get_options_from_ini(None, [])) + self.assertEqual({}, bandit._get_options_from_ini(None, [])) def test_get_options_from_ini_empty_directory_no_target(self): # Test that no config options are loaded when an empty directory is # provided as the ini path and no target directory is provided ini_directory = self.useFixture(fixtures.TempDir()).path - self.assertIsNone(bandit._get_options_from_ini(ini_directory, [])) + self.assertEqual({}, bandit._get_options_from_ini(ini_directory, [])) def test_get_options_from_ini_no_ini_path_no_bandit_files(self): # Test that no config options are loaded when no ini path is provided # and the target directory contains no bandit config files (.bandit) target_directory = self.useFixture(fixtures.TempDir()).path - self.assertIsNone(bandit._get_options_from_ini(None, - [target_directory])) + self.assertEqual({}, bandit._get_options_from_ini( + None, [target_directory])) def test_get_options_from_ini_no_ini_path_multi_bandit_files(self): # Test that bandit exits when no ini path is provided and the target @@ -124,27 +123,41 @@ def test_init_extensions(self): # Test that an extension loader manager is returned self.assertEqual(ext_loader.MANAGER, bandit._init_extensions()) - def test_log_option_source_arg_val(self): + def test_decide_option_source_arg_val(self): # Test that the command argument value is returned when provided arg_val = 'file' ini_val = 'vuln' option_name = 'aggregate' - self.assertEqual(arg_val, bandit._log_option_source(arg_val, ini_val, - option_name)) + self.assertEqual(arg_val, bandit._decide_option_source(arg_val, + ini_val, + None, + option_name)) - def test_log_option_source_ini_value(self): + def test_decide_option_source_ini_value(self): # Test that the ini value is returned when no command argument is # provided ini_val = 'vuln' option_name = 'aggregate' - self.assertEqual(ini_val, bandit._log_option_source(None, ini_val, - option_name)) + self.assertEqual(ini_val, bandit._decide_option_source(None, ini_val, + None, + option_name)) + + def test_decide_option_source_default_value(self): + # Test that the ini value is returned when no command argument is + # provided + default_val = 'vuln' + option_name = 'aggregate' + self.assertEqual(default_val, + bandit._decide_option_source(None, None, + default_val, + option_name)) - def test_log_option_source_no_values(self): + def test_decide_option_source_no_values(self): # Test that None is returned when no command argument or ini value are # provided option_name = 'aggregate' - self.assertIsNone(bandit._log_option_source(None, None, option_name)) + self.assertIsNone(bandit._decide_option_source( + None, None, None, option_name)) @mock.patch('sys.argv', ['bandit', '-c', 'bandit.yaml', 'test']) def test_main_config_unopenable(self): diff --git a/tests/unit/core/test_util.py b/tests/unit/core/test_util.py index 13c31f6a6..4d0c716a0 100644 --- a/tests/unit/core/test_util.py +++ b/tests/unit/core/test_util.py @@ -12,7 +12,6 @@ import tempfile import testtools - from bandit.core import utils as b_utils @@ -272,7 +271,7 @@ def test_parse_ini_file(self): 'expected': {'exclude': '/abc,/def'}}, {'content': '[Blabla]\nsomething=something', - 'expected': None}] + 'expected': {}}] with tempfile.NamedTemporaryFile('r+') as t: for test in tests: From ec786be23da6a3f665d579836fb9391982244b93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Mare=C5=A1?= Date: Wed, 13 Jan 2021 15:12:42 +0100 Subject: [PATCH 5/5] Fix import order --- tests/unit/cli/test_main.py | 1 + tests/unit/core/test_util.py | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/unit/cli/test_main.py b/tests/unit/cli/test_main.py index 2a099ac28..eba07a946 100644 --- a/tests/unit/cli/test_main.py +++ b/tests/unit/cli/test_main.py @@ -8,6 +8,7 @@ import fixtures import mock import testtools + from bandit.cli import main as bandit from bandit.core import extension_loader as ext_loader from bandit.core import utils diff --git a/tests/unit/core/test_util.py b/tests/unit/core/test_util.py index 4d0c716a0..f55ca3d10 100644 --- a/tests/unit/core/test_util.py +++ b/tests/unit/core/test_util.py @@ -12,6 +12,7 @@ import tempfile import testtools + from bandit.core import utils as b_utils