message:
Support for the TF `concat` function
In addition, function handling has been reworked to be more concise and allow easier addition of new functions.

diff:
+import copy
import json
import logging
import os
import re
from pathlib import Path
-from typing import Mapping, Optional, Dict, Any, List, Callable, Set, Tuple
-import copy
+from typing import Any, Callable, Dict, List, Mapping, Optional, Set, Tuple
+
import deep_merge
import hcl2
import jmespath
from checkov.common.runners.base_runner import filter_ignored_directories
from checkov.common.util.consts import DEFAULT_EXTERNAL_MODULES_DIR, RESOLVED_MODULE_ENTRY_NAME
-from checkov.common.util.type_forcers import convert_str_to_bool
from checkov.common.variables.context import EvaluationContext, VarReference
-from checkov.terraform.module_loading.registry import ModuleLoaderRegistry
-from checkov.terraform.module_loading.registry import module_loader_registry as default_ml_registry
-from checkov.terraform.parser_var_blocks import find_var_blocks, split_merge_args
+from checkov.terraform import parser_functions
+from checkov.terraform.module_loading.registry import module_loader_registry as default_ml_registry, \
+ ModuleLoaderRegistry
+from checkov.terraform.parser_functions import FUNCTION_FAILED
+from checkov.terraform.parser_utils import eval_string, find_var_blocks, to_string
LOGGER = logging.getLogger(__name__)
@@ -366,62 +368,15 @@ class Parser:
if match.full_str == altered_value:
altered_value = replaced
else:
- altered_value = altered_value.replace(match.full_str, str(replaced))
+ replace_str = f"'{match.full_str}'"
+ if isinstance(replaced, str) or replace_str not in altered_value:
+ replace_str = match.full_str
+ altered_value = altered_value.replace(replace_str, str(replaced))
prev_matches.append((match.full_str, replaced))
had_var_block_match = True
if not had_var_block_match:
- # tomap is annoying because the curly braces in the string break the regex. Rather than
- # coming up with something that's significantly more complex, we'll special case this
- # check. Only do this when there wasn't a pattern match to make sure there are no
- # variables lingering in the map value.
- # https://www.terraform.io/docs/configuration/functions/tomap.html
- if value.startswith("${tomap(") and value.endswith(")}"):
- trimmed = value[8:-2]
- trimmed = trimmed.replace(":", "=") # converted to colons by parser #shrug
- altered_value = _eval_string(trimmed)
- if altered_value is None:
- altered_value = value
- continue
-
- if not isinstance(altered_value, dict):
- continue
-
- # If there is a string and anything else, convert to string
- had_string = False
- had_something_else = False
- for k, v in altered_value.items():
- if v == "${True}":
- altered_value[k] = True
- v = True
- elif v == "${False}":
- altered_value[k] = False
- v = False
-
- if isinstance(v, str):
- had_string = True
- if had_something_else:
- break
- else:
- had_something_else = True
- if had_string:
- break
- if had_string and had_something_else:
- altered_value = {k: _tostring(v) for k, v in altered_value.items()}
- # Same as above, regex can blow this up
- # (see parser scenario: tostring_function, INNER_CURLY
- elif value.startswith("${tostring(\"") and value.endswith("\")}"):
- altered_value = value[12:-3]
-
- # Support HCL 0.11 optional boolean syntax - evaluate "true" to true and "false" to false
- #
- # `allow_str_bool_translation` exists because we want to prevent conversion in a dict
- # which is a direct value. See the "MIXED_BOOL" variable in the "tomap_function" parser
- # scenario for a situation which worked incorrectly without this.
- # NOTE: This is probably not a big deal to be removed if this causes problems in other
- # places. The MIXED_BOOL test case is technically correct with the TF spec, but
- # isn't essential operation for Checkov.
- elif allow_str_bool_translation and value == "true":
+ if allow_str_bool_translation and value == "true":
altered_value = True
elif allow_str_bool_translation and value == "false":
altered_value = False
@@ -625,71 +580,16 @@ def _handle_single_var_pattern(orig_variable: str, var_value_and_file_map: Dict[
var_value = _handle_indexing(orig_variable[6:], lambda r: locals_values.get(r))
if var_value is not None:
return var_value
- elif orig_variable.startswith("to") and orig_variable.endswith(")"):
- # https://www.terraform.io/docs/configuration/functions/tobool.html
- if orig_variable.startswith("tobool("):
- bool_variable = orig_variable[7:-1].lower()
- bool_value = convert_str_to_bool(bool_variable)
- if isinstance(bool_value, bool):
- return bool_value
- else:
- return orig_variable
- # https://www.terraform.io/docs/configuration/functions/tolist.html
- elif orig_variable.startswith("tolist("):
- altered_value = _eval_string(orig_variable[7:-1])
- if altered_value is None:
- return orig_variable
- return altered_value if isinstance(altered_value, list) else list(altered_value)
- # NOTE: tomap as handled outside this loop (see below)
- # https://www.terraform.io/docs/configuration/functions/tonumber.html
- elif orig_variable.startswith("tonumber("):
- num_variable = orig_variable[9:-1]
- if num_variable.startswith('"') and num_variable.endswith('"'):
- num_variable = num_variable[1:-1]
- try:
- if "." in num_variable:
- return float(num_variable)
- else:
- return int(num_variable)
- except ValueError:
- return orig_variable
- # https://www.terraform.io/docs/configuration/functions/toset.html
- elif orig_variable.startswith("toset("):
- altered_value = _eval_string(orig_variable[6:-1])
- if altered_value is None:
- return orig_variable
- return set(altered_value)
- # https://www.terraform.io/docs/configuration/functions/tostring.html
- elif orig_variable.startswith("tostring("):
- altered_value = orig_variable[9:-1]
- # Indicates a safe string, all good
- if altered_value.startswith('"') and altered_value.endswith('"'):
- return altered_value[1:-1]
- # Otherwise, need to check for valid types (number or bool)
- bool_value = convert_str_to_bool(altered_value)
- if isinstance(bool_value, bool):
- return bool_value
- else:
- try:
- if "." in altered_value:
- return str(float(altered_value))
- else:
- return str(int(altered_value))
- except ValueError:
- return orig_variable # no change
- elif orig_variable.startswith("merge(") and orig_variable.endswith(")"):
- altered_value = orig_variable[6:-1]
- args = split_merge_args(altered_value)
- if args is None:
- return orig_variable
- merged_map = {}
- for arg in args:
- if arg.startswith("{"):
- value = _map_string_to_native(arg)
- if value is None:
- return orig_variable
- else:
- value = _handle_single_var_pattern(arg,
+ elif orig_variable.endswith(")") and not orig_variable.startswith("(") and "(" in orig_variable:
+ # Function handling. See parser_functions for implementations.
+ paren_start = orig_variable.index("(")
+ function_name = orig_variable[:paren_start]
+ function = getattr(parser_functions, # call from `parser_functions` script
+ function_name, # name of the function which will be executed
+ function_nyi_handler) # function (in this file) called if lookup fails
+ value_before = orig_variable[paren_start + 1: -1]
+ value_after = function(value_before,
+ var_resolver=lambda v: _handle_single_var_pattern(v,
var_value_and_file_map,
locals_values,
resource_list,
@@ -697,17 +597,10 @@ def _handle_single_var_pattern(orig_variable: str, var_value_and_file_map: Dict[
module_data_retrieval,
eval_map_by_var_name,
context,
- arg,
- root_directory)
- if isinstance(value, dict):
- merged_map.update(value)
- else:
- return orig_variable # don't know what this is, blow out
- return merged_map
- # TODO - format() support, still in progress
- # elif orig_variable.startswith("format(") and orig_variable.endswith(")"):
- # format_tokens = orig_variable[7:-1].split(",")
- # return format_tokens[0].format([_to_native_value(t) for t in format_tokens[1:]])
+ v,
+ root_directory),
+ function_name=function_name)
+ return value_after if value_after != FUNCTION_FAILED else orig_variable
elif _RESOURCE_REF_PATTERN.match(orig_variable):
# Reference to resources, example: 'aws_s3_bucket.example.bucket'
@@ -823,36 +716,11 @@ def _clean_bad_definitions(tf_definition_list):
}
-def _eval_string(value: str) -> Optional[Any]:
- try:
- value_string = value.replace("'", '"')
- parsed = hcl2.loads(f'eval = {value_string}\n') # NOTE: newline is needed
- return parsed["eval"][0]
- except Exception:
- return None
-
-
def _to_native_value(value: str) -> Any:
if value.startswith('"') or value.startswith("'"):
return value[1:-1]
else:
- return _eval_string(value)
-
-
-def _tostring(value: Any) -> str:
- if value is True:
- return "true"
- elif value is False:
- return "false"
- return str(value)
-
-
-def _map_string_to_native(value: str) -> Optional[Dict]:
- try:
- value_string = value.replace("'", '"')
- return json.loads(value_string)
- except Exception:
- return None
+ return eval_string(value)
def _remove_module_dependency_in_path(path):
@@ -904,3 +772,10 @@ def _safe_index(sequence_hopefully, index) -> Optional[Any]:
logging.debug(f'Failed to parse index int ({index}) out of {sequence_hopefully}')
logging.debug(e, stack_info=True)
return None
+
+
+def function_nyi_handler(original, function_name, **_):
+ logging.debug("Function '%s' is not yet implemented and will not be handled. Please file a"
+ "feature request if it is important to your evaluation (value: '%s')",
+ function_name, original)
+ return FUNCTION_FAILED

diff:
+import logging
+
+from checkov.common.util.type_forcers import convert_str_to_bool
+from checkov.terraform.parser_utils import eval_string, split_merge_args, string_to_native, to_string
+
+#
+# Functions defined in this file implement terraform functions.
+#
+# Inputs:
+# - First arg (unnamed) - the value string provided to the function
+# - "var_resolver" - function pointer to resolve variable/local references and such
+# - "function_name" - name of the function being called (mainly useful for error reporting when a
+# function isn't defined)
+# These may be expanded over time, so accepting kwargs (via `**`) is recommended.
+#
+# If the value cannot be processed, `FUNCTION_FAILED` should be returned.
+#
+
+FUNCTION_FAILED = "____FUNCTION_FAILED____"
+
+
+def merge(original, var_resolver, **_):
+ args = split_merge_args(original)
+ if args is None:
+ return FUNCTION_FAILED
+ merged_map = {}
+ for arg in args:
+ if arg.startswith("{"):
+ arg_value = string_to_native(arg)
+ if arg_value is None:
+ return FUNCTION_FAILED
+ else:
+ arg_value = var_resolver(arg)
+ if isinstance(arg_value, dict):
+ merged_map.update(arg_value)
+ else:
+ return FUNCTION_FAILED # don't know what this is, blow out
+ return merged_map
+
+
+def concat(original, var_resolver, **_):
+ args = split_merge_args(original)
+ if args is None:
+ return FUNCTION_FAILED
+ merged_list = []
+ for arg in args:
+ if arg.startswith("["):
+ value = eval_string(arg)
+ if value is None:
+ logging.debug("Unable to convert to list: %s", arg)
+ return FUNCTION_FAILED
+ else:
+ value = var_resolver(arg)
+ if isinstance(value, list):
+ merged_list.extend(value)
+ else:
+ return FUNCTION_FAILED # don't know what this is, blow out
+ return merged_list
+
+
+def tobool(original, **_):
+ # https://www.terraform.io/docs/configuration/functions/tobool.html
+ bool_value = convert_str_to_bool(original)
+ return bool_value if isinstance(bool_value, bool) else FUNCTION_FAILED
+
+
+def tonumber(original, **_):
+ # https://www.terraform.io/docs/configuration/functions/tonumber.html
+ if original.startswith('"') and original.endswith('"'):
+ original = original[1:-1]
+ try:
+ if "." in original:
+ return float(original)
+ else:
+ return int(original)
+ except ValueError:
+ return FUNCTION_FAILED
+
+
+def tostring(original, **_):
+ # Indicates a safe string, all good
+ if original.startswith('"') and original.endswith('"'):
+ return original[1:-1]
+ # Otherwise, need to check for valid types (number or bool)
+ bool_value = convert_str_to_bool(original)
+ if isinstance(bool_value, bool):
+ return bool_value
+ else:
+ try:
+ if "." in original:
+ return str(float(original))
+ else:
+ return str(int(original))
+ except ValueError:
+ return FUNCTION_FAILED # no change
+
+
+def tolist(original, **_):
+ # https://www.terraform.io/docs/configuration/functions/tolist.html
+ altered_value = eval_string(original)
+ if altered_value is None:
+ return FUNCTION_FAILED
+ return altered_value if isinstance(altered_value, list) else list(altered_value)
+
+
+def toset(original, **_):
+ # https://www.terraform.io/docs/configuration/functions/toset.html
+ altered_value = eval_string(original)
+ if altered_value is None:
+ return FUNCTION_FAILED
+ return altered_value if isinstance(altered_value, set) else set(altered_value)
+
+
+def tomap(original, **_):
+ original = original.replace(":", "=") # converted to colons by parser #shrug
+
+ altered_value = eval_string(original)
+ if altered_value is None or not isinstance(altered_value, dict):
+ return FUNCTION_FAILED
+
+ # If there is a string and anything else, convert to string
+ had_string = False
+ had_something_else = False
+ for k, v in altered_value.items():
+ if v == "${True}":
+ altered_value[k] = True
+ v = True
+ elif v == "${False}":
+ altered_value[k] = False
+ v = False
+
+ if isinstance(v, str):
+ had_string = True
+ if had_something_else:
+ break
+ else:
+ had_something_else = True
+ if had_string:
+ break
+ if had_string and had_something_else:
+ return {k: to_string(v) for k, v in altered_value.items()}
+ return altered_value

diff:
+import json
from dataclasses import dataclass
from enum import Enum, IntEnum
-from typing import List, Optional
+from typing import Any, Dict, List, Optional
+
+import hcl2
@dataclass
@@ -221,3 +224,27 @@ def _str_parser_loop_collection_helper(c: str, inside_collection_stack: List[str
inside_collection_stack.insert(0, ")")
return processing_str_escape
+
+
+def eval_string(value: str) -> Optional[Any]:
+ try:
+ value_string = value.replace("'", '"')
+ parsed = hcl2.loads(f'eval = {value_string}\n') # NOTE: newline is needed
+ return parsed["eval"][0]
+ except Exception:
+ return None
+
+
+def string_to_native(value: str) -> Optional[Dict]:
+ try:
+ value_string = value.replace("'", '"')
+ return json.loads(value_string)
+ except Exception:
+ return None
+
+def to_string(value: Any) -> str:
+ if value is True:
+ return "true"
+ elif value is False:
+ return "false"
+ return str(value)
\ No newline at end of file

diff:
+{
+ "main.tf": {
+ "value": ["${not_a_real_function(1,2,3,4)}"]
+ }
+}
\ No newline at end of file

diff:
+value = not_a_real_function(1, 2, 3, 4)
\ No newline at end of file

diff:
+{
+ "main.tf": {
+ "locals": [
+ {
+ "inline_map": [
+ {
+ "key": "a_key",
+ "value": "a_value",
+ "propagate_at_launch": false
+ }
+ ]
+ },
+ {
+ "simple_list": [
+ [
+ "a",
+ "",
+ "b",
+ "c"
+ ]
+ ],
+ "simple_list2": [
+ [
+ "a",
+ "",
+ "b",
+ "c"
+ ]
+ ]
+ }
+ ],
+ "variable": [
+ {
+ "extra_tags": {
+ "default": [
+ [
+ {
+ "key": "Foo",
+ "value": "Bar",
+ "propagate_at_launch": true
+ },
+ {
+ "key": "Baz",
+ "value": "Bam",
+ "propagate_at_launch": true
+ }
+ ]
+ ]
+ }
+ }
+ ],
+ "resource": [
+ {
+ "aws_autoscaling_group": {
+ "bar": {
+ "name": [
+ "foobar3-terraform-test"
+ ],
+ "max_size": [
+ 5
+ ],
+ "min_size": [
+ 2
+ ],
+ "tags": [
+ [
+ {
+ "key": "interpolation1",
+ "value": "value3",
+ "propagate_at_launch": true
+ },
+ {
+ "key": "interpolation2",
+ "value": "value4",
+ "propagate_at_launch": true
+ },
+ {
+ "key": "a_key",
+ "value": "a_value",
+ "propagate_at_launch": false
+ },
+ {
+ "key": "Foo",
+ "value": "Bar",
+ "propagate_at_launch": true
+ },
+ {
+ "key": "Baz",
+ "value": "Bam",
+ "propagate_at_launch": true
+ }
+ ]
+ ]
+ }
+ }
+ }
+ ]
+ }
+}
\ No newline at end of file

diff:
+# Loosely from https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/autoscaling_group
+locals {
+ inline_map = {
+ key = "a_key"
+ value = "a_value"
+ propagate_at_launch = false
+ }
+}
+
+variable "extra_tags" {
+ default = [
+ {
+ key = "Foo"
+ value = "Bar"
+ propagate_at_launch = true
+ },
+ {
+ key = "Baz"
+ value = "Bam"
+ propagate_at_launch = true
+ }
+ ]
+}
+
+resource "aws_autoscaling_group" "bar" {
+ name = "foobar3-terraform-test"
+ max_size = 5
+ min_size = 2
+
+ tags = concat(
+ [
+ {
+ "key" = "interpolation1"
+ "value" = "value3"
+ "propagate_at_launch" = true
+ },
+ {
+ "key" = "interpolation2"
+ "value" = "value4"
+ "propagate_at_launch" = true
+ },
+ local.inline_map
+ ],
+ var.extra_tags,
+ )
+}
+
+# From https://www.terraform.io/docs/language/functions/concat.html
+locals {
+ simple_list = concat(["a", ""], ["b", "c"])
+ simple_list2 = concat(["a"], [""], ["b"], ["c"])
+}
\ No newline at end of file

diff:
@@ -6,4 +6,4 @@ from checkov.terraform import parser
class TestParserInternals(unittest.TestCase):
def test_eval_string_to_list(self):
expected = ["a", "b", "c"]
- assert parser._eval_string('["a", "b", "c"]') == expected
+ assert parser.eval_string('["a", "b", "c"]') == expected

diff:
@@ -40,6 +40,9 @@ class TestParserScenarios(unittest.TestCase):
def test_compound_local(self):
self.go("compound_local")
+ def test_concat_function(self):
+ self.go("concat_function")
+
def test_merge_function(self):
self.go("merge_function")
@@ -124,6 +127,9 @@ class TestParserScenarios(unittest.TestCase):
def test_account_dirs_and_modules(self):
self.go("account_dirs_and_modules")
+ def test_bogus_function(self):
+ self.go("bogus_function")
+
@staticmethod
def go(dir_name):
dir_path = os.path.join(os.path.dirname(os.path.realpath(__file__)),

diff:
@@ -3,7 +3,7 @@ import unittest
from typing import List, Tuple
-from checkov.terraform.parser_var_blocks import VarBlockMatch as VBM, split_merge_args, find_var_blocks
+from checkov.terraform.parser_utils import VarBlockMatch as VBM, split_merge_args, find_var_blocks
class TestParserInternals(unittest.TestCase):

original_message:
Support for the TF `concat` function (#922)

In addition, function handling has been reworked to be more concise and allow easier addition of new functions.


message:
Avoid None-type reference
Fixes:

change_type: MODIFY

diff:
@@ -415,6 +415,8 @@ class Parser:
continue
file_data = self.out_definitions.get(file)
+ if file_data is None:
+ continue
module_calls = file_data.get("module")
if not module_calls or not isinstance(module_calls, list):
continue

license: Apache License 2.0

repo: bridgecrewio/checkov

original_message:
Avoid None-type reference

Fixes: #9931


message: Implementation of `map` function

diff:
import logging
+from typing import Any, Dict, Optional
from checkov.common.util.type_forcers import convert_str_to_bool
from checkov.terraform.parser_utils import eval_string, split_merge_args, string_to_native, to_string
@@ -20,6 +21,7 @@ FUNCTION_FAILED = "____FUNCTION_FAILED____"
def merge(original, var_resolver, **_):
+ # https://www.terraform.io/docs/language/functions/merge.html
args = split_merge_args(original)
if args is None:
return FUNCTION_FAILED
@@ -39,6 +41,7 @@ def merge(original, var_resolver, **_):
def concat(original, var_resolver, **_):
+ # https://www.terraform.io/docs/language/functions/concat.html
args = split_merge_args(original)
if args is None:
return FUNCTION_FAILED
@@ -112,21 +115,41 @@ def toset(original, **_):
def tomap(original, **_):
+ # https://www.terraform.io/docs/language/functions/tomap.html
original = original.replace(":", "=") # converted to colons by parser #shrug
altered_value = eval_string(original)
if altered_value is None or not isinstance(altered_value, dict):
return FUNCTION_FAILED
+ return _check_map_type_consistency(altered_value)
+
+def map(original, **_):
+ # https://www.terraform.io/docs/language/functions/map.html
+
+ # NOTE: Splitting by commas is annoying due to possible commas in strings. To avoid
+ # the issue, act like it's a list (to allow comma separation) and let the HCL
+ # parser deal with it. Then iterating the list is easy.
+ converted_to_list = eval_string(f"[{original}]")
+ if converted_to_list is None or len(converted_to_list) & 1: # none or odd number of args
+ return FUNCTION_FAILED
+
+ new_map = {}
+ for i in range(0, len(converted_to_list), 2):
+ new_map[converted_to_list[i]] = converted_to_list[i + 1]
+ return _check_map_type_consistency(new_map)
+
+
+def _check_map_type_consistency(value: Dict) -> Dict:
# If there is a string and anything else, convert to string
had_string = False
had_something_else = False
- for k, v in altered_value.items():
+ for k, v in value.items():
if v == "${True}":
- altered_value[k] = True
+ value[k] = True
v = True
elif v == "${False}":
- altered_value[k] = False
+ value[k] = False
v = False
if isinstance(v, str):
@@ -138,5 +161,5 @@ def tomap(original, **_):
if had_string:
break
if had_string and had_something_else:
- return {k: to_string(v) for k, v in altered_value.items()}
- return altered_value
+ value = {k: to_string(v) for k, v in value.items()}
+ return value

diff:
@@ -242,6 +242,7 @@ def string_to_native(value: str) -> Optional[Dict]:
except Exception:
return None
+
def to_string(value: Any) -> str:
if value is True:
return "true"

diff:
"b",
"c"
]
+ ],
+ "single_item_list": [
+ ["a"]
+ ],
+ "single_item_trailing_list": [
+ ["a"]
]
}
],
]
}
}
+ },
+ {
+ "aws_autoscaling_group": {
+ "bar_simplified": {
+ "name": [
+ "bar_simplified_group"
+ ],
+ "max_size": [
+ 5
+ ],
+ "min_size": [
+ 2
+ ],
+ "tags": [
+ [
+ {
+ "key": "interpolation1",
+ "value": "value3",
+ "propagate_at_launch": true
+ },
+ {
+ "key": "interpolation2",
+ "value": "value4",
+ "propagate_at_launch": true
+ }
+ ]
+ ]
+ }
+ }
}
]
}

