bridgecrewio / checkov

Prevent cloud misconfigurations and find vulnerabilities during build-time in infrastructure as code, container images and open source packages with Checkov by Bridgecrew.
https://www.checkov.io/
Apache License 2.0
6.72k stars 1.08k forks source link

CKV_TF_2 false positive on Terraform Registry modules with pinned version #6335

Closed Constantin07 closed 1 month ago

Constantin07 commented 1 month ago

Describe the issue Despite for terraform registry modules use a pinned version(tag) the CKV_TF_2 check fails:

image

Examples

module "iam_github_oidc_role" {
  source  = "terraform-aws-modules/iam/aws//modules/iam-github-oidc-role"
  version = "5.39.1"

  name                 = var.role_name
  description          = "Role to assume from GitHub to allow assuming ${var.assumable_role_name} in serviced aws accounts"
  max_session_duration = var.max_session_duration

  subjects = [
    var.github_subject,
  ]

  policies = {
    AssumeRole = aws_iam_policy.allow_assume_role.arn
  }
}

Version (please complete the following information):

Additional context Using Terraform Registry modules https://developer.hashicorp.com/terraform/registry/modules/use#using-modules.

rwe-dtroup commented 1 month ago

https://github.com/bridgecrewio/checkov/issues/6307

bug or feature?

aschleifer commented 1 month ago

As mentioned in the comments of #6307 I see this clearly as a missing feature. Terraform itself encourages people to use the version field to specify how to pin the module. Then this check should also consider this field and not only a ref= reference in the source field.

Just to mention this up front. This shouldn't be a discussion if CKV_TF_1 or CKV_TF_2 is more secure or not. Right now we have disabled both as the follow the recommended way of Terraform to define a version field and we don't plan to move to the ref= part in the source field.

Also we use https://github.com/renovatebot/renovate/ to keep up-to-date with our dependencies like upstream terraform modules and I'm not sure if renovate would be able to work with the ref= part of the source field.

Also I personally find the dedicated version field more humand readable then having to scan he source string to find some kind of version/tag/commit-hash reference.

rwe-dtroup commented 1 month ago

I don't disagree, merely pointing out that this seems to be by design rather than a bug.

mikeurbanski1 commented 1 month ago

It was a miss on our side. We'll fix it.