Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring-your own-plugins (BYOP), via --custom-plugins option #308

Merged
merged 5 commits into from
Jul 9, 2020

Conversation

KevinHock
Copy link
Collaborator

This is an exact replica of #255, I just don't have the GitHub permissions to push to the branch any more, so I made this one. 馃槃

@KevinHock
Copy link
Collaborator Author

re: @OiCMudkips feedback, I changed the following. It's much nicer now :)

diff --git a/detect_secrets/core/audit.py b/detect_secrets/core/audit.py
index f50dd93..92d418b 100644
--- a/detect_secrets/core/audit.py
+++ b/detect_secrets/core/audit.py
@@ -290,13 +290,13 @@ def determine_audit_results(baseline, baseline_path):
         audit_results['stats'][audit_result]['count'] += 1
         audit_results['stats'][audit_result]['files'][filename] += 1
         total += 1
-    audit_results['stats']['signal'] = '{:.2f}'.format(
+    audit_results['stats']['signal'] = '{:.2f}%'.format(
         (
             float(audit_results['stats']['true-positives']['count'])
             /
             total
         ) * 100,
-    ) + '%'
+    )
 
     for plugin_config in baseline['plugins_used']:
         plugin_name = plugin_config['name']
@@ -339,7 +339,10 @@ def print_audit_results(baseline_filename):
 def _get_baseline_from_file(filename):  # pragma: no cover
     try:
         with open(filename) as f:
-            return json.loads(f.read())
+            baseline = json.loads(f.read())
+            if 'custom_plugin_paths' in baseline:
+                baseline['custom_plugin_paths'] = tuple(baseline['custom_plugin_paths'])
+            return baseline
     except (IOError, json.decoder.JSONDecodeError):
         print('Not a valid baseline file!', file=sys.stderr)
         return
@@ -507,8 +510,8 @@ def _print_context(  # pragma: no cover
         ...     },
         ... ]
 
-    :type custom_plugin_paths: List[str]
-    :param custom_plugin_paths: possibly empty list of paths that have custom plugins.
+    :type custom_plugin_paths: Tuple[str]
+    :param custom_plugin_paths: possibly empty tuple of paths that have custom plugins.
 
     :type additional_header_lines: str
     :param additional_header_lines: any additional lines to add to the
@@ -649,8 +652,8 @@ def _get_secret_with_context(
         ...     },
         ... ]
 
-    :type custom_plugin_paths: List[str]
-    :param custom_plugin_paths: possibly empty list of paths that have custom plugins.
+    :type custom_plugin_paths: Tuple[str]
+    :param custom_plugin_paths: possibly empty tuple of paths that have custom plugins.
 
     :type lines_of_context: int
     :param lines_of_context: number of lines displayed before and after
@@ -720,8 +723,8 @@ def get_raw_secret_value(
         ...     },
         ... ]
 
-    :type custom_plugin_paths: List[str]
-    :param custom_plugin_paths: possibly empty list of paths that have custom plugins.
+    :type custom_plugin_paths: Tuple[str]
+    :param custom_plugin_paths: possibly empty tuple of paths that have custom plugins.
 
     :type file_handle: file object
     :param file_handle: Open handle to file where the secret is
