From 936c4402cda1337e0467435565c739559e7645f1 Mon Sep 17 00:00:00 2001 From: Abdel Thafeer Date: Wed, 6 Jul 2022 03:19:49 -0700 Subject: [PATCH 1/3] fix is_verified not stored in PotentialSecret --- detect_secrets/core/scan.py | 17 +-- detect_secrets/plugins/base.py | 21 ++++ .../plugins/high_entropy_strings.py | 9 +- detect_secrets/plugins/keyword.py | 3 + tests/filters/common_filter_test.py | 8 +- tests/plugins/base_test.py | 103 ++++++++++++++++++ 6 files changed, 152 insertions(+), 9 deletions(-) diff --git a/detect_secrets/core/scan.py b/detect_secrets/core/scan.py index b9e70e92b..0dfd57a45 100644 --- a/detect_secrets/core/scan.py +++ b/detect_secrets/core/scan.py @@ -17,6 +17,7 @@ from ..types import NamedIO from ..types import SelfAwareCallable from ..util import git +from ..util.code_snippet import CodeSnippet from ..util.code_snippet import get_code_snippet from ..util.inject import call_function_with_arguments from ..util.path import get_relative_path @@ -112,6 +113,7 @@ def scan_line(line: str) -> Generator[PotentialSecret, None, None]: 'detect_secrets.filters.common.is_invalid_file', ) get_filters.cache_clear() + context = get_code_snippet(lines=[line], line_number=1) yield from ( secret @@ -122,6 +124,7 @@ def scan_line(line: str) -> Generator[PotentialSecret, None, None]: line=line, line_number=0, enable_eager_search=True, + context=context, ) if not _is_filtered_out( required_filter_parameters=['context'], @@ -129,10 +132,7 @@ def scan_line(line: str) -> Generator[PotentialSecret, None, None]: secret=secret.secret_value, plugin=plugin, line=line, - context=get_code_snippet( - lines=[line], - line_number=1, - ), + context=context, ) ) @@ -225,10 +225,11 @@ def _scan_for_allowlisted_secrets_in_lines( line_numbers, lines = zip(*lines) line_content = [line.rstrip() for line in lines] for line_number, line in zip(line_numbers, line_content): + context = get_code_snippet(line_content, line_number) if not is_line_allowlisted( filename, line, - context=get_code_snippet(line_content, line_number), + context=context, ): continue @@ -236,7 +237,7 @@ def _scan_for_allowlisted_secrets_in_lines( continue for plugin in get_plugins(): - yield from _scan_line(plugin, filename, line, line_number) + yield from _scan_line(plugin, filename, line, line_number, context) def _get_lines_from_file(filename: str) -> Generator[List[str], None, None]: @@ -323,7 +324,7 @@ def _process_line_based_plugins( yield from ( secret for plugin in get_plugins() - for secret in _scan_line(plugin, filename, line, line_number) + for secret in _scan_line(plugin, filename, line, line_number, code_snippet) if not _is_filtered_out( required_filter_parameters=['context'], filename=secret.filename, @@ -340,6 +341,7 @@ def _scan_line( filename: str, line: str, line_number: int, + context: CodeSnippet, **kwargs: Any, ) -> Generator[PotentialSecret, None, None]: # NOTE: We don't apply filter functions here yet, because we don't have any filters @@ -349,6 +351,7 @@ def _scan_line( filename=filename, line=line, line_number=line_number, + context=context, **kwargs, ) if not secrets: diff --git a/detect_secrets/plugins/base.py b/detect_secrets/plugins/base.py index c78cc5d3d..835bc26b8 100644 --- a/detect_secrets/plugins/base.py +++ b/detect_secrets/plugins/base.py @@ -21,6 +21,8 @@ from ..constants import VerifiedResult from ..core.potential_secret import PotentialSecret from ..settings import get_settings +from detect_secrets.util.code_snippet import CodeSnippet +from detect_secrets.util.inject import call_function_with_arguments class BasePlugin(metaclass=ABCMeta): @@ -46,17 +48,36 @@ def analyze_line( filename: str, line: str, line_number: int = 0, + context: CodeSnippet = None, **kwargs: Any ) -> Set[PotentialSecret]: """This examines a line and finds all possible secret values in it.""" output = set() for match in self.analyze_string(line, **kwargs): # type: ignore + is_verified: bool = False + # If the filter is disabled it means --no-verify flag was passed + # We won't run verification in that case + if( + 'detect_secrets.filters.common.is_ignored_due_to_verification_policies' + in get_settings().filters + ): + try: + verified_result = call_function_with_arguments( + self.verify, + secret=match, + context=context, + ) + is_verified = True if verified_result == VerifiedResult.VERIFIED_TRUE else False + except requests.exceptions.RequestException: + is_verified = False + output.add( PotentialSecret( type=self.secret_type, filename=filename, secret=match, line_number=line_number, + is_verified=is_verified, ), ) diff --git a/detect_secrets/plugins/high_entropy_strings.py b/detect_secrets/plugins/high_entropy_strings.py index 280ca25bd..5a352cb19 100644 --- a/detect_secrets/plugins/high_entropy_strings.py +++ b/detect_secrets/plugins/high_entropy_strings.py @@ -11,6 +11,7 @@ from ..core.potential_secret import PotentialSecret from .base import BasePlugin +from detect_secrets.util.code_snippet import CodeSnippet class HighEntropyStringsPlugin(BasePlugin, metaclass=ABCMeta): @@ -45,10 +46,16 @@ def analyze_line( filename: str, line: str, line_number: int = 0, + context: CodeSnippet = None, enable_eager_search: bool = False, **kwargs: Any, ) -> Set[PotentialSecret]: - output = super().analyze_line(filename=filename, line=line, line_number=line_number) + output = super().analyze_line( + filename=filename, + line=line, + line_number=line_number, + context=context, + ) if output or not enable_eager_search: # NOTE: We perform the limit filter at this layer (rather than analyze_string) so # that we can surface secrets that do not meet the limit criteria when diff --git a/detect_secrets/plugins/keyword.py b/detect_secrets/plugins/keyword.py index 441c88ab1..d5ee43014 100644 --- a/detect_secrets/plugins/keyword.py +++ b/detect_secrets/plugins/keyword.py @@ -36,6 +36,7 @@ from ..util.filetype import determine_file_type from ..util.filetype import FileType from .base import BasePlugin +from detect_secrets.util.code_snippet import CodeSnippet # Note: All values here should be lowercase @@ -306,6 +307,7 @@ def analyze_line( filename: str, line: str, line_number: int = 0, + context: CodeSnippet = None, **kwargs: Any, ) -> Set[PotentialSecret]: filetype = determine_file_type(filename) @@ -314,6 +316,7 @@ def analyze_line( filename=filename, line=line, line_number=line_number, + context=context, denylist_regex_to_group=denylist_regex_to_group, ) diff --git a/tests/filters/common_filter_test.py b/tests/filters/common_filter_test.py index be6bc6032..e746db400 100644 --- a/tests/filters/common_filter_test.py +++ b/tests/filters/common_filter_test.py @@ -1,4 +1,5 @@ import re +from unittest import mock import pytest import requests @@ -44,7 +45,12 @@ def test_supports_injection_of_context(): # NOTE: This test case relies on the fact that this file contains a multi-factor # AWS KeyPair. with register_plugin(ContextAwareMockPlugin()): - main_module.main(['scan', 'test_data/each_secret.py']) + with mock.patch( + 'detect_secrets.plugins.aws.verify_aws_secret_access_key', + return_value=False, + ): + + main_module.main(['scan', 'test_data/each_secret.py']) @staticmethod def test_handles_request_error_gracefully(): diff --git a/tests/plugins/base_test.py b/tests/plugins/base_test.py index 3628bdfea..518cb3346 100644 --- a/tests/plugins/base_test.py +++ b/tests/plugins/base_test.py @@ -1,4 +1,14 @@ +from typing import Generator + +import pytest +import requests + +from detect_secrets.constants import VerifiedResult from detect_secrets.core.plugins.util import get_mapping_from_secret_type_to_class +from detect_secrets.plugins.base import BasePlugin +from detect_secrets.settings import get_settings +from detect_secrets.util.code_snippet import CodeSnippet +from detect_secrets.util.code_snippet import get_code_snippet def test_ensure_all_plugins_have_unique_secret_types(): @@ -7,3 +17,96 @@ def test_ensure_all_plugins_have_unique_secret_types(): secret_types.add(plugin_type.secret_type) assert len(secret_types) == len(get_mapping_from_secret_type_to_class()) + + +class MockPlugin(BasePlugin): + secret_type = 'MockPlugin' + + def __init__(self, verify_result: VerifiedResult): + self.verify_result = verify_result + self.verify_call_count = 0 + + def verify(self, secret: str, context: CodeSnippet) -> VerifiedResult: + self.verify_call_count += 1 + return self.verify_result + + def analyze_string(self, string: str) -> Generator[str, None, None]: + yield string + + +class MockExceptionRaisingPlugin(BasePlugin): + secret_type = 'MockExceptionRaisingPlugin' + + def analyze_string(self, string: str) -> Generator[str, None, None]: + yield string + + def verify(self, secret: str, context: CodeSnippet): + raise requests.exceptions.Timeout + + +class TestAnalyzeLine(): + def setup(self): + self.line = 'some-secret' + self.filename = 'secrets.py' + self.context = get_code_snippet(lines=[self.line], line_number=1) + + @pytest.mark.parametrize( + 'verified_result ,is_verified', + [ + (VerifiedResult.UNVERIFIED, False), + (VerifiedResult.VERIFIED_FALSE, False), + (VerifiedResult.VERIFIED_TRUE, True), + ], + ) + def test_potential_secret_constructed_correctly(self, verified_result, is_verified): + self._enable_filter() + plugin = MockPlugin(verified_result) + output = plugin.analyze_line( + filename=self.filename, + line=self.line, + line_number=1, + context=self.context, + ) + secret = list(output)[0] + assert secret.secret_value == self.line + assert secret.type == plugin.secret_type + assert secret.filename == self.filename + assert secret.line_number == 1 + assert secret.is_verified == is_verified + + def test_no_verification_call_if_verification_filter_is_disabled(self): + self._disable_filter() + plugin = MockPlugin(VerifiedResult.VERIFIED_TRUE) + output = plugin.analyze_line( + filename=self.filename, + line=self.line, + line_number=1, + context=self.context, + ) + secret = list(output)[0] + assert secret.is_verified is False + assert plugin.verify_call_count == 0 + + def test_handle_verify_exception_gracefully(self): + self._enable_filter() + plugin = MockExceptionRaisingPlugin() + output = plugin.analyze_line( + filename=self.filename, + line=self.line, + line_number=1, + context=self.context, + ) + secret = list(output)[0] + assert secret.is_verified is False + + def _enable_filter(self): + get_settings().filters[ + 'detect_secrets.filters.common.is_ignored_due_to_verification_policies' + ] = { + 'min_value': 0, + } + + def _disable_filter(self): + get_settings().disable_filters( + 'detect_secrets.filters.common.is_ignored_due_to_verification_policies', + ) From da42fccf07873510ac4d8622e881660ce82c3031 Mon Sep 17 00:00:00 2001 From: Abdel Thafeer Date: Mon, 11 Jul 2022 02:37:02 -0700 Subject: [PATCH 2/3] refactor code and fix test --- detect_secrets/core/scan.py | 20 ++++++++++++++++---- tests/main_test.py | 1 + 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/detect_secrets/core/scan.py b/detect_secrets/core/scan.py index 0dfd57a45..03bda2179 100644 --- a/detect_secrets/core/scan.py +++ b/detect_secrets/core/scan.py @@ -227,8 +227,8 @@ def _scan_for_allowlisted_secrets_in_lines( for line_number, line in zip(line_numbers, line_content): context = get_code_snippet(line_content, line_number) if not is_line_allowlisted( - filename, - line, + filename=filename, + line=line, context=context, ): continue @@ -237,7 +237,13 @@ def _scan_for_allowlisted_secrets_in_lines( continue for plugin in get_plugins(): - yield from _scan_line(plugin, filename, line, line_number, context) + yield from _scan_line( + plugin=plugin, + filename=filename, + line=line, + line_number=line_number, + context=context, + ) def _get_lines_from_file(filename: str) -> Generator[List[str], None, None]: @@ -324,7 +330,13 @@ def _process_line_based_plugins( yield from ( secret for plugin in get_plugins() - for secret in _scan_line(plugin, filename, line, line_number, code_snippet) + for secret in _scan_line( + plugin=plugin, + filename=filename, + line=line, + line_number=line_number, + context=code_snippet, + ) if not _is_filtered_out( required_filter_parameters=['context'], filename=secret.filename, diff --git a/tests/main_test.py b/tests/main_test.py index 106d0c4bf..be3abda83 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -84,6 +84,7 @@ def test_works_from_different_directory(): class TestSlimScan: @staticmethod def test_basic(): + os.environ['no_proxy'] = '*' with mock_printer(main_module) as printer: main_module.main(['scan', '--slim', 'test_data']) From a071ff1a5ef39925d449883109608461b82e17d0 Mon Sep 17 00:00:00 2001 From: Abdel Thafeer Date: Mon, 11 Jul 2022 05:11:13 -0700 Subject: [PATCH 3/3] move no_proxy to tox --- tests/main_test.py | 1 - tox.ini | 4 ++++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/main_test.py b/tests/main_test.py index be3abda83..106d0c4bf 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -84,7 +84,6 @@ def test_works_from_different_directory(): class TestSlimScan: @staticmethod def test_basic(): - os.environ['no_proxy'] = '*' with mock_printer(main_module) as printer: main_module.main(['scan', '--slim', 'test_data']) diff --git a/tox.ini b/tox.ini index 14d2c8439..7c0e4a9bb 100644 --- a/tox.ini +++ b/tox.ini @@ -7,6 +7,10 @@ tox_pip_extensions_ext_venv_update = true [testenv] passenv = SSH_AUTH_SOCK +# NO_PROXY is needed to call requests API within a forked process +# when using macOS and python version 3.6/3.7 +setenv = + NO_PROXY = '*' deps = -rrequirements-dev.txt whitelist_externals = coverage commands =