From 851eadb31333b248e7f4129557cc82e3b1e72b87 Mon Sep 17 00:00:00 2001 From: inbalavital Date: Sun, 18 Aug 2024 11:59:31 +0300 Subject: [PATCH] fix(general): revert support multiple frameworks in one custom policy (#6664) --- .../features/custom_policies_integration.py | 15 ++-- checkov/common/checks_infra/registry.py | 30 ++----- checkov/common/graph/graph_builder/consts.py | 4 - .../test_custom_policies_integration.py | 83 +++---------------- 4 files changed, 25 insertions(+), 107 deletions(-) diff --git a/checkov/common/bridgecrew/integration_features/features/custom_policies_integration.py b/checkov/common/bridgecrew/integration_features/features/custom_policies_integration.py index ce80ce6d90f..269c1eb158d 100644 --- a/checkov/common/bridgecrew/integration_features/features/custom_policies_integration.py +++ b/checkov/common/bridgecrew/integration_features/features/custom_policies_integration.py @@ -11,7 +11,7 @@ from checkov.common.bridgecrew.platform_integration import bc_integration from checkov.common.bridgecrew.severities import Severities from checkov.common.checks_infra.checks_parser import GraphCheckParser -from checkov.common.checks_infra.registry import Registry, get_graph_checks_registry, get_all_graph_checks_registries +from checkov.common.checks_infra.registry import Registry, get_graph_checks_registry from checkov.common.util.data_structures_utils import pickle_deepcopy if TYPE_CHECKING: @@ -79,16 +79,17 @@ def pre_scan(self) -> None: if check.frameworks: for f in check.frameworks: if f.lower() == "cloudformation": - get_graph_checks_registry("cloudformation").custom_policies_checks.append(check) + get_graph_checks_registry("cloudformation").checks.append(check) elif f.lower() == "terraform": - get_graph_checks_registry("terraform").custom_policies_checks.append(check) + get_graph_checks_registry("terraform").checks.append(check) elif f.lower() == "kubernetes": - get_graph_checks_registry("kubernetes").custom_policies_checks.append(check) + get_graph_checks_registry("kubernetes").checks.append(check) elif f.lower() == "bicep": - get_graph_checks_registry("bicep").custom_policies_checks.append(check) + get_graph_checks_registry("bicep").checks.append(check) + elif re.match(CFN_RESOURCE_TYPE_IDENTIFIER, check.resource_types[0]): + get_graph_checks_registry("cloudformation").checks.append(check) else: - for registry in get_all_graph_checks_registries(): - registry.custom_policies_checks.append(check) + get_graph_checks_registry("terraform").checks.append(check) except Exception: logging.debug(f"Failed to load policy id: {policy.get('id')}", exc_info=True) logging.debug(f'Found {len(policies)} custom policies from the platform.') diff --git a/checkov/common/checks_infra/registry.py b/checkov/common/checks_infra/registry.py index 63337dbb957..3670e0f6102 100644 --- a/checkov/common/checks_infra/registry.py +++ b/checkov/common/checks_infra/registry.py @@ -11,7 +11,6 @@ from checkov.common.checks_infra.checks_parser import GraphCheckParser from checkov.common.graph.checks_infra.base_parser import BaseGraphCheckParser from checkov.common.graph.checks_infra.registry import BaseRegistry -from checkov.common.graph.graph_builder.consts import GraphSource from checkov.common.resource_code_logger_filter import add_resource_code_filter_to_logger from checkov.runner_filter import RunnerFilter from checkov.common.checks_infra.resources_types import resources_types @@ -21,10 +20,6 @@ CHECKS_POSSIBLE_ENDING = {".json", ".yaml", ".yml"} -GraphSupportedIACFrameworks = [GraphSource.TERRAFORM, GraphSource.CLOUDFORMATION, GraphSource.KUBERNETES, - GraphSource.TERRAFORM_PLAN, GraphSource.KUSTOMIZE, GraphSource.BICEP, - GraphSource.GITHUB_ACTION, GraphSource.HELM, GraphSource.ANSIBLE] - class Registry(BaseRegistry): def __init__(self, checks_dir: str, parser: BaseGraphCheckParser | None = None) -> None: @@ -32,7 +27,6 @@ def __init__(self, checks_dir: str, parser: BaseGraphCheckParser | None = None) super().__init__(parser) self.checks: list[BaseGraphCheck] = [] - self.custom_policies_checks: list[BaseGraphCheck] = [] self.checks_dir = checks_dir self.logger = logging.getLogger(__name__) add_resource_code_filter_to_logger(self.logger) @@ -43,7 +37,6 @@ def load_checks(self) -> None: return self._load_checks_from_dir(self.checks_dir, False) - self.checks += self.custom_policies_checks def _load_checks_from_dir(self, directory: str, external_check: bool) -> None: dir = os.path.expanduser(directory) @@ -72,8 +65,7 @@ def _load_checks_from_dir(self, directory: str, external_check: bool) -> None: continue check = self.parser.parse_raw_check( - check_json, resources_types=self._get_resource_types(check_json), - check_path=f'{root}/{file}' + check_json, resources_types=self._get_resource_types(check_json), check_path=f'{root}/{file}' ) if not any(c for c in self.checks if check.id == c.id): if external_check: @@ -93,22 +85,10 @@ def _get_resource_types(check_json: dict[str, dict[str, Any]]) -> list[str] | No _registry_instances: dict[str, Registry] = {} -def _initialize_registry(check_type: str) -> None: - _registry_instances[check_type] = Registry( - parser=GraphCheckParser(), - checks_dir=f"{Path(__file__).parent.parent.parent}/{check_type}/checks/graph_checks", - ) - - def get_graph_checks_registry(check_type: str) -> Registry: if not _registry_instances.get(check_type): - _initialize_registry(check_type) + _registry_instances[check_type] = Registry( + parser=GraphCheckParser(), + checks_dir=f"{Path(__file__).parent.parent.parent}/{check_type}/checks/graph_checks", + ) return _registry_instances[check_type] - - -def get_all_graph_checks_registries() -> list[Registry]: - graph_supported_iac_frameworks = [framework.value.lower() for framework in GraphSupportedIACFrameworks] - for framework in graph_supported_iac_frameworks: - if not _registry_instances.get(framework): - _initialize_registry(framework) - return list(_registry_instances[framework] for framework in graph_supported_iac_frameworks) diff --git a/checkov/common/graph/graph_builder/consts.py b/checkov/common/graph/graph_builder/consts.py index 3bd63043bd4..ea1e3719888 100644 --- a/checkov/common/graph/graph_builder/consts.py +++ b/checkov/common/graph/graph_builder/consts.py @@ -13,10 +13,6 @@ class GraphSource(str, Enum): GITHUB_ACTIONS = "GitHubActions" KUBERNETES = "Kubernetes" TERRAFORM = "Terraform" - TERRAFORM_PLAN = "terraform_plan" - KUSTOMIZE = "kustomize" - GITHUB_ACTION = "github_actions" - HELM = "helm" def __str__(self) -> str: # needed, because of a Python 3.11 change diff --git a/tests/common/integration_features/test_custom_policies_integration.py b/tests/common/integration_features/test_custom_policies_integration.py index 3f67c48bd79..b37ebf9ba96 100644 --- a/tests/common/integration_features/test_custom_policies_integration.py +++ b/tests/common/integration_features/test_custom_policies_integration.py @@ -7,7 +7,7 @@ CustomPoliciesIntegration from checkov.common.bridgecrew.platform_integration import BcPlatformIntegration from checkov.common.checks_infra.checks_parser import GraphCheckParser -from checkov.common.checks_infra.registry import Registry, get_all_graph_checks_registries, get_graph_checks_registry +from checkov.common.checks_infra.registry import Registry, get_graph_checks_registry from checkov.common.models.enums import CheckResult from checkov.common.output.record import Record from checkov.common.output.report import Report @@ -19,10 +19,10 @@ class TestCustomPoliciesIntegration(unittest.TestCase): def tearDown(self) -> None: - get_graph_checks_registry("cloudformation").custom_policies_checks = [] - get_graph_checks_registry("terraform").custom_policies_checks = [] - get_graph_checks_registry("kubernetes").custom_policies_checks = [] - get_graph_checks_registry("bicep").custom_policies_checks = [] + get_graph_checks_registry("cloudformation").checks = [] + get_graph_checks_registry("terraform").checks = [] + get_graph_checks_registry("kubernetes").checks = [] + get_graph_checks_registry("bicep").checks = [] def test_integration_valid(self): instance = BcPlatformIntegration() @@ -170,7 +170,7 @@ def test_policy_load(self): registry = Registry(parser=GraphCheckParser(), checks_dir=str( Path(__file__).parent.parent.parent.parent / "checkov" / "terraform" / "checks" / "graph_checks")) checks = [parser.parse_raw_check(CustomPoliciesIntegration._convert_raw_check(p)) for p in policies] - registry.custom_policies_checks = checks # simulate that the policy downloader will do + registry.checks = checks # simulate that the policy downloader will do tf_runner = TerraformRunner(external_registries=[registry]) cfn_runner = CFNRunner(external_registries=[registry]) @@ -216,10 +216,10 @@ def test_pre_scan_with_cloned_checks(self): instance.customer_run_config_response = mock_custom_policies_response() custom_policies_integration.pre_scan() - cfn_registry = get_graph_checks_registry("cloudformation").custom_policies_checks - tf_registry = get_graph_checks_registry("terraform").custom_policies_checks - k8s_registry = get_graph_checks_registry("kubernetes").custom_policies_checks - bicep_registry = get_graph_checks_registry("bicep").custom_policies_checks + cfn_registry = get_graph_checks_registry("cloudformation").checks + tf_registry = get_graph_checks_registry("terraform").checks + k8s_registry = get_graph_checks_registry("kubernetes").checks + bicep_registry = get_graph_checks_registry("bicep").checks self.assertEqual(1, len(custom_policies_integration.bc_cloned_checks)) self.assertEqual('kpande_AZR_1648821862291', tf_registry[0].id, cfn_registry[0].id) self.assertEqual('kpande_AZR_1648821862291', tf_registry[0].bc_id, cfn_registry[0].bc_id) @@ -228,25 +228,6 @@ def test_pre_scan_with_cloned_checks(self): self.assertEqual('kpande_bicep_1650378013212', bicep_registry[0].id) self.assertEqual('kpande_bicep_1650378013212', bicep_registry[0].bc_id) - def test_pre_scan_with_multiple_frameworks_graph_check(self): - instance = BcPlatformIntegration() - instance.skip_download = False - instance.platform_integration_configured = True - custom_policies_integration = CustomPoliciesIntegration(instance) - - instance.customer_run_config_response = mock_multiple_frameworks_custom_policy_response() - - custom_policies_integration.pre_scan() - bicep_registry_custom_policies_checks = get_graph_checks_registry("bicep").custom_policies_checks - all_graph_checks = get_all_graph_checks_registries() - for registry in all_graph_checks: - multiple_frameworks_custom_policy_exist = False - for check in registry.custom_policies_checks: - if check.bc_id == 'multiple_frameworks_policy_1625063607541': - multiple_frameworks_custom_policy_exist = True - self.assertEqual(True, multiple_frameworks_custom_policy_exist) - self.assertEqual(2, len(bicep_registry_custom_policies_checks)) - def test_post_runner_with_cloned_checks(self): instance = BcPlatformIntegration() instance.skip_download = False @@ -493,8 +474,8 @@ def test_policy_load_with_resources_types_as_str(self): Path(__file__).parent.parent.parent.parent / "checkov" / "terraform" / "checks" / "graph_checks")) checks = [parser.parse_raw_check(CustomPoliciesIntegration._convert_raw_check(p)) for p in policies] registry.checks = checks # simulate that the policy downloader will do - - + + def mock_custom_policies_response(): return { "customPolicies": [ @@ -575,45 +556,5 @@ def mock_custom_policies_response(): } -def mock_multiple_frameworks_custom_policy_response(): - return { - "customPolicies": [ - { - "id": "kpande_bicep_1650378013212", - "code": "{\"operator\":\"exists\",\"attribute\":\"spec.runAsUser.rule\",\"cond_type\":\"attribute\"," - "\"resource_types\":[\"PodSecurityPolicy\"]}", - "title": "bicep policy", - "guideline": "meaningful guideline for bicep policy", - "severity": "HIGH", - "pcSeverity": None, - "category": "bicep", - "pcPolicyId": None, - "additionalPcPolicyIds": None, - "sourceIncidentId": None, - "benchmarks": {}, - "frameworks": [ - "bicep" - ] - }, - { - "id": "multiple_frameworks_policy_1625063607541", - "title": "multiple frameworks policy", - "code": "{\"and\":[{\"operator\":\"exists\",\"cond_type\":\"connection\",\"resource_types\":[" - "\"azurerm_subnet_network_security_group_association\"],\"connected_resource_types\":[" - "\"azurerm_subnet\",\"azurerm_network_security_group\"]},{\"value\":[\"azurerm_subnet\"]," - "\"operator\":\"within\",\"attribute\":\"resource_type\",\"cond_type\":\"filter\"}]}", - "severity": "CRITICAL", - "category": "General", - "frameworks": [], - "resourceTypes": ["aws_s3_bucket", "PodSecurityPolicy"], - "guideline": "multiple_frameworks_policy_1625063607541", - "benchmarks": {}, - "createdBy": "mike+policies@bridgecrew.io", - "sourceIncidentId": None - } - ] - } - - if __name__ == '__main__': unittest.main()