diff --git a/detect_secrets/core/baseline.py b/detect_secrets/core/baseline.py
index a6f12e1..d74b7db 100644
--- a/detect_secrets/core/baseline.py
+++ b/detect_secrets/core/baseline.py
@@ -29,8 +29,8 @@ def initialize(
     :type plugins: tuple of detect_secrets.plugins.base.BasePlugin
     :param plugins: rules to initialize the SecretsCollection with.
 
-    :type custom_plugin_paths: List[str]
-    :param custom_plugin_paths: possibly empty list of paths that have custom plugins.
+    :type custom_plugin_paths: Tuple[str]
+    :param custom_plugin_paths: possibly empty tuple of paths that have custom plugins.
 
     :type exclude_files_regex: str|None
     :type exclude_lines_regex: str|None
diff --git a/detect_secrets/core/secrets_collection.py b/detect_secrets/core/secrets_collection.py
index 04184ef..b76e2f5 100644
--- a/detect_secrets/core/secrets_collection.py
+++ b/detect_secrets/core/secrets_collection.py
@@ -28,8 +28,8 @@ class SecretsCollection:
         :type plugins: tuple of detect_secrets.plugins.base.BasePlugin
         :param plugins: rules to determine whether a string is a secret
 
-        :type custom_plugin_paths: List[str]|None
-        :param custom_plugin_paths: possibly empty list of paths that have custom plugins.
+        :type custom_plugin_paths: Tuple[str]|None
+        :param custom_plugin_paths: possibly empty tuple of paths that have custom plugins.
 
         :type exclude_files: str|None
         :param exclude_files: optional regex for ignored paths.
@@ -47,7 +47,7 @@ class SecretsCollection:
         self.version = VERSION
 
         self.plugins = plugins
-        self.custom_plugin_paths = custom_plugin_paths or []
+        self.custom_plugin_paths = custom_plugin_paths or ()
         self.exclude_files = exclude_files
         self.exclude_lines = exclude_lines
         self.word_list_file = word_list_file
@@ -116,7 +116,7 @@ class SecretsCollection:
                 automaton, result.word_list_hash = build_automaton(result.word_list_file)
 
         # In v0.12.8 the `--custom-plugins` option got added
-        result.custom_plugin_paths = data.get('custom_plugin_paths', [])
+        result.custom_plugin_paths = data.get('custom_plugin_paths', ())
 
         plugins = []
         for plugin in data['plugins_used']:
diff --git a/detect_secrets/core/usage.py b/detect_secrets/core/usage.py
index 149c554..eacbf71 100644
--- a/detect_secrets/core/usage.py
+++ b/detect_secrets/core/usage.py
@@ -4,7 +4,6 @@ from collections import namedtuple
 from functools import lru_cache
 
 from detect_secrets import VERSION
-from detect_secrets.plugins.common.util import change_custom_plugin_paths_to_tuple
 from detect_secrets.plugins.common.util import import_plugins
 
 
@@ -28,11 +27,33 @@ def add_word_list_argument(parser):
     )
 
 
