Use different regexes in KeywordDetector to improve accuracy

detect_secrets/core/audit.py

@@ -9,6 +9,7 @@
 
 from ..plugins.core import initialize
 from ..plugins.high_entropy_strings import HighEntropyStringsPlugin
+from ..plugins.keyword import KeywordDetector
 from .baseline import format_baseline_for_output
 from .baseline import merge_results
 from .bidirectional_iterator import BidirectionalIterator
@@ -518,7 +519,13 @@ def _get_secret_with_context(
     )
 
 
-def _highlight_secret(secret_line, secret_lineno, secret, filename, plugin_settings):
+def _highlight_secret(
+    secret_line,
+    secret_lineno,
+    secret,
+    filename,
+    plugin_settings,
+):
     """
     :type secret_line: str
     :param secret_line: the line on which the secret is found
@@ -544,7 +551,11 @@ def _highlight_secret(secret_line, secret_lineno, secret, filename, plugin_setti
         plugin_settings,
     )
 
-    for raw_secret in _raw_secret_generator(plugin, secret_line):
+    for raw_secret in _raw_secret_generator(
+        plugin,
+        secret_line,
+        is_php_file=filename.endswith('.php'),
+    ):
         secret_obj = PotentialSecret(
             plugin.secret_type,
             filename,
@@ -572,10 +583,14 @@ def _highlight_secret(secret_line, secret_lineno, secret, filename, plugin_setti
     )
 
 
-def _raw_secret_generator(plugin, secret_line):
+def _raw_secret_generator(plugin, secret_line, is_php_file):
     """Generates raw secrets by re-scanning the line, with the specified plugin"""
-    for raw_secret in plugin.secret_generator(secret_line):
-        yield raw_secret
+    if isinstance(plugin, KeywordDetector):
+        for raw_secret in plugin.secret_generator(secret_line, is_php_file):
+            yield raw_secret
+    else:
+        for raw_secret in plugin.secret_generator(secret_line):
+            yield raw_secret
 
     if issubclass(plugin.__class__, HighEntropyStringsPlugin):
         with plugin.non_quoted_string_regex(strict=False):

detect_secrets/core/usage.py

@@ -348,9 +348,7 @@ def _add_opt_out_options(self):
                 plugin.disable_flag_text,
                 action='store_true',
                 help=plugin.disable_help_text,
-                # Temporarily disabling the KeywordDetector
-                # Until we can test its effectiveness on more repositories
-                default=True if plugin.disable_flag_text == '--no-keyword-scan' else False,
+                default=False,
             )
 
         return self

detect_secrets/plugins/base.py

@@ -53,6 +53,9 @@ def secret_generator(self, string):
 
         :type string: str
         :param string: the secret to scan
+
+        :rtype: iter
+        :returns: Of all the identifiers found
         """
         raise NotImplementedError
 

detect_secrets/plugins/keyword.py