diff:
@@ -45,8 +45,30 @@ resource "aws_autoscaling_group" "bar" {
)
}
+resource "aws_autoscaling_group" "bar_simplified" {
+ name = "bar_simplified_group"
+ max_size = 5
+ min_size = 2
+ tags = concat(
+ [
+ {
+ "key" = "interpolation1"
+ "value" = "value3"
+ "propagate_at_launch" = true
+ },
+ {
+ "key" = "interpolation2"
+ "value" = "value4"
+ "propagate_at_launch" = true
+ }
+ ]
+ )
+}
+
# From https://www.terraform.io/docs/language/functions/concat.html
locals {
simple_list = concat(["a", ""], ["b", "c"])
simple_list2 = concat(["a"], [""], ["b"], ["c"])
+ single_item_list = concat(["a"])
+ single_item_trailing_list = concat(["a"],)
}
\ No newline at end of file

change_type: ADD

diff:
+locals {
+ INTS = map("a", 1, "b", 2)
+ FLOATS = map("a", 1.1, "b", 2.2)
+ STRINGS = map("a", "one", "b", "two")
+ BOOLS = map("a", true, "b", false)
+
+ MIXED_BOOL = map("a", "foo", "b", true)
+ MIXED_FLOAT = map("a", "foo", "b", 1.2)
+
+ ANNOYING_SPLIT = map("this, is", "really, annoying")
+
+ INVALID_ODD_ARGS = map("only one")
+
+ common_tags = "${map(
+ "App", "my_app",
+ "Product", "my_product",
+ "Team", "my_team",
+ )}"
+}
\ No newline at end of file

diff:
@@ -58,6 +58,9 @@ class TestParserScenarios(unittest.TestCase):
def test_tomap_function(self):
self.go("tomap_function")
+ def test_map_function(self):
+ self.go("map_function")
+
def test_tonumber_function(self):
self.go("tonumber_function")