+def _is_valid_path(path):  # pragma: no cover
+    if not os.path.exists(path):
+        raise argparse.ArgumentTypeError(
+            'Invalid path: {}'.format(path),
+        )
+
+    return path
+
+
+class TupleAction(argparse.Action):
+    def __call__(self, parser, namespace, values, options_string=None):
+        existing_values = getattr(
+            namespace,
+            self.dest,
+        )
+        setattr(
+            namespace,
+            self.dest,
+            existing_values + (values,),
+        )
+
+
 def add_custom_plugins_argument(parser):
     parser.add_argument(
         '--custom-plugins',
-        action='append',
-        default=[],
+        action=TupleAction,
+        default=(),
         dest='custom_plugin_paths',
         help=(
             'Custom plugin Python files, or directories containing them. '
@@ -42,15 +63,6 @@ def add_custom_plugins_argument(parser):
     )
 
 
-def _is_valid_path(path):  # pragma: no cover
-    if not os.path.exists(path):
-        raise argparse.ArgumentTypeError(
-            'Invalid path: {}'.format(path),
-        )
-
-    return path
-
-
 def add_use_all_plugins_argument(parser):
     parser.add_argument(
         '--use-all-plugins',
@@ -378,7 +390,6 @@ class PluginDescriptor(
         return 'Disables {}'.format(line)
 
 
-@change_custom_plugin_paths_to_tuple
 @lru_cache(maxsize=1)
 def get_all_plugin_descriptors(custom_plugin_paths):
     return [
diff --git a/detect_secrets/plugins/common/initialize.py b/detect_secrets/plugins/common/initialize.py
index 135e3b8..a09c27e 100644
--- a/detect_secrets/plugins/common/initialize.py
+++ b/detect_secrets/plugins/common/initialize.py
@@ -16,8 +16,8 @@ def from_parser_builder(
     :param plugins_dict: plugins dictionary received from ParserBuilder.
         See example in tests.core.usage_test.
 
-    :type custom_plugin_paths: List[str]
-    :param custom_plugin_paths: possibly empty list of paths that have custom plugins.
+    :type custom_plugin_paths: Tuple[str]
+    :param custom_plugin_paths: possibly empty tuple of paths that have custom plugins.
 
     :type exclude_lines_regex: str|None
     :param exclude_lines_regex: optional regex for ignored lines.
@@ -163,8 +163,8 @@ def from_plugin_classname(
     :type plugin_classname: str
     :param plugin_classname: subclass of BasePlugin.
 
-    :type custom_plugin_paths: List[str]
-    :param custom_plugin_paths: possibly empty list of paths that have custom plugins.
+    :type custom_plugin_paths: Tuple[str]
+    :param custom_plugin_paths: possibly empty tuple of paths that have custom plugins.
 
     :type exclude_lines_regex: str|None
     :param exclude_lines_regex: optional regex for ignored lines.
@@ -219,8 +219,8 @@ def from_secret_type(secret_type, plugins_used, custom_plugin_paths):
         ...     },
         ... ]
 
-    :type custom_plugin_paths: List[str]
-    :param custom_plugin_paths: possibly empty list of paths that have custom plugins.
+    :type custom_plugin_paths: Tuple[str]
+    :param custom_plugin_paths: possibly empty tuple of paths that have custom plugins.
     """
     mapping = get_mapping_from_secret_type_to_class_name(custom_plugin_paths)
     try:
diff --git a/detect_secrets/plugins/common/util.py b/detect_secrets/plugins/common/util.py
index 6891446..3210925 100644
--- a/detect_secrets/plugins/common/util.py
+++ b/detect_secrets/plugins/common/util.py
@@ -8,21 +8,6 @@ from detect_secrets.plugins.base import BasePlugin
 from detect_secrets.util import get_root_directory
 
 
-def change_custom_plugin_paths_to_tuple(custom_plugin_paths_function):
-    """
-    :type custom_plugin_paths_function: function
-    A function that takes one argument named custom_plugin_paths
-
-    :returns: function
-    The custom_plugin_paths_function with it's arg changed to a tuple
-    """
-    def wrapper_of_custom_plugin_paths_function(custom_plugin_paths):
-        return custom_plugin_paths_function(tuple(custom_plugin_paths))
-
-    return wrapper_of_custom_plugin_paths_function
-
-
-@change_custom_plugin_paths_to_tuple
 @lru_cache(maxsize=1)
 def get_mapping_from_secret_type_to_class_name(custom_plugin_paths):
     """Returns dictionary of secret_type => plugin classname"""
@@ -64,7 +49,6 @@ def _is_valid_concrete_plugin_class(attr):
     )
 
 
-@change_custom_plugin_paths_to_tuple
 @lru_cache(maxsize=1)
 def import_plugins(custom_plugin_paths):
     """
diff --git a/testing/util.py b/testing/util.py
index aa9743e..fbddb09 100644
--- a/testing/util.py
+++ b/testing/util.py
@@ -20,7 +20,7 @@ def uncolor(text):
 def get_regex_based_plugins():
     return {
         name: plugin
-        for name, plugin in import_plugins(custom_plugin_paths=[]).items()
+        for name, plugin in import_plugins(custom_plugin_paths=()).items()
         if issubclass(plugin, RegexBasedDetector)
     }
 
diff --git a/tests/core/audit_test.py b/tests/core/audit_test.py
index 64419ae..063388e 100644
--- a/tests/core/audit_test.py
+++ b/tests/core/audit_test.py
@@ -240,7 +240,7 @@ class TestAuditBaseline:
     @property
     def baseline(self):
         return {
-            'custom_plugin_paths': [],
+            'custom_plugin_paths': (),
             'generated_at': 'some timestamp',
             'plugins_used': [
                 {
@@ -273,7 +273,7 @@ class TestAuditBaseline:
     @property
     def leapfrog_baseline(self):
         return {
-            'custom_plugin_paths': [],
+            'custom_plugin_paths': (),
             'generated_at': 'some timestamp',
             'plugins_used': [
                 {
@@ -399,7 +399,7 @@ class TestCompareBaselines:
     @property
     def old_baseline(self):
         return {
-            'custom_plugin_paths': [],
+            'custom_plugin_paths': (),
             'plugins_used': [
                 {
                     'name': 'Base64HighEntropyString',
@@ -444,7 +444,7 @@ class TestCompareBaselines:
     @property
     def new_baseline(self):
         return {
-            'custom_plugin_paths': [],
+            'custom_plugin_paths': (),
             'plugins_used': [
                 {
                     'name': 'Base64HighEntropyString',
@@ -528,7 +528,7 @@ class TestDetermineAuditResults:
         audited.
         """
         baseline_fixture = {
-            'custom_plugin_paths': [],
+            'custom_plugin_paths': (),
             'plugins_used': plugins_used,
             'results': {
                 'mocked_file': [
@@ -762,7 +762,7 @@ class TestPrintContext:
             audit._print_context(
                 filename=secret.filename,
                 secret=secret.json(),
-                custom_plugin_paths=[],
+                custom_plugin_paths=(),
                 count=1,
                 total=2,
                 plugins_used=plugins_used,
diff --git a/tests/core/baseline_test.py b/tests/core/baseline_test.py
index bf4a533..22b10cb 100644
--- a/tests/core/baseline_test.py
+++ b/tests/core/baseline_test.py
@@ -35,7 +35,7 @@ class TestInitializeBaseline:
         return baseline.initialize(
             path,
             self.plugins,
-            custom_plugin_paths=[],
+            custom_plugin_paths=(),
             exclude_files_regex=exclude_files_regex,
             should_scan_all_files=scan_all_files,
         ).json()
diff --git a/tests/core/secrets_collection_test.py b/tests/core/secrets_collection_test.py
index cb411e9..76e078f 100644
--- a/tests/core/secrets_collection_test.py
+++ b/tests/core/secrets_collection_test.py
@@ -350,7 +350,7 @@ class TestBaselineInputOutput:
 
         # v0.12.8+ assertions
         assert 'custom_plugin_paths' not in original
-        assert secrets['custom_plugin_paths'] == []
+        assert secrets['custom_plugin_paths'] == ()
 
         # v0.12.7+ assertions
         assert original['word_list']['file'] == secrets['word_list']['file']
@@ -390,7 +390,7 @@ class TestBaselineInputOutput:
     def get_point_twelve_point_eight_and_later_baseline_dict(self, gmtime):
         # In v0.12.8 --custom-plugins got added
         baseline = self.get_point_twelve_point_seven_and_later_baseline_dict(gmtime)
-        baseline['custom_plugin_paths'] = []
+        baseline['custom_plugin_paths'] = ()
         return baseline
 
     def get_point_twelve_point_seven_and_later_baseline_dict(self, gmtime):
diff --git a/tests/core/usage_test.py b/tests/core/usage_test.py
index 345f6d5..94a29f2 100644
--- a/tests/core/usage_test.py
+++ b/tests/core/usage_test.py
@@ -19,7 +19,7 @@ class TestPluginOptions:
 
         regex_based_plugins = {
             key: {}
-            for key in import_plugins(custom_plugin_paths=[])
+            for key in import_plugins(custom_plugin_paths=())
         }
         regex_based_plugins.update({
             'HexHighEntropyString': {
@@ -65,3 +65,22 @@ class TestPluginOptions:
         else:
             with pytest.raises(SystemExit):
                 parse_pre_commit_args_with_correct_prog(argument_string)
+
+    @pytest.mark.parametrize(
+        'argument_string,expected_value',
+        [
+            ('--custom-plugins testing', ('testing',)),
+            ('--custom-plugins does_not_exist', None),
+        ],
+    )
+    def test_custom_plugins(self, argument_string, expected_value):
+        if expected_value is not None:
+            args = parse_pre_commit_args_with_correct_prog(argument_string)
+
+            assert (
+                args.custom_plugin_paths
+                == expected_value
+            )
+        else:
+            with pytest.raises(SystemExit):
+                parse_pre_commit_args_with_correct_prog(argument_string)
diff --git a/tests/main_test.py b/tests/main_test.py
index 004817c..182e1bf 100644
--- a/tests/main_test.py
+++ b/tests/main_test.py
@@ -30,7 +30,7 @@ def get_list_of_plugins(include=None, exclude=None):
         ]
 
     output = []
-    for name, plugin in import_plugins(custom_plugin_paths=[]).items():
+    for name, plugin in import_plugins(custom_plugin_paths=()).items():
         if (
             name in included_plugins or
             exclude and name in exclude
@@ -59,7 +59,7 @@ def get_plugin_report(extra=None):
 
     longest_name_length = max([
         len(name)
-        for name in import_plugins(custom_plugin_paths=[])
+        for name in import_plugins(custom_plugin_paths=())
     ])
 
     return '\n'.join(
@@ -68,7 +68,7 @@ def get_plugin_report(extra=None):
                 name=name + ' ' * (longest_name_length - len(name)),
                 result='False' if name not in extra else extra[name],
             )
-            for name in import_plugins(custom_plugin_paths=[])
+            for name in import_plugins(custom_plugin_paths=())
         ]),
     ) + '\n'
 
@@ -84,7 +84,7 @@ class TestMain:
 
         mock_baseline_initialize.assert_called_once_with(
             plugins=Any(tuple),
-            custom_plugin_paths=Any(list),
+            custom_plugin_paths=Any(tuple),
             exclude_files_regex=None,
             exclude_lines_regex=None,
             path='.',
@@ -99,7 +99,7 @@ class TestMain:
 
         mock_baseline_initialize.assert_called_once_with(
             plugins=Any(tuple),
-            custom_plugin_paths=Any(list),
+            custom_plugin_paths=Any(tuple),
             exclude_files_regex=None,
             exclude_lines_regex=None,
             path=['test_data'],
@@ -116,7 +116,7 @@ class TestMain:
 
         mock_baseline_initialize.assert_called_once_with(
             plugins=Any(tuple),
-            custom_plugin_paths=Any(list),
+            custom_plugin_paths=Any(tuple),
             exclude_files_regex='some_pattern_here',
             exclude_lines_regex='other_patt',
             path='.',
@@ -183,7 +183,7 @@ class TestMain:
 
         mock_baseline_initialize.assert_called_once_with(
             plugins=Any(tuple),
-            custom_plugin_paths=Any(list),
+            custom_plugin_paths=Any(tuple),
             exclude_files_regex=None,
             exclude_lines_regex=None,
             path='.',
@@ -492,6 +492,7 @@ class TestMain:
             baseline = printer_shim.message
 
         baseline_dict = json.loads(baseline)
+        baseline_dict['custom_plugin_paths'] = tuple(baseline_dict['custom_plugin_paths'])
         with mock_stdin(), mock.patch(
             # To pipe in printer_shim
             'detect_secrets.core.audit._get_baseline_from_file',
@@ -559,6 +560,7 @@ class TestMain:
             baseline = printer_shim.message
 
         baseline_dict = json.loads(baseline)
+        baseline_dict['custom_plugin_paths'] = tuple(baseline_dict['custom_plugin_paths'])
         with mock.patch(
             'detect_secrets.core.audit._get_baseline_from_file',
             return_value=baseline_dict,
diff --git a/tests/plugins/common/initialize_test.py b/tests/plugins/common/initialize_test.py
index 6205a72..b9ef5d6 100644
--- a/tests/plugins/common/initialize_test.py
+++ b/tests/plugins/common/initialize_test.py
@@ -11,7 +11,7 @@ class TestFromPluginClassname:
     def test_success(self):
         plugin = initialize.from_plugin_classname(
             plugin_classname='HexHighEntropyString',
-            custom_plugin_paths=[],
+            custom_plugin_paths=(),
             hex_limit=4,
         )
 
@@ -27,7 +27,7 @@ class TestFromPluginClassname:
         with pytest.raises(TypeError):
             initialize.from_plugin_classname(
                 plugin_classname='NotABasePlugin',
-                custom_plugin_paths=[],
+                custom_plugin_paths=(),
             )
 
     def test_fails_on_bad_initialization(self):
@@ -40,7 +40,7 @@ class TestFromPluginClassname:
         ):
             initialize.from_plugin_classname(
                 plugin_classname='HexHighEntropyString',
-                custom_plugin_paths=[],
+                custom_plugin_paths=(),
                 hex_limit=4,
             )
 
@@ -62,7 +62,7 @@ class TestFromSecretType:
         plugin = initialize.from_secret_type(
             'Base64 High Entropy String',
             plugins_used=self.plugins_used,
-            custom_plugin_paths=[],
+            custom_plugin_paths=(),
         )
         # Dynamically imported classes have different
         # addresses for the same functions as statically
@@ -76,12 +76,12 @@ class TestFromSecretType:
         assert not initialize.from_secret_type(
             'some random secret_type',
             plugins_used=self.plugins_used,
-            custom_plugin_paths=[],
+            custom_plugin_paths=(),
         )
 
     def test_secret_type_not_in_settings(self):
         assert not initialize.from_secret_type(
             'Base64 High Entropy String',
             plugins_used=[],
-            custom_plugin_paths=[],
+            custom_plugin_paths=(),
         )

@KevinHock
Copy link
Collaborator Author

This is ready for review @OiCMudkips (no hurry though)

- 馃悕 Add add_shared_arguments() function in usage.py for DRYness
- 馃悰 Fix issue Yelp#242 via passing `should_verify_secrets=not args.no_verify` to `from_parser_builder` call
- 馃悰 Fix sorting issue in format_baseline_for_output() where --update and regular scan had different secret order
- 馃挴 All non-separated out files again :D
- 馃帗 Mention `--custom-plugins` in README
- 馃帗 Standardize NOTE -> Note
- 馃悰 Fix test pollution due to `all_plugins` cls attribute
- 馃悕 Change all relative imports to absolute, to avoid broken imports if someone copies an existing plugin to make a custom plugin
- 馃悰 Remove unused named argument in cloudant.py and keyword.py

馃檲 Hacks located in `def parse_args` of usage.py
@OiCMudkips OiCMudkips merged commit 7776f4b into Yelp:master Jul 9, 2020
@KevinHock
Copy link
Collaborator Author

\o/

Thanks!!!

@KevinHock KevinHock deleted the byop_feature branch July 9, 2020 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants