diff --git a/checkov/arm/graph_manager.py b/checkov/arm/graph_manager.py index e1ec830757..50e5a5b0bc 100644 --- a/checkov/arm/graph_manager.py +++ b/checkov/arm/graph_manager.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Optional from checkov.arm.graph_builder.local_graph import ArmLocalGraph from checkov.arm.utils import get_scannable_file_paths, get_files_definitions @@ -21,7 +21,7 @@ def build_graph_from_source_directory( local_graph_class: type[ArmLocalGraph] = ArmLocalGraph, render_variables: bool = False, parsing_errors: dict[str, Exception] | None = None, - download_external_modules: bool = False, + download_external_modules: Optional[bool] = False, excluded_paths: list[str] | None = None, **kwargs: Any, ) -> tuple[ArmLocalGraph, dict[str, dict[str, Any]]]: diff --git a/checkov/bicep/graph_manager.py b/checkov/bicep/graph_manager.py index ad1908d7de..b45a17ed25 100644 --- a/checkov/bicep/graph_manager.py +++ b/checkov/bicep/graph_manager.py @@ -1,7 +1,7 @@ from __future__ import annotations from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Optional from checkov.bicep.parser import Parser from checkov.bicep.utils import get_scannable_file_paths @@ -23,9 +23,9 @@ def build_graph_from_source_directory( source_dir: str, local_graph_class: type[BicepLocalGraph] = BicepLocalGraph, render_variables: bool = True, - parsing_errors: dict[str, Exception] | None = None, - download_external_modules: bool = False, - excluded_paths: list[str] | None = None, + parsing_errors: Optional[dict[str, Exception]] = None, + download_external_modules: Optional[bool] = False, + excluded_paths: Optional[list[str]] = None, **kwargs: Any, ) -> tuple[BicepLocalGraph, dict[Path, BicepJson]]: file_paths = get_scannable_file_paths(root_folder=source_dir) diff --git a/checkov/cloudformation/graph_manager.py b/checkov/cloudformation/graph_manager.py index 627b5d899d..090054e696 100644 --- a/checkov/cloudformation/graph_manager.py +++ b/checkov/cloudformation/graph_manager.py @@ -25,7 +25,7 @@ def build_graph_from_source_directory( local_graph_class: type[CloudformationLocalGraph] = CloudformationLocalGraph, render_variables: bool = True, parsing_errors: Optional[Dict[str, Exception]] = None, - download_external_modules: bool = False, + download_external_modules: Optional[bool] = False, excluded_paths: Optional[List[str]] = None, **kwargs: Any, ) -> Tuple[CloudformationLocalGraph, dict[str, dict[str, Any]]]: diff --git a/checkov/common/graph/graph_manager.py b/checkov/common/graph/graph_manager.py index b0e253f08e..7890b095db 100644 --- a/checkov/common/graph/graph_manager.py +++ b/checkov/common/graph/graph_manager.py @@ -1,7 +1,7 @@ from __future__ import annotations from abc import abstractmethod -from typing import Type, TYPE_CHECKING, TypeVar, Generic, Any +from typing import Type, TYPE_CHECKING, TypeVar, Generic, Any, Optional if TYPE_CHECKING: from checkov.common.graph.graph_builder.local_graph import LocalGraph # noqa @@ -24,9 +24,9 @@ def build_graph_from_source_directory( source_dir: str, local_graph_class: Type[_LocalGraph], render_variables: bool = True, - parsing_errors: dict[str, Exception] | None = None, - download_external_modules: bool = False, - excluded_paths: list[str] | None = None, + parsing_errors: Optional[dict[str, Exception]] = None, + download_external_modules: Optional[bool] = False, + excluded_paths: Optional[list[str]] = None, **kwargs: Any, ) -> tuple[_LocalGraph | None, _Definitions]: pass diff --git a/checkov/common/runners/graph_manager.py b/checkov/common/runners/graph_manager.py index 7852c8363e..6451f07cc9 100644 --- a/checkov/common/runners/graph_manager.py +++ b/checkov/common/runners/graph_manager.py @@ -1,7 +1,7 @@ from __future__ import annotations from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Optional from checkov.common.runners.graph_builder.local_graph import ObjectLocalGraph from checkov.common.graph.graph_manager import GraphManager @@ -20,9 +20,9 @@ def build_graph_from_source_directory( source_dir: str, local_graph_class: type[ObjectLocalGraph] = ObjectLocalGraph, render_variables: bool = True, - parsing_errors: dict[str, Exception] | None = None, - download_external_modules: bool = False, - excluded_paths: list[str] | None = None, + parsing_errors: Optional[dict[str, Exception]] = None, + download_external_modules: Optional[bool] = False, + excluded_paths: Optional[list[str]] = None, **kwargs: Any, ) -> tuple[ObjectLocalGraph, dict[str | Path, dict[str, Any] | list[dict[str, Any]]]]: definitions = local_graph_class.get_files_definitions(root_folder=source_dir) diff --git a/checkov/common/runners/runner_registry.py b/checkov/common/runners/runner_registry.py index 6b14853e54..c1f1d4a22d 100644 --- a/checkov/common/runners/runner_registry.py +++ b/checkov/common/runners/runner_registry.py @@ -730,7 +730,8 @@ def enrich_report_with_guidelines(scan_report: Report) -> None: @staticmethod def get_enriched_resources( - repo_roots: list[str | Path], download_external_modules: bool + repo_roots: list[str | Path], + download_external_modules: Optional[bool] ) -> dict[str, dict[str, Any]]: from checkov.terraform.modules.module_objects import TFDefinitionKey diff --git a/checkov/common/util/ext_argument_parser.py b/checkov/common/util/ext_argument_parser.py index d4d3d1c65b..fff06dfa6d 100644 --- a/checkov/common/util/ext_argument_parser.py +++ b/checkov/common/util/ext_argument_parser.py @@ -378,7 +378,7 @@ def add_parser_args(self) -> None: self.add( "--download-external-modules", help="download external terraform modules from public git repositories and terraform registry", - default=False, + default=None, env_var="DOWNLOAD_EXTERNAL_MODULES", ) self.add( diff --git a/checkov/common/util/type_forcers.py b/checkov/common/util/type_forcers.py index 7db5987920..92453051d9 100644 --- a/checkov/common/util/type_forcers.py +++ b/checkov/common/util/type_forcers.py @@ -4,7 +4,7 @@ import logging import typing from json import JSONDecodeError -from typing import TypeVar, overload, Any, Tuple, List +from typing import TypeVar, overload, Any, Tuple, List, Optional import yaml @@ -45,6 +45,12 @@ def force_float(var: Any) -> float | None: return None +def convert_str_to_optional_bool(s: Optional[bool | str]) -> Optional[bool]: + if s is None or (isinstance(s, str) and not s): + return None + return convert_str_to_bool(s) + + def convert_str_to_bool(bool_str: bool | str) -> bool: if isinstance(bool_str, str): bool_str_lower = bool_str.lower() diff --git a/checkov/dockerfile/graph_manager.py b/checkov/dockerfile/graph_manager.py index 0ccaf5ae13..431d9730ac 100644 --- a/checkov/dockerfile/graph_manager.py +++ b/checkov/dockerfile/graph_manager.py @@ -1,7 +1,7 @@ from __future__ import annotations import os -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Optional from checkov.common.graph.graph_builder.consts import GraphSource from checkov.common.graph.graph_manager import GraphManager @@ -22,9 +22,9 @@ def build_graph_from_source_directory( source_dir: str, local_graph_class: type[DockerfileLocalGraph] = DockerfileLocalGraph, render_variables: bool = True, - parsing_errors: dict[str, Exception] | None = None, - download_external_modules: bool = False, - excluded_paths: list[str] | None = None, + parsing_errors: Optional[dict[str, Exception]] = None, + download_external_modules: Optional[bool] = False, + excluded_paths: Optional[list[str]] = None, **kwargs: Any, ) -> tuple[DockerfileLocalGraph, dict[str, dict[str, list[_Instruction]]]]: file_paths = get_scannable_file_paths(root_folder=source_dir, excluded_paths=excluded_paths) diff --git a/checkov/kubernetes/graph_manager.py b/checkov/kubernetes/graph_manager.py index 3eaec23330..a0b046f406 100644 --- a/checkov/kubernetes/graph_manager.py +++ b/checkov/kubernetes/graph_manager.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import Any, TYPE_CHECKING +from typing import Any, TYPE_CHECKING, Optional from checkov.common.graph.graph_builder.consts import GraphSource from checkov.common.graph.graph_manager import GraphManager @@ -22,9 +22,9 @@ def build_graph_from_source_directory( source_dir: str, local_graph_class: type[KubernetesLocalGraph] = KubernetesLocalGraph, render_variables: bool = True, - parsing_errors: dict[str, Exception] | None = None, - download_external_modules: bool = False, - excluded_paths: list[str] | None = None, + parsing_errors: Optional[dict[str, Exception]] = None, + download_external_modules: Optional[bool] = False, + excluded_paths: Optional[list[str]] = None, **kwargs: Any, ) -> tuple[KubernetesLocalGraph, dict[str, list[dict[str, Any]]]]: definitions, definitions_raw = get_folder_definitions(source_dir, excluded_paths) diff --git a/checkov/main.py b/checkov/main.py index 1d382f867a..e8840434df 100755 --- a/checkov/main.py +++ b/checkov/main.py @@ -57,7 +57,7 @@ from checkov.common.util.config_utils import get_default_config_paths from checkov.common.util.ext_argument_parser import ExtArgumentParser, flatten_csv from checkov.common.util.runner_dependency_handler import RunnerDependencyHandler -from checkov.common.util.type_forcers import convert_str_to_bool +from checkov.common.util.type_forcers import convert_str_to_bool, convert_str_to_optional_bool from checkov.common.util.env_vars_config import env_vars_config from checkov.contributor_metrics import report_contributor_metrics from checkov.dockerfile.runner import Runner as dockerfile_runner @@ -326,7 +326,7 @@ def run(self, banner: str = checkov_banner, tool: str = default_tool, source_typ checks=self.config.check, skip_checks=self.config.skip_check, include_all_checkov_policies=self.config.include_all_checkov_policies, - download_external_modules=bool(convert_str_to_bool(self.config.download_external_modules)), + download_external_modules=convert_str_to_optional_bool(self.config.download_external_modules), external_modules_download_path=self.config.external_modules_download_path, evaluate_variables=bool(convert_str_to_bool(self.config.evaluate_variables)), runners=checkov_runners, diff --git a/checkov/runner_filter.py b/checkov/runner_filter.py index 8391169493..a7441a10ed 100644 --- a/checkov/runner_filter.py +++ b/checkov/runner_filter.py @@ -35,7 +35,7 @@ def __init__( checks: Union[str, List[str], None] = None, skip_checks: Union[str, List[str], None] = None, include_all_checkov_policies: bool = True, - download_external_modules: bool = False, + download_external_modules: Optional[bool] = False, external_modules_download_path: str = DEFAULT_EXTERNAL_MODULES_DIR, evaluate_variables: bool = True, runners: Optional[List[str]] = None, @@ -360,8 +360,6 @@ def from_dict(obj: Dict[str, Any]) -> RunnerFilter: if include_all_checkov_policies is None: include_all_checkov_policies = True download_external_modules = obj.get('download_external_modules') - if download_external_modules is None: - download_external_modules = False external_modules_download_path = obj.get('external_modules_download_path') if external_modules_download_path is None: external_modules_download_path = DEFAULT_EXTERNAL_MODULES_DIR diff --git a/checkov/serverless/graph_manager.py b/checkov/serverless/graph_manager.py index a59d349aa5..980595bba2 100644 --- a/checkov/serverless/graph_manager.py +++ b/checkov/serverless/graph_manager.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Optional from checkov.serverless.graph_builder.local_graph import ServerlessLocalGraph from checkov.common.graph.graph_builder.consts import GraphSource @@ -20,9 +20,9 @@ def build_graph_from_source_directory( source_dir: str, local_graph_class: type[ServerlessLocalGraph] = ServerlessLocalGraph, render_variables: bool = False, - parsing_errors: dict[str, Exception] | None = None, - download_external_modules: bool = False, - excluded_paths: list[str] | None = None, + parsing_errors: Optional[dict[str, Exception]] = None, + download_external_modules: Optional[bool] = False, + excluded_paths: Optional[list[str]] = None, **kwargs: Any, ) -> tuple[ServerlessLocalGraph, dict[str, dict[str, Any]]]: file_paths = get_scannable_file_paths(root_folder=source_dir, excluded_paths=excluded_paths) diff --git a/checkov/terraform/graph_manager.py b/checkov/terraform/graph_manager.py index aa30739ae7..1c6372f2eb 100644 --- a/checkov/terraform/graph_manager.py +++ b/checkov/terraform/graph_manager.py @@ -2,7 +2,7 @@ import logging import os -from typing import Type, Any, TYPE_CHECKING, overload +from typing import Type, Any, TYPE_CHECKING, overload, Optional from checkov.common.util.consts import DEFAULT_EXTERNAL_MODULES_DIR from checkov.terraform.graph_builder.local_graph import TerraformLocalGraph @@ -27,12 +27,12 @@ def build_multi_graph_from_source_directory( source_dir: str, local_graph_class: Type[TerraformLocalGraph] = TerraformLocalGraph, render_variables: bool = True, - parsing_errors: dict[str, Exception] | None = None, - download_external_modules: bool = False, - excluded_paths: list[str] | None = None, + parsing_errors: Optional[dict[str, Exception]] = None, + download_external_modules: Optional[bool] = False, + excluded_paths: Optional[list[str]] = None, external_modules_download_path: str = DEFAULT_EXTERNAL_MODULES_DIR, vars_files: list[str] | None = None, - external_modules_content_cache: dict[str, Any] | None = None, + external_modules_content_cache: Optional[dict[str, Any]] = None, ) -> tuple[list[tuple[TerraformLocalGraph, list[dict[TFDefinitionKey, dict[str, Any]]], str]], dict[str, str]]: logging.info("Parsing HCL files in source dir to multi graph") modules_with_definitions = self.parser.parse_multi_graph_hcl_module( @@ -63,9 +63,9 @@ def build_graph_from_source_directory( source_dir: str, local_graph_class: Type[TerraformLocalGraph] = TerraformLocalGraph, render_variables: bool = True, - parsing_errors: dict[str, Exception] | None = None, - download_external_modules: bool = False, - excluded_paths: list[str] | None = None, + parsing_errors: Optional[dict[str, Exception]] = None, + download_external_modules: Optional[bool] = False, + excluded_paths: Optional[list[str]] = None, **kwargs: Any, ) -> tuple[TerraformLocalGraph, dict[TFDefinitionKey, dict[str, Any]]]: logging.info("Parsing HCL files in source dir to graph") @@ -113,11 +113,11 @@ def build_multi_graph_from_definitions( self, definitions: dict[TFDefinitionKey, dict[str, Any]], render_variables: bool = True, - ) -> list[tuple[str | None, TerraformLocalGraph]]: + ) -> list[tuple[Optional[str], TerraformLocalGraph]]: module, tf_definitions = self.parser.parse_hcl_module_from_tf_definitions(definitions, "", self.source) dirs_to_definitions = self.parser.create_definition_by_dirs(tf_definitions) - graphs: list[tuple[str | None, TerraformLocalGraph]] = [] + graphs: list[tuple[Optional[str], TerraformLocalGraph]] = [] for source_path, dir_definitions in dirs_to_definitions.items(): module, parsed_tf_definitions = self.parser.parse_hcl_module_from_multi_tf_definitions(dir_definitions, source_path, self.source) local_graph = TerraformLocalGraph(module) diff --git a/checkov/terraform/module_loading/module_finder.py b/checkov/terraform/module_loading/module_finder.py index 68c374934f..aecb722fa9 100644 --- a/checkov/terraform/module_loading/module_finder.py +++ b/checkov/terraform/module_loading/module_finder.py @@ -143,10 +143,11 @@ def _download_module(ml_registry: ModuleLoaderRegistry, module_download: ModuleD tf_managed=module_download.tf_managed, ) if content is None or not content.loaded(): - log_message = f'Failed to download module {module_download.address}' - if not ml_registry.download_external_modules: - log_message += ' (for external modules, the --download-external-modules flag is required)' - logging.warning(log_message) + if ml_registry.download_external_modules is not False: + log_message = f'Failed to download module {module_download.address}' + if ml_registry.download_external_modules is None: + log_message += ' (for external modules, the --download-external-modules flag is required)' + logging.warning(log_message) return False except Exception as e: logging.warning(f"Unable to load module ({module_download.address}): {e}") diff --git a/checkov/terraform/module_loading/registry.py b/checkov/terraform/module_loading/registry.py index f3ae08795b..61604d3d5d 100644 --- a/checkov/terraform/module_loading/registry.py +++ b/checkov/terraform/module_loading/registry.py @@ -19,7 +19,9 @@ class ModuleLoaderRegistry: module_content_cache: Dict[str, Optional[ModuleContent]] = {} # noqa: CCE003 def __init__( - self, download_external_modules: bool = False, external_modules_folder_name: str = DEFAULT_EXTERNAL_MODULES_DIR + self, + download_external_modules: Optional[bool] = False, + external_modules_folder_name: str = DEFAULT_EXTERNAL_MODULES_DIR ) -> None: self.logger = logging.getLogger(__name__) add_resource_code_filter_to_logger(self.logger) diff --git a/checkov/terraform/tf_parser.py b/checkov/terraform/tf_parser.py index d95c707b75..30555659cf 100644 --- a/checkov/terraform/tf_parser.py +++ b/checkov/terraform/tf_parser.py @@ -59,7 +59,7 @@ def _init(self, directory: str, out_evaluations_context: Dict[TFDefinitionKey, Dict[str, EvaluationContext]] | None, out_parsing_errors: Dict[str, Exception] | None, env_vars: Mapping[str, str] | None, - download_external_modules: bool, + download_external_modules: Optional[bool], external_modules_download_path: str, excluded_paths: Optional[List[str]] = None, tf_var_files: Optional[List[str]] = None) -> None: @@ -92,7 +92,7 @@ def parse_directory( out_evaluations_context: Dict[TFDefinitionKey, Dict[str, EvaluationContext]] | None = None, out_parsing_errors: Dict[str, Exception] | None = None, env_vars: Mapping[str, str] | None = None, - download_external_modules: bool = False, + download_external_modules: Optional[bool] = False, external_modules_download_path: str = DEFAULT_EXTERNAL_MODULES_DIR, excluded_paths: Optional[List[str]] = None, vars_files: Optional[List[str]] = None, @@ -324,7 +324,7 @@ def parse_hcl_module( self, source_dir: str, source: str, - download_external_modules: bool = False, + download_external_modules: Optional[bool] = False, external_modules_download_path: str = DEFAULT_EXTERNAL_MODULES_DIR, parsing_errors: dict[str, Exception] | None = None, excluded_paths: list[str] | None = None, @@ -349,7 +349,7 @@ def parse_multi_graph_hcl_module( self, source_dir: str, source: str, - download_external_modules: bool = False, + download_external_modules: Optional[bool] = False, external_modules_download_path: str = DEFAULT_EXTERNAL_MODULES_DIR, parsing_errors: dict[str, Exception] | None = None, excluded_paths: list[str] | None = None, diff --git a/tests/terraform/module_loading/test_tf_module_finder.py b/tests/terraform/module_loading/test_tf_module_finder.py index 847ba62bbb..333416332e 100644 --- a/tests/terraform/module_loading/test_tf_module_finder.py +++ b/tests/terraform/module_loading/test_tf_module_finder.py @@ -1,11 +1,14 @@ import os import shutil import unittest +import logging from pathlib import Path from unittest import mock from checkov.common.util.consts import DEFAULT_EXTERNAL_MODULES_DIR from checkov.terraform.module_loading.module_finder import ( + ModuleDownload, + _download_module, find_modules, should_download, load_tf_modules, @@ -51,6 +54,30 @@ def test_downloader(self): self.assertEqual(len(distinct_roots), 1) +def test_dem_warning(caplog): + """ + Test that the --download-external-modules flag warning message is only + logged if the flag is not specified on the command line, and that + module download warnings are not logged if the flag is set to False. + """ + caplog.set_level(logging.WARNING) + module_loader_registry.download_external_modules = None + _download_module(module_loader_registry, ModuleDownload('xxx')) + assert 'Failed to download module' in caplog.text + assert '--download-external-modules flag' in caplog.text + caplog.clear() + + module_loader_registry.download_external_modules = True + _download_module(module_loader_registry, ModuleDownload('xxx')) + assert 'Failed to download module' in caplog.text + assert '--download-external-modules flag' not in caplog.text + caplog.clear() + + module_loader_registry.download_external_modules = False + _download_module(module_loader_registry, ModuleDownload('xxx')) + assert 'Failed to download module' not in caplog.text + assert '--download-external-modules flag' not in caplog.text + @mock.patch.dict(os.environ, {"CHECKOV_EXPERIMENTAL_TERRAFORM_MANAGED_MODULES": "True"}) def test_tf_managed_and_comment_out_modules(): # this test leverages the modules, which Terraform downloads on its own diff --git a/tests/test_main.py b/tests/test_main.py index 82f9e7b110..0271f80eeb 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -10,6 +10,7 @@ from checkov import main from checkov.common.runners.base_runner import BaseRunner from checkov.common.runners.runner_registry import RunnerRegistry +from checkov.common.util.type_forcers import convert_str_to_optional_bool from checkov.main import DEFAULT_RUNNERS, Checkov from checkov.runner_filter import RunnerFilter @@ -176,3 +177,15 @@ def test_run_without_custom_severity(): for report in ckv.scan_reports: assert report.failed_checks[0].check_id == "CUSTOM_WITHOUT_SEVERITY" assert not report.failed_checks[0].severity + +def test_optional_download_external_modules(): + args=[ + ['-d', '.', '--framework', 'all'], + ['-d', '.', '--framework', 'all', '--download-external-modules', 'true'], + ['-d', '.', '--framework', 'all', '--download-external-modules', 'false'] + ] + + assert convert_str_to_optional_bool(Checkov(argv=args[0]).config.download_external_modules) is None + assert convert_str_to_optional_bool(Checkov(argv=args[1]).config.download_external_modules) is True + assert convert_str_to_optional_bool(Checkov(argv=args[2]).config.download_external_modules) is False +