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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(secrets): Add _prioritise_secrets by 3 levels of severity #6390

Merged
merged 2 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions checkov/secrets/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@
'Hex High Entropy String': 'CKV_SECRET_19'
}

ENTROPY_CHECK_IDS = ('CKV_SECRET_6', 'CKV_SECRET_19', 'CKV_SECRET_80')
ENTROPY_CHECK_IDS = {'CKV_SECRET_6', 'CKV_SECRET_19', 'CKV_SECRET_80'}
GENERIC_PRIVATE_KEY_CHECK_IDS = {'CKV_SECRET_10', 'CKV_SECRET_13'}

CHECK_ID_TO_SECRET_TYPE = {v: k for k, v in SECRET_TYPE_TO_ID.items()}

Expand Down Expand Up @@ -244,9 +245,8 @@ def run(
f"Removing secret due to UUID filtering: {hashlib.sha256(secret.secret_value.encode('utf-8')).hexdigest()}")
continue
if secret_key in secret_records.keys():
if secret_records[secret_key].check_id in ENTROPY_CHECK_IDS and check_id not in ENTROPY_CHECK_IDS:
secret_records.pop(secret_key)
else:
is_prioritise = self._prioritise_secrets(secret_records, secret_key, check_id)
if not is_prioritise:
continue
bc_check_id = metadata_integration.get_bc_id(check_id)
if bc_check_id in secret_suppressions_id:
Expand Down Expand Up @@ -319,6 +319,17 @@ def run(
self._modify_invalid_secrets_check_result_to_skipped(report)
return report

@staticmethod
def _prioritise_secrets(secret_records: Dict[str, SecretsRecord], secret_key: str, check_id: str) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def _prioritise_secrets(secret_records: Dict[str, SecretsRecord], secret_key: str, check_id: str) -> bool:
def _should_prioritise_secrets(secret_records: Dict[str, SecretsRecord], secret_key: str, check_id: str) -> bool:

either rename or change the return type to not be a boolean

if secret_records[secret_key].check_id in ENTROPY_CHECK_IDS and check_id not in ENTROPY_CHECK_IDS:
secret_records.pop(secret_key)
return True
if secret_records[secret_key].check_id in GENERIC_PRIVATE_KEY_CHECK_IDS:
if check_id not in GENERIC_PRIVATE_KEY_CHECK_IDS | ENTROPY_CHECK_IDS:
secret_records.pop(secret_key)
return True
return False

def cleanup_plugin_files(
self,
work_path: str,
Expand Down
54 changes: 54 additions & 0 deletions tests/secrets/test_prioritise_secrets.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import unittest

from checkov.common.models.enums import CheckResult
from checkov.common.output.secrets_record import SecretsRecord
from checkov.secrets.runner import Runner, ENTROPY_CHECK_IDS, GENERIC_PRIVATE_KEY_CHECK_IDS


class TestPrioritiseSecrets(unittest.TestCase):
def setUp(self):
self.secret_records = {
'key1': SecretsRecord(check_id='CKV_SECRET_6', check_name='foo',
check_result={"result": CheckResult.FAILED}, code_block=[(1, 'baz')],
file_path='qux', file_line_range=[1, 2], resource='resource', evaluations=None,
check_class='CheckClass', file_abs_path='abs_path'),
'key2': SecretsRecord(check_id='CKV_SECRET_10', check_name='foo',
check_result={"result": CheckResult.FAILED},
code_block=[(1, 'baz')], file_path='qux', file_line_range=[1, 2], resource='resource',
evaluations=None, check_class='CheckClass', file_abs_path='abs_path'),
'key3': SecretsRecord(check_id='CKV_SECRET_18', check_name='foo',
check_result={"result": CheckResult.FAILED}, code_block=[(1, 'baz')],
file_path='qux', file_line_range=[1, 2], resource='resource', evaluations=None,
check_class='CheckClass', file_abs_path='abs_path'),
}
self.ENTROPY_CHECK_IDS = ENTROPY_CHECK_IDS
self.GENERIC_PRIVATE_KEY_CHECK_IDS = GENERIC_PRIVATE_KEY_CHECK_IDS

def test_entropy_check_id_removed(self):
result = Runner._prioritise_secrets(self.secret_records, 'key1', 'CKV_SECRET_18')
self.assertTrue(result)
self.assertNotIn('key1', self.secret_records)

def test_generic_private_key_check_id_removed(self):
result = Runner._prioritise_secrets(self.secret_records, 'key2', 'CKV_SECRET_18')
self.assertTrue(result)
self.assertNotIn('key2', self.secret_records)

def test_no_removal_entropy_check_id(self):
result = Runner._prioritise_secrets(self.secret_records, 'key1', 'CKV_SECRET_6')
self.assertFalse(result)
self.assertIn('key1', self.secret_records)

def test_no_removal_generic_private_key_check_id(self):
result = Runner._prioritise_secrets(self.secret_records, 'key2', 'CKV_SECRET_10')
self.assertFalse(result)
self.assertIn('key2', self.secret_records)

def test_no_removal_other_check_id(self):
result = Runner._prioritise_secrets(self.secret_records, 'key3', 'CKV_SECRET_1000')
self.assertFalse(result)
self.assertIn('key3', self.secret_records)


if __name__ == '__main__':
unittest.main()
Loading