Bump parser version to avoid catastrophic backtracking (bridgecrewio#1677)

* Add OS-level tests

* Fix example and really bump parser version

* Fix parser

* Bump in setup.py

* Run python UT without coverage to avoid windows failure

* Bumping

* Update tests files to hcl2 syntax

* Fix path based tests to handle windows correctly

* Remove fail-fast

* Move windows tests to IT instead of UT

* Use bash -c

* Another windows fix

* Fix IT file path

* Fix IT file path

* Fix IT file path

* prepare_data.sh supporting windows

* Use bash

* Log error in helm failures

* Handle non-relative path in helm runner

* Add logs

* Skip helm windows test and update build.yml

* Skip helm test for windows

* Revert unnecessary changes

* Merge fix

* Fix UTs

Author: nimrodkor

.github/workflows/build.yml

@@ -16,22 +16,23 @@ jobs:
       fail-fast: true
       matrix:
         python: [3.7, 3.8, 3.9]
-    runs-on: ubuntu-latest
+        os: [ubuntu-latest, macos-latest, windows-latest]
+    runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@v2
       - uses: actions/setup-python@v2
         with:
           python-version: ${{ matrix.python }}
-      - uses: dschep/install-pipenv-action@v1
       - uses: actions/setup-node@v2
       - 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 --upgrade pip==21.1.1
           pipenv run pip install pytest
           pipenv run python setup.py sdist bdist_wheel
-          pipenv run pip install dist/checkov-*.whl
+          bash -c 'pipenv run pip install dist/checkov-*.whl'
       - name: Clone Terragoat - vulnerable terraform
         run: git clone https://github.com/bridgecrewio/terragoat
       - name: Clone Cfngoat - vulnerable cloudformation
@@ -40,9 +41,10 @@ jobs:
         run: git clone https://github.com/madhuakula/kubernetes-goat
       - name: Create checkov reports
         run: |
-          sleep $((RANDOM % 11))
-          ./integration_tests/prepare_data.sh ${{ matrix.python }}
+          # Just making sure the API key tests don't run on PRs
+          bash -c './integration_tests/prepare_data.sh ${{ matrix.os }} 3.8'
         env:
+          LOG_LEVEL: INFO
           BC_KEY: ${{ secrets.BC_API_KEY }}
       - name: Run integration tests
         run: |

.github/workflows/pr-test.yml

@@ -22,11 +22,10 @@ jobs:
       fail-fast: true
       matrix:
         python: [3.7, 3.8, 3.9]
-        os: [ubuntu-latest, macos-latest, windows-latest]
-    runs-on: ${{ matrix.os }}
+    runs-on: ubuntu-latest
     timeout-minutes: 30
     steps:
-      - uses: actions/checkout@v1
+      - uses: actions/checkout@v2
       - name: Set up Python ${{ matrix.python }}
         uses: actions/setup-python@v2
         with:
@@ -40,16 +39,15 @@ jobs:
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
-          pipenv run python -m coverage run -m pytest tests
-          pipenv run python -m coverage report
-          pipenv run python -m coverage html
+          pipenv run python -m pytest tests
 
   integration-tests:
     strategy:
       fail-fast: true
       matrix:
         python: [3.7, 3.8, 3.9]
-    runs-on: ubuntu-latest
+        os: [ubuntu-latest, macos-latest, windows-latest]
+    runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@v2
       - uses: actions/setup-python@v2
@@ -64,7 +62,7 @@ jobs:
           pipenv run pip install --upgrade pip==21.1.1
           pipenv run pip install pytest
           pipenv run python setup.py sdist bdist_wheel
-          pipenv run pip install dist/checkov-*.whl
+          bash -c 'pipenv run pip install dist/checkov-*.whl'
       - name: Clone Terragoat - vulnerable terraform
         run: git clone https://github.com/bridgecrewio/terragoat
       - name: Clone Cfngoat - vulnerable cloudformation
@@ -76,8 +74,8 @@ jobs:
           LOG_LEVEL: INFO
           BC_KEY: ${{ secrets.BC_API_KEY }}
         run: |
-          sleep $((RANDOM % 11))
-          ./integration_tests/prepare_data.sh 3.8  # Just making sure the API key tests don't run on PRs
+          # Just making sure the API key tests don't run on PRs
+          bash -c './integration_tests/prepare_data.sh ${{ matrix.os }} 3.8'
       - name: Run integration tests
         run: |
           pipenv run pytest integration_tests -k 'not api_key'

Pipfile

@@ -14,6 +14,7 @@ GitPython = "*"
 bandit = "*"
 urllib3-mock = "*"
 jsonschema = "*"
+atomicwrites = "*"
 
 [packages]
 #

Pipfile.lock

@@ -191,7 +191,8 @@ def run(self, root_folder, external_checks_dir=None, files=None, runner_filter=R
                     report.skipped_checks += chart_results.skipped_checks
                     report.resources.update(chart_results.resources)
 
-                except:
+                except Exception as e:
+                    logging.warning(e, stack_info=True)
                     with tempfile.TemporaryDirectory() as save_error_dir:
                         logging.debug(
                             f"Error running k8s scan on {chart_meta['name']}. Scan dir: {target_dir}. Saved context dir: {save_error_dir}")

checkov/helm/runner.py

@@ -8,7 +8,6 @@
 import sys
 from copy import deepcopy
 from json import dumps, loads, JSONEncoder
-from multiprocessing.connection import Connection
 from pathlib import Path
 from typing import Optional, Dict, Mapping, Set, Tuple, Callable, Any, List
 
@@ -652,63 +651,22 @@ def _load_or_die_quietly(file: os.PathLike, parsing_errors: Dict,
     try:
         logging.debug(f"Parsing {file_path}")
 
-        if file_name.endswith(".json"):
-            with open(file_name, "r") as f:
+        with open(file_path, "r") as f:
+            if file_name.endswith(".json"):
                 return json.load(f)
-        else:
-            raw_data = _hcl2_load_with_timeout(file_path)
-            non_malformed_definitions = validate_malformed_definitions(raw_data)
-            if clean_definitions:
-                return clean_bad_definitions(non_malformed_definitions)
             else:
-                return non_malformed_definitions
-
+                raw_data = hcl2.load(f)
+                non_malformed_definitions = validate_malformed_definitions(raw_data)
+                if clean_definitions:
+                    return clean_bad_definitions(non_malformed_definitions)
+                else:
+                    return non_malformed_definitions
     except Exception as e:
         logging.debug(f'failed while parsing file {file_path}', exc_info=e)
         parsing_errors[file_path] = e
         return None
 
 
-def _hcl2_load_with_timeout(filename: str) -> Dict:
-    # Start bar as a process
-    raw_data = None
-    reader, writer = multiprocessing.Pipe(duplex=False)  # used by the child process to return its result
-
-    # certain Python and OS versions (for sure 3.8/3.9 + Mac) have issues with 'multiprocessing.Process'
-    # see: https://github.com/pytorch/pytorch/issues/36515
-    # https://bugs.python.org/issue33725
-    # "Fork" is the correct approach on 3.8+ on Mac, and should be compatible with *nix, and generally 3.7+
-    # It is faster to open processes, and therefore we prefer it. However, it is not supported on Windows
-    if sys.platform == 'win32':
-        p = multiprocessing.Process(target=_hcl2_load, args=(writer, filename))
-    else:
-        p = multiprocessing.get_context("fork").Process(target=_hcl2_load, args=(writer, filename))
-    p.start()
-
-    # Wait until the file is parsed, up to 60 seconds, and fetch the raw data
-    is_input_available = reader.poll(timeout=60)
-    if is_input_available:
-        raw_data = reader.recv()
-
-    # Resources cleanup
-    if p.is_alive():
-        p.terminate()
-    writer.close()
-
-    return raw_data
-
-
-def _hcl2_load(writer: Connection, filename: str) -> None:
-    with open(filename, 'r') as f:
-        try:
-            raw_data = hcl2.load(f)
-            writer.send(raw_data)
-        except Exception as e:
-            logging.error(f'Failed to parse file {f.name}. Error:')
-            logging.error(e, exc_info=True)
-            writer.send(None)
-
-
 def _is_valid_block(block):
     if not isinstance(block, dict):
         return True

checkov/terraform/parser.py

@@ -1,14 +1,27 @@
 #!/bin/bash
 
-pipenv run checkov -s --framework terraform -d terragoat/terraform/ -o json > checkov_report_terragoat.json
-pipenv run checkov -s --framework terraform -d terragoat/terraform/ -o junitxml > checkov_report_terragoat.xml
-pipenv run checkov -s -d cfngoat/ -o json --external-checks-dir ./checkov/cloudformation/checks/graph_checks/aws > checkov_report_cfngoat.json
-pipenv run checkov -s -d kubernetes-goat/ --framework kubernetes -o json > checkov_report_kubernetes-goat.json
-pipenv run checkov -s -d kubernetes-goat/ --framework helm -o json > checkov_report_kubernetes-goat-helm.json
-pipenv run checkov -s --framework terraform --skip-check CKV_AWS_33,CKV_AWS_41 -d terragoat/terraform/ -o json > checkov_report_terragoat_with_skip.json
-pipenv run checkov -s -d cfngoat/ -o json --quiet > checkov_report_cfngoat_quiet.json
-pipenv run checkov -s -d terragoat/terraform/ --config-file integration_tests/example_config_files/config.yaml -o json > checkov_config_report_terragoat.json
-if [[ "$1" == "3.7" ]]
+if [[ "$1" == "windows-latest" ]]
+then
+  pipenv run checkov -s --framework terraform -d terragoat\\terraform\\ -o json > checkov_report_terragoat.json
+  pipenv run checkov -s --framework terraform -d terragoat\\terraform\\ -o junitxml > checkov_report_terragoat.xml
+  pipenv run checkov -s -d cfngoat\\ -o json --external-checks-dir .\\checkov\\cloudformation\\checks\\graph_checks\\aws > checkov_report_cfngoat.json
+  pipenv run checkov -s -d kubernetes-goat\\ --framework kubernetes -o json > checkov_report_kubernetes-goat.json
+#  LOG_LEVEL=DEBUG pipenv run checkov -s -d kubernetes-goat\\ --framework helm -o json > checkov_report_kubernetes-goat-helm.json
+  pipenv run checkov -s --framework terraform --skip-check CKV_AWS_33,CKV_AWS_41 -d terragoat\\terraform\\ -o json > checkov_report_terragoat_with_skip.json
+  pipenv run checkov -s -d cfngoat\\ -o json --quiet > checkov_report_cfngoat_quiet.json
+  pipenv run checkov -s -d terragoat\\terraform\\ --config-file integration_tests\\example_config_files\\config.yaml -o json > checkov_config_report_terragoat.json
+else
+  pipenv run checkov -s --framework terraform -d terragoat/terraform/ -o json > checkov_report_terragoat.json
+  pipenv run checkov -s --framework terraform -d terragoat/terraform/ -o junitxml > checkov_report_terragoat.xml
+  pipenv run checkov -s -d cfngoat/ -o json --external-checks-dir ./checkov/cloudformation/checks/graph_checks/aws > checkov_report_cfngoat.json
+  pipenv run checkov -s -d kubernetes-goat/ --framework kubernetes -o json > checkov_report_kubernetes-goat.json
+  pipenv run checkov -s -d kubernetes-goat/ --framework helm -o json > checkov_report_kubernetes-goat-helm.json
+  pipenv run checkov -s --framework terraform --skip-check CKV_AWS_33,CKV_AWS_41 -d terragoat/terraform/ -o json > checkov_report_terragoat_with_skip.json
+  pipenv run checkov -s -d cfngoat/ -o json --quiet > checkov_report_cfngoat_quiet.json
+  pipenv run checkov -s -d terragoat/terraform/ --config-file integration_tests/example_config_files/config.yaml -o json > checkov_config_report_terragoat.json
+fi
+
+if [[ "$2" == "3.7" ]]
 then
   pipenv run checkov -s -f terragoat/terraform/aws/s3.tf --bc-api-key $BC_KEY > checkov_report_s3_singlefile_api_key_terragoat.txt
   pipenv run checkov -s -d terragoat/terraform/azure/ --bc-api-key $BC_KEY > checkov_report_azuredir_api_key_terragoat.txt

integration_tests/prepare_data.sh

@@ -8,11 +8,11 @@
 class TestCheckovJsonReport(unittest.TestCase):
 
     def test_terragoat_report_dir_api_key(self):
-        report_path = current_dir + "/../checkov_report_azuredir_api_key_terragoat.txt"
+        report_path = os.path.join(current_dir, '..', 'checkov_report_azuredir_api_key_terragoat.txt')
         self.validate_report(os.path.abspath(report_path))
 
     def test_terragoat_report_file_api_key(self):
-        report_path = current_dir + "/../checkov_report_s3_singlefile_api_key_terragoat.txt"
+        report_path = os.path.join(current_dir, '..', 'checkov_report_s3_singlefile_api_key_terragoat.txt')
         self.validate_report(os.path.abspath(report_path))
 
     def validate_report(self, report_path):

integration_tests/test_checkov_cli_integration_report.py

@@ -12,7 +12,7 @@ def test_terragoat_report(self):
         # checkov -d path/to/terragoat --config-file \
         # path/to/checkov/integration_tests/example_config_files/config.yaml \
         # > path/to/checkov/checkov_config_report_terragoat.json
-        report_path = current_dir + "/../checkov_config_report_terragoat.json"
+        report_path = os.path.join(os.path.dirname(current_dir), "checkov_config_report_terragoat.json")
         with open(report_path) as json_file:
             data = json.load(json_file)
             self.assertEqual(data["summary"]["parsing_errors"], 0,

integration_tests/test_checkov_config.py

@@ -1,6 +1,7 @@
 import itertools
 import json
 import os
+import sys
 import unittest
 
 current_dir = os.path.dirname(os.path.realpath(__file__))
@@ -9,24 +10,25 @@
 class TestCheckovJsonReport(unittest.TestCase):
 
     def test_terragoat_report(self):
-        report_path = current_dir + "/../checkov_report_terragoat.json"
+        report_path = os.path.join(os.path.dirname(current_dir), 'checkov_report_terragoat.json')
         self.validate_report(os.path.abspath(report_path))
 
     def test_cfngoat_report(self):
-        report_path = current_dir + "/../checkov_report_cfngoat.json"
+        report_path = os.path.join(os.path.dirname(current_dir), 'checkov_report_cfngoat.json')
         self.validate_report(os.path.abspath(report_path))
         self.validate_check_in_report(report_path, "CKV2_AWS_26")
 
     def test_k8goat_report(self):
-        report_path = current_dir + "/../checkov_report_kubernetes-goat.json"
+        report_path = os.path.join(os.path.dirname(current_dir), 'checkov_report_kubernetes-goat.json')
         self.validate_report(os.path.abspath(report_path))
 
     def test_k8goat_report(self):
-        report_path = current_dir + "/../checkov_report_kubernetes-goat-helm.json"
-        self.validate_report(os.path.abspath(report_path))
+        if not sys.platform.startswith('win'):
+            report_path = os.path.join(os.path.dirname(current_dir), 'checkov_report_kubernetes-goat-helm.json')
+            self.validate_report(os.path.abspath(report_path))
 
     def test_checkov_report_terragoat_with_skip(self):
-        report_path = current_dir + "/../checkov_report_terragoat_with_skip.json"
+        report_path = os.path.join(os.path.dirname(current_dir), 'checkov_report_terragoat_with_skip.json')
         checkov2_graph_findings = 0
         with open(report_path) as json_file:
             data = json.load(json_file)
@@ -53,7 +55,7 @@ def validate_report_not_empty(self, report):
                            f"expecting more than 1 failed checks, got: {report['summary']['failed']}")
 
     def validate_json_quiet(self):
-        report_path = current_dir + "/../checkov_report_cfngoat_quiet.json"
+        report_path = os.path.join(os.path.dirname(current_dir), 'checkov_report_cfngoat_quiet.json')
         with open(report_path) as json_file:
             data = json.load(json_file)
             self.assertTrue(data["results"]["failed_checks"])