Skip to content

Commit

Permalink
update comment parsing
Browse files Browse the repository at this point in the history
- consolidating the test regex to limit the number of passes we need to make
- use a regex to capture the nosec comment anchor rather than multiple find calls
- remove unicode ellipses in example file
  • Loading branch information
mikespallino committed Jan 11, 2022
1 parent 433fb15 commit b4b7222
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 53 deletions.
76 changes: 28 additions & 48 deletions bandit/core/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@


LOG = logging.getLogger(__name__)
NOSEC_SPECIFIC_IDS_IGNORE = re.compile(r"([bB][\d]+),?[ ]?")
NOSEC_SPECIFIC_NAMES_IGNORE = re.compile(r"([a-z_]+),?[ ]?")
NOSEC_COMMENT = re.compile(r"#\s*nosec:?\s*(?P<tests>[^#]+)?#?")
NOSEC_COMMENT_TESTS = re.compile(r"(?:(B\d+|[a-z_]+),?)+", re.IGNORECASE)


class BanditManager(object):
Expand Down Expand Up @@ -416,61 +416,41 @@ def _find_candidate_matches(unmatched_issues, results_list):
return issue_candidates


def _get_tests_from_nosec_comment(comment):
nosec_no_space = '#nosec'
nosec_space = '# nosec'
nospace = comment.find(nosec_no_space)
space = comment.find(nosec_space)
if nospace > -1:
start_comment = nospace
nosec_length = len(nosec_no_space)
elif space > -1:
start_comment = space
nosec_length = len(nosec_space)
else:
# None is explicitly different to empty set, None indicates no
# nosec comment on a line
return None

# find the next # separator
end = comment.find('#', start_comment + 1)
# if we don't find one, set end index to the length
if end == -1:
end = len(comment)

# extract text after #[ ]?nosec and the end index, this is just the
# list of potential test ids or names
return comment[start_comment + nosec_length:end]
def _find_test_id_from_nosec_string(extman, match):
plugin_id = extman.check_id(match)
if plugin_id:
return match
# Finding by short_id didn't work, let's check the plugin name
plugin_id = extman.get_plugin_id(match)
if not plugin_id:
# Name and short id didn't work:
LOG.warning(
"Test in comment: %s is not a test name or id, ignoring",
match
)
return plugin_id # We want to return None or the string here regardless


def _parse_nosec_comment(comment):
nosec_tests = _get_tests_from_nosec_comment(comment)
if nosec_tests is None:
found_no_sec_comment = NOSEC_COMMENT.search(comment)
if not found_no_sec_comment:
# there was no nosec comment
return None

matches = found_no_sec_comment.groupdict()
nosec_tests = matches.get("tests", set())

# empty set indicates that there was a nosec comment without specific
# test ids or names
test_ids = set()
if nosec_tests:
extman = extension_loader.MANAGER
# check for IDS to ignore (e.g. B602)
for test in re.finditer(NOSEC_SPECIFIC_IDS_IGNORE,
nosec_tests):
test_id_match = test.group(1)
plugin_id = extman.check_id(test_id_match)
if plugin_id:
test_ids.add(test_id_match)
else:
LOG.warning("Test in comment: %s is not a test id",
test_id_match)
# check for NAMES to ignore (e.g. assert_used)
for test in re.finditer(NOSEC_SPECIFIC_NAMES_IGNORE,
# lookup tests by short code or name
for test in re.finditer(NOSEC_COMMENT_TESTS,
nosec_tests):
plugin_id = extman.get_plugin_id(test.group(1))
if plugin_id:
test_ids.add(plugin_id)
else:
LOG.warning(
"Test in comment: %s is not a test name, ignoring",
test.group(1))
test_match = test.group(1)
test_id = _find_test_id_from_nosec_string(extman, test_match)
if test_id:
test_ids.add(test_id)

return test_ids
3 changes: 2 additions & 1 deletion bandit/core/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ def note_nosec_by_test(self, num=1):
self.current['nosec_by_test'] += num

def note_failed_nosec_by_test(self, num=1):
"""Note a test failed not caught when specific tests in comment used
"""Note a test failed when using a nosec comment with specific tests.
e.g. # nosec B102, B607, but B602 failed.
Increment the currently active metrics failed_nosec_by_test count.
:param num: number of failed nosecs seen, defaults to 1
Expand Down
5 changes: 3 additions & 2 deletions examples/nosec.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
subprocess.Popen('/bin/ls *',
shell=True) #nosec (on the specific kwarg line)
subprocess.Popen('#nosec', shell=True)
subprocess.Popen('/bin/ls *', shell=True) # type: … # nosec # noqa: E501 ; pylint: disable=line-too-long
subprocess.Popen('/bin/ls *', shell=True) # type: ... # nosec # noqa: E501 ; pylint: disable=line-too-long
subprocess.Popen('/bin/ls *', shell=True) # type: ... # nosec B607 # noqa: E501 ; pylint: disable=line-too-long
subprocess.Popen('/bin/ls *', shell=True) #nosec subprocess_popen_with_shell_equals_true (on the line)
subprocess.Popen('#nosec', shell=True) # nosec B607, B602
subprocess.Popen('#nosec', shell=True) # nosec B607 B602
subprocess.Popen('/bin/ls *', shell=True) # nosec subprocess_popen_with_shell_equals_true start_process_with_partial_path
subprocess.Popen('/bin/ls *', shell=True) # type: # noqa: E501 ; pylint: disable=line-too-long # nosec
subprocess.Popen('/bin/ls *', shell=True) # type: ... # noqa: E501 ; pylint: disable=line-too-long # nosec
subprocess.Popen('#nosec', shell=True) # nosec B607, B101
subprocess.Popen('#nosec', shell=True) # nosec B602, subprocess_popen_with_shell_equals_true
4 changes: 2 additions & 2 deletions tests/functional/test_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -722,8 +722,8 @@ def test_flask_debug_true(self):

def test_nosec(self):
expect = {
'SEVERITY': {'UNDEFINED': 0, 'LOW': 4, 'MEDIUM': 0, 'HIGH': 0},
'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 4}
'SEVERITY': {'UNDEFINED': 0, 'LOW': 5, 'MEDIUM': 0, 'HIGH': 0},
'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 5}
}
self.check_example('nosec.py', expect)

Expand Down

0 comments on commit b4b7222

Please sign in to comment.