CKV_TF_2 doesn't validate against version field

[egmar]

Describe the issue
CKV_TF_2 doesn't validate against version field. In Terraform when referencing a module, in addition to source it is possible to also specify version. This is specifically useful when using Terraform registries.

Examples

module "naming" {
  source  = "Azure/naming/azurerm"
  version = "0.4.1"
}

Version (please complete the following information):

• Checkov Version 3.2.92

Additional context
CKV_TF_2 was introduced by #6213

[cmgrayb]

Seeing the same. The examples in the PR explicitly put the tf_registry example as an expected failure, and the check is only looking at the URL for a version ref. This check appears to be overly opinionated.

[hulquest]

Agree. The code is good when version is not present. The version attribute should be checked prior to this check.

[tsmithv11]

version is not immutable and while git tags are not immutable either, they are less likely to have modules altered under the same tag than version. Neither are as good as using a hash (CKV_TF_1)

[egmar]

version is not immutable and while git tags are not immutable either, they are less likely to have modules altered under the same tag than version. Neither are as good as using a hash (CKV_TF_1)

Thanks. The issue is not necessary whether version is immutable or not, nor if tag is better or not. CKV_TF_2 is effectively not even checking, if the version field is present. Public Terraform registry is not the only one, and assuming source starting with https:// is a Git URL is generally wrong.

What is the point of this check, if first thing to do in our pipelines is to disable it globally?

[hulquest]

I'll put up an issue and PR that reworks CHK_TF_2. It's just wrong. As an example, here is my code. Please share how CHK_TF_2 provides any value other than being a false negative.

module "eks_node_groups" {
  source  = "gitlab-nnn.com/terraform-at-nnn/terraform-aws-eks/local"
  version = "2.1.2"

[mcantinqc]

I understand that using a reference tag hash as in CKV_TF_1 is a better security approach. However, since specifying a version or using a reference are equally secure in practice, CKV_TF_2 should support both methods.

[matthewmrichter]

@tsmithv11 I feel like this makes the checks less secure in practice, because now most of us are just going to skip this check altogether. Version field is pretty widely accepted.

[cmgrayb]

There are exactly two checks in CKV_TF. Both are part of tsmithv11’s personal crusade against registries and SemVer in general with no regard to the obvious problem of someone being able to convince others to use the unmerged commit hash in their branch with malicious code from your repository, which Taylor calls “more secure”, or regard for the very concept of controlled releases and being able to determine if the commit you are on is the latest. When you challenge either of the checks, the issue is closed, untouched. It’s grown tiresome. All of the Terraform specific checks are disabled for us because they require an alternate reality to exist to comply with them, and we use other tools instead for Terraform compliance. Taylor is going to personally destroy checkov use for Terraform if something doesn’t change soon.

@hulquest I hope you are successful. I’d prefer not to have to migrate to another tool.