original_message: Implementation of `map` function (#937)


message: remove funky char causing issue every time it shows on windows w/colorama

diff:
@@ -14,5 +14,5 @@ By bridgecrew.io | version: {} """.format(version)
new_version = check_for_update("checkov", version)
if new_version:
- banner = "\n" + banner + "\nUpdate available " + colored(version,"grey") + " > " + colored(new_version, "green") + "\nRun " + colored(
+ banner = "\n" + banner + "\nUpdate available " + colored(version,"grey") + " -> " + colored(new_version, "green") + "\nRun " + colored(
"pip3 install -U checkov", "magenta") + " to update \n"


diff:
@@ -71,7 +71,9 @@ class Runner(BaseRunner):
for file in files:
if file.endswith(".tf"):
file_parsing_errors = {}
- self.tf_definitions[file] = self.parser.parse_file(file=file, parsing_errors=file_parsing_errors)
+ parse_result = self.parser.parse_file(file=file, parsing_errors=file_parsing_errors)
+ if parse_result is not None:
+ self.tf_definitions[file] = parse_result
if file_parsing_errors:
parsing_errors.update(file_parsing_errors)
continue
@@ -101,6 +103,8 @@ class Runner(BaseRunner):
def run_all_blocks(self, definition, definitions_context, full_file_path, root_folder, report,
scanned_file, runner_filter, module_referrer: Optional[str]):
+ if not definition:
+ return
for block_type in definition.keys():
if block_type in CHECK_BLOCK_TYPES:
self.run_block(definition[block_type], definitions_context,

original_message:
Protection against None reference

Fixes: #941


message: Debug message when run_all_blocks aborts

diff:
@@ -104,6 +104,7 @@ class Runner(BaseRunner):
def run_all_blocks(self, definition, definitions_context, full_file_path, root_folder, report,
scanned_file, runner_filter, module_referrer: Optional[str]):
if not definition:
+ logger.debug("Empty definition, skipping run")
return
for block_type in definition.keys():
if block_type in CHECK_BLOCK_TYPES:


message: Additional comment detail (really, just to re-kick build)

diff:
@@ -104,7 +104,7 @@ class Runner(BaseRunner):
def run_all_blocks(self, definition, definitions_context, full_file_path, root_folder, report,
scanned_file, runner_filter, module_referrer: Optional[str]):
if not definition:
- logging.debug("Empty definition, skipping run")
+ logging.debug("Empty definition, skipping run (root_folder=%s)", root_folder)
return
for block_type in definition.keys():
if block_type in CHECK_BLOCK_TYPES:


diff:
+# Configuration for probot-stale - https://github.com/probot/stale
+
+# Number of days of inactivity before an Issue or Pull Request becomes stale
+daysUntilStale: 180
+
+# Number of days of inactivity before an Issue or Pull Request with the stale label is closed.
+# Set to false to disable. If disabled, issues still need to be closed manually, but will remain marked as stale.
+daysUntilClose: 14
+
+# Only issues or pull requests with all of these labels are check if stale. Defaults to `[]` (disabled)
+onlyLabels: []
+
+# Issues or Pull Requests with these labels will never be considered stale. Set to `[]` to disable
+exemptLabels:
+ - pinned
+ - security
+ - nostale
+
+# Set to true to ignore issues in a project (defaults to false)
+exemptProjects: false
+
+# Set to true to ignore issues in a milestone (defaults to false)
+exemptMilestones: false
+
+# Set to true to ignore issues with an assignee (defaults to false)
+exemptAssignees: false
+
+# Label to use when marking as stale
+staleLabel: stale
+
+# Comment to post when marking as stale. Set to `false` to disable
+markComment: >
+ Thanks for contributing to Checkov!
+ We've automatically marked this issue as stale to keep our issues list tidy,
+ because it has not had any activity for 6 months.
+ It will be closed in 14 days if no further activity occurs.
+ Commenting on this issue will remove the stale tag.
+ If you want to talk through the issue or help us understand the priority and context,
+ feel free to add a comment or join us in the Checkov slack channel at https://slack.bridgecrew.io
+
+ Thanks!
+
+# Comment to post when removing the stale label.
+# unmarkComment: >
+# Your comment here.
+
+# Comment to post when closing a stale Issue or Pull Request.
+closeComment: >
+ Closing issue due to inactivity.
+ If you feel this is in error, please re-open, or reach out to the community via slack:
+ https://slack.bridgecrew.io
+ Thanks!
+
+# Limit the number of actions per hour, from 1-30. Default is 30
+limitPerRun: 30
+
+# Limit to only `issues` or `pulls`
+# only: issues
+
+# Optionally, specify configuration settings that are specific to just 'issues' or 'pulls':
+# pulls:
+# daysUntilStale: 30
+# markComment: >
+# This pull request has been automatically marked as stale because it has not had
+# recent activity. It will be closed if no further activity occurs. Thank you
+# for your contributions.
+
+# issues:
+# exemptLabels:
+# - confirmed
\ No newline at end of file

original_message: activating stalebot with initial 6mo/14 days config (#1011)


message: Matrix tests for python 3.7, 3.8, 3.9, artifacts left on 3.7 for now

diff:
@@ -6,16 +6,21 @@ on:
- master
jobs:
integration-tests:
+ strategy:
+ fail-fast: true
+ matrix:
+ python: [3.7, 3.8, 3.9]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v2
with:
- python-version: 3.7
+ python-version: ${{ matrix.python }}
- uses: dschep/install-pipenv-action@v1
- uses: azure/setup-helm@v1
- name: Build & install checkov package
run: |
+ pipenv --python ${{ matrix.python }}
pipenv run pip install pytest
pipenv run python setup.py sdist bdist_wheel
pipenv run pip install --upgrade pip==20.2

diff:
@@ -18,16 +18,21 @@ jobs:
cfn-lint tests/cloudformation/checks/resource/aws/**/* -i W
unit-tests:
+ strategy:
+ fail-fast: true
+ matrix:
+ python: [3.7, 3.8, 3.9]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- - name: Set up Python 3.7
+ - name: Set up Python ${{ matrix.python }}
uses: actions/setup-python@v2
with:
- python-version: 3.7
+ python-version: ${{ matrix.python }}
- uses: dschep/install-pipenv-action@v1
- name: Install dependencies
run: |
+ pipenv --python ${{ matrix.python }}
pipenv install --dev
- name: Unit tests
env:
@@ -38,16 +43,21 @@ jobs:
pipenv run python -m coverage html
integration-tests:
+ strategy:
+ fail-fast: true
+ matrix:
+ python: [3.7, 3.8, 3.9]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v2
with:
- python-version: 3.7
+ python-version: ${{ matrix.python }}
- uses: azure/setup-helm@v1
- uses: dschep/install-pipenv-action@v1
- name: Build & install checkov package
run: |
+ pipenv --python ${{ matrix.python }}
pipenv run pip install pytest
pipenv run python setup.py sdist bdist_wheel
pipenv run pip install --upgrade pip==20.2


message: Updated HELM documentation

diff:
+---
+layout: default
+published: true
+title: Helm
+nav_order: 9
+---
+
+# Scan Helm charts with Checkov
+
+Checkov is able to autodetect helm charts by the presence of a `Chart.yaml` file, if found, the helm framework will automatically be used to template out the helm chart (with it's default values) into resulting Kubernetes manifests, which will then be scanned by all Checkovs' Kubernetes policies.
+
+
+```
+ _ _
+ ___| |__ ___ ___| | _______ __
+ / __| '_ \ / _ \/ __| |/ / _ \ \ / /
+ | (__| | | | __/ (__| < (_) \ V /
+ \___|_| |_|\___|\___|_|\_\___/ \_/
+
+By bridgecrew.io | version: 2.0.587
+
+helm scan results:
+
+Passed checks: 370, Failed checks: 90, Skipped checks: 0
+
+Check: CKV_K8S_27: "Do not expose the docker daemon socket to containers"
+ PASSED for resource: Deployment.RELEASE-NAME-nextcloud.default
+ File: /nextcloud/templates/deployment.yaml:3-107
+ Guide: https://docs.bridgecrew.io/docs/bc_k8s_26
+
+
+...
+```
+
+To do this, we use the helm binary, if the helm binary (v3) is not available via the users `$PATH` for Checkov, Checkov will automatically skip the helm framework, with a message informing the user as below. Checkov will then run with exactly the same behaviour as if running with `checkov --skip-framework helm`.
+
+```
+The following frameworks will automatically be disabled due to missing system dependencies: helm
+```
+
+This auto-detection behaviour is to protect existing CI pipelines which pull the latest version of Checkov, which may not have the helm binary available.
+
+# Scan Helm values.yaml files with Checkov
+
+If you are consuming third party charts, it is unlikley you will have a `Chart.yaml` file for Checkov to auto-detect.
+
+For example, you may have run:
+
+```
+helm repo add gocd https://gocd.github.io/helm-chart
+helm inspect values gocd/gocd > gocd.yaml
+```
+
+Then edited your custom values in `gocd.yaml` to suit your deployment needs.
+
+In this case, you can use your custom values from your values file (`gocd.yml` in this case) to generate kubernetes output with Helm, and pass the templated output to checkov to receive the same policy information. This would also be trivial to set up in CI for automated scanning of this scenario:
+
+```
+helm template gocd/gocd -f gocd.yaml > k8s-template.yaml
+checkov -f k8s-template.yaml --framework kubernetes --skip-check CKV_K8S_21
+```
+
+Note we skip check `CKV_K8S_21` for this process, which alerts on default namespace usage within Kubernetes manifests.
+Since HELM manages our namespaces, we always skip this internally when using the HELM framework, so we want to replicate the same behaviour here.
+

original_message: Updated HELM documentation


message: caps changes

diff:
@@ -61,5 +61,5 @@ checkov -f k8s-template.yaml --framework kubernetes --skip-check CKV_K8S_21
```
Note we skip check `CKV_K8S_21` for this process, which alerts on default namespace usage within Kubernetes manifests.
-Since HELM manages our namespaces, we always skip this internally when using the HELM framework, so we want to replicate the same behaviour here.
+Since helm manages our namespaces, we always skip this internally when using the helm framework, so we want to replicate the same behaviour here.

original_message: caps changes


diff:
@@ -152,7 +152,7 @@ class Runner(BaseRunner):
if 'bases' in fileContent:
metadata['referenced_bases'] = fileContent['bases']
- elif 'bases' in fileContent.get:
+ elif 'bases' in fileContent:
logging.debug(f"Kustomization contains bases: section. Likley an overlay/env. {parseKustomizationData}/kustomization.yaml")
metadata['type'] = "overlay"
metadata['referenced_bases'] = fileContent['bases']

original_message: Correct incorrect codepath for certain bases (#2280)


message: Remove confusing BASEDIR warning to improve UX

diff:
@@ -254,14 +254,14 @@ class Runner(BaseRunner):
self.kustomizeProcessedFolderAndMeta[filePath]['overlay_name'] = checkovKustomizeEnvNameByPath
logging.debug(f"Overlay based on {self.kustomizeProcessedFolderAndMeta[filePath]['validated_base']}, naming overlay {checkovKustomizeEnvNameByPath} for Checkov Results.")
else:
- checkovKustomizeEnvNameByPath = f"UNVALIDATEDBASEDIR/{pathlib.Path(filePath).stem}"
+ checkovKustomizeEnvNameByPath = f"{pathlib.Path(filePath).stem}"
self.kustomizeProcessedFolderAndMeta[filePath]['overlay_name'] = checkovKustomizeEnvNameByPath
- logging.warning(f"Could not confirm base dir for Kustomize overlay/env. Using {checkovKustomizeEnvNameByPath} for Checkov Results.")
+ logging.debug(f"Could not confirm base dir for Kustomize overlay/env. Using {checkovKustomizeEnvNameByPath} for Checkov Results.")
except KeyError:
- checkovKustomizeEnvNameByPath = f"UNVALIDATEDBASEDIR/{pathlib.Path(filePath).stem}"
+ checkovKustomizeEnvNameByPath = f"{pathlib.Path(filePath).stem}"
self.kustomizeProcessedFolderAndMeta[filePath]['overlay_name'] = checkovKustomizeEnvNameByPath
- logging.warning(f"Could not confirm base dir for Kustomize overlay/env. Using {checkovKustomizeEnvNameByPath} for Checkov Results.")
+ logging.debug(f"Could not confirm base dir for Kustomize overlay/env. Using {checkovKustomizeEnvNameByPath} for Checkov Results.")
if self.templateRendererCommand == "kubectl":


diff:
@@ -42,7 +42,7 @@ class K8sKustomizeRunner(K8sRunner):
record = Record(
check_id=check.id, bc_check_id=check.bc_id, check_name=check.name,
check_result=check_result, code_block=entity_context.get("code_lines"), file_path=realKustomizeEnvMetadata['filePath'],
- file_line_range=[],
+ file_line_range=[0,0],
resource=kustomizeResourceID, evaluations=variable_evaluations,
check_class=check.__class__.__module__, file_abs_path=realKustomizeEnvMetadata['filePath'])
record.set_guideline(check.guideline)
@@ -80,7 +80,7 @@ class K8sKustomizeRunner(K8sRunner):
check_result=check_result,
code_block=entity_context.get("code_lines"),
file_path=realKustomizeEnvMetadata['filePath'],
- file_line_range=[],
+ file_line_range=[0,0],
resource=kustomizeResourceID, # entity.get(CustomAttributes.ID),
evaluations={},
check_class=check.__class__.__module__,

original_message: Prevent runner tripping up Sarif output formatter (#2365)


message:
Move Kustomize into path. Enabling Kustomize output
Enables kustomize output for docker users
Enables kustomize output for GHA users

old_path: Dockerfile

new_path: Dockerfile

diff:
@@ -7,7 +7,7 @@ RUN apk add --no-cache --virtual .build_deps build-base libffi-dev \
&& apk del .build_deps
RUN wget -q -O get_helm.sh https://raw.githubusercontent.com/helm/helm/master/scripts/get-helm-3; chmod 700 get_helm.sh; VERIFY_CHECKSUM=true ./get_helm.sh; rm ./get_helm.sh
-RUN wget -q -O get_kustomize.sh https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh; chmod 700 get_kustomize.sh; ./get_kustomize.sh; rm ./get_kustomize.sh
+RUN wget -q -O get_kustomize.sh https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh; chmod 700 get_kustomize.sh; ./get_kustomize.sh; mv /kustomize /usr/bin/kustomize; rm ./get_kustomize.sh
COPY ./github_action_resources/entrypoint.sh /entrypoint.sh
COPY ./github_action_resources/checkov-problem-matcher.json /usr/local/lib/checkov-problem-matcher.json

original_message:
Move Kustomize into path. Enabling Kustomize output (#2366)

- Enables kustomize output for docker users
- Enables kustomize output for GHA users


message: Initial CircleCI config.yaml pipelines runner

diff: +from checkov.circleci_pipelines.checks import * # noqa

diff:
+from checkov.common.checks.base_check import BaseCheck
+
+from checkov.common.models.enums import CheckCategories
+from checkov.circleci_pipelines.registry import registry
+
+
+class BaseCircleCIPipelinesCheck(BaseCheck):
+ def __init__(self, name, id, supported_entities, block_type, path=None):
+ categories = [CheckCategories.SUPPLY_CHAIN]
+
+ super().__init__(
+ name=name,
+ id=id,
+ categories=categories,
+ supported_entities=supported_entities,
+ block_type=block_type,
+ )
+ self.path = path
+ registry.register(self)

diff:
+from pathlib import Path
+
+modules = Path(__file__).parent.glob("*.py")
+__all__ = [f.stem for f in modules if f.is_file() and not f.stem == "__init__"]

diff:
+from operator import contains
+from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
+from checkov.common.models.enums import CheckResult
+from checkov.yaml_doc.enums import BlockType
+
+
+class ImageReferenceHashVersion(BaseCircleCIPipelinesCheck):
+ def __init__(self):
+ name = "Ensure the pipeline image version is referenced via hash not arbitrary tag."
+ id = "CKV_CIRCLECIPIPELINES_2"
+ super().__init__(
+ name=name,
+ id=id,
+ block_type=BlockType.ARRAY,
+ supported_entities=['jobs.*.docker[].{image: image, __startline__: __startline__, __endline__:__endline__}']
+ )
+
+ def scan_entity_conf(self, conf):
+ if not isinstance(conf, dict):
+ return
+ image = conf.get("image", None)
+ if not image:
+ return
+ if isinstance(image, str):
+ if "@" in image:
+ return CheckResult.PASSED, conf
+
+ return CheckResult.FAILED, conf
+
+
+check = ImageReferenceHashVersion()

diff:
+from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
+from checkov.common.models.enums import CheckResult
+from checkov.yaml_doc.enums import BlockType
+
+class ImageReferenceLatestTag(BaseCircleCIPipelinesCheck):
+ def __init__(self):
+ name = "Ensure the pipeline image uses a non latest version tag"
+ id = "CKV_CIRCLECIPIPELINES_1"
+ super().__init__(
+ name=name,
+ id=id,
+ block_type=BlockType.ARRAY,
+ supported_entities=['jobs.*.docker[].{image: image, __startline__: __startline__, __endline__:__endline__}']
+ )
+
+ def scan_entity_conf(self, conf):
+ if not isinstance(conf, dict):
+ return
+ image = conf.get("image", None)
+ if not image:
+ return
+ if isinstance(image, str):
+ if image.endswith(":latest"):
+ return CheckResult.FAILED, conf
+
+ return CheckResult.PASSED, conf
+
+
+check = ImageReferenceLatestTag()

diff:
+from asyncio.log import logger
+from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
+from checkov.common.models.enums import CheckResult
+from checkov.yaml_doc.enums import BlockType
+
+class PreventDevelopmentOrbs(BaseCircleCIPipelinesCheck):
+ def __init__(self):
+ name = "Ensure mutable development orbs are not used."
+ id = "CKV_CIRCLECIPIPELINES_3"
+ super().__init__(
+ name=name,
+ id=id,
+ block_type=BlockType.ARRAY,
+ supported_entities=['orbs']
+ )
+
+ def scan_entity_conf(self, conf):
+ if isinstance(conf, str):
+ if "@dev:" in conf:
+ return CheckResult.FAILED, conf
+ else:
+ return CheckResult.PASSED, conf
+ return CheckResult.PASSED, conf
+
+
+check = PreventDevelopmentOrbs()

diff:
+from asyncio.log import logger
+from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
+from checkov.common.models.enums import CheckResult
+from checkov.yaml_doc.enums import BlockType
+
+class PreventDevelopmentOrbs(BaseCircleCIPipelinesCheck):
+ def __init__(self):
+ name = "Ensure unversioned volatile orbs are not used."
+ id = "CKV_CIRCLECIPIPELINES_4"
+ super().__init__(
+ name=name,
+ id=id,
+ block_type=BlockType.ARRAY,
+ supported_entities=['orbs']
+ )
+
+ def scan_entity_conf(self, conf):
+ if isinstance(conf, str):
+ if "@volatile" in conf:
+ return CheckResult.FAILED, conf
+ else:
+ return CheckResult.PASSED, conf
+ return CheckResult.PASSED, conf
+
+
+check = PreventDevelopmentOrbs()

diff:
+from checkov.yaml_doc.base_registry import Registry
+
+registry = Registry()

diff:
+from __future__ import annotations
+from asyncio.log import logger
+import jmespath
+import os
+from typing import TYPE_CHECKING, Any
+from checkov.common.images.image_referencer import ImageReferencer, Image
+from checkov.common.output.report import CheckType
+from checkov.circleci_pipelines.registry import registry
+from checkov.yaml_doc.runner import Runner as YamlRunner
+
+if TYPE_CHECKING:
+ from checkov.common.checks.base_check_registry import BaseCheckRegistry
+
+WORKFLOW_DIRECTORY = "circleci"
+
+class Runner(YamlRunner, ImageReferencer):
+ check_type = CheckType.CIRCLECI_PIPELINES # noqa: CCE003 # a static attribute
+
+ def __init__(self):
+ super().__init__()
+
+ def require_external_checks(self):
+ return False
+
+ def import_registry(self):
+ return registry
+
+ def _parse_file(self, f):
+ if self.is_workflow_file(f):
+ return super()._parse_file(f)
+
+ def is_workflow_file(self, file_path):
+ """
+ :return: True if the file mentioned is named config.yml/yaml in .circleci dir. Otherwise: False
+ """
+ abspath = os.path.abspath(file_path)
+ return WORKFLOW_DIRECTORY in abspath and abspath.endswith(("config.yml", "config.yaml"))
+
+ def get_images(self, file_path):
+ """
+ Get container images mentioned in a file
+ :param file_path: File to be inspected
+
+ File sample that will return 5 Image objects:
+ # # Use the latest 2.1 version of CircleCI pipeline process engine.
+ # # See: https://circleci.com/docs/2.0/configuration-reference
+ # version: 2.1
+ # # Define a job to be invoked later in a workflow.
+ # # See: https://circleci.com/docs/2.0/configuration-reference/#jobs
+ # jobs:
+ # say-hello:
+ # docker:
+ # - image: buildpack-deps:latest # primary container
+ # auth:
+ # username: mydockerhub-user
+ # password: $DOCKERHUB_PASSWORD # context / project UI env-var reference
+ # environment:
+ # ENV: CI
+ # - image: mongo:2.6.8
+ # auth:
+ # username: mydockerhub-user
+ # password: $DOCKERHUB_PASSWORD # context / project UI env-var reference
+ # command: [--smallfiles]
+ # - image: postgres:14.2
+ # auth:
+ # username: mydockerhub-user
+ # password: $DOCKERHUB_PASSWORD # context / project UI env-var reference
+ # environment:
+ # POSTGRES_USER: user
+ # - image: redis@sha256:54057dd7e125ca41afe526a877e8bd35ec2cdd33b9217e022ed37bdcf7d09673
+ # auth:
+ # username: mydockerhub-user
+ # password: $DOCKERHUB_PASSWORD # context / project UI env-var reference
+ # - image: acme-private/private-image:321
+ # auth:
+ # username: mydockerhub-user
+ # password: $DOCKERHUB_PASSWORD # context / project UI env-var reference
+ # # Invoke jobs via workflows
+ # # See: https://circleci.com/docs/2.0/configuration-reference/#workflows
+ # workflows:
+ # say-hello-workflow:
+ # jobs:
+ # - say-hello
+ :return: List of container image objects mentioned in the file.
+
+ """
+
+ images = set()
+
+ workflow, workflow_line_numbers = self._parse_file(file_path)
+ self.add_docker_job_images(workflow, images, file_path)
+
+ return images
+
+ def add_docker_job_images(self, workflow: dict, images: set, file_path: str) -> None:
+ """
+
+ :param workflow: parsed workflow file
+ :param images: set of images to be updated
+ :param file_path: path of analyzed workflow
+ """
+
+ # Only 'docker' executor paths are supported. (machine, windows & osx executors do not yeild OCI compatible images see https://circleci.com/docs/2.0/configuration-reference#docker-machine-macos-windows-executor)
+ keywords = [
+ 'jobs.*.docker[].{image: image, __startline__: __startline__, __endline__:__endline__}']
+ for keyword in keywords:
+ results = jmespath.search(keyword, workflow)
+ for result in results:
+ image_name = result.get("image", None)
+ if image_name:
+ image_id = self.inspect(image_name)
+ image_obj = Image(file_path=file_path, name=image_name, image_id=image_id,
+ start_line=result["__startline__"],
+ end_line=result["__endline__"])
+ images.add(image_obj)
+
+ # def add_root_image(self, file_path: str, images: set,
+ # workflow_line_numbers: dict, workflow: dict) -> None:
+ # root_image = workflow.get("image", "")
+
+ # if root_image:
+ # for line_number, line_txt in workflow_line_numbers:
+ # if "image" in line_txt and not line_txt.startswith(' '):
+ # image_id = self.inspect(root_image)
+ # image_obj = Image(
+ # file_path=file_path,
+ # name=root_image,
+ # image_id=image_id,
+ # start_line=line_number,
+ # end_line=line_number,
+ # )
+ # images.add(image_obj)

diff:
@@ -32,6 +32,7 @@ init(autoreset=True)
@dataclass
class CheckType:
BITBUCKET_PIPELINES = "bitbucket_pipelines"
+ CIRCLECI_PIPELINES = "circleci_pipelines"
ARGO_WORKFLOWS = "argo_workflows"
ARM = "arm"
BICEP = "bicep"

diff:
@@ -61,7 +61,7 @@ from checkov.version import version
from checkov.yaml_doc.runner import Runner as yaml_runner
from checkov.bicep.runner import Runner as bicep_runner
from checkov.openapi.runner import Runner as openapi_runner
-
+from checkov.circleci_pipelines.runner import Runner as circleci_pipelines_runner
signal.signal(signal.SIGINT, lambda x, y: sys.exit(''))
outer_registry = None
@@ -93,6 +93,7 @@ DEFAULT_RUNNERS = (
openapi_runner(),
sca_image_runner(),
argo_workflows_runner(),
+ circleci_pipelines_runner(),
)


message: Debug linenumbers orbs

diff:
@@ -11,7 +11,7 @@ class PreventDevelopmentOrbs(BaseCircleCIPipelinesCheck):
name=name,
id=id,
block_type=BlockType.ARRAY,
- supported_entities=['orbs']
+ supported_entities=['orbs[].{image: image, __startline__: __startline__, __endline__:__endline__}']
)
def scan_entity_conf(self, conf):

original_message: Debug linenumbers orbs


message: CircleCI runner with Injection and Image checks + tests

diff:
+from asyncio.log import logger
+from operator import contains
+from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
+from checkov.common.models.enums import CheckResult
+from checkov.yaml_doc.enums import BlockType
+import re
+
+
+class ReverseShellNetcat(BaseCircleCIPipelinesCheck):
+ def __init__(self):
+ name = "Suspicious use of netcat with IP address"
+ id = "CKV_CIRCLECIPIPELINES_5"
+ super().__init__(
+ name=name,
+ id=id,
+ block_type=BlockType.ARRAY,
+ supported_entities=['jobs.*.steps[]']
+ )
+
+ def scan_entity_conf(self, conf):
+ if "run" not in conf:
+ return CheckResult.PASSED, conf
+ run = conf.get("run", "")
+ if type(run) == dict:
+ run = run.get("command", "")
+ if re.search(r'(nc|netcat) (\d{1,3}).(\d{1,3}).(\d{1,3}).(\d{1,3})', run):
+ return CheckResult.FAILED, conf
+ return CheckResult.PASSED, conf
+
+
+check = ReverseShellNetcat()

diff:
+from asyncio.log import logger
+from operator import contains
+from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
+from checkov.circleci_pipelines.common.shell_injection_list import terms as bad_inputs
+from checkov.common.models.enums import CheckResult
+from checkov.yaml_doc.enums import BlockType
+import re
+
+
+class DontAllowShellInjection(BaseCircleCIPipelinesCheck):
+ def __init__(self):
+ name = "Ensure run commands are not vulnerable to shell injection"
+ id = "CKV_CIRCLECIPIPELINES_6"
+ super().__init__(
+ name=name,
+ id=id,
+ block_type=BlockType.ARRAY,
+ #supported_entities=['jobs','jobs.*.steps[]']
+ supported_entities=['jobs.*.steps[]']
+ )
+
+ def scan_entity_conf(self, conf):
+ if "run" not in conf:
+ return CheckResult.PASSED, conf
+ run = conf.get("run", "")
+ if type(run) == dict:
+ run = run.get("command", "")
+ for term in bad_inputs:
+ if re.search(term, run):
+ return CheckResult.FAILED, conf
+
+ for term in bad_inputs:
+ if re.search(term, run):
+ return CheckResult.FAILED, conf
+
+ return CheckResult.PASSED, conf
+
+
+check = DontAllowShellInjection()

diff:
+from asyncio.log import logger
+from operator import contains
+from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
+from checkov.common.models.enums import CheckResult
+from checkov.yaml_doc.enums import BlockType
+
+class SuspectCurlInScript(BaseCircleCIPipelinesCheck):
+ def __init__(self):
+ name = "Suspicious use of curl in run task"
+ id = "CKV_CIRCLECIPIPELINES_7"
+ super().__init__(
+ name=name,
+ id=id,
+ block_type=BlockType.ARRAY,
+ supported_entities=['jobs.*.steps[]']
+ )
+
+ def scan_entity_conf(self, conf):
+ if "run" not in conf:
+ return CheckResult.PASSED, conf
+ run = conf.get("run", "")
+ if type(run) == dict:
+ run = run.get("command", "")
+ if "curl" in run:
+ badstuff = ['curl', 'POST']
+ lines = run.split("\n")
+ for line in lines:
+ if all(x in line for x in badstuff):
+ return CheckResult.FAILED, conf
+ return CheckResult.PASSED, conf
+
+check = SuspectCurlInScript()

diff:
from asyncio.log import logger
+from operator import contains
from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
from checkov.common.models.enums import CheckResult
from checkov.yaml_doc.enums import BlockType
@@ -11,16 +12,22 @@ class PreventDevelopmentOrbs(BaseCircleCIPipelinesCheck):
name=name,
id=id,
block_type=BlockType.ARRAY,
- supported_entities=['orbs[].{image: image, __startline__: __startline__, __endline__:__endline__}']
+ supported_entities=["orbs.{orbs: @}"]
)
def scan_entity_conf(self, conf):
- if isinstance(conf, str):
- if "@dev:" in conf:
+ badOrbInBlock = False
+ for orb in conf:
+ if type(conf[orb]) == str:
+ #Special __ vars show up in this dict too.
+ if "@dev" in conf[orb]:
+ badOrbInBlock = True
+ # We only get one return per orb: section, regardless of how many orbs, so set a flag and error later.
+ # Potentially more JMEpath reflection-foo can resolve this so we end up with a call to scan_entity_conf per orb.
+
+ if badOrbInBlock:
return CheckResult.FAILED, conf
else:
return CheckResult.PASSED, conf
- return CheckResult.PASSED, conf
-
check = PreventDevelopmentOrbs()

diff:
+from asyncio.log import logger
+from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
+from checkov.common.models.enums import CheckResult
+from checkov.yaml_doc.enums import BlockType
+
+class PreventVolatileOrbs(BaseCircleCIPipelinesCheck):
+ def __init__(self):
+ name = "Ensure unversioned volatile orbs are not used."
+ id = "CKV_CIRCLECIPIPELINES_4"
+ super().__init__(
+ name=name,
+ id=id,
+ block_type=BlockType.ARRAY,
+ supported_entities=["orbs.{orbs: @}"]
+ )
+
+ def scan_entity_conf(self, conf):
+ badOrbInBlock = False
+ for orb in conf:
+ if type(conf[orb]) == str:
+ #Special __ vars show up in this dict too.
+ if "@volatile" in conf[orb]:
+ badOrbInBlock = True
+ # We only get one return per orb: section, regardless of how many orbs, so set a flag and error later.
+ # Potentially more JMEpath reflection-foo can resolve this so we end up with a call to scan_entity_conf per orb.
+
+ if badOrbInBlock:
+ return CheckResult.FAILED, conf
+ else:
+ return CheckResult.PASSED, conf
+
+check = PreventVolatileOrbs()

diff:
+# flake8: noqa
+terms = [
+ "\$*CIRCLE_PR_REPONAME*",
+ "\$*CIRCLE_PR_USERNAME*",
+ "\$*CIRCLE_PULL_REQUESTS*",
+ "\$*CIRCLE_TAG*",
+ "\$*CIRCLE_BRANCH*"
+]

change_type: DELETE

diff:
-from asyncio.log import logger
-from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
-from checkov.common.models.enums import CheckResult
-from checkov.yaml_doc.enums import BlockType
-
-class PreventDevelopmentOrbs(BaseCircleCIPipelinesCheck):
- def __init__(self):
- name = "Ensure unversioned volatile orbs are not used."
- id = "CKV_CIRCLECIPIPELINES_4"
- super().__init__(
- name=name,
- id=id,
- block_type=BlockType.ARRAY,
- supported_entities=['orbs']
- )
-
- def scan_entity_conf(self, conf):
- if isinstance(conf, str):
- if "@volatile" in conf:
- return CheckResult.FAILED, conf
- else:
- return CheckResult.PASSED, conf
- return CheckResult.PASSED, conf
-
-
-check = PreventDevelopmentOrbs()

diff:
@@ -25,13 +25,16 @@ class Runner(YamlRunner, ImageReferencer):
def import_registry(self):
return registry
+ def included_paths(self):
+ return [".circleci"]
+
def _parse_file(self, f):
if self.is_workflow_file(f):
return super()._parse_file(f)
def is_workflow_file(self, file_path):
"""
- :return: True if the file mentioned is named config.yml/yaml in .circleci dir. Otherwise: False
+ :return: True if the file mentioned is named config.yml/yaml in .circleci dir from included_paths(). Otherwise: False
"""
abspath = os.path.abspath(file_path)
return WORKFLOW_DIRECTORY in abspath and abspath.endswith(("config.yml", "config.yaml"))

diff:
+---
+orbs:
+ neworb: "whtever/orbname@goodorb"
+ someorb: "orbns/orbname@dev:blah"
+jobs:
+ test-docker-hash-img:
+ docker:
+ -
+ image: "redis@sha256:54057dd7e125ca41afe526a877e8bd35ec2cdd33b9217e022ed37bdcf7d09673"
+ -
+ auth:
+ password: $DOCKERHUB_PASSWORD
+ username: mydockerhub-user
+ test-docker-latest-img:
+ docker:
+ -
+ image: "buildpack-deps:latest"
+ -
+ auth:
+ password: $DOCKERHUB_PASSWORD
+ username: mydockerhub-user
+ command:
+ - "--smallfiles"
+ test-docker-versioned-img:
+ docker:
+ -
+ image: "mongo:2.6.8"
+ -
+ auth:
+ password: $DOCKERHUB_PASSWORD
+ username: mydockerhub-user
+ environment:
+ POSTGRES_USER: user
+ image: "postgres:14.2"
+ test-echo:
+ docker:
+ -
+ image: "cimg/python:latest"
+ steps:
+ - checkout
+ -
+ run: "echo \"this is an echo in a script.\""
+ test-inject:
+ docker:
+ -
+ image: "cimg/python:latest"
+ steps:
+ - checkout
+ - run:
+ command: |
+ curl -sSJL https://www.mongodb.org/static/pgp/server-4.2.asc | sudo apt-key add -
+ echo ${CIRCLE_BRANCH}
+ name: "Multi-line run with injection via vars"
+ test-inject2:
+ docker:
+ -
+ image: "cimg/python:latest"
+ steps:
+ - checkout
+ -
+ run:
+ command: |
+ curl -sSJL https://www.mongodb.org/static/pgp/server-4.2.asc | sudo apt-key add -
+ echo $CIRCLE_BRANCH
+ name: "Multi-line run with injection via vars"
+ test-curl-secret:
+ docker:
+ -
+ image: "cimg/python:latest"
+ steps:
+ - checkout
+ - run:
+ command: |
+ curl -x POST someurl $SECRET
+ name: "Multi-line export secret"
+ test-inject-ci-vars:
+ docker:
+ -
+ image: "cimg/python:latest"
+ steps:
+ - checkout
+ -
+ run:
+ command: |
+ echo ${CIRCLE_PR_REPONAME}
+ name: "Echo the PR Reponame"
+version: 2.1
+workflows:
+ say-hello-workflow:
+ jobs:
+ - test-docker-hash-img
+ - test-docker-latest-img
+ - test-docker-versioned-img
+ - test-echo
+ - test-inject
+ - test-inject2
+ - test-inject-ci-vars
+

diff:
+import os
+import unittest
+
+from checkov.circleci_pipelines.runner import Runner
+from checkov.runner_filter import RunnerFilter
+
+
+class TestRunnerValid(unittest.TestCase):
+
+ def test_runner(self):
+ current_dir = os.path.dirname(os.path.realpath(__file__))
+ valid_dir_path = os.path.join(current_dir, "resources")
+ runner = Runner()
+ checks = []
+ report = runner.run(
+ root_folder=valid_dir_path,
+ runner_filter=RunnerFilter(framework=['circleci_pipelines'], checks=checks)
+ )
+ self.assertEqual(len(report.failed_checks), 19)
+ self.assertEqual(report.parsing_errors, [])
+ self.assertEqual(len(report.passed_checks), 19)
+ self.assertEqual(report.skipped_checks, [])
+ report.print_console()
+
+
+if __name__ == "__main__":
+ unittest.main()


message: Doc generator addition for circleci

diff:
@@ -40,6 +40,7 @@ def test_get_checks_returned_check_number():
"gitlab_configuration",
"bitbucket_configuration",
"bitbucket_pipelines",
+ "circleci_pipelines",
"github_actions",
"OpenAPI",
"gitlab_ci",
@@ -59,6 +60,7 @@ def test_get_checks_returned_check_number():
"Terraform",
"github_configuration",
"bitbucket_pipelines",
+ "circleci_pipelines",
"gitlab_configuration",
"bitbucket_configuration",
"github_actions",

original_message: Doc generator addition for circleci


message: Add CircleCI to build and checkov-wide runner tests

diff:
@@ -151,7 +151,7 @@ jobs:
export PYTHONPATH='.'
git pull
- for i in cloudformation terraform kubernetes serverless arm dockerfile secrets github_configuration gitlab_configuration bitbucket_configuration github_actions gitlab_ci bicep openapi bitbucket_pipelines all
+ for i in cloudformation terraform kubernetes serverless arm dockerfile secrets github_configuration gitlab_configuration bitbucket_configuration github_actions gitlab_ci bicep openapi bitbucket_pipelines circleci_pipelines all
do
export scansdoc="docs/5.Policy Index/$i.md"
echo "---" > "$scansdoc"

diff:
@@ -13,6 +13,7 @@ from checkov.bicep.checks.param.registry import registry as bicep_param_registry
from checkov.bicep.checks.resource.registry import registry as bicep_resource_registry
from checkov.bitbucket.registry import registry as bitbucket_configuration_registry
from checkov.bitbucket_pipelines.registry import registry as bitbucket_pipelines_registry
+from checkov.circleci_pipelines.registry import registry as circleci_pipelines_registry
from checkov.cloudformation.checks.resource.registry import cfn_registry as cfn_registry
from checkov.common.checks.base_check_registry import BaseCheckRegistry
from checkov.common.checks_infra.registry import BaseRegistry as BaseGraphRegistry, get_graph_checks_registry
@@ -118,6 +119,8 @@ def get_checks(frameworks: Optional[List[str]] = None, use_bc_ids: bool = False,
add_from_repository(bitbucket_configuration_registry, "bitbucket_configuration", "bitbucket_configuration")
if any(x in framework_list for x in ("all", "bitbucket_pipelines")):
add_from_repository(bitbucket_pipelines_registry, "bitbucket_pipelines", "bitbucket_pipelines")
+ if any(x in framework_list for x in ("all", "circleci_pipelines")):
+ add_from_repository(circleci_pipelines_registry, "circleci_pipelines", "circleci_pipelines")
if any(x in framework_list for x in ("all", "argo_workflows")):
add_from_repository(argo_workflows_registry, "argo_workflows", "Argo Workflows")
if any(x in framework_list for x in ("all", "arm")):

diff:
@@ -34,6 +34,7 @@ from checkov.runner_filter import RunnerFilter
"gitlab_ci",
"bitbucket_configuration",
"bitbucket_pipelines",
+ "circleci_pipelines",
"kustomize",
"sca_package",
"openapi",


message: Linter happyness

diff:
from __future__ import annotations
import jmespath
import os
-from typing import TYPE_CHECKING
from checkov.common.images.image_referencer import ImageReferencer, Image
from checkov.common.output.report import CheckType
from checkov.circleci_pipelines.registry import registry

original_message: Linter happyness


message: Tidy up commented code

diff:
@@ -111,20 +111,3 @@ class Runner(YamlRunner, ImageReferencer):
start_line=result["__startline__"],
end_line=result["__endline__"])
images.add(image_obj)
\ No newline at end of file
-
- # def add_root_image(self, file_path: str, images: set,
- # workflow_line_numbers: dict, workflow: dict) -> None:
- # root_image = workflow.get("image", "")
-
- # if root_image:
- # for line_number, line_txt in workflow_line_numbers:
- # if "image" in line_txt and not line_txt.startswith(' '):
- # image_id = self.inspect(root_image)
- # image_obj = Image(
- # file_path=file_path,
- # name=root_image,
- # image_id=image_id,
- # start_line=line_number,
- # end_line=line_number,
- # )
- # images.add(image_obj)

original_message: Tidy up commented code


message: WIP Annotations cc/

diff:
from __future__ import annotations
import jmespath
import os
+from typing import TYPE_CHECKING, Any
+
from checkov.common.images.image_referencer import ImageReferencer, Image
from checkov.common.output.report import CheckType
from checkov.circleci_pipelines.registry import registry
from checkov.yaml_doc.runner import Runner as YamlRunner
+if TYPE_CHECKING:
+ from checkov.common.checks.base_check_registry import BaseCheckRegistry
+
WORKFLOW_DIRECTORY = "circleci"
class Runner(YamlRunner, ImageReferencer):
check_type = CheckType.CIRCLECI_PIPELINES # noqa: CCE003 # a static attribute
- def __init__(self):
- super().__init__()
-
- def require_external_checks(self):
+ def require_external_checks(self) -> bool:
return False
- def import_registry(self):
+ def import_registry(self) -> BaseCheckRegistry:
return registry
- def included_paths(self):
+ def included_paths(self) -> list[str]:
return [".circleci"]
- def _parse_file(self, f):
+ def _parse_file(self, f: str) -> tuple[dict[str, Any] | list[dict[str, Any]], list[tuple[int, str]]] | None:
if self.is_workflow_file(f):
return super()._parse_file(f)
- def is_workflow_file(self, file_path):
+ def is_workflow_file(self, file_path: str) -> bool:
"""
:return: True if the file mentioned is named config.yml/yaml in .circleci dir from included_paths(). Otherwise: False
"""
abspath = os.path.abspath(file_path)
return WORKFLOW_DIRECTORY in abspath and abspath.endswith(("config.yml", "config.yaml"))
- def get_images(self, file_path):
+ def get_images(self, file_path: str) -> set[Image]:
"""
Get container images mentioned in a file
:param file_path: File to be inspected
@@ -83,14 +85,14 @@ class Runner(YamlRunner, ImageReferencer):
"""
- images = set()
+ images: set[Image] = set()
workflow, workflow_line_numbers = self._parse_file(file_path)
- self.add_docker_job_images(workflow, images, file_path)
+ images.union(self.add_docker_job_images(workflow, images, file_path))
return images
- def add_docker_job_images(self, workflow: dict, images: set, file_path: str) -> None:
+ def add_docker_job_images(self, workflow: dict, images: set, file_path: str) -> set[Image]:
"""
:param workflow: parsed workflow file
@@ -111,3 +113,4 @@ class Runner(YamlRunner, ImageReferencer):
start_line=result["__startline__"],
end_line=result["__endline__"])
images.add(image_obj)
+ return images
\ No newline at end of file

