Merge pull request #388 from ericwb/nosec

Properly handle nosec strings in code

bandit/core/manager.py

@@ -20,8 +20,11 @@
 import logging
 import os
 import sys
+import tokenize
 import traceback
 
+import six
+
 from bandit.core import constants as b_constants
 from bandit.core import extension_loader
 from bandit.core import issue
@@ -271,10 +274,18 @@ def _parse_file(self, fname, fdata, new_files_list):
             if self.ignore_nosec:
                 nosec_lines = set()
             else:
-                nosec_lines = set(
-                    lineno + 1 for
-                    (lineno, line) in enumerate(lines)
-                    if b'#nosec' in line or b'# nosec' in line)
+                try:
+                    fdata.seek(0)
+                    if six.PY2:
+                        tokens = tokenize.generate_tokens(fdata.readline)
+                    else:
+                        tokens = tokenize.tokenize(fdata.readline)
+                    nosec_lines = set(
+                        lineno for toktype, tokval, (lineno, _), _, _ in tokens
+                        if toktype == tokenize.COMMENT and
+                        '#nosec' in tokval or '# nosec' in tokval)
+                except tokenize.TokenError as e:
+                    nosec_lines = set()
             score = self._execute_ast_visitor(fname, data, nosec_lines)
             self.scores.append(score)
             self.metrics.count_issues([score, ])

examples/nosec.py

@@ -3,3 +3,5 @@
                  shell=True)
 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

pylintrc

@@ -12,6 +12,7 @@
 # TODO(browne): fix these in the future
 # C0103: invalid-name
 # E1101: no-member
+# E1111: assignment-from-no-return
 # R0902: too-many-instance-attributes
 # R0912: too-many-branches
 # R0913: too-many-arguments
@@ -26,7 +27,7 @@
 # W0613: unused-argument
 # W0621: redefined-outer-name
 # W0703: broad-except
-disable=C0111,C0301,C0325,F0401,W0511,W0142,W0622,C0103,E1101,R0902,R0912,R0913,R0914,R0915,W0110,W0141,W0201,W0401,W0603,W0212,W0613,W0621,W0703
+disable=C0111,C0301,C0325,F0401,W0511,W0142,W0622,C0103,E1101,E1111,R0902,R0912,R0913,R0914,R0915,W0110,W0141,W0201,W0401,W0603,W0212,W0613,W0621,W0703
 
 [Basic]
 # Variable names can be 1 to 31 characters long, with lowercase and underscores

tests/functional/test_functional.py

@@ -723,8 +723,8 @@ def test_flask_debug_true(self):
 
     def test_nosec(self):
         expect = {
-            'SEVERITY': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 0},
-            'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 0}
+            'SEVERITY': {'UNDEFINED': 0, 'LOW': 2, 'MEDIUM': 0, 'HIGH': 0},
+            'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 2}
         }
         self.check_example('nosec.py', expect)
 

tests/functional/test_runtime.py

@@ -15,6 +15,7 @@
 import os
 import subprocess
 
+import six
 import testtools
 
 
@@ -111,11 +112,13 @@ def test_example_nonsense2(self):
             ['bandit', ], ['nonsense2.py', ]
         )
         self.assertEqual(0, retcode)
-        self.assertIn(
-            "Exception occurred when executing tests against", output
-        )
         self.assertIn("Files skipped (1):", output)
-        self.assertIn("nonsense2.py (exception while scanning file)", output)
+        if six.PY2:
+            self.assertIn("nonsense2.py (exception while scanning file)",
+                          output)
+        else:
+            self.assertIn("nonsense2.py (syntax error while parsing AST",
+                          output)
 
     def test_example_imports(self):
         (retcode, output) = self._test_example(['bandit', ], ['imports.py', ])