From 10370b6a78fb9488499cc5665776dacba3d7bab3 Mon Sep 17 00:00:00 2001 From: Omri Yoffe Date: Sun, 24 Nov 2024 13:30:38 +0200 Subject: [PATCH 01/12] add arm breadcrumbs --- checkov/arm/runner.py | 15 ++++++++++++--- checkov/arm/utils.py | 5 +++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/checkov/arm/runner.py b/checkov/arm/runner.py index e95915fa456..cd3b36aa322 100644 --- a/checkov/arm/runner.py +++ b/checkov/arm/runner.py @@ -3,6 +3,7 @@ import logging import os from collections.abc import Iterable +from pathlib import Path from typing import TYPE_CHECKING, Any, cast from typing_extensions import TypeAlias # noqa[TC002] @@ -11,11 +12,12 @@ from checkov.arm.graph_builder.local_graph import ArmLocalGraph from checkov.arm.graph_manager import ArmGraphManager from checkov.arm.registry import arm_resource_registry, arm_parameter_registry -from checkov.arm.utils import get_scannable_file_paths, get_files_definitions, ARM_POSSIBLE_ENDINGS, ArmElements +from checkov.arm.utils import get_scannable_file_paths, get_files_definitions, ARM_POSSIBLE_ENDINGS, ArmElements, clean_file_path from checkov.common.checks_infra.registry import get_graph_checks_registry from checkov.common.graph.graph_builder import CustomAttributes from checkov.common.graph.graph_builder.consts import GraphSource from checkov.common.output.extra_resource import ExtraResource +from checkov.common.output.graph_record import GraphRecord from checkov.common.output.record import Record from checkov.common.output.report import Report from checkov.common.bridgecrew.check_type import CheckType @@ -263,7 +265,7 @@ def add_graph_check_results(self, report: Report, runner_filter: RunnerFilter) - for check, check_results in graph_checks_results.items(): for check_result in check_results: entity = check_result["entity"] - entity_file_path: str = entity[CustomAttributes.FILE_PATH] + entity_file_path = entity[CustomAttributes.FILE_PATH] start_line = entity[START_LINE] - 1 end_line = entity[END_LINE] - 1 @@ -272,7 +274,7 @@ def add_graph_check_results(self, report: Report, runner_filter: RunnerFilter) - check=check, check_result=check_result, code_block=self.definitions_raw[entity_file_path][start_line:end_line], - file_path=entity_file_path, + file_path=self.extract_file_path_from_abs_path(clean_file_path(entity_file_path)), file_abs_path=os.path.abspath(entity_file_path), file_line_range=[start_line - 1, end_line - 1], resource_id=entity[CustomAttributes.ID], @@ -304,5 +306,12 @@ def build_record( file_abs_path=file_abs_path, severity=check.severity, ) + if self.breadcrumbs: + breadcrumb = self.breadcrumbs.get(record.file_path, {}).get(record.resource) + if breadcrumb: + record = GraphRecord(record, breadcrumb) record.set_guideline(check.guideline) report.add_record(record=record) + + def extract_file_path_from_abs_path(self, path: Path) -> str: + return f"/{os.path.relpath(path, self.root_folder)}" diff --git a/checkov/arm/utils.py b/checkov/arm/utils.py index 2d3ac8f8388..46309a0b802 100644 --- a/checkov/arm/utils.py +++ b/checkov/arm/utils.py @@ -115,3 +115,8 @@ def extract_resource_name_from_reference_func(reference: str) -> str: def clean_string(input: str) -> str: return input.replace("'", '').replace(" ", "") + +def clean_file_path(file_path: Path) -> Path: + path_parts = [part for part in file_path.parts if part not in (".", "..")] + + return Path(*path_parts) From 8b012674e3d3f5d7766f35d54f7c1220a5976ea2 Mon Sep 17 00:00:00 2001 From: Omri Yoffe Date: Sun, 24 Nov 2024 13:58:03 +0200 Subject: [PATCH 02/12] fix file paths --- checkov/arm/runner.py | 9 +++++---- checkov/arm/utils.py | 1 + 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/checkov/arm/runner.py b/checkov/arm/runner.py index cd3b36aa322..983e317f9bb 100644 --- a/checkov/arm/runner.py +++ b/checkov/arm/runner.py @@ -140,6 +140,7 @@ def add_python_check_results(self, report: Report, runner_filter: RunnerFilter, path_to_convert = (os.path.join(root_folder, arm_file)) if root_folder else arm_file file_abs_path = os.path.abspath(path_to_convert) + cleaned_path = clean_file_path(Path(arm_file)) if isinstance(self.definitions[arm_file], dict): arm_context_parser = ContextParser(arm_file, self.definitions[arm_file], self.definitions_raw[arm_file]) @@ -197,7 +198,7 @@ def add_python_check_results(self, report: Report, runner_filter: RunnerFilter, check_name=check.name, check_result=check_result, code_block=entity_code_lines, - file_path=arm_file, + file_path=self.extract_file_path_from_abs_path(cleaned_path), file_line_range=entity_lines_range, resource=resource_id, evaluations=variable_evaluations, @@ -212,7 +213,7 @@ def add_python_check_results(self, report: Report, runner_filter: RunnerFilter, report.extra_resources.add( ExtraResource( file_abs_path=file_abs_path, - file_path=arm_file, + file_path=self.extract_file_path_from_abs_path(cleaned_path), resource=resource_id, ) ) @@ -247,7 +248,7 @@ def add_python_check_results(self, report: Report, runner_filter: RunnerFilter, check=check, check_result=check_result, code_block=censored_code_lines, - file_path=arm_file, + file_path=self.extract_file_path_from_abs_path(cleaned_path), file_abs_path=file_abs_path, file_line_range=entity_lines_range, resource_id=resource_id, @@ -314,4 +315,4 @@ def build_record( report.add_record(record=record) def extract_file_path_from_abs_path(self, path: Path) -> str: - return f"/{os.path.relpath(path, self.root_folder)}" + return f"${os.path.sep}{os.path.relpath(path, self.root_folder)}" diff --git a/checkov/arm/utils.py b/checkov/arm/utils.py index 46309a0b802..ec2dac1eb35 100644 --- a/checkov/arm/utils.py +++ b/checkov/arm/utils.py @@ -116,6 +116,7 @@ def extract_resource_name_from_reference_func(reference: str) -> str: def clean_string(input: str) -> str: return input.replace("'", '').replace(" ", "") + def clean_file_path(file_path: Path) -> Path: path_parts = [part for part in file_path.parts if part not in (".", "..")] From 16013f957fe2942460a6bb36af0b76f5874749ff Mon Sep 17 00:00:00 2001 From: Omri Yoffe Date: Sun, 24 Nov 2024 14:52:07 +0200 Subject: [PATCH 03/12] fix paths --- checkov/arm/runner.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/checkov/arm/runner.py b/checkov/arm/runner.py index 983e317f9bb..7f7151ebc49 100644 --- a/checkov/arm/runner.py +++ b/checkov/arm/runner.py @@ -140,7 +140,6 @@ def add_python_check_results(self, report: Report, runner_filter: RunnerFilter, path_to_convert = (os.path.join(root_folder, arm_file)) if root_folder else arm_file file_abs_path = os.path.abspath(path_to_convert) - cleaned_path = clean_file_path(Path(arm_file)) if isinstance(self.definitions[arm_file], dict): arm_context_parser = ContextParser(arm_file, self.definitions[arm_file], self.definitions_raw[arm_file]) @@ -198,7 +197,7 @@ def add_python_check_results(self, report: Report, runner_filter: RunnerFilter, check_name=check.name, check_result=check_result, code_block=entity_code_lines, - file_path=self.extract_file_path_from_abs_path(cleaned_path), + file_path=arm_file, file_line_range=entity_lines_range, resource=resource_id, evaluations=variable_evaluations, @@ -213,7 +212,7 @@ def add_python_check_results(self, report: Report, runner_filter: RunnerFilter, report.extra_resources.add( ExtraResource( file_abs_path=file_abs_path, - file_path=self.extract_file_path_from_abs_path(cleaned_path), + file_path=arm_file, resource=resource_id, ) ) @@ -315,4 +314,4 @@ def build_record( report.add_record(record=record) def extract_file_path_from_abs_path(self, path: Path) -> str: - return f"${os.path.sep}{os.path.relpath(path, self.root_folder)}" + return f"{os.path.sep}{os.path.relpath(path, self.root_folder)}" From 22d76ff83977c2aa20180057efd9235fbcdfaa24 Mon Sep 17 00:00:00 2001 From: Omri Yoffe Date: Sun, 24 Nov 2024 14:55:30 +0200 Subject: [PATCH 04/12] fix --- checkov/arm/runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkov/arm/runner.py b/checkov/arm/runner.py index 7f7151ebc49..430ff009e23 100644 --- a/checkov/arm/runner.py +++ b/checkov/arm/runner.py @@ -247,7 +247,7 @@ def add_python_check_results(self, report: Report, runner_filter: RunnerFilter, check=check, check_result=check_result, code_block=censored_code_lines, - file_path=self.extract_file_path_from_abs_path(cleaned_path), + file_path=arm_file, file_abs_path=file_abs_path, file_line_range=entity_lines_range, resource_id=resource_id, From aaf474397809813bb99121b73c6408a24824237c Mon Sep 17 00:00:00 2001 From: Omri Yoffe Date: Sun, 24 Nov 2024 15:06:57 +0200 Subject: [PATCH 05/12] path --- checkov/arm/runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkov/arm/runner.py b/checkov/arm/runner.py index 430ff009e23..52b23c8492c 100644 --- a/checkov/arm/runner.py +++ b/checkov/arm/runner.py @@ -274,7 +274,7 @@ def add_graph_check_results(self, report: Report, runner_filter: RunnerFilter) - check=check, check_result=check_result, code_block=self.definitions_raw[entity_file_path][start_line:end_line], - file_path=self.extract_file_path_from_abs_path(clean_file_path(entity_file_path)), + file_path=self.extract_file_path_from_abs_path(clean_file_path(Path(entity_file_path))), file_abs_path=os.path.abspath(entity_file_path), file_line_range=[start_line - 1, end_line - 1], resource_id=entity[CustomAttributes.ID], From a1647935f30877f71b65a4ac67f7b9308cfcc3cd Mon Sep 17 00:00:00 2001 From: Omri Yoffe Date: Sun, 24 Nov 2024 18:57:26 +0200 Subject: [PATCH 06/12] remove name fn --- checkov/arm/utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/checkov/arm/utils.py b/checkov/arm/utils.py index ec2dac1eb35..c926eba11fb 100644 --- a/checkov/arm/utils.py +++ b/checkov/arm/utils.py @@ -55,7 +55,6 @@ def create_definitions( if root_folder: file_paths = get_scannable_file_paths(root_folder, runner_filter.excluded_paths) - filepath_fn = lambda f: f"/{os.path.relpath(f, os.path.commonprefix((root_folder, f)))}" definitions, definitions_raw, parsing_errors = get_files_definitions(files=file_paths, filepath_fn=filepath_fn) if parsing_errors: From f1e4263ff75aae71210e8e91faf3f91ea40a04a9 Mon Sep 17 00:00:00 2001 From: Omri Yoffe Date: Sun, 24 Nov 2024 19:04:28 +0200 Subject: [PATCH 07/12] remove name fn --- checkov/arm/graph_manager.py | 3 +-- checkov/arm/runner.py | 4 +--- checkov/arm/utils.py | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/checkov/arm/graph_manager.py b/checkov/arm/graph_manager.py index e2475869ce2..72c96c388b2 100644 --- a/checkov/arm/graph_manager.py +++ b/checkov/arm/graph_manager.py @@ -26,8 +26,7 @@ def build_graph_from_source_directory( excluded_paths: list[str] | None = None, ) -> tuple[ArmLocalGraph, dict[str, dict[str, Any]]]: file_paths = get_scannable_file_paths(root_folder=source_dir, excluded_paths=excluded_paths) - filepath_fn = lambda f: f"/{os.path.relpath(f, os.path.commonprefix((source_dir, f)))}" - definitions, _, _ = get_files_definitions(files=file_paths, filepath_fn=filepath_fn) + definitions, _, _ = get_files_definitions(files=file_paths) local_graph = self.build_graph_from_definitions(definitions=definitions) diff --git a/checkov/arm/runner.py b/checkov/arm/runner.py index 52b23c8492c..6e80251cfda 100644 --- a/checkov/arm/runner.py +++ b/checkov/arm/runner.py @@ -79,7 +79,6 @@ def run( report = Report(self.check_type) if not self.context or not self.definitions: files_list: "Iterable[str]" = [] - filepath_fn = None if external_checks_dir: for directory in external_checks_dir: arm_resource_registry.load_external_checks(directory) @@ -91,12 +90,11 @@ def run( files_list = files.copy() if root_folder: - filepath_fn = lambda f: f"/{os.path.relpath(f, os.path.commonprefix((root_folder, f)))}" self.root_folder = root_folder files_list = get_scannable_file_paths(root_folder=root_folder, excluded_paths=runner_filter.excluded_paths) - self.definitions, self.definitions_raw, parsing_errors = get_files_definitions(files_list, filepath_fn) + self.definitions, self.definitions_raw, parsing_errors = get_files_definitions(files_list) self.context = build_definitions_context(definitions=self.definitions, definitions_raw=self.definitions_raw) report.add_parsing_errors(parsing_errors) diff --git a/checkov/arm/utils.py b/checkov/arm/utils.py index c926eba11fb..0cd8bf99a3d 100644 --- a/checkov/arm/utils.py +++ b/checkov/arm/utils.py @@ -55,7 +55,7 @@ def create_definitions( if root_folder: file_paths = get_scannable_file_paths(root_folder, runner_filter.excluded_paths) - definitions, definitions_raw, parsing_errors = get_files_definitions(files=file_paths, filepath_fn=filepath_fn) + definitions, definitions_raw, parsing_errors = get_files_definitions(files=file_paths) if parsing_errors: logging.warning(f"[arm] found errors while parsing definitions: {parsing_errors}") From f5e075eebc765a24d4822b405f5945af32f72274 Mon Sep 17 00:00:00 2001 From: Omri Yoffe Date: Mon, 25 Nov 2024 09:41:44 +0200 Subject: [PATCH 08/12] fix unit test --- checkov/arm/runner.py | 12 ++---------- tests/arm/runner/test_runner.py | 6 +++--- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/checkov/arm/runner.py b/checkov/arm/runner.py index 6e80251cfda..65bf9bce651 100644 --- a/checkov/arm/runner.py +++ b/checkov/arm/runner.py @@ -128,16 +128,8 @@ def add_python_check_results(self, report: Report, runner_filter: RunnerFilter, for arm_file in self.definitions.keys(): self.pbar.set_additional_data({"Current File Scanned": os.path.relpath(arm_file, root_folder)}) - # There are a few cases here. If -f was used, there could be a leading / because it's an absolute path, - # or there will be no leading slash; root_folder will always be none. - # If -d is used, root_folder will be the value given, and -f will start with a / (hardcoded above). - # The goal here is simply to get a valid path to the file (which arm_file does not always give). - if arm_file[0] == "/": - path_to_convert = (root_folder + arm_file) if root_folder else arm_file - else: - path_to_convert = (os.path.join(root_folder, arm_file)) if root_folder else arm_file - - file_abs_path = os.path.abspath(path_to_convert) + + file_abs_path = os.path.abspath(arm_file) if isinstance(self.definitions[arm_file], dict): arm_context_parser = ContextParser(arm_file, self.definitions[arm_file], self.definitions_raw[arm_file]) diff --git a/tests/arm/runner/test_runner.py b/tests/arm/runner/test_runner.py index 754d80c6871..f7e7a710d76 100644 --- a/tests/arm/runner/test_runner.py +++ b/tests/arm/runner/test_runner.py @@ -66,7 +66,7 @@ def test_record_relative_path_with_relative_dir(self): self.assertGreater(len(all_checks), 0) # ensure that the assertions below are going to do something for record in all_checks: # no need to join with a '/' because the CFN runner adds it to the start of the file path - self.assertEqual(record.repo_file_path, f'/{dir_rel_path}{record.file_path}') + self.assertEqual(record.repo_file_path, f'/{record.file_path}') def test_record_relative_path_with_abs_dir(self): @@ -88,8 +88,8 @@ def test_record_relative_path_with_abs_dir(self): all_checks = report.failed_checks + report.passed_checks self.assertGreater(len(all_checks), 0) # ensure that the assertions below are going to do something for record in all_checks: - # no need to join with a '/' because the CFN runner adds it to the start of the file path - self.assertEqual(record.repo_file_path, f'/{dir_rel_path}{record.file_path}') + file_name = record.file_path.split('/')[-1] + self.assertEqual(record.repo_file_path, f'/{dir_rel_path}/{file_name}') def test_record_relative_path_with_relative_file(self): From a3da04d31d600bf88744eabe5e448cd21dbafd42 Mon Sep 17 00:00:00 2001 From: Omri Yoffe Date: Mon, 25 Nov 2024 09:42:14 +0200 Subject: [PATCH 09/12] remove deps --- checkov/arm/graph_manager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/checkov/arm/graph_manager.py b/checkov/arm/graph_manager.py index 72c96c388b2..feb7b5189b3 100644 --- a/checkov/arm/graph_manager.py +++ b/checkov/arm/graph_manager.py @@ -1,6 +1,5 @@ from __future__ import annotations -import os from typing import TYPE_CHECKING, Any from checkov.arm.graph_builder.local_graph import ArmLocalGraph From d49edb71a497d3720a8f435fd7434d96ed4a5ce8 Mon Sep 17 00:00:00 2001 From: Omri Yoffe Date: Mon, 25 Nov 2024 10:03:19 +0200 Subject: [PATCH 10/12] empty commit --- tests/arm/runner/test_runner.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/arm/runner/test_runner.py b/tests/arm/runner/test_runner.py index f7e7a710d76..8547366d023 100644 --- a/tests/arm/runner/test_runner.py +++ b/tests/arm/runner/test_runner.py @@ -92,7 +92,6 @@ def test_record_relative_path_with_abs_dir(self): self.assertEqual(record.repo_file_path, f'/{dir_rel_path}/{file_name}') def test_record_relative_path_with_relative_file(self): - # test whether the record's repo_file_path is correct, relative to the CWD (with a / at the start). # this is just constructing the scan dir as normal From 2dad48a18a380ff4d9c46cc2c1932fb909796f0c Mon Sep 17 00:00:00 2001 From: Omri Yoffe Date: Tue, 26 Nov 2024 13:19:03 +0200 Subject: [PATCH 11/12] rename resource --- checkov/arm/runner.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/checkov/arm/runner.py b/checkov/arm/runner.py index 65bf9bce651..ee4187d6961 100644 --- a/checkov/arm/runner.py +++ b/checkov/arm/runner.py @@ -161,7 +161,9 @@ def add_python_check_results(self, report: Report, runner_filter: RunnerFilter, logging.debug(f"Could not determine 'resource_id' of Resource {resource}") continue - report.add_resource(f"{arm_file}:{resource_id}") + cleaned_path = clean_file_path(Path(arm_file)) + + report.add_resource(f"{cleaned_path}:{resource_id}") entity_lines_range, entity_code_lines = arm_context_parser.extract_arm_resource_code_lines( resource ) @@ -187,7 +189,7 @@ def add_python_check_results(self, report: Report, runner_filter: RunnerFilter, check_name=check.name, check_result=check_result, code_block=entity_code_lines, - file_path=arm_file, + file_path=self.extract_file_path_from_abs_path(cleaned_path), file_line_range=entity_lines_range, resource=resource_id, evaluations=variable_evaluations, @@ -202,7 +204,7 @@ def add_python_check_results(self, report: Report, runner_filter: RunnerFilter, report.extra_resources.add( ExtraResource( file_abs_path=file_abs_path, - file_path=arm_file, + file_path=self.extract_file_path_from_abs_path(cleaned_path), resource=resource_id, ) ) @@ -232,12 +234,13 @@ def add_python_check_results(self, report: Report, runner_filter: RunnerFilter, entity_config=parameter_details, resource_attributes_to_omit=runner_filter.resource_attr_to_omit, ) + cleaned_path = clean_file_path(Path(arm_file)) self.build_record( report=report, check=check, check_result=check_result, code_block=censored_code_lines, - file_path=arm_file, + file_path=self.extract_file_path_from_abs_path(cleaned_path), file_abs_path=file_abs_path, file_line_range=entity_lines_range, resource_id=resource_id, From 6f66d38eeb70df62d1da4c3bacee6598e52094da Mon Sep 17 00:00:00 2001 From: Omri Yoffe Date: Tue, 26 Nov 2024 13:54:09 +0200 Subject: [PATCH 12/12] adjust ut --- tests/arm/runner/test_runner.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/arm/runner/test_runner.py b/tests/arm/runner/test_runner.py index 8547366d023..a04af3cdb03 100644 --- a/tests/arm/runner/test_runner.py +++ b/tests/arm/runner/test_runner.py @@ -66,7 +66,7 @@ def test_record_relative_path_with_relative_dir(self): self.assertGreater(len(all_checks), 0) # ensure that the assertions below are going to do something for record in all_checks: # no need to join with a '/' because the CFN runner adds it to the start of the file path - self.assertEqual(record.repo_file_path, f'/{record.file_path}') + self.assertEqual(record.repo_file_path, f'/{dir_rel_path}{record.file_path}') def test_record_relative_path_with_abs_dir(self): @@ -88,8 +88,7 @@ def test_record_relative_path_with_abs_dir(self): all_checks = report.failed_checks + report.passed_checks self.assertGreater(len(all_checks), 0) # ensure that the assertions below are going to do something for record in all_checks: - file_name = record.file_path.split('/')[-1] - self.assertEqual(record.repo_file_path, f'/{dir_rel_path}/{file_name}') + self.assertEqual(record.repo_file_path, f'/{dir_rel_path}{record.file_path}') def test_record_relative_path_with_relative_file(self): # test whether the record's repo_file_path is correct, relative to the CWD (with a / at the start).