original_message: WIP Annotations cc/ @antongrubel


message: Type Annotation WIP

diff:
+from __future__ import annotations
+
+from typing import Iterable
from checkov.common.checks.base_check import BaseCheck
from checkov.common.models.enums import CheckCategories
@@ -5,7 +8,14 @@ from checkov.circleci_pipelines.registry import registry
class BaseCircleCIPipelinesCheck(BaseCheck):
- def __init__(self, name, id, supported_entities, block_type, path=None):
+ def __init__(
+ self,
+ name: str,
+ id: str,
+ supported_entities: Iterable[str],
+ block_type: str,
+ path: str | None = None,
+ ) -> None:
categories = [CheckCategories.SUPPLY_CHAIN]
super().__init__(

diff:
+from __future__ import annotations
+from typing import Any
+
from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
from checkov.common.models.enums import CheckResult
from checkov.yaml_doc.enums import BlockType
@@ -5,7 +8,7 @@ import re
class ReverseShellNetcat(BaseCircleCIPipelinesCheck):
- def __init__(self):
+ def __init__(self) -> None:
name = "Suspicious use of netcat with IP address"
id = "CKV_CIRCLECIPIPELINES_5"
super().__init__(
@@ -15,7 +18,7 @@ class ReverseShellNetcat(BaseCircleCIPipelinesCheck):
supported_entities=['jobs.*.steps[]']
)
- def scan_entity_conf(self, conf):
+ def scan_entity_conf(self, conf: dict[str, Any] ) -> tuple[CheckResult, dict[str, Any]]:
if "run" not in conf:
return CheckResult.PASSED, conf
run = conf.get("run", "")

diff:
+from __future__ import annotations
+from typing import Any
+
from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
from checkov.circleci_pipelines.common.shell_injection_list import terms as bad_inputs
from checkov.common.models.enums import CheckResult
@@ -6,7 +9,7 @@ import re
class DontAllowShellInjection(BaseCircleCIPipelinesCheck):
- def __init__(self):
+ def __init__(self) -> None:
name = "Ensure run commands are not vulnerable to shell injection"
id = "CKV_CIRCLECIPIPELINES_6"
super().__init__(
@@ -16,7 +19,7 @@ class DontAllowShellInjection(BaseCircleCIPipelinesCheck):
supported_entities=['jobs.*.steps[]']
)
- def scan_entity_conf(self, conf):
+ def scan_entity_conf(self, conf: dict[str, Any] ) -> tuple[CheckResult, dict[str, Any]]:
if "run" not in conf:
return CheckResult.PASSED, conf
run = conf.get("run", "")

diff:
+from __future__ import annotations
+from typing import Any
+
from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
from checkov.common.models.enums import CheckResult
from checkov.yaml_doc.enums import BlockType
class SuspectCurlInScript(BaseCircleCIPipelinesCheck):
- def __init__(self):
+ def __init__(self) -> None:
name = "Suspicious use of curl in run task"
id = "CKV_CIRCLECIPIPELINES_7"
super().__init__(
@@ -13,7 +16,7 @@ class SuspectCurlInScript(BaseCircleCIPipelinesCheck):
supported_entities=['jobs.*.steps[]']
)
- def scan_entity_conf(self, conf):
+ def scan_entity_conf(self, conf: dict[str, Any] ) -> tuple[CheckResult, dict[str, Any]]:
if "run" not in conf:
return CheckResult.PASSED, conf
run = conf.get("run", "")

diff:
+from __future__ import annotations
+from typing import Any
+
from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
from checkov.common.models.enums import CheckResult
from checkov.yaml_doc.enums import BlockType
class ImageReferenceHashVersion(BaseCircleCIPipelinesCheck):
- def __init__(self):
+ def __init__(self) -> None:
name = "Ensure the pipeline image version is referenced via hash not arbitrary tag."
id = "CKV_CIRCLECIPIPELINES_2"
super().__init__(
@@ -14,12 +17,12 @@ class ImageReferenceHashVersion(BaseCircleCIPipelinesCheck):
supported_entities=['jobs.*.docker[].{image: image, __startline__: __startline__, __endline__:__endline__}']
)
- def scan_entity_conf(self, conf):
+ def scan_entity_conf(self, conf: dict[str, Any] ) -> tuple[CheckResult, dict[str, Any]]:
if not isinstance(conf, dict):
- return
+ return CheckResult.PASSED, conf
image = conf.get("image", None)
if not image:
- return
+ return CheckResult.PASSED, conf
if isinstance(image, str):
if "@" in image:
return CheckResult.PASSED, conf

diff:
+from __future__ import annotations
+from typing import Any
+
from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
from checkov.common.models.enums import CheckResult
from checkov.yaml_doc.enums import BlockType
class ImageReferenceLatestTag(BaseCircleCIPipelinesCheck):
- def __init__(self):
+ def __init__(self) -> None:
name = "Ensure the pipeline image uses a non latest version tag"
id = "CKV_CIRCLECIPIPELINES_1"
super().__init__(
@@ -13,12 +16,12 @@ class ImageReferenceLatestTag(BaseCircleCIPipelinesCheck):
supported_entities=['jobs.*.docker[].{image: image, __startline__: __startline__, __endline__:__endline__}']
)
- def scan_entity_conf(self, conf):
+ def scan_entity_conf(self, conf: dict[str, Any] ) -> tuple[CheckResult, dict[str, Any]] | None:
if not isinstance(conf, dict):
- return
+ return CheckResult.PASSED, conf
image = conf.get("image", None)
if not image:
- return
+ return CheckResult.PASSED, conf
if isinstance(image, str):
if image.endswith(":latest"):
return CheckResult.FAILED, conf

diff:
+from __future__ import annotations
+from typing import Any
+
from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
from checkov.common.models.enums import CheckResult
from checkov.yaml_doc.enums import BlockType
class PreventDevelopmentOrbs(BaseCircleCIPipelinesCheck):
- def __init__(self):
+ def __init__(self) -> None:
name = "Ensure mutable development orbs are not used."
id = "CKV_CIRCLECIPIPELINES_3"
super().__init__(
@@ -13,7 +16,7 @@ class PreventDevelopmentOrbs(BaseCircleCIPipelinesCheck):
supported_entities=["orbs.{orbs: @}"]
)
- def scan_entity_conf(self, conf):
+ def scan_entity_conf(self, conf: dict[str, Any] ) -> tuple[CheckResult, dict[str, Any]]:
badOrbInBlock = False
for orb in conf:
if type(conf[orb]) == str:

diff:
+from __future__ import annotations
+from typing import Any
+
from checkov.circleci_pipelines.base_circleci_pipelines_check import BaseCircleCIPipelinesCheck
from checkov.common.models.enums import CheckResult
from checkov.yaml_doc.enums import BlockType
class PreventVolatileOrbs(BaseCircleCIPipelinesCheck):
- def __init__(self):
+ def __init__(self) -> None:
name = "Ensure unversioned volatile orbs are not used."
id = "CKV_CIRCLECIPIPELINES_4"
super().__init__(
@@ -13,7 +16,7 @@ class PreventVolatileOrbs(BaseCircleCIPipelinesCheck):
supported_entities=["orbs.{orbs: @}"]
)
- def scan_entity_conf(self, conf):
+ def scan_entity_conf(self, conf: dict[str, Any] ) -> tuple[CheckResult, dict[str, Any]]:
badOrbInBlock = False
for orb in conf:
if type(conf[orb]) == str:

original_message: Type Annotation WIP


message:
Remove duplicate violation on latest images
Update tests based on new expected failed tests numbers.

diff:
@@ -26,6 +26,10 @@ class ImageReferenceHashVersion(BaseCircleCIPipelinesCheck):
if isinstance(image, str):
if "@" in image:
return CheckResult.PASSED, conf
+ if "latest" in image:
+ return CheckResult.PASSED, conf
+ # We pass on "latest" as we have a specific check with a more informative violation description.
+ # See CKV_CIRCLECIPIPELINES_1
return CheckResult.FAILED, conf

diff:
@@ -16,9 +16,9 @@ class TestRunnerValid(unittest.TestCase):
root_folder=valid_dir_path,
runner_filter=RunnerFilter(framework=['circleci_pipelines'], checks=checks)
)
- self.assertEqual(len(report.failed_checks), 19)
+ self.assertEqual(len(report.failed_checks), 13)
self.assertEqual(report.parsing_errors, [])
- self.assertEqual(len(report.passed_checks), 19)
+ self.assertEqual(len(report.passed_checks), 29)
self.assertEqual(report.skipped_checks, [])
report.print_console()


