checkov icon indicating copy to clipboard operation
checkov copied to clipboard

[CKV_SECRET_6] Only report some violations when run on a formatted JSON plan

Open AliSoftware opened this issue 10 months ago • 5 comments

Describe the issue

We run checkov on our tfplan.json terraform plan (as opposed to running it on the source .tf files)

  • When we don't format the JSON generated by terraform show -json output/tfplan.bin > output/tfplan.json, running checkov --file tfplan.json doesn't find any violations.
  • But formatting the generated JSON via terraform show -json output/tfplan.bin | jq > output/tfplan.json and running the exact same checkov --file tfplan.json on the formatted JSON now finds two CKV_SECRET_6 violations (one of them being a false positive btw, but that's another problem)

Examples This is quite a bit difficult to share an example in public because that would consist of sharing a terraform state file, which are quite large and with lots of information, so not as easy to redact or even create a small reproducible case (especially given that file is generated so not easy to create a valid yet trimmed out example file)

Version (please complete the following information):

  • Checkov Version: 3.2.53 (via the ghcr.io/bridgecrewio/checkov:3.2.53 Docker image)

AliSoftware avatar Apr 17 '24 16:04 AliSoftware

Hey @AliSoftware there is a line length limit of 10k chars, that's why you don't get any result for the unformatted JSON file. This was added to keep the performance of secret scanning to an acceptable level.

gruebel avatar Apr 18 '24 05:04 gruebel

Ah, that explains it! Thanks for the quick answer.

Could this information be added to the docs maybe? (Or is it already and I missed it?) Seems it'd be useful to mention in https://www.checkov.io/7.Scan%20Examples/Terraform%20Plan%20Scanning.html in particular?

Also I wonder if it would also be a good idea to make checkov print a warning about it in its output when the file it operates on encounters such >10k lines and suggest to use jq in those cases. Otherwise it's very easy to have some violations being missed completely silently, which feels dangerous (especially if checkov ends up green while missing security- or secret-related checks)? Wdyt?

AliSoftware avatar Apr 18 '24 06:04 AliSoftware

I can't remember why we didn't add an info log for it, maybe it would be too spammy?! jq is not always a valid solution, because long lines can also occur in other file types, but could be identified during scanning 🙂

Honestly, I have no strong opinion on it.

gruebel avatar Apr 19 '24 05:04 gruebel

jq is not always a valid solution, because long lines can also occur in other file types Ah, good point. Honestly, I was quite surprised that the check seemed to run on the raw text content of the files, as opposed to checkov parsing the JSON file as an object and then traverse it. But I am guessing now that this is so that CKV_SECRET_* checks can detect secrets present anywhere in any type of file, as opposed to make it parse whatever input format the file to scan was in and scan that?

I can't remember why we didn't add an info log for it, maybe it would be too spammy?! […] but could be identified during scanning 🙂

Maybe. Though my intuition makes me doubt that this would end up spammy, because regardless of the type of input file those checks are scanning, I'd expect that in most cases if the input file happens to have line(s) that are >10k, it'll likely because that input file just contains a single, giant line—as opposed to many lines, multiple of which being that long? So the warning would thus occur only once per file scanned.

Honestly, I have no strong opinion on it.

I still think it's important to at least document it and make it more visible, because the impact of those violations not being reported can be pretty important, especially for CKV_SECRET_* checks, where a run of checkov would report no violation and let you think you were guaranteed no secret was leaked, while in reality there might be but the user wasn't cautioned about parts of the file (>10k lines) not being scanned and potentially missing some. That could lessen our confidence in the checkov tool properly securing our setup, and have potentially security implications (by leaking secrets accidentally while being convinced—from checkov being green—that we hadn't)

AliSoftware avatar Apr 19 '24 09:04 AliSoftware

From a quick look at the code, seems we're supposed to log those cases?

Yet when running checkov --file output/tfplan.json on our unformatted, all-in-one-line state file (with .checkov.yml defining output: [cli, junitxml]) I don't see any such warning/log in the output 🤔

AliSoftware avatar Apr 19 '24 10:04 AliSoftware