From 646c1e07b056e493b430fd5fca4d91bf63c31d73 Mon Sep 17 00:00:00 2001 From: Kevin Hock Date: Thu, 22 Aug 2019 13:19:42 -0700 Subject: [PATCH 1/6] :telescope: [Keyword Plugin] Require quotes for JS and Swift --- detect_secrets/plugins/common/filetype.py | 7 +++++-- detect_secrets/plugins/keyword.py | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/detect_secrets/plugins/common/filetype.py b/detect_secrets/plugins/common/filetype.py index 883ab52d1..73fd549db 100644 --- a/detect_secrets/plugins/common/filetype.py +++ b/detect_secrets/plugins/common/filetype.py @@ -8,8 +8,9 @@ class FileType(Enum): JAVASCRIPT = 3 PHP = 4 PYTHON = 5 - YAML = 6 - OTHER = 7 + SWIFT = 6 + YAML = 7 + OTHER = 8 def determine_file_type(filename): @@ -30,6 +31,8 @@ def determine_file_type(filename): return FileType.PHP elif filename.endswith('.py'): return FileType.PYTHON + elif filename.endswith('.swift'): + return FileType.SWIFT elif ( filename.endswith( ('.eyaml', '.yaml', '.yml'), diff --git a/detect_secrets/plugins/keyword.py b/detect_secrets/plugins/keyword.py index 5716c3381..ce54bd2d8 100644 --- a/detect_secrets/plugins/keyword.py +++ b/detect_secrets/plugins/keyword.py @@ -199,14 +199,16 @@ FOLLOWED_BY_QUOTES_AND_SEMICOLON_REGEX: 3, } GOLANG_DENYLIST_REGEX_TO_GROUP = { + FOLLOWED_BY_COLON_EQUAL_SIGNS_REGEX: 4, FOLLOWED_BY_EQUAL_SIGNS_REGEX: 4, FOLLOWED_BY_QUOTES_AND_SEMICOLON_REGEX: 3, - FOLLOWED_BY_COLON_EQUAL_SIGNS_REGEX: 4, } QUOTES_REQUIRED_FILETYPES = { FileType.CLS, FileType.JAVA, + FileType.JAVASCRIPT, FileType.PYTHON, + FileType.SWIFT, } From 85f0c74ce34216376053194f27991cbdc8c3f404 Mon Sep 17 00:00:00 2001 From: Kevin Hock Date: Thu, 22 Aug 2019 13:23:02 -0700 Subject: [PATCH 2/6] :telescope: [Keyword Plugin] Remove FP heuristics for JS These are no longer necessary as we made quotes required in the last commit. --- detect_secrets/plugins/keyword.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/detect_secrets/plugins/keyword.py b/detect_secrets/plugins/keyword.py index ce54bd2d8..699a9364e 100644 --- a/detect_secrets/plugins/keyword.py +++ b/detect_secrets/plugins/keyword.py @@ -281,14 +281,6 @@ def probably_false_positive(lowered_secret, filetype): or 'forgot' in lowered_secret or lowered_secret in FALSE_POSITIVES or ( - filetype == FileType.JAVASCRIPT - and ( - lowered_secret.startswith('this.') - or lowered_secret.startswith('fs.read') - or lowered_secret.startswith('options.') - or lowered_secret == 'new' - ) - ) or ( filetype == FileType.PHP and lowered_secret[0] == '$' ) or ( From 04c6d34c4a3fb8e8ef3dbf4d73192e49537de599 Mon Sep 17 00:00:00 2001 From: Kevin Hock Date: Thu, 22 Aug 2019 19:14:33 -0700 Subject: [PATCH 3/6] :telescope: [Keyword Plugin] Add custom regex for Obj-C - :snake: Alphabetize variables - :snake: Replace `elif` chain with a dict - :snake: Update QUOTES_REQUIRED_FILE_EXTENSIONS in the test --- detect_secrets/plugins/common/filetype.py | 51 +++++----- detect_secrets/plugins/keyword.py | 55 +++++++---- tests/plugins/keyword_test.py | 112 +++++++++++++++------- 3 files changed, 140 insertions(+), 78 deletions(-) diff --git a/detect_secrets/plugins/common/filetype.py b/detect_secrets/plugins/common/filetype.py index 73fd549db..abc3ee3bb 100644 --- a/detect_secrets/plugins/common/filetype.py +++ b/detect_secrets/plugins/common/filetype.py @@ -1,3 +1,4 @@ +import os from enum import Enum @@ -7,10 +8,26 @@ class FileType(Enum): JAVA = 2 JAVASCRIPT = 3 PHP = 4 - PYTHON = 5 - SWIFT = 6 - YAML = 7 - OTHER = 8 + OBJECTIVE_C = 5 + PYTHON = 6 + SWIFT = 7 + YAML = 8 + OTHER = 9 + + +EXTENSION_TO_FILETYPE = { + '.cls': FileType.CLS, + '.eyaml': FileType.YAML, + '.go': FileType.GO, + '.java': FileType.JAVA, + '.js': FileType.JAVASCRIPT, + '.m': FileType.OBJECTIVE_C, + '.php': FileType.PHP, + '.py': FileType.PYTHON, + '.swift': FileType.SWIFT, + '.yaml': FileType.YAML, + '.yml': FileType.YAML, +} def determine_file_type(filename): @@ -19,24 +36,8 @@ def determine_file_type(filename): :rtype: FileType """ - if filename.endswith('.cls'): - return FileType.CLS - elif filename.endswith('.go'): - return FileType.GO - elif filename.endswith('.java'): - return FileType.JAVA - elif filename.endswith('.js'): - return FileType.JAVASCRIPT - elif filename.endswith('.php'): - return FileType.PHP - elif filename.endswith('.py'): - return FileType.PYTHON - elif filename.endswith('.swift'): - return FileType.SWIFT - elif ( - filename.endswith( - ('.eyaml', '.yaml', '.yml'), - ) - ): - return FileType.YAML - return FileType.OTHER + _, file_extension = os.path.splitext(filename) + return EXTENSION_TO_FILETYPE.get( + file_extension, + FileType.OTHER, + ) diff --git a/detect_secrets/plugins/keyword.py b/detect_secrets/plugins/keyword.py index 699a9364e..d89f9ded9 100644 --- a/detect_secrets/plugins/keyword.py +++ b/detect_secrets/plugins/keyword.py @@ -118,15 +118,26 @@ 'true;', '{', } -QUOTE = r'[\'"]' -# includes ], ', " as closing +# Includes ], ', " as closing CLOSING = r'[]\'"]{0,2}' -# non-greedy match +DENYLIST_REGEX = r'|'.join(DENYLIST) +# Non-greedy match OPTIONAL_WHITESPACE = r'\s*?' OPTIONAL_NON_WHITESPACE = r'[^\s]*?' +QUOTE = r'[\'"]' SECRET = r'[^\s]+' -DENYLIST_REGEX = r'|'.join(DENYLIST) +SQUARE_BRACKETS = r'(\[\])' +FOLLOWED_BY_COLON_EQUAL_SIGNS_REGEX = re.compile( + # e.g. my_password := "bar" or my_password := bar + r'({denylist})({closing})?{whitespace}:=?{whitespace}({quote}?)({secret})(\3)'.format( + denylist=DENYLIST_REGEX, + closing=CLOSING, + quote=QUOTE, + whitespace=OPTIONAL_WHITESPACE, + secret=SECRET, + ), +) FOLLOWED_BY_COLON_REGEX = re.compile( # e.g. api_key: foo r'({denylist})({closing})?:{whitespace}({quote}?)({secret})(\3)'.format( @@ -147,6 +158,17 @@ secret=SECRET, ), ) +FOLLOWED_BY_EQUAL_SIGNS_OPTIONAL_BRACKETS_OPTIONAL_AT_SIGN_QUOTES_REQUIRED_REGEX = re.compile( + # e.g. my_password = "bar" + # e.g. my_password = @"bar" + # e.g. my_password[] = "bar"; + r'({denylist})({square_brackets})?{optional_whitespace}={optional_whitespace}(@)?(")({secret})(\5)'.format( # noqa: E501 + denylist=DENYLIST_REGEX, + square_brackets=SQUARE_BRACKETS, + optional_whitespace=OPTIONAL_WHITESPACE, + secret=SECRET, + ), +) FOLLOWED_BY_EQUAL_SIGNS_REGEX = re.compile( # e.g. my_password = bar r'({denylist})({closing})?{whitespace}={whitespace}({quote}?)({secret})(\3)'.format( @@ -178,31 +200,24 @@ secret=SECRET, ), ) -FOLLOWED_BY_COLON_EQUAL_SIGNS_REGEX = re.compile( - # e.g. my_password := "bar" or my_password := bar - r'({denylist})({closing})?{whitespace}:=?{whitespace}({quote}?)({secret})(\3)'.format( - denylist=DENYLIST_REGEX, - closing=CLOSING, - quote=QUOTE, - whitespace=OPTIONAL_WHITESPACE, - secret=SECRET, - ), -) DENYLIST_REGEX_TO_GROUP = { FOLLOWED_BY_COLON_REGEX: 4, FOLLOWED_BY_EQUAL_SIGNS_REGEX: 4, FOLLOWED_BY_QUOTES_AND_SEMICOLON_REGEX: 3, } -QUOTES_REQUIRED_DENYLIST_REGEX_TO_GROUP = { - FOLLOWED_BY_COLON_QUOTES_REQUIRED_REGEX: 5, - FOLLOWED_BY_EQUAL_SIGNS_QUOTES_REQUIRED_REGEX: 4, - FOLLOWED_BY_QUOTES_AND_SEMICOLON_REGEX: 3, -} GOLANG_DENYLIST_REGEX_TO_GROUP = { FOLLOWED_BY_COLON_EQUAL_SIGNS_REGEX: 4, FOLLOWED_BY_EQUAL_SIGNS_REGEX: 4, FOLLOWED_BY_QUOTES_AND_SEMICOLON_REGEX: 3, } +OBJECTIVE_C_DENYLIST_REGEX_TO_GROUP = { + FOLLOWED_BY_EQUAL_SIGNS_OPTIONAL_BRACKETS_OPTIONAL_AT_SIGN_QUOTES_REQUIRED_REGEX: 6, +} +QUOTES_REQUIRED_DENYLIST_REGEX_TO_GROUP = { + FOLLOWED_BY_COLON_QUOTES_REQUIRED_REGEX: 5, + FOLLOWED_BY_EQUAL_SIGNS_QUOTES_REQUIRED_REGEX: 4, + FOLLOWED_BY_QUOTES_AND_SEMICOLON_REGEX: 3, +} QUOTES_REQUIRED_FILETYPES = { FileType.CLS, FileType.JAVA, @@ -259,6 +274,8 @@ def secret_generator(self, string, filetype): denylist_regex_to_group = QUOTES_REQUIRED_DENYLIST_REGEX_TO_GROUP elif filetype == FileType.GO: denylist_regex_to_group = GOLANG_DENYLIST_REGEX_TO_GROUP + elif filetype == FileType.OBJECTIVE_C: + denylist_regex_to_group = OBJECTIVE_C_DENYLIST_REGEX_TO_GROUP else: denylist_regex_to_group = DENYLIST_REGEX_TO_GROUP diff --git a/tests/plugins/keyword_test.py b/tests/plugins/keyword_test.py index a7f2759cf..db5de456e 100644 --- a/tests/plugins/keyword_test.py +++ b/tests/plugins/keyword_test.py @@ -11,8 +11,41 @@ QUOTES_REQUIRED_FILE_EXTENSIONS = ( '.cls', '.java', + '.js', '.py', + '.swift', ) +FOLLOWED_BY_COLON_EQUAL_SIGNS_RE = { + 'negatives': { + 'quotes_required': [ + 'theapikey := ""', # Nothing in the quotes + 'theapikey := "somefakekey"', # 'fake' in the secret + ], + 'quotes_not_required': [ + 'theapikeyforfoo := hopenobodyfindsthisone', # Characters between apikey and := + ], + }, + 'positives': { + 'quotes_required': [ + 'apikey := "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey :="{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey := "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + "apikey := '{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", + "apikey :='{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", + 'apikey:= "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey:="{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + "apikey:= '{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", + "apikey:='{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", + "apikey:= '{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", + ], + 'quotes_not_required': [ + 'apikey := {{h}o)p${e]nob(ody[finds>-_$#thisone}}', + 'apikey :={{h}o)p${e]nob(ody[finds>-_$#thisone}}', + 'apikey:= {{h}o)p${e]nob(ody[finds>-_$#thisone}}', + 'apikey:={{h}o)p${e]nob(ody[finds>-_$#thisone}}', + ], + }, +} FOLLOWED_BY_COLON_RE = { 'negatives': { 'quotes_required': [ @@ -38,6 +71,26 @@ ], }, } +FOLLOWED_BY_EQUAL_SIGNS_OPTIONAL_BRACKETS_OPTIONAL_AT_SIGN_QUOTES_REQUIRED_REGEX = { + 'negatives': { + 'quotes_required': [ + 'theapikey[] = ""', # Nothing in the quotes + 'theapikey = @"somefakekey"', # 'fake' in the secret + ], + }, + 'positives': { + 'quotes_required': [ + 'apikey = "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey ="{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey = "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey = @"{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey =@"{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey = @"{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey[]= "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey[]="{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + ], + }, +} FOLLOWED_BY_EQUAL_SIGNS_RE = { 'negatives': { 'quotes_required': [ @@ -93,49 +146,21 @@ ], }, } -FOLLOWED_BY_COLON_EQUAL_SIGNS_RE = { - 'negatives': { - 'quotes_required': [ - 'theapikey := ""', # Nothing in the quotes - 'theapikey := "somefakekey"', # 'fake' in the secret - ], - 'quotes_not_required': [ - 'theapikeyforfoo := hopenobodyfindsthisone', # Characters between apikey and := - ], - }, - 'positives': { - 'quotes_required': [ - 'apikey := "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - 'apikey :="{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - 'apikey := "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - "apikey := '{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", - "apikey :='{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", - 'apikey:= "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - 'apikey:="{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - "apikey:= '{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", - "apikey:='{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", - "apikey:= '{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", - ], - 'quotes_not_required': [ - 'apikey := {{h}o)p${e]nob(ody[finds>-_$#thisone}}', - 'apikey :={{h}o)p${e]nob(ody[finds>-_$#thisone}}', - 'apikey:= {{h}o)p${e]nob(ody[finds>-_$#thisone}}', - 'apikey:={{h}o)p${e]nob(ody[finds>-_$#thisone}}', - ], - }, -} STANDARD_NEGATIVES = [] STANDARD_POSITIVES = [] STANDARD_NEGATIVES.extend( - FOLLOWED_BY_COLON_RE.get('negatives').get('quotes_required') + FOLLOWED_BY_COLON_EQUAL_SIGNS_RE.get('negatives').get('quotes_required') + + FOLLOWED_BY_COLON_EQUAL_SIGNS_RE.get('negatives').get('quotes_not_required') + + FOLLOWED_BY_COLON_RE.get('negatives').get('quotes_required') + FOLLOWED_BY_COLON_RE.get('negatives').get('quotes_not_required') + FOLLOWED_BY_EQUAL_SIGNS_RE.get('negatives').get('quotes_required') + FOLLOWED_BY_EQUAL_SIGNS_RE.get('negatives').get('quotes_not_required') + FOLLOWED_BY_QUOTES_AND_SEMICOLON_RE.get('negatives').get('quotes_required') - + FOLLOWED_BY_COLON_EQUAL_SIGNS_RE.get('negatives').get('quotes_required') - + FOLLOWED_BY_COLON_EQUAL_SIGNS_RE.get('negatives').get('quotes_not_required'), + + FOLLOWED_BY_EQUAL_SIGNS_OPTIONAL_BRACKETS_OPTIONAL_AT_SIGN_QUOTES_REQUIRED_REGEX.get( + 'negatives', + ).get('quotes_required'), ) STANDARD_POSITIVES.extend( FOLLOWED_BY_COLON_RE.get('positives').get('quotes_required') @@ -222,6 +247,25 @@ def test_analyze_go_positives(self, file_content): PotentialSecret.hash_secret('{{h}o)p${e]nob(ody[finds>-_$#thisone}}') ) + @pytest.mark.parametrize( + 'file_content', + FOLLOWED_BY_EQUAL_SIGNS_OPTIONAL_BRACKETS_OPTIONAL_AT_SIGN_QUOTES_REQUIRED_REGEX.get( + 'positives', + ).get('quotes_required'), + ) + def test_analyze_objective_c_positives(self, file_content): + logic = KeywordDetector() + + f = mock_file_object(file_content) + output = logic.analyze(f, 'mock_filename.m') + assert len(output) == 1 + for potential_secret in output: + assert 'mock_filename.m' == potential_secret.filename + assert ( + potential_secret.secret_hash == + PotentialSecret.hash_secret('{{h}o)p${e]nob(ody[finds>-_$#thisone}}') + ) + @pytest.mark.parametrize( 'file_content', STANDARD_NEGATIVES, From 2f2ccb0e68889e2a56bc1ba15bae85610452f5fc Mon Sep 17 00:00:00 2001 From: Kevin Hock Date: Fri, 23 Aug 2019 12:11:14 -0700 Subject: [PATCH 4/6] :telescope: [Keyword Plugin] Add format strings FP heuristic e.g. "secret": "{secret}" - Change keyword test to not have the positives start with { and end with } - :snake: Make .pyi files have FileType.PYTHON --- detect_secrets/plugins/common/filetype.py | 1 + detect_secrets/plugins/keyword.py | 4 + tests/plugins/keyword_test.py | 120 +++++++++++----------- 3 files changed, 66 insertions(+), 59 deletions(-) diff --git a/detect_secrets/plugins/common/filetype.py b/detect_secrets/plugins/common/filetype.py index abc3ee3bb..8b6a2dc49 100644 --- a/detect_secrets/plugins/common/filetype.py +++ b/detect_secrets/plugins/common/filetype.py @@ -24,6 +24,7 @@ class FileType(Enum): '.m': FileType.OBJECTIVE_C, '.php': FileType.PHP, '.py': FileType.PYTHON, + '.pyi': FileType.PYTHON, '.swift': FileType.SWIFT, '.yaml': FileType.YAML, '.yml': FileType.YAML, diff --git a/detect_secrets/plugins/keyword.py b/detect_secrets/plugins/keyword.py index d89f9ded9..67c4d99cf 100644 --- a/detect_secrets/plugins/keyword.py +++ b/detect_secrets/plugins/keyword.py @@ -297,7 +297,11 @@ def probably_false_positive(lowered_secret, filetype): 'fake' in lowered_secret or 'forgot' in lowered_secret or lowered_secret in FALSE_POSITIVES + # For e.g. "secret": "{secret}" or ( + lowered_secret[0] == '{' + and lowered_secret[-1] == '}' + ) or ( filetype == FileType.PHP and lowered_secret[0] == '$' ) or ( diff --git a/tests/plugins/keyword_test.py b/tests/plugins/keyword_test.py index db5de456e..375e1d3a1 100644 --- a/tests/plugins/keyword_test.py +++ b/tests/plugins/keyword_test.py @@ -8,13 +8,6 @@ from testing.mocks import mock_file_object -QUOTES_REQUIRED_FILE_EXTENSIONS = ( - '.cls', - '.java', - '.js', - '.py', - '.swift', -) FOLLOWED_BY_COLON_EQUAL_SIGNS_RE = { 'negatives': { 'quotes_required': [ @@ -27,22 +20,22 @@ }, 'positives': { 'quotes_required': [ - 'apikey := "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - 'apikey :="{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - 'apikey := "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - "apikey := '{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", - "apikey :='{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", - 'apikey:= "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - 'apikey:="{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - "apikey:= '{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", - "apikey:='{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", - "apikey:= '{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", + 'apikey := "m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey :="m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey := "m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + "apikey := 'm{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", + "apikey :='m{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", + 'apikey:= "m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey:="m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + "apikey:= 'm{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", + "apikey:='m{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", + "apikey:= 'm{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", ], 'quotes_not_required': [ - 'apikey := {{h}o)p${e]nob(ody[finds>-_$#thisone}}', - 'apikey :={{h}o)p${e]nob(ody[finds>-_$#thisone}}', - 'apikey:= {{h}o)p${e]nob(ody[finds>-_$#thisone}}', - 'apikey:={{h}o)p${e]nob(ody[finds>-_$#thisone}}', + 'apikey := m{{h}o)p${e]nob(ody[finds>-_$#thisone}}', + 'apikey :=m{{h}o)p${e]nob(ody[finds>-_$#thisone}}', + 'apikey:= m{{h}o)p${e]nob(ody[finds>-_$#thisone}}', + 'apikey:=m{{h}o)p${e]nob(ody[finds>-_$#thisone}}', ], }, } @@ -59,15 +52,15 @@ }, 'positives': { 'quotes_required': [ - "'theapikey': '{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", - '"theapikey": "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - 'apikey: "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - "apikey: '{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", + "'theapikey': 'm{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", + '"theapikey": "m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey: "m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + "apikey: 'm{{h}o)p${e]nob(ody[finds>-_$#thisone}}'", ], 'quotes_not_required': [ - 'apikey: {{h}o)p${e]nob(ody[finds>-_$#thisone}}', - 'apikey:{{h}o)p${e]nob(ody[finds>-_$#thisone}}', - 'theapikey:{{h}o)p${e]nob(ody[finds>-_$#thisone}}', + 'apikey: m{{h}o)p${e]nob(ody[finds>-_$#thisone}}', + 'apikey:m{{h}o)p${e]nob(ody[finds>-_$#thisone}}', + 'theapikey:m{{h}o)p${e]nob(ody[finds>-_$#thisone}}', ], }, } @@ -80,14 +73,14 @@ }, 'positives': { 'quotes_required': [ - 'apikey = "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - 'apikey ="{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - 'apikey = "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - 'apikey = @"{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - 'apikey =@"{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - 'apikey = @"{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - 'apikey[]= "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - 'apikey[]="{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey = "m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey ="m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey = "m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey = @"m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey =@"m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey = @"m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey[]= "m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'apikey[]="m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', ], }, } @@ -110,18 +103,18 @@ }, 'positives': { 'quotes_required': [ - 'some_dict["secret"] = "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', - 'the_password= "{{h}o)p${e]nob(ody[finds>-_$#thisone}}"\n', - 'the_password=\'{{h}o)p${e]nob(ody[finds>-_$#thisone}}\'\n', + 'some_dict["secret"] = "m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"', + 'the_password= "m{{h}o)p${e]nob(ody[finds>-_$#thisone}}"\n', + 'the_password=\'m{{h}o)p${e]nob(ody[finds>-_$#thisone}}\'\n', ], 'quotes_not_required': [ - "some_dict['secret'] = {{h}o)p${e]nob(ody[finds>-_$#thisone}}", - 'my_password={{h}o)p${e]nob(ody[finds>-_$#thisone}}', - 'my_password= {{h}o)p${e]nob(ody[finds>-_$#thisone}}', - 'my_password ={{h}o)p${e]nob(ody[finds>-_$#thisone}}', - 'my_password = {{h}o)p${e]nob(ody[finds>-_$#thisone}}', - 'my_password ={{h}o)p${e]nob(ody[finds>-_$#thisone}}', - 'the_password={{h}o)p${e]nob(ody[finds>-_$#thisone}}\n', + "some_dict['secret'] = m{{h}o)p${e]nob(ody[finds>-_$#thisone}}", + 'my_password=m{{h}o)p${e]nob(ody[finds>-_$#thisone}}', + 'my_password= m{{h}o)p${e]nob(ody[finds>-_$#thisone}}', + 'my_password =m{{h}o)p${e]nob(ody[finds>-_$#thisone}}', + 'my_password = m{{h}o)p${e]nob(ody[finds>-_$#thisone}}', + 'my_password =m{{h}o)p${e]nob(ody[finds>-_$#thisone}}', + 'the_password=m{{h}o)p${e]nob(ody[finds>-_$#thisone}}\n', ], }, } @@ -137,16 +130,24 @@ }, 'positives': { 'quotes_required': [ - 'apikey "{{h}o)p${e]nob(ody[finds>-_$#thisone}}";', # Double-quotes - 'fooapikeyfoo "{{h}o)p${e]nob(ody[finds>-_$#thisone}}";', # Double-quotes - 'fooapikeyfoo"{{h}o)p${e]nob(ody[finds>-_$#thisone}}";', # Double-quotes - 'private_key \'{{h}o)p${e]nob(ody[finds>-_$#thisone}}\';', # Single-quotes - 'fooprivate_keyfoo\'{{h}o)p${e]nob(ody[finds>-_$#thisone}}\';', # Single-quotes - 'fooprivate_key\'{{h}o)p${e]nob(ody[finds>-_$#thisone}}\';', # Single-quotes + 'apikey "m{{h}o)p${e]nob(ody[finds>-_$#thisone}}";', # Double-quotes + 'fooapikeyfoo "m{{h}o)p${e]nob(ody[finds>-_$#thisone}}";', # Double-quotes + 'fooapikeyfoo"m{{h}o)p${e]nob(ody[finds>-_$#thisone}}";', # Double-quotes + 'private_key \'m{{h}o)p${e]nob(ody[finds>-_$#thisone}}\';', # Single-quotes + 'fooprivate_keyfoo\'m{{h}o)p${e]nob(ody[finds>-_$#thisone}}\';', # Single-quotes + 'fooprivate_key\'m{{h}o)p${e]nob(ody[finds>-_$#thisone}}\';', # Single-quotes ], }, } +QUOTES_REQUIRED_FILE_EXTENSIONS = ( + '.cls', + '.java', + '.js', + '.py', + '.swift', +) + STANDARD_NEGATIVES = [] STANDARD_POSITIVES = [] @@ -187,7 +188,7 @@ def test_analyze_standard_positives(self, file_content): assert 'mock_filename' == potential_secret.filename assert ( potential_secret.secret_hash - == PotentialSecret.hash_secret('{{h}o)p${e]nob(ody[finds>-_$#thisone}}') + == PotentialSecret.hash_secret('m{{h}o)p${e]nob(ody[finds>-_$#thisone}}') ) @pytest.mark.parametrize( @@ -223,7 +224,7 @@ def test_analyze_quotes_required_positives(self, file_content, file_extension): assert mock_filename == potential_secret.filename assert ( potential_secret.secret_hash - == PotentialSecret.hash_secret('{{h}o)p${e]nob(ody[finds>-_$#thisone}}') + == PotentialSecret.hash_secret('m{{h}o)p${e]nob(ody[finds>-_$#thisone}}') ) @pytest.mark.parametrize( @@ -244,7 +245,7 @@ def test_analyze_go_positives(self, file_content): assert 'mock_filename.go' == potential_secret.filename assert ( potential_secret.secret_hash == - PotentialSecret.hash_secret('{{h}o)p${e]nob(ody[finds>-_$#thisone}}') + PotentialSecret.hash_secret('m{{h}o)p${e]nob(ody[finds>-_$#thisone}}') ) @pytest.mark.parametrize( @@ -263,7 +264,7 @@ def test_analyze_objective_c_positives(self, file_content): assert 'mock_filename.m' == potential_secret.filename assert ( potential_secret.secret_hash == - PotentialSecret.hash_secret('{{h}o)p${e]nob(ody[finds>-_$#thisone}}') + PotentialSecret.hash_secret('m{{h}o)p${e]nob(ody[finds>-_$#thisone}}') ) @pytest.mark.parametrize( @@ -341,8 +342,8 @@ def test_analyze_quotes_required_negatives(self, file_content, file_extension): @pytest.mark.parametrize( 'file_content, file_extension', ( - (yaml_negative, file_extension) - for yaml_negative in STANDARD_POSITIVES + (standard_positive, file_extension) + for standard_positive in STANDARD_POSITIVES for file_extension in ( '.yaml', '.yml', @@ -352,7 +353,8 @@ def test_analyze_quotes_required_negatives(self, file_content, file_extension): def test_analyze_yaml_negatives(self, file_content, file_extension): logic = KeywordDetector() - f = mock_file_object(file_content) + # Make it start with `{{`, (and end with `}}`) so it hits our false-positive check + f = mock_file_object(file_content.replace('m{', '{')) output = logic.analyze( f, 'mock_filename{}'.format(file_extension), From a53d12e65c450f5daea44290e7ba0cee744fe686 Mon Sep 17 00:00:00 2001 From: Kevin Hock Date: Fri, 23 Aug 2019 14:08:19 -0700 Subject: [PATCH 5/6] :telescope: [Keyword Plugin] Accuracy improvements - Add FP check for '/etc/' in secret - Add FP heuristic for secrets that look like directories - Add FP heuristic for .example files - Add more to FALSE_POSITIVES dict - Make all non-quotes required filetypes skip secrets starting with a $ - Make quotes required for Terraform files --- detect_secrets/plugins/common/filetype.py | 22 +++++++----- detect_secrets/plugins/keyword.py | 41 ++++++++++++++++++----- tests/plugins/keyword_test.py | 19 +++++++++++ 3 files changed, 65 insertions(+), 17 deletions(-) diff --git a/detect_secrets/plugins/common/filetype.py b/detect_secrets/plugins/common/filetype.py index 8b6a2dc49..687805b56 100644 --- a/detect_secrets/plugins/common/filetype.py +++ b/detect_secrets/plugins/common/filetype.py @@ -4,19 +4,22 @@ class FileType(Enum): CLS = 0 - GO = 1 - JAVA = 2 - JAVASCRIPT = 3 - PHP = 4 - OBJECTIVE_C = 5 - PYTHON = 6 - SWIFT = 7 - YAML = 8 - OTHER = 9 + EXAMPLE = 1 + GO = 2 + JAVA = 3 + JAVASCRIPT = 4 + PHP = 5 + OBJECTIVE_C = 6 + PYTHON = 7 + SWIFT = 8 + TERRAFORM = 9 + YAML = 10 + OTHER = 11 EXTENSION_TO_FILETYPE = { '.cls': FileType.CLS, + '.example': FileType.EXAMPLE, '.eyaml': FileType.YAML, '.go': FileType.GO, '.java': FileType.JAVA, @@ -26,6 +29,7 @@ class FileType(Enum): '.py': FileType.PYTHON, '.pyi': FileType.PYTHON, '.swift': FileType.SWIFT, + '.tf': FileType.TERRAFORM, '.yaml': FileType.YAML, '.yml': FileType.YAML, } diff --git a/detect_secrets/plugins/keyword.py b/detect_secrets/plugins/keyword.py index 67c4d99cf..97f0c243c 100644 --- a/detect_secrets/plugins/keyword.py +++ b/detect_secrets/plugins/keyword.py @@ -66,8 +66,9 @@ "'this", '(nsstring', '-default}', - '/etc/passwd:ro', '::', + '<%=', + '', '= 3 # For e.g. "secret": "{secret}" or ( lowered_secret[0] == '{' and lowered_secret[-1] == '}' ) or ( - filetype == FileType.PHP + filetype not in QUOTES_REQUIRED_FILETYPES and lowered_secret[0] == '$' ) or ( - filetype == FileType.YAML - and lowered_secret.startswith('{{') - and lowered_secret.endswith('}}') + filetype == FileType.EXAMPLE + and lowered_secret[0] == '<' + and lowered_secret[-1] == '>' ) ): return True diff --git a/tests/plugins/keyword_test.py b/tests/plugins/keyword_test.py index 375e1d3a1..3b7eb4bf0 100644 --- a/tests/plugins/keyword_test.py +++ b/tests/plugins/keyword_test.py @@ -124,6 +124,8 @@ 'private_key "";', # Nothing in the quotes 'private_key \'"no spaces\';', # Has whitespace in the secret 'private_key "fake";', # 'fake' in the secret + 'private_key "some/dir/aint/a/secret";', # 3 or more / + 'private_key "${FOO}";', # Starts with ${ and ends with } 'private_key "hopenobodyfindsthisone\';', # Double-quote does not match single-quote 'private_key \'hopenobodyfindsthisone";', # Single-quote does not match double-quote ], @@ -360,3 +362,20 @@ def test_analyze_yaml_negatives(self, file_content, file_extension): 'mock_filename{}'.format(file_extension), ) assert len(output) == 0 + + @pytest.mark.parametrize( + 'file_content', + STANDARD_POSITIVES, + ) + def test_analyze_example_negatives(self, file_content): + logic = KeywordDetector() + + # Make it start with `<`, (and end with `>`) so it hits our false-positive check + f = mock_file_object( + file_content.replace('m{', '<').replace('}', '>'), + ) + output = logic.analyze( + f, + 'mock_filename.example', + ) + assert len(output) == 0 From 5de43e278169ba8f040f6cf24b7b41e134d265fc Mon Sep 17 00:00:00 2001 From: Kevin Hock Date: Fri, 23 Aug 2019 16:33:02 -0700 Subject: [PATCH 6/6] :mortar_board: [Keyword Plugin] Comment FP directory heuristic --- detect_secrets/plugins/keyword.py | 1 + 1 file changed, 1 insertion(+) diff --git a/detect_secrets/plugins/keyword.py b/detect_secrets/plugins/keyword.py index 97f0c243c..e9cca6bc7 100644 --- a/detect_secrets/plugins/keyword.py +++ b/detect_secrets/plugins/keyword.py @@ -321,6 +321,7 @@ def probably_false_positive(lowered_secret, filetype): 'forgot', ) ) or lowered_secret in FALSE_POSITIVES + # For e.g. private_key "some/dir/that/is/not/a/secret"; or lowered_secret.count('/') >= 3 # For e.g. "secret": "{secret}" or (