Merge pull request #248 from Yelp/easy-plugin-development

Easy plugin development

Author: domanchi

Diff for: CONTRIBUTING.md

@@ -78,23 +78,19 @@ There are many examples of existing plugins to reference, under
    Be sure to write comments about **why** your particular regex was crafted
    as it is!
 
-3. Register your plugin
-
-   Once your plugin is written and tested, you need to register it so that
-   it can be disabled if other users don't need it. Be sure to add it to
-   `detect_secrets.core.usage.PluginOptions` as a new option for users to
-   use.
-
-   Check out the following PRs for examples:
-     - https://github.com/Yelp/detect-secrets/pull/74/files
-     - https://github.com/Yelp/detect-secrets/pull/157/files
-
-4. Update documentation
+3. Update documentation
 
    Be sure to add your changes to the `README.md` and `CHANGELOG.md` so that
    it will be easier for maintainers to bump the version and for other
    downstream consumers to get the latest information about plugins available.
 
+### Tips
+
+- There should be a total of three modified files in a minimal new plugin: the
+  plugin file, it's corresponding test, and an updated README.
+- If your plugin uses customizable options (e.g. entropy limit in `HighEntropyStrings`)
+  be sure to add default options to the plugin's `default_options`.
+
 ## Running Tests
 
 ### Running the Entire Test Suite

Diff for: detect_secrets/core/baseline.py

@@ -65,7 +65,7 @@ def initialize(
         elif os.path.isfile(element):
             files_to_scan.append(element)
         else:
-            log.error('detect-secrets: ' + element + ': No such file or directory')
+            log.error('detect-secrets: %s: No such file or directory', element)
 
     if not files_to_scan:
         return output

Diff for: detect_secrets/core/usage.py

@@ -4,6 +4,7 @@
 from collections import namedtuple
 
 from detect_secrets import VERSION
+from detect_secrets.plugins.common.util import import_plugins
 
 
 def add_exclude_lines_argument(parser):
@@ -279,7 +280,6 @@ class PluginDescriptor(
         ],
     ),
 ):
-
     def __new__(cls, related_args=None, **kwargs):
         if not related_args:
             related_args = []
@@ -290,74 +290,46 @@ def __new__(cls, related_args=None, **kwargs):
             **kwargs
         )
 