message: Changes for image ver latest runner to UNKNOWN and related test changes

diff:
@@ -28,7 +28,7 @@ class ImageReferenceHashVersion(BaseCircleCIPipelinesCheck):
return CheckResult.PASSED, conf
if "latest" in image:
return CheckResult.UNKNOWN, conf
- # We pass on "latest" as we have a specific check with a more informative violation description.
+ # We UNKNOWN on "latest" as we have a specific check with a more informative violation description.
# See CKV_CIRCLECIPIPELINES_1
return CheckResult.FAILED, conf

diff:
@@ -18,7 +18,7 @@ class TestRunnerValid(unittest.TestCase):
)
self.assertEqual(len(report.failed_checks), 13)
self.assertEqual(report.parsing_errors, [])
- self.assertEqual(len(report.passed_checks), 29)
+ self.assertEqual(len(report.passed_checks), 23)
self.assertEqual(report.skipped_checks, [])
report.print_console()


message: Fixups to check logic after review changes.

diff:
@@ -26,6 +26,10 @@ class ReverseShellNetcat(BaseCircleCIPipelinesCheck):
command = run.get("command", "")
if re.search(r'(nc|netcat) (\d{1,3}).(\d{1,3}).(\d{1,3}).(\d{1,3})', command):
return CheckResult.FAILED, conf
+ else:
+ if re.search(r'(nc|netcat) (\d{1,3}).(\d{1,3}).(\d{1,3}).(\d{1,3})', run):
+ return CheckResult.FAILED, conf
+
return CheckResult.PASSED, conf

