[3rd party: checkov] Checkov runs for all files, not just .tf

[mj3c]

Describe the bug

I have a problem with the checkov hook running against the entire repo, checking all files, instead of only .tf files and specifically a subdirectory of them. For example, if you have this:

project
│   Dockerfile  
│   .pre-commit-config.yaml
│
└───terraform
        main.tf

And if you run pre-commit run -a, checkov will fail at any Dockerfile errors.

How can we reproduce it?

Dockerfile example that will fail checkov:

FROM ubuntu:18.04

Pre-commit config:

repos:
  - repo: git://github.com/antonbabenko/pre-commit-terraform
    rev: v1.56.0
    hooks:
      - id: checkov

Environment information

• OS: Ubuntu 20.04

• uname -a and/or systeminfo | Select-String "^OS" output:

Linux 5.11.0-38-generic #42~20.04.1-Ubuntu SMP Tue Sep 28 20:41:07 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

• Tools availability and versions:

pre-commit 2.15.0
Terraform v1.0.7
Python 3.8.10
checkov 2.0.580

Additional information

I believe this is because of the following setting for the checkov hook:

  entry: checkov -d .

Even though files: is set, checkov still seems to check all files. The checkov CLI supports regex in the --skip-path argument, so perhaps this can be resolved with something like the following?

  entry: checkov -d . --skip-path '.*(?<!\.tf)$'

[yermulnik]

And if you run pre-commit run -a, checkov will fail at any Dockerfile errors.

Could you please provide a snippet from the execution output to demonstrate details? Along with checkov hook config blocks from your .pre-commit-config.yaml and .pre-commit-hooks.yaml files.

As far as you can see from https://github.com/antonbabenko/pre-commit-terraform#checkov, this hook's args can be configured per individual use cases (alongside any other per-hook pre-commit config options: https://pre-commit.com/#plugins).

Even though files: is set, checkov still seems to check all files.

I lean to think that's the expected behavior for the defaults of pass_filenames: false passed to checkov hook config here: https://github.com/antonbabenko/pre-commit-terraform/blob/master/.pre-commit-hooks.yaml#L100

[mj3c]

I have posted the full .pre-commit-config.yaml file in my question already.

Here is the command I execute and the output:

$ pre-commit run --files ./terraform/*

Checkov..................................................................Failed
- hook id: checkov
- exit code: 1

dockerfile scan results:

Passed checks: 1, Failed checks: 2, Skipped checks: 0

Check: CKV_DOCKER_7: "Ensure the base image uses a non latest version tag"
	PASSED for resource: /Dockerfile.
	File: /Dockerfile:1-1
	Guide: https://docs.bridgecrew.io/docs/ensure-the-base-image-uses-a-non-latest-version-tag

Check: CKV_DOCKER_2: "Ensure that HEALTHCHECK instructions have been added to container images"
	FAILED for resource: /Dockerfile.
	File: /Dockerfile:1-1
	Guide: https://docs.bridgecrew.io/docs/ensure-that-healthcheck-instructions-have-been-added-to-container-images

		1 | FROM ubuntu:18.04


Check: CKV_DOCKER_3: "Ensure that a user for the container has been created"
	FAILED for resource: /Dockerfile.
	File: /Dockerfile:1-1
	Guide: https://docs.bridgecrew.io/docs/ensure-that-a-user-for-the-container-has-been-created

		1 | FROM ubuntu:18.04

I did manage to work around this by updating the .pre-commit-config.yaml like so:

repos:
  - repo: git://github.com/antonbabenko/pre-commit-terraform
    rev: v1.56.0
    hooks:
      - id: checkov
        args:
          - --skip-path
          - .*(?<!terraform\/)$
#          - .*(?<!\.tf)$    # something like this can also do the trick

This is okay in my case, it was just not intuitive at first. I would expect it to not scan the Dockefile, or really anything but the ./terraform/* files, but maybe that's just me. I guess it is because of the default behavior (pass_filenames: false) as you mentioned.

---

P.S. I don't believe you can pass the -d <dir> argument to checkov hook as mentioned in the README, because it is set as a default here: https://github.com/antonbabenko/pre-commit-terraform/blob/master/.pre-commit-hooks.yaml#L98

From pre-commit docs:

entry | the entry point - the executable to run. entry can also contain arguments that will not be overridden such as entry: autopep8 -i.

[yermulnik]

This is okay in my case, it was just not intuitive at first.

Good to know you've worked around this.
If you have an idea of how to improve REAMDE to eliminate any confusions consumers may encounter, we'd appreciate PR(s) — realworld use cases are what in my opinion are the best building bricks for the howto's describe in README.

I would expect it to not scan the Dockefile, or really anything but the ./terraform/* files, but maybe that's just me.

Since the pre-commit-terraform is supposed to be used for the repo which contains specifically and exclusively Terraform configurations (I assume), which may have non-TF files as part of configuration (e.g. files used as templates etc including Docker files), I'd say it may be an expected behavior.

Should we close this issue at this point?

[mj3c]

Yup, that also makes sense.

The confusion came from the fact that the rest of the terraform hooks seem to respect the --files <path> argument when they are checking files (you can check only a subdirectory, if you want), but checkov is an exception, and by default checks everything in the repo.

Thanks for the quick responses!

[entscheidungsproblem]

Can this be reopened to allow the -d flag to be passed to checkov? This looks like a workaround since, as @mj3c mentioned, the -d flag currently cannot be overridden.

[MaxymVlasov]

@entscheidungsproblem did you try to override it using the example in https://github.com/antonbabenko/pre-commit-terraform#checkov ?

If not, #290 should add that possibility