@@ -26,21 +26,58 @@
 """
 from __future__ import absolute_import
 
+import re
+
 from .base import BasePlugin
 from detect_secrets.core.potential_secret import PotentialSecret
 
 
+# Note: All values here should be lowercase
 BLACKLIST = (
-    # NOTE all values here should be lowercase,
-    # otherwise _secret_generator can fail to match them
-    'pass =',
+    'apikey',
+    'api_key',
+    'pass',
     'password',
     'passwd',
-    'pwd',
+    'private_key',
     'secret',
     'secrete',
-    'token',
 )
+FALSE_POSITIVES = (
+    "''",
+    "''):",
+    '""',
+    '""):',
+    'false',
+    'none',
+    'not',
+    'password)',
+    'password},',
+    'true',
+)
+FOLLOWED_BY_COLON_RE = re.compile(
+    # e.g. token:
+    r'({})(("|\')?):(\s*?)(("|\')?)([^\s]+)(\5)'.format(
+        r'|'.join(BLACKLIST),
+    ),
+)
+FOLLOWED_BY_EQUAL_SIGNS_RE = re.compile(
+    # e.g. my_password =
+    r'({})()(\s*?)=(\s*?)(("|\')?)([^\s]+)(\5)'.format(
+        r'|'.join(BLACKLIST),
+    ),
+)
+FOLLOWED_BY_QUOTES_AND_SEMICOLON_RE = re.compile(
+    # e.g. private_key "something";
+    r'({})([^\s]*?)(\s*?)("|\')([^\s]+)(\4);'.format(
+        r'|'.join(BLACKLIST),
+    ),
+)
+BLACKLIST_REGEX_TO_GROUP = {
+    FOLLOWED_BY_COLON_RE: 7,
+    FOLLOWED_BY_EQUAL_SIGNS_RE: 7,
+    FOLLOWED_BY_QUOTES_AND_SEMICOLON_RE: 5,
+}
 
 
 class KeywordDetector(BasePlugin):
@@ -53,7 +90,10 @@ class KeywordDetector(BasePlugin):
     def analyze_string(self, string, line_num, filename):
         output = {}
 
-        for identifier in self.secret_generator(string):
+        for identifier in self.secret_generator(
+            string,
+            is_php_file=filename.endswith('.php'),
+        ):
             secret = PotentialSecret(
                 self.secret_type,
                 filename,
@@ -64,10 +104,47 @@ def analyze_string(self, string, line_num, filename):
 
         return output
 
-    def _secret_generator(self, lowercase_string):
-        for line in BLACKLIST:
-            if line in lowercase_string:
-                yield line
+    def secret_generator(self, string, is_php_file):
+        lowered_string = string.lower()
+
+        for REGEX, group_number in BLACKLIST_REGEX_TO_GROUP.items():
+            match = REGEX.search(lowered_string)
+            if match:
+                lowered_secret = match.group(group_number)
+
+                # ([^\s]+) guarantees lowered_secret is not ''
+                if not probably_false_positive(lowered_secret, is_php_file):
+                    yield lowered_secret
+
+
+def probably_false_positive(lowered_secret, is_php_file):
+    if (
+        'fake' in lowered_secret or
+        lowered_secret in FALSE_POSITIVES or
+        # If it is a .php file, do not report $variables
+        (
+            is_php_file and
+            lowered_secret[0] == '$'
+        )
+    ):
+        return True
+
+    # No function calls
+    try:
+        if (
+            lowered_secret.index('(') < lowered_secret.index(')')
+        ):
+            return True
+    except ValueError:
+        pass
+
+    # Skip over e.g. request.json_body['hey']
+    try:
+        if (
+            lowered_secret.index('[') < lowered_secret.index(']')
+        ):
+            return True
+    except ValueError:
+        pass
 
-    def secret_generator(self, string):
-        return self._secret_generator(string.lower())
+    return False

test_data/short_files/first_line.py → test_data/short_files/first_line.php

@@ -1,4 +1,4 @@
-secret = 'BEEF0123456789a'
+seecret = 'BEEF0123456789a'
 skipped_sequential_false_positive = '0123456789a'
 print('second line')
 var = 'third line'

tests/core/audit_test.py

@@ -65,7 +65,7 @@ def test_making_decisions(self, mock_printer):
     def test_quit_half_way(self, mock_printer):
         modified_baseline = deepcopy(self.baseline)
 
-        for secrets in modified_baseline['results'].values():
+        for secrets in modified_baseline['results'].values():  # pragma: no cover
             secrets[0]['is_secret'] = False
             break
 
@@ -147,8 +147,7 @@ def test_go_back_several_steps(self, mock_printer):
         for secrets in modified_baseline['results'].values():
             for secret in secrets:
                 value = values_to_inject.pop(0)
-                if value is not None:
-                    secret['is_secret'] = value
+                secret['is_secret'] = value
 
         self.run_logic(
             ['s', 'y', 'b', 's', 'b', 'b', 'n', 'n', 'n'],
@@ -174,10 +173,7 @@ def run_logic(self, inputs, modified_baseline=None, input_baseline=None):
         ) as m:
             audit.audit_baseline('will_be_mocked')
 
-            if not modified_baseline:
-                assert m.call_args[0][1] == self.baseline
-            else:
-                assert m.call_args[0][1] == modified_baseline
+            assert m.call_args[0][1] == modified_baseline
 
     @contextmanager
     def mock_env(self, user_inputs=None, baseline=None):
@@ -335,7 +331,7 @@ def test_compare(self, mock_printer):
         # These files come after, because filenames are sorted first
         assert headers[2] == textwrap.dedent("""
             Secret:      3 of 4
-            Filename:    test_data/short_files/first_line.py
+            Filename:    test_data/short_files/first_line.php
             Secret Type: Hex High Entropy String
             Status:      >> REMOVED <<
         """)[1:]
@@ -406,7 +402,7 @@ def old_baseline(self):
                 ],
 
                 # This entire file will be "removed"
-                'test_data/short_files/first_line.py': [
+                'test_data/short_files/first_line.php': [
                     {
                         'hashed_secret': '0de9a11b3f37872868ca49ecd726c955e25b6e21',
                         'line_number': 1,

tests/core/secrets_collection_test.py

@@ -350,7 +350,7 @@ def assert_loaded_collection_is_original_collection(self, original, new):
             assert original[key] == new[key]
 
 
-class MockBasePlugin(BasePlugin):
+class MockBasePlugin(BasePlugin):  # pragma: no cover
     """Abstract testing class, to implement abstract methods."""
 
     def analyze_string(self, value):

tests/core/usage_test.py

@@ -32,6 +32,7 @@ def test_consolidates_output_basic(self):
             'Base64HighEntropyString': {
                 'base64_limit': 4.5,
             },
+            'KeywordDetector': {},
             'PrivateKeyDetector': {},
             'AWSKeyDetector': {},
         }

tests/main_test.py

@@ -91,6 +91,7 @@ def test_scan_string_basic(self, mock_baseline_initialize):
                 Base64HighEntropyString: False (3.459)
                 BasicAuthDetector      : False
                 HexHighEntropyString   : True  (3.459)
+                KeywordDetector        : False
                 PrivateKeyDetector     : False
             """)[1:]
 
@@ -108,6 +109,7 @@ def test_scan_string_cli_overrides_stdin(self):
                 Base64HighEntropyString: False (2.585)
                 BasicAuthDetector      : False
                 HexHighEntropyString   : False (2.121)
+                KeywordDetector        : False
                 PrivateKeyDetector     : False
             """)[1:]
 
@@ -192,9 +194,9 @@ def test_old_baseline_ignored_with_update_flag(
         'filename, expected_output',
         [
             (
-                'test_data/short_files/first_line.py',
+                'test_data/short_files/first_line.php',
                 textwrap.dedent("""
-                    1:secret = 'BEEF0123456789a'
+                    1:seecret = 'BEEF0123456789a'
                     2:skipped_sequential_false_positive = '0123456789a'
                     3:print('second line')
                     4:var = 'third line'

tests/plugins/base_test.py

@@ -6,7 +6,7 @@
 
 
 def test_fails_if_no_secret_type_defined():
-    class MockPlugin(BasePlugin):
+    class MockPlugin(BasePlugin):  # pragma: no cover
         def analyze_string(self, *args, **kwargs):
             pass