diff:
@@ -29,6 +29,7 @@ class DontAllowShellInjection(BaseCircleCIPipelinesCheck):
if re.search(term, command):
return CheckResult.FAILED, conf
+ else:
for term in bad_inputs:
if re.search(term, run):
return CheckResult.FAILED, conf

diff:
@@ -17,18 +17,14 @@ class PreventVolatileOrbs(BaseCircleCIPipelinesCheck):
)
def scan_conf(self, conf: dict[str, Any]) -> tuple[CheckResult, dict[str, Any]]:
- badOrbInBlock = False
- for orb in conf:
- if type(conf[orb]) == str:
+ for orb in conf.values():
+ if isinstance(orb, str):
# Special __ vars show up in this dict too.
- if "@volatile" in conf[orb]:
- badOrbInBlock = True
- # We only get one return per orb: section, regardless of how many orbs, so set a flag and error later.
+ if "@volitile" in orb:
+ # We only get one return per orb: section, regardless of how many orbs.
# Potentially more JMEpath reflection-foo can resolve this so we end up with a call to scan_entity_conf per orb.
-
- if badOrbInBlock:
return CheckResult.FAILED, conf
- else:
+
return CheckResult.PASSED, conf
check = PreventVolatileOrbs()


message:
[Logging][Mining] Add BCLog::MINING category.
Adds a new logging category MINING to aid debugging mining specifically
without including block template creation logs that may be noisier.
Converts most logs in BitcoinMiner and RandomXMiner to use this category
instead of BLOCKCREATION.
Fixes one unlikely logging statement to include the function name.

