Add secret line number (#722)

* add line number

* add reporting line number

* fix unit testing

* fix error message

* change script output

* fix mypy errors

* add changelog

CHANGELOG.md

@@ -1,4 +1,5 @@
 # Changelog
+* Added line number to secrets' path in **secrets** command report.
 * Fixed an issue where **init** a community pack did not present the valid support URL.
 * Fixed an issue where **init** offered a non relevant pack support type.
 * Fixed an issue where **lint** did not pull docker images for powershell.

demisto_sdk/commands/secrets/secrets.py

@@ -3,6 +3,8 @@
 import math
 import os
 import string
+from collections import defaultdict
+from typing import DefaultDict
 
 import PyPDF2
 from bs4 import BeautifulSoup
@@ -75,20 +77,23 @@ def __init__(
         self.ignore_entropy = ignore_entropy
 
     def get_secrets(self, branch_name, is_circle):
-        secrets_found = {}
+        secret_to_location_mapping = {}
         if self.input_paths:
             secrets_file_paths = self.input_paths
         else:
             secrets_file_paths = self.get_all_diff_text_files(branch_name, is_circle)
         # If a input path supplied, should not run on git. If not supplied make sure not in middle of merge.
         if not run_command('git rev-parse -q --verify MERGE_HEAD') or self.input_paths:
-            secrets_found = self.search_potential_secrets(secrets_file_paths, self.ignore_entropy)
-            if secrets_found:
+            secret_to_location_mapping = self.search_potential_secrets(secrets_file_paths, self.ignore_entropy)
+            if secret_to_location_mapping:
                 secrets_found_string = 'Secrets were found in the following files:'
-                for file_name in secrets_found:
-                    secrets_found_string += ('\n\nIn File: ' + file_name + '\n')
-                    secrets_found_string += '\nThe following expressions were marked as secrets: \n'
-                    secrets_found_string += self.reformat_secrets_output(secrets_found[file_name])
+                for file_name in secret_to_location_mapping:
+                    for line in sorted(secret_to_location_mapping[file_name]):
+                        secrets_found_string += ('\nIn File: ' + f'{file_name}:{line}' + '\n')
+                        if len(secret_to_location_mapping[file_name][line]) == 1:
+                            secrets_found_string += f'Secret found: {secret_to_location_mapping[file_name][line][0]}\n'
+                        else:
+                            secrets_found_string += f'Secrets found: {secret_to_location_mapping[file_name][line]}\n'
 
                 if not is_circle:
                     secrets_found_string += '\n\nRemove or whitelist secrets in order to proceed, then re-commit\n'
@@ -100,7 +105,7 @@ def get_secrets(self, branch_name, is_circle):
                 secrets_found_string += 'For more information about whitelisting visit: ' \
                                         'https://github.com/demisto/internal-content/tree/master/documentation/secrets'
                 print_error(secrets_found_string)
-        return secrets_found
+        return secret_to_location_mapping
 
     def reformat_secrets_output(self, secrets_list):
         """
@@ -159,7 +164,7 @@ def search_potential_secrets(self, secrets_file_paths: list, ignore_entropy: boo
 
         :return: dictionary(filename: (list)secrets) of strings sorted by file name for secrets found in files
         """
-        secrets_found = {}
+        secret_to_location_mapping: DefaultDict[str, defaultdict] = defaultdict(lambda: defaultdict(list))
         for file_path in secrets_file_paths:
             # Get if file path in pack and pack name
             is_pack = is_file_path_in_pack(file_path)
@@ -173,8 +178,6 @@ def search_potential_secrets(self, secrets_file_paths: list, ignore_entropy: boo
                 continue
             # Init vars for current loop
             file_name = os.path.basename(file_path)
-            high_entropy_strings = []
-            secrets_found_with_regex = []
             _, file_extension = os.path.splitext(file_path)
             skip_secrets = {'skip_once': False, 'skip_multi': False}
             # get file contents
@@ -189,7 +192,7 @@ def search_potential_secrets(self, secrets_file_paths: list, ignore_entropy: boo
                 temp_white_list = self.create_temp_white_list(yml_file_contents if yml_file_contents else file_contents)
                 secrets_white_list = secrets_white_list.union(temp_white_list)
             # Search by lines after strings with high entropy / IoCs regex as possibly suspicious
-            for line in file_contents.split('\n'):
+            for line_num, line in enumerate(file_contents.split('\n')):
                 # if detected disable-secrets comments, skip the line/s
                 skip_secrets = self.is_secrets_disabled(line, skip_secrets)
                 if skip_secrets['skip_once'] or skip_secrets['skip_multi']:
@@ -199,7 +202,7 @@ def search_potential_secrets(self, secrets_file_paths: list, ignore_entropy: boo
                 regex_secrets, false_positives = self.regex_for_secrets(line)
                 for regex_secret in regex_secrets:
                     if not any(ioc.lower() in regex_secret.lower() for ioc in ioc_white_list):
-                        secrets_found_with_regex.append(regex_secret)
+                        secret_to_location_mapping[file_path][line_num + 1].append(regex_secret)
                 # added false positives into white list array before testing the strings in line
 
                 secrets_white_list = secrets_white_list.union(false_positives)
@@ -219,14 +222,9 @@ def search_potential_secrets(self, secrets_file_paths: list, ignore_entropy: boo
 
                             entropy = self.calculate_shannon_entropy(string_)
                             if entropy >= ENTROPY_THRESHOLD:
-                                high_entropy_strings.append(string_)
-
-            if high_entropy_strings or secrets_found_with_regex:
-                # uniquify identical matches between lists
-                file_secrets = list(set(high_entropy_strings + secrets_found_with_regex))
-                secrets_found[file_path] = file_secrets
+                                secret_to_location_mapping[file_path][line_num + 1].append(string_)
 
-        return secrets_found
+        return secret_to_location_mapping
 
     @staticmethod
     def remove_whitelisted_items_from_file(file_content: str, secrets_white_list: set) -> str:

demisto_sdk/commands/secrets/tests/secrets_test.py

@@ -74,8 +74,8 @@ def test_is_text_file(self):
         assert is_txt is True
 
     def test_search_potential_secrets__no_secrets_found(self):
-        secrets_found = self.validator.search_potential_secrets([self.TEST_YML_FILE])
-        assert not secrets_found
+        secret_to_location = self.validator.search_potential_secrets([self.TEST_YML_FILE])
+        assert not secret_to_location
 
     def test_search_potential_secrets__secrets_found(self):
         create_empty_whitelist_secrets_file(os.path.join(TestSecrets.TEMP_DIR, TestSecrets.WHITE_LIST_FILE_NAME))
@@ -100,7 +100,7 @@ def test_search_potential_secrets__secrets_found(self):
             ''')
 
         secrets_found = validator.search_potential_secrets([self.TEST_FILE_WITH_SECRETS])
-        assert secrets_found[self.TEST_FILE_WITH_SECRETS] == ['OIifdsnsjkgnj3254nkdfsjKNJD0345']
+        assert secrets_found[self.TEST_FILE_WITH_SECRETS] == {7: ['OIifdsnsjkgnj3254nkdfsjKNJD0345']}
 
     def test_ignore_entropy(self):
         """
@@ -136,7 +136,7 @@ def test_ignore_entropy(self):
             ''')
 
         secrets_found = validator.search_potential_secrets([self.TEST_FILE_WITH_SECRETS], True)
-        assert secrets_found[self.TEST_FILE_WITH_SECRETS] == ['fooo@someorg.com']
+        assert secrets_found[self.TEST_FILE_WITH_SECRETS] == {4: ['fooo@someorg.com']}
 
     def test_two_files_with_same_name(self):
         """
@@ -171,8 +171,8 @@ def test_two_files_with_same_name(self):
 
 ''')
         secrets_found = validator.search_potential_secrets([file1_path, file2_path], True)
-        assert secrets_found[os.path.join(dir1_path, file_name)] == ['fooo@someorg.com']
-        assert secrets_found[os.path.join(dir2_path, file_name)] == ['fooo@someorg.com']
+        assert secrets_found[os.path.join(dir1_path, file_name)] == {4: ['fooo@someorg.com']}
+        assert secrets_found[os.path.join(dir2_path, file_name)] == {4: ['fooo@someorg.com']}
 
     def test_remove_white_list_regex(self):
         white_list = '155.165.45.232'