+    @classmethod
+    def from_plugin_class(cls, plugin, name):
+        """
+        :type plugin: Type[TypeVar('Plugin', bound=BasePlugin)]
+        :type name: str
+        """
+        related_args = None
+        if plugin.default_options:
+            related_args = []
+            for arg_name, value in plugin.default_options.items():
+                related_args.append((
+                    '--{}'.format(arg_name.replace('_', '-')),
+                    value,
+                ))
+
+        return cls(
+            classname=name,
+            disable_flag_text='--{}'.format(plugin.disable_flag_text),
+            disable_help_text=cls.get_disabled_help_text(plugin),
+            related_args=related_args,
+        )
+
+    @staticmethod
+    def get_disabled_help_text(plugin):
+        for line in plugin.__doc__.splitlines():
+            line = line.strip().lstrip()
+            if line:
+                break
+        else:
+            raise NotImplementedError('Plugins must declare a docstring.')
+
+        line = line[0].lower() + line[1:]
+        return 'Disables {}'.format(line)
+
 
 class PluginOptions(object):
 
     all_plugins = [
-        PluginDescriptor(
-            classname='HexHighEntropyString',
-            disable_flag_text='--no-hex-string-scan',
-            disable_help_text='Disables scanning for hex high entropy strings',
-            related_args=[
-                ('--hex-limit', 3),
-            ],
-        ),
-        PluginDescriptor(
-            classname='Base64HighEntropyString',
-            disable_flag_text='--no-base64-string-scan',
-            disable_help_text='Disables scanning for base64 high entropy strings',
-            related_args=[
-                ('--base64-limit', 4.5),
-            ],
-        ),
-        PluginDescriptor(
-            classname='PrivateKeyDetector',
-            disable_flag_text='--no-private-key-scan',
-            disable_help_text='Disables scanning for private keys.',
-        ),
-        PluginDescriptor(
-            classname='BasicAuthDetector',
-            disable_flag_text='--no-basic-auth-scan',
-            disable_help_text='Disables scanning for Basic Auth formatted URIs.',
-        ),
-        PluginDescriptor(
-            classname='KeywordDetector',
-            disable_flag_text='--no-keyword-scan',
-            disable_help_text='Disables scanning for secret keywords.',
-            related_args=[
-                ('--keyword-exclude', None),
-            ],
-        ),
-        PluginDescriptor(
-            classname='AWSKeyDetector',
-            disable_flag_text='--no-aws-key-scan',
-            disable_help_text='Disables scanning for AWS keys.',
-        ),
-        PluginDescriptor(
-            classname='SlackDetector',
-            disable_flag_text='--no-slack-scan',
-            disable_help_text='Disables scanning for Slack tokens.',
-        ),
-        PluginDescriptor(
-            classname='ArtifactoryDetector',
-            disable_flag_text='--no-artifactory-scan',
-            disable_help_text='Disable scanning for Artifactory credentials',
-        ),
-        PluginDescriptor(
-            classname='StripeDetector',
-            disable_flag_text='--no-stripe-scan',
-            disable_help_text='Disable scanning for Stripe keys',
-        ),
-        PluginDescriptor(
-            classname='MailchimpDetector',
-            disable_flag_text='--no-mailchimp-scan',
-            disable_help_text='Disable scanning for Mailchimp keys',
-        ),
-        PluginDescriptor(
-            classname='JwtTokenDetector',
-            disable_flag_text='--no-jwt-scan',
-            disable_help_text='Disable scanning for JWTs',
-        ),
+        PluginDescriptor.from_plugin_class(plugin, name)
+        for name, plugin in import_plugins().items()
     ]
 
     def __init__(self, parser):

Diff for: detect_secrets/plugins/artifactory.py

@@ -6,7 +6,7 @@
 
 
 class ArtifactoryDetector(RegexBasedDetector):