diff:
@@ -123,6 +123,7 @@ const CLogCategoryDesc LogCategories[] =
{BCLog::BLOCKCREATION, "blockcreation"},
{BCLog::CHAINSCORE, "chainscore"},
{BCLog::STAGING, "staging"},
+ {BCLog::MINING, "mining"},
{BCLog::ALL, "1"},
{BCLog::ALL, "all"},
};

diff:
@@ -1011,7 +1011,7 @@ void BitcoinMiner(std::shared_ptr<CReserveScript> coinbaseScript, bool fProofOfS
++pblock->nNonce;
}
} else {
- LogPrintf("%s: Unknown hashing algorithm found!\n");
+ LogPrintf("%s: Unknown hashing algorithm found!\n", __func__);
return;
}
@@ -1023,8 +1023,8 @@ void BitcoinMiner(std::shared_ptr<CReserveScript> coinbaseScript, bool fProofOfS
if (!nTimeDuration) nTimeDuration = 1;
nHashSpeed = arith_uint256(nHashes/1000/nTimeDuration).getdouble();
}
- LogPrint(BCLog::BLOCKCREATION, "%s: PoW Hashspeed %d kh/s\n", __func__, nHashSpeed);
+ LogPrint(BCLog::MINING, "%s: PoW Hashspeed %d kh/s\n", __func__, nHashSpeed);
if (nTries == nInnerLoopCount) {
continue;
}
@@ -1032,7 +1032,7 @@ void BitcoinMiner(std::shared_ptr<CReserveScript> coinbaseScript, bool fProofOfS
std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(*pblock);
if (!ProcessNewBlock(Params(), shared_pblock, true, nullptr)) {
- LogPrint(BCLog::BLOCKCREATION, "%s : Failed to process new block\n", __func__);
+ LogPrint(BCLog::MINING, "%s: Failed to process new block\n", __func__);
continue;
}
@@ -1142,8 +1142,8 @@ void BitcoinRandomXMiner(std::shared_ptr<CReserveScript> coinbaseScript, int vm_
nHashSpeed = arith_uint256(nHashes/nTimeDuration).getdouble();
}
}
- LogPrint(BCLog::BLOCKCREATION, "%s: RandomX PoW Hashspeed %d hashes/s\n", __func__, nHashSpeed);
+ LogPrint(BCLog::MINING, "%s: RandomX PoW Hashspeed %d hashes/s\n", __func__, nHashSpeed);
if (nTries == nInnerLoopCount) {
continue;
}
@@ -1155,7 +1155,7 @@ void BitcoinRandomXMiner(std::shared_ptr<CReserveScript> coinbaseScript, int vm_
std::shared_ptr<const CBlock> shared_pblock = std::make_shared<const CBlock>(*pblock);
if (!ProcessNewBlock(Params(), shared_pblock, true, nullptr)) {
- LogPrint(BCLog::BLOCKCREATION, "%s: Failed to process new block\n", __func__);
+ LogPrint(BCLog::MINING, "%s: Failed to process new block\n", __func__);
continue;
}

license: MIT License

repo: veil-project/veil


message:
[Utils] Use gmtime_s with _WIN32
This allows the project to build in WSL/Ubuntu 20.04 x64
and potentially other Windows versions.

diff:
@@ -79,7 +79,7 @@ void MilliSleep(int64_t n)
std::string FormatISO8601DateTime(int64_t nTime) {
struct tm ts;
time_t time_val = nTime;
-#ifdef _MSC_VER
+#if defined(_MSC_VER) || defined(_WIN32)
gmtime_s(&ts, &time_val);
#else
gmtime_r(&time_val, &ts);
@@ -90,7 +90,7 @@ std::string FormatISO8601DateTime(int64_t nTime) {
std::string FormatISO8601Date(int64_t nTime) {
struct tm ts;
time_t time_val = nTime;
-#ifdef _MSC_VER
+#if defined(_MSC_VER) || defined(_WIN32)
gmtime_s(&ts, &time_val);
#else
gmtime_r(&time_val, &ts);
@@ -101,7 +101,7 @@ std::string FormatISO8601Date(int64_t nTime) {
std::string FormatISO8601Time(int64_t nTime) {
struct tm ts;
time_t time_val = nTime;
-#ifdef _MSC_VER
+#if defined(_MSC_VER) || defined(_WIN32)
gmtime_s(&ts, &time_val);
#else
gmtime_r(&time_val, &ts);


message:
[Logging] Define a STAKING log category.
This allows separation of log messages related to staking from log messages
about block creation, which currently includes all generic block creation
messages as well as DarkGravityWave difficulty measurements.

diff:
@@ -124,6 +124,7 @@ const CLogCategoryDesc LogCategories[] =
{BCLog::CHAINSCORE, "chainscore"},
{BCLog::STAGING, "staging"},
{BCLog::MINING, "mining"},
+ {BCLog::STAKING, "staking"},
{BCLog::ALL, "1"},
{BCLog::ALL, "all"},
};

diff:
@@ -515,7 +515,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
}
auto spend = TxInToZerocoinSpend(pblock->vtx[1]->vin[0]);
if (!spend) {
- LogPrint(BCLog::BLOCKCREATION, "%s: failed to get spend for txin", __func__);
+ LogPrint(BCLog::STAKING, "%s: failed to get spend for txin", __func__);
return nullptr;
}
@@ -525,17 +525,17 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
#ifdef ENABLE_WALLET
if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET) || !pwalletMain->GetZerocoinKey(bnSerial, key)) {
#endif
- LogPrint(BCLog::BLOCKCREATION, "%s: Failed to get zerocoin key from wallet!\n", __func__);
+ LogPrint(BCLog::STAKING, "%s: Failed to get zerocoin key from wallet!\n", __func__);
return nullptr;
#ifdef ENABLE_WALLET
}
#endif
if (!key.Sign(pblock->GetHash(), pblock->vchBlockSig)) {
- LogPrint(BCLog::BLOCKCREATION, "%s: Failed to sign block hash\n", __func__);
+ LogPrint(BCLog::STAKING, "%s: Failed to sign block hash\n", __func__);
return nullptr;
}
- LogPrint(BCLog::BLOCKCREATION, "%s: FOUND STAKE!!\n block: \n%s\n", __func__, pblock->ToString());
+ LogPrint(BCLog::STAKING, "%s: FOUND STAKE!!\n block: \n%s\n", __func__, pblock->ToString());
}
if (pindexPrev && pindexPrev != chainActive.Tip()) {
@@ -932,7 +932,7 @@ void BitcoinMiner(std::shared_ptr<CReserveScript> coinbaseScript, bool fProofOfS
auto it = mapStakeHashCounter.find(nHeight);
if (it != mapStakeHashCounter.end() && it->second != nStakeHashesLast) {
nStakeHashesLast = it->second;
- LogPrint(BCLog::BLOCKCREATION, "%s: Tried %d stake hashes for block %d last=%d\n", __func__, nStakeHashesLast, nHeight+1, mapHashedBlocks.at(hashBestBlock));
+ LogPrint(BCLog::STAKING, "%s: Tried %d stake hashes for block %d last=%d\n", __func__, nStakeHashesLast, nHeight+1, mapHashedBlocks.at(hashBestBlock));
}
// wait half of the nHashDrift with max wait of 3 minutes
int rand = GetRandInt(20); // add small randomness to prevent all nodes from being on too similar of timing

diff:
@@ -3908,7 +3908,7 @@ bool CWallet::CreateCoinStake(const CBlockIndex* pindexBest, unsigned int nBits,
LOCK(cs_main);
//Double check that this will pass time requirements
if (nTxNewTime <= nTimeMinBlock) {
- LogPrint(BCLog::BLOCKCREATION, "CreateCoinStake() : kernel found, but it is too far in the past \n");
+ LogPrint(BCLog::STAKING, "%s : kernel found, but it is too far in the past \n", __func__);
continue;
}
nHeight = chainActive.Height();
@@ -4008,7 +4008,7 @@ bool CWallet::SelectStakeCoins(std::list<std::unique_ptr<ZerocoinStake> >& listI
}
}
- LogPrint(BCLog::BLOCKCREATION, "%s: FOUND %d STAKABLE ZEROCOINS\n", __func__, listInputs.size());
+ LogPrint(BCLog::STAKING, "%s: FOUND %d STAKABLE ZEROCOINS\n", __func__, listInputs.size());
return true;
}


message:
[Mining] Use -miningaddress for in-wallet mining.
Replaces the hook to get a mining address from the wallet
with a retrieval of the address set as `-miningaddress`.
Performs an additional safety check at startup that the wallet
actually owns the given address.

diff:
@@ -1933,6 +1933,17 @@ bool AppInitMain()
// Link thread groups
LinkAutoSpendThreadGroup(&threadGroupAutoSpend);
+ // Validate wallet mining address.
+ std::string sAddress = gArgs.GetArg("-miningaddress", "");
+ if (!sAddress.empty()) {
+ CTxDestination dest = DecodeDestination(sAddress);
+
+ auto pt = GetMainWallet();
+ if (pt && pt->IsMine(dest) != ISMINE_SPENDABLE) {
+ return InitError(strprintf(_("Invalid -miningaddress: '%s' not owned by wallet"), sAddress));
+ }
+ }
+
// Start wallet CPU mining if the -gen=<n> parameter is given
int nThreads = gArgs.GetArg("-gen", 0);
if (nThreads) {

diff:
@@ -4702,7 +4702,7 @@ void CWallet::MarkReserveKeysAsUsed(int64_t keypool_id)
}
}
-void CWallet::GetScriptForMining(std::shared_ptr<CReserveScript> &script)
+void CWallet::GetScriptForMiningReserveKey(std::shared_ptr<CReserveScript> &script)
{
std::shared_ptr<CReserveKey> rKey = std::make_shared<CReserveKey>(this);
CPubKey pubkey;
@@ -4713,6 +4713,19 @@ void CWallet::GetScriptForMining(std::shared_ptr<CReserveScript> &script)
script->reserveScript = CScript() << ToByteVector(pubkey) << OP_CHECKSIG;
}
+void CWallet::GetScriptForMining(std::shared_ptr<CReserveScript> &script)
+{
+ std::string sAddress = gArgs.GetArg("-miningaddress", "");
+ if (!sAddress.empty()) {
+ CTxDestination dest = DecodeDestination(sAddress);
+
+ script = std::make_shared<CReserveScript>();
+ script->reserveScript = GetScriptForDestination(dest);
+ return;
+ }
+ return GetScriptForMiningReserveKey(script);
+}
+
void CWallet::LockCoin(const COutPoint& output)
{
AssertLockHeld(cs_wallet); // setLockedCoins

diff:
@@ -1221,6 +1221,7 @@ public:
const std::string& GetLabelName(const CScript& scriptPubKey) const;
+ void GetScriptForMiningReserveKey(std::shared_ptr<CReserveScript> &script);
void GetScriptForMining(std::shared_ptr<CReserveScript> &script);
unsigned int GetKeyPoolSize() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet)


message:
[Trivial] Placate clang warning string-plus-int
Replace eg `"tv"+2` with the suggested syntax of `&"tv"[2]`.

diff:
@@ -301,8 +301,8 @@ public:
nZerocoinAccount = 100000;
//sv for "stealth veil" & bv for "basecoin veil"
- bech32Prefixes[STEALTH_ADDRESS].assign("sv","sv"+2);
- bech32Prefixes[BASE_ADDRESS].assign("bv", "bv"+2);
+ bech32Prefixes[STEALTH_ADDRESS].assign("sv",&"sv"[2]);
+ bech32Prefixes[BASE_ADDRESS].assign("bv", &"bv"[2]);
bech32_hrp_stealth = "sv";
bech32_hrp_base = "bv";
@@ -491,8 +491,8 @@ public:
base58Prefixes[EXT_PUBLIC_KEY] = {0x04, 0x35, 0x87, 0xCF};
base58Prefixes[EXT_SECRET_KEY] = {0x04, 0x35, 0x83, 0x94};
- bech32Prefixes[STEALTH_ADDRESS].assign("tps","tps"+3);
- bech32Prefixes[BASE_ADDRESS].assign("tv", "tv"+2);
+ bech32Prefixes[STEALTH_ADDRESS].assign("tps",&"tps"[3]);
+ bech32Prefixes[BASE_ADDRESS].assign("tv", &"tv"[2]);
nBIP44ID = 0x80000001;
nRingCTAccount = 20000;
nZerocoinAccount = 100000;
@@ -662,8 +662,8 @@ public:
base58Prefixes[EXT_PUBLIC_KEY] = {0x04, 0x35, 0x87, 0xCF};
base58Prefixes[EXT_SECRET_KEY] = {0x04, 0x35, 0x83, 0x94};
- bech32Prefixes[STEALTH_ADDRESS].assign("tps","tps"+3);
- bech32Prefixes[BASE_ADDRESS].assign("tv", "tv"+2);
+ bech32Prefixes[STEALTH_ADDRESS].assign("tps",&"tps"[3]);
+ bech32Prefixes[BASE_ADDRESS].assign("tv", &"tv"[2]);
nBIP44ID = 0x80000001;
nRingCTAccount = 20000;
nZerocoinAccount = 100000;
@@ -832,8 +832,8 @@ public:
base58Prefixes[EXT_PUBLIC_KEY] = {0x04, 0x35, 0x87, 0xCF};
base58Prefixes[EXT_SECRET_KEY] = {0x04, 0x35, 0x83, 0x94};
- bech32Prefixes[STEALTH_ADDRESS].assign("tps","tps"+3);
- bech32Prefixes[BASE_ADDRESS].assign("tv", "tv"+2);
+ bech32Prefixes[STEALTH_ADDRESS].assign("tps", &"tps"[3]);
+ bech32Prefixes[BASE_ADDRESS].assign("tv", &"tv"[2]);
nBIP44ID = 0x80000001;
nRingCTAccount = 20000;
nZerocoinAccount = 100000;


message:
Templatify AssertLockHeld to placate clang warning
By changing the type of the lock from void* (which has no capability)
to the actual type (which does).
Minimize the amount of code we have to move into sync.h by pointing
the AssertLockHeldInternal template at the original function that takes
void*.

diff:
@@ -152,7 +152,7 @@ std::string LocksHeld()
return result;
}
-void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs)
+void _AssertLockHeld(const char* pszName, const char* pszFile, int nLine, void* cs)
{
for (const std::pair<void*, CLockLocation>& i : g_lockstack)
if (i.first == cs)

diff:
@@ -74,14 +74,21 @@ public:
void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false);
void LeaveCritical();
std::string LocksHeld();
-void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs);
void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs);
void DeleteLock(void* cs);
+
+void _AssertLockHeld(const char* pszName, const char* pszFile, int nLine, void* cs);
+template <typename T>
+void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, T* cs) ASSERT_EXCLUSIVE_LOCK(cs) {
+ _AssertLockHeld(pszName, pszFile, nLine, (void*)cs);
+}
#else
void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {}
void static inline LeaveCritical() {}
-void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs) {}
-void static inline AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {}
+template <typename T>
+void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, T* cs) ASSERT_EXCLUSIVE_LOCK(cs) {}
+template <typename T>
+void static inline AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, T* cs) {}
void static inline DeleteLock(void* cs) {}
#endif
#define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs)


