Improve handling nosec for multi-line strings

bandit/core/node_visitor.py

@@ -200,13 +200,6 @@ def pre_visit(self, node):
         if hasattr(node, "lineno"):
             self.context["lineno"] = node.lineno
 
-            # explicitly check for empty set to skip all tests for a line
-            nosec_tests = self.nosec_lines.get(node.lineno)
-            if nosec_tests is not None and not len(nosec_tests):
-                LOG.debug("skipped, nosec without test number")
-                self.metrics.note_nosec()
-                return False
-
         if hasattr(node, "col_offset"):
             self.context["col_offset"] = node.col_offset
         if hasattr(node, "end_col_offset"):

bandit/core/tester.py

@@ -76,12 +76,15 @@ def run_tests(self, raw_context, checktype):
 
                     # don't skip the test if there was no nosec comment
                     if nosec_tests_to_skip is not None:
-                        # if the set is empty or the test id is in the set of
-                        # tests to skip, log and increment the skip by test
-                        # count
-                        if not nosec_tests_to_skip or (
-                            result.test_id in nosec_tests_to_skip
-                        ):
+                        # If the set is empty then it means that nosec was
+                        # used without test number -> update nosecs counter.
+                        # If the test id is in the set of tests to skip,
+                        # log and increment the skip by test count.
+                        if not nosec_tests_to_skip:
+                            LOG.debug("skipped, nosec without test number")
+                            self.metrics.note_nosec()
+                            continue
+                        elif result.test_id in nosec_tests_to_skip:
                             LOG.debug(
                                 "skipped, nosec for test %s" % result.test_id
                             )
@@ -129,7 +132,7 @@ def _get_nosecs_from_contexts(self, context, test_result=None):
             if test_result
             else None
         )
-        context_tests = self.nosec_lines.get(context["lineno"], None)
+        context_tests = utils.get_nosec(self.nosec_lines, context)
 
         # if both are none there were no comments
         # this is explicitly different from being empty.

bandit/core/utils.py

@@ -370,3 +370,11 @@ def check_ast_node(name):
         pass
 
     raise TypeError("Error: %s is not a valid node type in AST" % name)
+
+
+def get_nosec(nosec_lines, context):
+    for lineno in context["linerange"]:
+        nosec = nosec_lines.get(lineno, None)
+        if nosec is not None:
+            return nosec
+    return None

examples/sql_multiline_statements.py

@@ -121,3 +121,62 @@ def b():
 
 a()("""SELECT %s
 FROM foo""" % val)
+
+# skip
+query = """SELECT *
+FROM foo WHERE id = '%s'""" % identifier  # nosec
+query = """SELECT *
+FROM foo WHERE id = '%s'""" % identifier  # nosec B608
+query = """
+SELECT *
+FROM foo
+WHERE id = '%s'
+""" % identifier  # nosec B608
+
+query = f"""
+SELECT *
+FROM foo
+WHERE id = {identifier}
+"""  # nosec
+query = f"""
+SELECT *
+FROM foo
+WHERE id = {identifier}
+"""  # nosec B608
+
+query = f"""
+SELECT *
+FROM foo
+WHERE id = {identifier}"""  # nosec
+query = f"""
+SELECT *
+FROM foo
+WHERE id = {identifier}"""  # nosec B608
+
+cur.execute("SELECT * "  # nosec
+            "FROM foo "
+            f"WHERE id = {identifier}")
+cur.execute("SELECT * "  # nosec B608
+            "FROM foo "
+            f"WHERE id = {identifier}")
+
+query = ("SELECT * "  # nosec
+         "FROM foo "
+         f"WHERE id = {identifier}")
+query = ("SELECT * "  # nosec B608
+         "FROM foo "
+         f"WHERE id = {identifier}")
+
+# nosec is not recognized for the 4 below cases in python 3.7
+query = ("SELECT * "
+         "FROM foo "  # nosec
+         f"WHERE id = {identifier}")
+query = ("SELECT * "
+         "FROM foo "  # nosec B608
+         f"WHERE id = {identifier}")
+query = ("SELECT * "
+         "FROM foo "
+         f"WHERE id = {identifier}")  # nosec
+query = ("SELECT * "
+         "FROM foo "
+         f"WHERE id = {identifier}")  # nosec B608

tests/functional/test_functional.py

@@ -457,21 +457,42 @@ def test_multiline_sql_statements(self):
         multi-line strings.
         """
         example_file = "sql_multiline_statements.py"
+        confidence_low_tests = 13
+        severity_medium_tests = 26
+        nosec_tests = 7
+        skipped_tests = 8
+        if sys.version_info[:2] <= (3, 7):
+            # In the case of implicit concatenation in python 3.7,
+            # we know only the first line of multi-line string.
+            # Thus, cases like:
+            # query = ("SELECT * "
+            #          "FROM foo "  # nosec
+            #          f"WHERE id = {identifier}")
+            # are not skipped but reported as errors.
+            confidence_low_tests = 17
+            severity_medium_tests = 30
+            nosec_tests = 5
+            skipped_tests = 6
         expect = {
             "SEVERITY": {
                 "UNDEFINED": 0,
                 "LOW": 0,
-                "MEDIUM": 26,
+                "MEDIUM": severity_medium_tests,
                 "HIGH": 0,
             },
             "CONFIDENCE": {
                 "UNDEFINED": 0,
-                "LOW": 13,
+                "LOW": confidence_low_tests,
                 "MEDIUM": 13,
                 "HIGH": 0,
             },
         }
+        expect_stats = {
+            "nosec": nosec_tests,
+            "skipped_tests": skipped_tests,
+        }
         self.check_example(example_file, expect)
+        self.check_metrics(example_file, expect_stats)
 
     def test_ssl_insecure_version(self):
         """Test for insecure SSL protocol versions."""