checkov icon indicating copy to clipboard operation
checkov copied to clipboard

checkov -f plan.json does not download external terraform modules for enrichment

Open coffeecoco opened this issue 3 years ago • 6 comments

with a sample config when file is used for tfplan.json i can see it does not download the external terraform modules ( my scenario is a git source ) i can see when --directory is used it does download the external terraform module

I thought the enrichment should download the external module so the enrichment can work and suppression and so forth I only use the #comment suppression.

download-external-modules: True
evaluate-variables: True
external-modules-download-path: '.external_modules'
file:
  - 'tfplan.json'
repo-root-for-plan-enrichment: '.'
framework: 'all'
no-guide: True
output: 'cli'
skip-fixes: True
skip-suppressions: True

coffeecoco avatar Sep 28 '21 02:09 coffeecoco

I have the same behaviour when running the latest version from my Windows OS. when we run checkov via GH actions it does download the external modules to the ./external_modules folder and scans the files. Doing it locally that process does not happen.

nicholas-marchini avatar Nov 11 '21 14:11 nicholas-marchini

Same here, I assumed having the plan file and the stuff in the .terraform directory is already enough for Checkov

trallnag avatar Feb 17 '22 13:02 trallnag

Maybe to enrich this issue with a bit of debug output, since we are also running into this issue and want to use skips on specific Terraform definition blocks. Calling checkov -d . --download-external-modules True --framework terraform -o junitxml will work fine and result in the following

2022-03-09 11:16:19,804 [MainThread  ] [INFO ]  Downloading module git::https://<GITREPOURI>/module-tagging.git?ref=tags/v1.1.0:None
2022-03-09 11:16:19,804 [MainThread  ] [DEBUG]  Cache miss for git::https://<GITREPOURI>/module-tagging.git?ref=tags/v1.1.0:latest
2022-03-09 11:16:19,805 [MainThread  ] [DEBUG]  getting module git::https://<GITREPOURI>/module-tagging.git?ref=tags/v1.1.0 version: latest
2022-03-09 11:16:19,805 [MainThread  ] [DEBUG]  cloning https://<GITREPOURI>/module-tagging.git to /mnt/runner-workdir/builds/P4teuDUM/0/<SOMEWORKDIR>/.external_modules/<GITREPOURI>/module-tagging/tags/v1.1.0
2022-03-09 11:16:19,805 [MainThread  ] [DEBUG]  Popen(['git', 'clone', '-v', '--depth=1', '-b', 'v1.1.0', 'https://<GITREPOURI>/module-tagging.git', '/mnt/runner-workdir/builds/P4teuDUM/0/<SOMEWORKDIR>/.external_modules/<GITREPOURI>/module-tagging/tags/v1.1.0'], cwd=/mnt/runner-workdir/builds/P4teuDUM/0/<SOMEWORKDIR> , universal_newlines=True, shell=None, istream=None)

Calling checkov -f plan.cache.json --framework terraform_plan --download-external-modules True --repo-root-for-plan-enrichment . -o junitxml will fail with the following Debug:

2022-03-09 12:39:31,992 [MainThread  ] [INFO ]  Downloading module git::https://<GITREPOURI>/module-tagging.git?ref=tags/v1.1.0:None
2022-03-09 12:39:31,992 [MainThread  ] [DEBUG]  Cache miss for git::https://<GITREPOURI>/module-tagging.git?ref=tags/v1.1.0:latest
2022-03-09 12:39:31,992 [MainThread  ] [WARNI]  Failed to download module git::https://<GITREPOURI>/module-tagging.git?ref=tags/v1.1.0:None (for external modules, the --download-external-modules flag is required)

AndreasMWalter avatar Mar 09 '22 12:03 AndreasMWalter

So this looks like it is because of the plan enrichment. Specifically, this call to parse_directory which doesn't pass along the configuration for download-external-modules (or external-modules-download-path):

https://github.com/bridgecrewio/checkov/blob/d73fd4bd7096d48ab3434a92a177bcc55605460a/checkov/common/runners/runner_registry.py#L250

Parser().parse_directory(
                directory=repo_root,  # assume plan file is in the repo-root
                out_definitions=tf_definitions,
                out_parsing_errors=parsing_errors,
            )

However, I'm not sure modifying that would help with suppression, since as best I can tell, this stage only happens after checkov has determined any failures already, and is only used for output formatting.

In my particular use case, I have a terraform module which creates an aws_acm_certificate, with the create_before_destroy lifecycle rule, however CKV_AWS_233 "Ensure Create before destroy for ACM certificates" still fires when checking against the plan since that information isn't contained within the plan file.

Enriching the plan (and forcing it to download modules by modifying the code I linked above) changes the output from a JSON map of the acm certificate resource to the full terraform code of the resource, but does nothing to correct the false positive.

melbit-michaelw avatar Mar 10 '22 23:03 melbit-michaelw

So at present I see that the plan enrichment works... but only for the root config and not the modules being called. As a workaround we included the skips to the actual module calls in the root config and now it will at least not report the issues on the plan step anymore. However this effect might not be wanted, example being now the Check CKV_AZURE_118 ignores all 4 Interfaces of a particular NVA even though it only needs to be ignored on two of those interfaces. This is just a simple example, there might be more critical examples.

AndreasMWalter avatar Mar 11 '22 07:03 AndreasMWalter

Thank you @melbit-michaelw for your investigation. This was a blocker for me so I ended up patching it after install:

--- runner_registry.py	2022-05-12 12:13:47.000000000 +0100
+++ runner_registry_fixed.py	2022-05-18 16:44:19.000000000 +0100
@@ -277,6 +277,7 @@
                 directory=repo_root,  # assume plan file is in the repo-root
                 out_definitions=tf_definitions,
                 out_parsing_errors=parsing_errors,
+                download_external_modules=True,
             )
             repo_definitions[repo_root] = {'tf_definitions': tf_definitions, 'parsing_errors': parsing_errors}
patch runner_registry.py < /tmp/checkov-runner-registry.patch

jSherz avatar May 19 '22 09:05 jSherz

Same issue here https://github.com/bridgecrewio/checkov/issues/3878 Just want to comment the patch did work for me, fixed the problem

coffeecoco avatar Nov 15 '22 07:11 coffeecoco