feature: have option for terraform_tfsec.sh to only run in relevant modified directories

[nkazarian-spokeo]

This will allow fine-grain control in .pre-commit-config.yml for those who implement the hook to specify tfsec to run in only certain directories like so:

- id: terraform_tfsec
  files: ^dir1/

Maybe someone can point me to the rationale behind the comment stating that "$files should be ignored since tfsec will run recursively anyway" since I was able to get a working example with these changes. Perhaps there was a time when tfsec only ran in a recursive fashion no matter what?

@jon-proietti-nutrien would like your input on this, as I noticed that you are the only contributor to this file on this repository.

Thanks.

EDIT: actually I believe this is more of an enhancement, as leaving out files: key will have tfsec run recursively, but now we have the option to filter it down since there's no --include type flag that you can pass to tfsec itself (they just offer --exclude-dir). I changed the title to appropriately reflect my intent.

[jon-proietti-nutrien]

@nkazarian-spokeo I hadn't considered using $files as a means to specify which sub-directories to run in (even if it will then run recursively), so this looks okay to me.
The key files might be a bit misleading though. root_dirs would more accurately describe the behavior IMO... --exclude-dir also wasn't an option at the time. Could probably come up with some interesting uses for that.

[nkazarian-spokeo]

@nkazarian-spokeo I hadn't considered using $files as a means to specify which sub-directories to run in (even if it will then run recursively), so this looks okay to me.
The key files might be a bit misleading though. root_dirs would more accurately describe the behavior IMO... --exclude-dir also wasn't an option at the time. Could probably come up with some interesting uses for that.

Thank you for your reply @jon-proietti-nutrien. I took files from the pre-commit documentation as it seemed to fit the bill: https://pre-commit.com/#config-files. Unless you are referring to the naming convention of files in the bash script.

[jon-proietti-nutrien]

Sorry, my reference was to the pre-commit files parameter. I would suggest an edit to README point 1 to capture the change being introduced.
EDIT: Meant to add that I think the use of files is fine. Just need an update to the README.

[nkazarian-spokeo]

Okay, I have updated this PR with the README.md changes, please review.

[nkazarian-spokeo]

On branch SE-8155
Your branch is up to date with 'origin/SE-8155'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   oregon/sandbox/ec2-starter/elb.tf

no changes added to commit (use "git add" and/or "git commit -a")
--------------------------------------------------------------------------------------------------------------------------------------------------------------
~/git/Spokeo/terraform(SE-8155*) » ga .                                                                                       narekkazarian@MB190-NKazarian-2
--------------------------------------------------------------------------------------------------------------------------------------------------------------
~/git/Spokeo/terraform(SE-8155*) » gc -m "test tfsec"                                                                         narekkazarian@MB190-NKazarian-2
Terraform fmt............................................................Passed
Terraform validate with tflint...........................................Passed
Terraform validate with tfsec............................................Failed
- hook id: terraform_tfsec
- exit code: 1

FILES: oregon/sandbox/ec2-starter/elb.tf
WARNING: skipped AWS008 check due to error(s): not a string

2 potential problems detected:

Problem 1

  [AWS009][WARNING] Resource 'aws_security_group.elb' defines a fully open egress security group.
  /Users/narekkazarian/git/Spokeo/terraform/oregon/sandbox/ec2-starter/elb.tf:131

     128 |     from_port   = 0
     129 |     to_port     = 0
     130 |     protocol    = "-1"
     131 |     cidr_blocks = ["0.0.0.0/0"]
     132 |   }
     133 |
     134 |   tags = {

  See https://github.com/liamg/tfsec/wiki/AWS009 for more information.

Problem 2

  [AWS009][WARNING] Resource 'aws_security_group.custom' defines a fully open egress security group.
  /Users/narekkazarian/git/Spokeo/terraform/oregon/sandbox/ec2-starter/security_groups.tf:73

      70 |     from_port   = 0
      71 |     to_port     = 0
      72 |     protocol    = "-1"
      73 |     cidr_blocks = ["0.0.0.0/0"]
      74 |   }
      75 |
      76 |   tags = {

  See https://github.com/liamg/tfsec/wiki/AWS009 for more information.

Here is an example of me modifying a single file and tfsec only running against the directory that includes this file. Note that FILES: output comes from me adding an echo statement to the wrapper bash script.

[nkazarian-spokeo]

@jon-proietti-nutrien

The only oddity I noticed (and this might be a bug with tfsec) is that when you specify multiple positional arguments and the first argument is not a directory, it falls back to running recursively:

tfsec .pre-commit-config.yaml common/

Whereas when the first argument is a valid directory, the behavior is correct and expected:

tfsec common/

The workaround is easy enough, just exclude: the problematic file such that pre-commit does not pass it to the underlying hook

Either way I believe this change is an enhancement because it won't fall back to always running recursively. In the proposed change, if more than one (unique) directory is modified, it will be a separate call to tfsec anyway.

[jon-proietti-nutrien]

Can you add a quick example of how this will look in the config file, similar to the other examples in the readme, and then callout how the example will change which directories will be scanned by tfsec?

hooks:
    - id: terraform_tfsec
      files: 'prd-infra/'

Will instruct tfsec to recursively scan the ./prd-infra folder, but will ignore any other folders at the root level.

[nkazarian-spokeo]

Done! Please review when you get a moment.

[jon-proietti-nutrien]

I do wish that tfsec could adhere a bit more closely to how pre-commit works, which is to say that it would be nice if we could pass in to it a list of files we'd like to have scanned. It is also an open source project if you're feeling ambitious today :D

Thanks again for the PR. I'll leave it here for @antonbabenko

[nkazarian-spokeo]

I do wish that tfsec could adhere a bit more closely to how pre-commit works, which is to say that it would be nice if we could pass in to it a list of files we'd like to have scanned. It is also an open source project if you're feeling ambitious today :D

Thanks again for the PR. I'll leave it here for @antonbabenko

Yes, this! They managed to implement a blacklist with --exclude-dir, but ignored the use case of a whitelist...

[jon-proietti-nutrien]

LGTM

[nkazarian-spokeo]

@antonbabenko can we get your blessings on this? Our internal team wants to move forward with using tfsec and we are excited to incorporate this hook into our pre-commit flow. We have an extremely nested Terraform repository structure and defaulting to running recursively does not scale well for us, hence this change.

Thanks a bunch.

[nkazarian-spokeo]

@antonbabenko can you look at this again? I have resolved conflicts resulting from recent PR that involved changes to this file and back-ported my changes on top of it.

[antonbabenko]

@nkazarian-spokeo Thanks for this PR and final rebase!

v1.36.0 has been just released.

[nkazarian-spokeo]

Awesome!!! Appreciate it @antonbabenko. Thanks for looking.