-
+    """Scans for Artifactory credentials."""
     secret_type = 'Artifactory Credentials'
 
     denylist = [

Diff for: detect_secrets/plugins/aws.py

@@ -12,18 +12,23 @@
 
 import requests
 
+from .base import classproperty
 from .base import RegexBasedDetector
 from detect_secrets.core.constants import VerifiedResult
 
 
 class AWSKeyDetector(RegexBasedDetector):
-
+    """Scans for AWS keys."""
     secret_type = 'AWS Access Key'
 
     denylist = (
         re.compile(r'AKIA[0-9A-Z]{16}'),
     )
 
+    @classproperty
+    def disable_flag_text(cls):
+        return 'no-aws-key-scan'
+
     def verify(self, token, content):
         secret_access_key_candidates = get_secret_access_keys(content)
         if not secret_access_key_candidates:

Diff for: detect_secrets/plugins/base.py

@@ -19,12 +19,39 @@
 LINES_OF_CONTEXT = 5
 
 
+class classproperty(property):
+    def __get__(self, cls, owner):
+        return classmethod(self.fget).__get__(None, owner)()
+
+
 class BasePlugin(object):
-    """This is an abstract class to define Plugins API"""
+    """
+    This is an abstract class to define Plugins API.
+
+    :type secret_type: str
+    :param secret_type: uniquely identifies the type of secret found in the baseline.
+        e.g. {
+            "hashed_secret": <hash>,
+            "line_number": 123,
+            "type": <secret_type>,
+        }
+
+        Be warned of modifying the `secret_type` once rolled out to clients since
+        the hashed_secret uses this value to calculate a unique hash (and the baselines
+        will no longer match).
 
+    :type disable_flag_text: str
+    :param disable_flag_text: text used as an command line argument flag to disable
+        this specific plugin scan. does not include the `--` prefix.
+
+    :type default_options: Dict[str, Any]
+    :param default_options: configurable options to modify plugin behavior
+    """
     __metaclass__ = ABCMeta
 
-    secret_type = None
+    @abstractproperty
+    def secret_type(self):
+        raise NotImplementedError
 
     def __init__(self, exclude_lines_regex=None, should_verify=False, **kwargs):
         """
@@ -33,15 +60,31 @@ def __init__(self, exclude_lines_regex=None, should_verify=False, **kwargs):
 
         :type should_verify: bool
         """
-        if not self.secret_type:
-            raise ValueError('Plugins need to declare a secret_type.')
-
         self.exclude_lines_regex = None
         if exclude_lines_regex:
             self.exclude_lines_regex = re.compile(exclude_lines_regex)
 
         self.should_verify = should_verify
 
+    @classproperty
+    def disable_flag_text(cls):
+        name = cls.__name__
+        if name.endswith('Detector'):
+            name = name[:-len('Detector')]
+
+        # turn camel case into hyphenated strings
+        name_hyphen = ''
+        for letter in name:
+            if letter.upper() == letter and name_hyphen:
+                name_hyphen += '-'
+            name_hyphen += letter.lower()
+
+        return 'no-{}-scan'.format(name_hyphen)
+
+    @classproperty
+    def default_options(cls):
+        return {}
+
     def analyze(self, file, filename):
         """
         :param file:     The File object itself.
@@ -213,10 +256,6 @@ class FooDetector(RegexBasedDetector):
     """
     __metaclass__ = ABCMeta
 
-    @abstractproperty
-    def secret_type(self):
-        raise NotImplementedError
-
     @abstractproperty
     def denylist(self):
         raise NotImplementedError

Diff for: detect_secrets/plugins/basic_auth.py

@@ -15,7 +15,7 @@
 
 
 class BasicAuthDetector(RegexBasedDetector):
-
+    """Scans for Basic Auth formatted URIs."""
     secret_type = 'Basic Auth Credentials'
 
     denylist = [

Diff for: detect_secrets/plugins/common/initialize.py

@@ -1,17 +1,6 @@
 """Intelligent initialization of plugins."""
-from ..artifactory import ArtifactoryDetector               # noqa: F401
-from ..aws import AWSKeyDetector                            # noqa: F401
-from ..base import BasePlugin
-from ..basic_auth import BasicAuthDetector                  # noqa: F401
-from ..common.util import get_mapping_from_secret_type_to_class_name
-from ..high_entropy_strings import Base64HighEntropyString  # noqa: F401
-from ..high_entropy_strings import HexHighEntropyString     # noqa: F401
-from ..jwt import JwtTokenDetector                          # noqa: F401
-from ..keyword import KeywordDetector                       # noqa: F401
-from ..mailchimp import MailchimpDetector                   # noqa: F401
-from ..private_key import PrivateKeyDetector                # noqa: F401
-from ..slack import SlackDetector                           # noqa: F401
-from ..stripe import StripeDetector                         # noqa: F401
+from .util import get_mapping_from_secret_type_to_class_name
+from .util import import_plugins
 from detect_secrets.core.log import log
 from detect_secrets.core.usage import PluginOptions
 
@@ -173,10 +162,10 @@ def from_plugin_classname(
 
     :type should_verify_secrets: bool
     """
-    klass = globals()[plugin_classname]
-
-    # Make sure the instance is a BasePlugin type, before creating it.
-    if not issubclass(klass, BasePlugin):  # pragma: no cover
+    try:
+        klass = import_plugins()[plugin_classname]
+    except KeyError:
+        log.warning('No such plugin to initialize.')
         raise TypeError
 
     try:
@@ -187,9 +176,7 @@ def from_plugin_classname(
             **kwargs
         )
     except TypeError:
-        log.warning(
-            'Unable to initialize plugin!',
-        )
+        log.warning('Unable to initialize plugin!')
         raise
 
     return instance