From 5c997901d49ca18e653c88080e37c0504ffcb55b Mon Sep 17 00:00:00 2001 From: Jerzy Kozera Date: Mon, 8 Oct 2018 17:21:08 +0200 Subject: [PATCH 1/5] Fix keyword plugin failing to find the secret in audit mode --- detect_secrets/core/audit.py | 7 +++++-- detect_secrets/plugins/keyword.py | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/detect_secrets/core/audit.py b/detect_secrets/core/audit.py index a1b553f34..d3c2bb3ab 100644 --- a/detect_secrets/core/audit.py +++ b/detect_secrets/core/audit.py @@ -324,11 +324,14 @@ def _highlight_secret(secret_line, secret_lineno, secret, filename, plugin_setti else: raise SecretNotFoundOnSpecifiedLineError(secret_lineno) - index_of_secret = secret_line.index(raw_secret) + index_of_secret = secret_line.lower().index(raw_secret.lower()) + end_of_secret = index_of_secret + len(raw_secret) return '{}{}{}'.format( secret_line[:index_of_secret], BashColor.color( - raw_secret, + # copy the secret out of the line because .lower() from secret + # generator may be different from the original value: + secret_line[index_of_secret:end_of_secret], Color.RED, ), secret_line[index_of_secret + len(raw_secret):], diff --git a/detect_secrets/plugins/keyword.py b/detect_secrets/plugins/keyword.py index cfe723d53..7f22c3eef 100644 --- a/detect_secrets/plugins/keyword.py +++ b/detect_secrets/plugins/keyword.py @@ -68,5 +68,5 @@ def analyze_string(self, string, line_num, filename): def secret_generator(self, string): for line in BLACKLIST: - if line in string: + if line.lower() in string.lower(): yield line From 02e93b8299095829e51193b8096cbc945a07fdb2 Mon Sep 17 00:00:00 2001 From: Jerzy Kozera Date: Mon, 8 Oct 2018 17:50:29 +0200 Subject: [PATCH 2/5] Fix handling of files without \n at end of file --- detect_secrets/core/audit.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/detect_secrets/core/audit.py b/detect_secrets/core/audit.py index d3c2bb3ab..49987b613 100644 --- a/detect_secrets/core/audit.py +++ b/detect_secrets/core/audit.py @@ -248,8 +248,12 @@ def _get_secret_with_context( if len(output) < end_line - start_line + 1: # This handles the case of a short file. num_lines_in_file = int(subprocess.check_output([ - 'wc', - '-l', + # https://stackoverflow.com/a/38870057 + # - 'wc -l' cannot be used here because if the last char + # of the file isn't \n, then the last line isn't counted + 'grep', + '-c', + '', filename, ]).decode('utf-8').split()[0]) From 081f3d8a580a734691f727f32c786d2c09236a4c Mon Sep 17 00:00:00 2001 From: Jerzy Kozera Date: Mon, 8 Oct 2018 18:04:19 +0200 Subject: [PATCH 3/5] Simplify the keyword plugin `.lower()` is now done in `secret_generator` so there is no need to do it twice. --- detect_secrets/plugins/keyword.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/detect_secrets/plugins/keyword.py b/detect_secrets/plugins/keyword.py index 7f22c3eef..f59af98a4 100644 --- a/detect_secrets/plugins/keyword.py +++ b/detect_secrets/plugins/keyword.py @@ -55,7 +55,7 @@ def analyze_string(self, string, line_num, filename): if WHITELIST_REGEX.search(string): return output - for identifier in self.secret_generator(string.lower()): + for identifier in self.secret_generator(string): secret = PotentialSecret( self.secret_type, filename, From 191bbb53667efeed0679ab4dadefc8b0ac6d97d5 Mon Sep 17 00:00:00 2001 From: Jerzy Kozera Date: Mon, 8 Oct 2018 22:05:07 +0200 Subject: [PATCH 4/5] Simplify and improve performance of the keyword plugin As per @domanchi comments for the PR #83, doing `lower()` once is enough, and should also be faster. --- detect_secrets/plugins/keyword.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/detect_secrets/plugins/keyword.py b/detect_secrets/plugins/keyword.py index f59af98a4..5ce4d461b 100644 --- a/detect_secrets/plugins/keyword.py +++ b/detect_secrets/plugins/keyword.py @@ -32,7 +32,9 @@ BLACKLIST = ( - 'PASS =', + # NOTE all values here should be lowercase, + # otherwise _secret_generator can fail to match them + 'pass =', 'password', 'passwd', 'pwd', @@ -66,7 +68,10 @@ def analyze_string(self, string, line_num, filename): return output - def secret_generator(self, string): + def _secret_generator(self, lowercase_string): for line in BLACKLIST: - if line.lower() in string.lower(): + if line in lowercase_string: yield line + + def secret_generator(self, string): + return self._secret_generator(string.lower()) From 01cde918f5f471cb0e03964db37f905e5bcdd1cf Mon Sep 17 00:00:00 2001 From: Jerzy Kozera Date: Mon, 8 Oct 2018 22:07:21 +0200 Subject: [PATCH 5/5] Add a test case for uppercase keyword --- tests/plugins/keyword_test.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/plugins/keyword_test.py b/tests/plugins/keyword_test.py index 5d72e32f5..94ddc648f 100644 --- a/tests/plugins/keyword_test.py +++ b/tests/plugins/keyword_test.py @@ -18,6 +18,9 @@ class TestKeywordDetector(object): ( 'token = "noentropy"' ), + ( + 'PASSWORD = "verysimple"' + ), ], ) def test_analyze(self, file_content): @@ -28,3 +31,5 @@ def test_analyze(self, file_content): assert len(output) == 1 for potential_secret in output: assert 'mock_filename' == potential_secret.filename + generated = list(logic.secret_generator(file_content)) + assert len(generated) == len(output)