message:
Add locking on the chain index vector.
This alleviates a data race reported by tsan (concurrent access to
size(), operator[], and resize()).

diff:
* CChain implementation
*/
void CChain::SetTip(CBlockIndex *pindex) {
+ LOCK(cs_vchain);
if (pindex == nullptr) {
vChain.clear();
return;
@@ -68,6 +69,7 @@ const CBlockIndex *CChain::FindFork(const CBlockIndex *pindex) const {
CBlockIndex* CChain::FindEarliestAtLeast(int64_t nTime) const
{
+ LOCK(cs_vchain);
std::vector<CBlockIndex*>::const_iterator lower = std::lower_bound(vChain.begin(), vChain.end(), nTime,
[](CBlockIndex* pBlock, const int64_t& time) -> bool { return pBlock->GetBlockTimeMax() < time; });
return (lower == vChain.end() ? nullptr : *lower);

diff:
@@ -725,21 +725,25 @@ public:
/** An in-memory indexed chain of blocks. */
class CChain {
private:
- std::vector<CBlockIndex*> vChain;
+ mutable CCriticalSection cs_vchain;
+ std::vector<CBlockIndex*> vChain GUARDED_BY(cs_vchain);
public:
/** Returns the index entry for the genesis block of this chain, or nullptr if none. */
CBlockIndex *Genesis() const {
+ LOCK(cs_vchain);
return vChain.size() > 0 ? vChain[0] : nullptr;
}
/** Returns the index entry for the tip of this chain, or nullptr if none. */
CBlockIndex *Tip() const {
+ LOCK(cs_vchain);
return vChain.size() > 0 ? vChain[vChain.size() - 1] : nullptr;
}
/** Returns the index entry at a particular height in this chain, or nullptr if no such height exists. */
CBlockIndex *operator[](int nHeight) const {
+ LOCK(cs_vchain);
if (nHeight < 0 || nHeight >= (int)vChain.size())
return nullptr;
return vChain[nHeight];
@@ -747,6 +751,10 @@ public:
/** Compare two chains efficiently. */
friend bool operator==(const CChain &a, const CChain &b) {
+ // Maintain consistent lock order.
+ if (&b < &a) return b == a;
+
+ LOCK2(a.cs_vchain, b.cs_vchain);
return a.vChain.size() == b.vChain.size() &&
a.vChain[a.vChain.size() - 1] == b.vChain[b.vChain.size() - 1];
}
@@ -758,6 +766,7 @@ public:
/** Find the successor of a block in this chain, or nullptr if the given index is not found or is the tip. */
CBlockIndex *Next(const CBlockIndex *pindex) const {
+ LOCK(cs_vchain);
if (Contains(pindex))
return (*this)[pindex->nHeight + 1];
else
@@ -766,6 +775,7 @@ public:
/** Return the maximal height in the chain. Is equal to chain.Tip() ? chain.Tip()->nHeight : -1. */
int Height() const {
+ LOCK(cs_vchain);
return vChain.size() - 1;
}


message:
Resolve misc clang lock annotation warnings.
Double lock of cs_main can be replaced with an assertion.
blockheaderToJSON requires **not** holding cs_main.
cs_main is required for: GetBlocksToMaturity, GetAddressBalances,
IsSpent, and TestBlockValidity.

diff:
@@ -235,6 +235,7 @@ WalletTx MakeWalletTx(CHDWallet& wallet, MapRecords_t::const_iterator irtx)
//! Construct wallet tx status struct.
WalletTxStatus MakeWalletTxStatus(const CWalletTx& wtx)
{
+ LOCK(cs_main);
WalletTxStatus result;
auto mi = ::mapBlockIndex.find(wtx.hashBlock);
CBlockIndex* block = mi != ::mapBlockIndex.end() ? mi->second : nullptr;

diff:
@@ -499,7 +499,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
if(fProofOfFullNode && !fProofOfStake)
LogPrint(BCLog::BLOCKCREATION, "%s: A block can not be proof of full node and proof of work.\n", __func__);
else if(fProofOfFullNode && fProofOfStake) {
- LOCK(cs_main);
+ AssertLockHeld(cs_main);
pblock->hashPoFN = veil::GetFullNodeHash(*pblock, pindexPrev);
}

diff:
@@ -960,6 +960,9 @@ static UniValue getblockheader(const JSONRPCRequest& request)
+ HelpExampleRpc("getblockheader", "\"00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09\"")
);
+ const CBlockIndex* pChainTip;
+ const CBlockIndex* pblockindex;
+ {
LOCK(cs_main);
std::string strHash = request.params[0].get_str();
@@ -969,7 +972,7 @@ static UniValue getblockheader(const JSONRPCRequest& request)
if (!request.params[1].isNull())
fVerbose = request.params[1].get_bool();
- const CBlockIndex* pblockindex = LookupBlockIndex(hash);
+ pblockindex = LookupBlockIndex(hash);
if (!pblockindex) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found");
}
@@ -982,7 +985,10 @@ static UniValue getblockheader(const JSONRPCRequest& request)
return strHex;
}
- return blockheaderToJSON(chainActive.Tip(), pblockindex);
+ pChainTip = chainActive.Tip();
+ }
+
+ return blockheaderToJSON(pChainTip, pblockindex);
}
static CBlock GetBlockChecked(const CBlockIndex* pblockindex)

diff:
@@ -721,7 +721,7 @@ static UniValue listaddresses(const JSONRPCRequest& request)
throw JSONRPCError(RPC_INVALID_PARAMETER, type + " is not a valid address type");
}
}
- LOCK(pwallet->cs_wallet);
+ LOCK2(cs_main, pwallet->cs_wallet);
UniValue results(UniValue::VARR);
AnonWallet* pAnonWallet = wallet->GetAnonWallet();
@@ -2110,11 +2110,19 @@ static void ListTransactions(CWallet* const pwallet, const CWalletTx& wtx, const
{
if (wtx.GetDepthInMainChain() < 1)
entry.pushKV("category", "orphan");
- else if (wtx.GetBlocksToMaturity() > 0)
+ else
+ {
+ int nBlocksToMaturity;
+ {
+ LOCK(cs_main);
+ nBlocksToMaturity = wtx.GetBlocksToMaturity();
+ }
+ if (nBlocksToMaturity > 0)
entry.pushKV("category", "immature");
else
entry.pushKV("category", "generate");
}
+ }
else
{
entry.pushKV("category", "receive");

diff:
@@ -771,8 +771,10 @@ bool CWallet::IsSpent(const uint256& hash, unsigned int n) const
}
}
- if (setAnonTx.count(hash))
+ if (setAnonTx.count(hash)) {
+ LOCK(cs_main);
return pAnonWalletMain->IsSpent(hash, n);
+ }
return false;
}
@@ -2310,9 +2312,12 @@ CAmount CWalletTx::GetDebit(const isminefilter& filter) const
CAmount CWalletTx::GetCredit(const isminefilter& filter, bool fResetCache) const
{
+ {
+ LOCK(cs_main);
// Must wait until coinbase is safely deep enough in the chain before valuing it
if (IsCoinBase() && GetBlocksToMaturity() > 0)
return 0;
+ }
CAmount credit = 0;
if (filter & ISMINE_SPENDABLE)
@@ -2345,6 +2350,7 @@ CAmount CWalletTx::GetCredit(const isminefilter& filter, bool fResetCache) const
CAmount CWalletTx::GetImmatureCredit(bool fUseCache) const
{
+ LOCK(cs_main);
if (IsCoinBase() && GetBlocksToMaturity() > 0 && IsInMainChain())
{
if (fUseCache && fImmatureCreditCached)
@@ -2362,9 +2368,12 @@ CAmount CWalletTx::GetAvailableCredit(bool fUseCache, const isminefilter& filter
if (pwallet == nullptr)
return 0;
+ {
+ LOCK(cs_main);
// Must wait until coinbase is safely deep enough in the chain before valuing it
if (IsCoinBase() && GetBlocksToMaturity() > 0)
return 0;
+ }
CAmount* cache = nullptr;
bool* cache_used = nullptr;
@@ -2408,6 +2417,7 @@ CAmount CWalletTx::GetAvailableCredit(bool fUseCache, const isminefilter& filter
CAmount CWalletTx::GetImmatureWatchOnlyCredit(const bool fUseCache) const
{
+ LOCK(cs_main);
if (IsCoinBase() && GetBlocksToMaturity() > 0 && IsInMainChain())
{
if (fUseCache && fImmatureWatchCreditCached)
@@ -4496,7 +4506,7 @@ std::map<CTxDestination, CAmount> CWallet::GetAddressBalances()
std::map<CTxDestination, CAmount> balances;
{
- LOCK(cs_wallet);
+ LOCK2(cs_main, cs_wallet);
for (const auto& walletEntry : mapWallet)
{
const CWalletTx *pcoin = &walletEntry.second;

original_message:
Resolve misc clang lock annotation warnings.

- Double lock of cs_main can be replaced with an assertion.
- blockheaderToJSON requires **not** holding cs_main.
- cs_main is required for: GetBlocksToMaturity, GetAddressBalances,
  IsSpent, and TestBlockValidity.


message:
Improve hash mismatch warning log message.
Previously, it was logging "Block 2: ..." when 2 was the status, which
is fairly misleading. Now, output both the height and the status and
make clear which is which.

diff:
@@ -285,8 +285,8 @@ bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams,
// ignore any duplicates and mark them to be erased
uint256 hashBlock = diskindex.GetBlockHash();
if (hashBlock != key.second) {
- LogPrintf("%s: Skipping Block %d: %s - Block Hash does not match Index Key\n",
- __func__, diskindex.nStatus, hashBlock.GetHex());
+ LogPrintf("%s: Skipping Block %d (status=%d): %s - Block Hash does not match Index Key\n",
+ __func__, diskindex.nHeight, diskindex.nStatus, hashBlock.GetHex());
pcursor->Next();
continue;
}


message:
Clear the calculated hash speed at mining start.
This avoids having the hash speed affected by a gap in mining
or a change in the number of mining threads, without requiring
users to call `setminingalgo` on the same algorithm.

diff:
@@ -4989,6 +4989,7 @@ UniValue generatecontinuous(const JSONRPCRequest& request)
result.pushKV("threads", 0);
result.pushKV("message", "Mining stopped");
} else {
+ ClearHashSpeed();
result.pushKV("threads", nThreads);
if (sWarning.compare(""))
result.pushKV("message", strprintf("Warning: %s", sWarning.c_str()));


message:
Fix clang warning: 'csize' not initialized.
Initialize the keyValueMap directly from s instead.

diff:
@@ -45,7 +45,7 @@ private:
CCriticalSection cs_mycache;
public:
- SimpleLRUCache(int s) : csize(s < 1 ? 10 : s), keyValuesMap(csize) {}
+ SimpleLRUCache(int s) : csize(s < 1 ? 10 : s), keyValuesMap(s < 1 ? 10 : s) {}
void set(const K key, const V value) {
LOCK(cs_mycache);