bug(terraform): TFPlan parser overwrittes resources with same type and name

[ivangonzalezz]

This bug is an extension of #7265 , where the parser overwrites resources with the same type:

The map entry is initialized for each child module and overrides the resources from modules up the tree

https://github.com/Checkmarx/kics/blob/c17d26eb1d582e6cf8c169f476b494421cef00a2/pkg/parser/json/tfplan.go#L67

In this issue, when two resources have the same type and the same name, they are overwritten and only the last one is kept: https://github.com/Checkmarx/kics/blob/67de28ccf7e0f984a34d8af6576d5645fda16809/pkg/parser/json/tfplan.go#L70-L73

For example, the following TFPlan with two resources of type aws_s3_bucket, each of them with name this, as it's expected from the official aws_s3_bucket module:

{
    "format_version": "1.2",
    "terraform_version": "1.9.0",
    "planned_values": {
        "root_module": {
            "child_modules": [
                {
                    "resources": [
                        {
                            "resources": [
                                {
                                    "address": "module.s3.module.s3_bucket.aws_s3_bucket.this[0]",
                                    "mode": "managed",
                                    "type": "aws_s3_bucket",
                                    "name": "this",
                                    "index": 0,
                                    "provider_name": "registry.terraform.io/hashicorp/aws",
                                    "schema_version": 0,
                                    "values": {},
                                    "sensitive_values": {}
                                }
                            ],
                            "address": "module.s3.module.s3_bucket"
                        },
                        {
                            "resources": [
                                {
                                    "address": "module.s3.module.log_bucket.aws_s3_bucket.this[0]",
                                    "mode": "managed",
                                    "type": "aws_s3_bucket",
                                    "name": "this",
                                    "index": 0,
                                    "provider_name": "registry.terraform.io/hashicorp/aws",
                                    "schema_version": 0,
                                    "values": {},
                                    "sensitive_values": {}
                                }
                            ],
                            "address": "module.s3.module.log_bucket"
                        }
                    ]
                }
            ]
        }
    }
}

Maybe it could be considered to change the data model from a map identified by the resource name to a slice/array of resources.

[kicsbot]

Please, follow the guideline for an issue title:

For bug:

bug(<scope>): <title starting with lowercase letter>

For query:

query(<platform>): <title starting with lowercase letter>

For feature request:

feat(<scope>): <title starting with lowercase letter>

Thank you! KICS Team

[cx-artur-ribeiro]

Hi @ivangonzalezz,

Sorry for the delayed response, and thank you for reporting this with a detailed example!

We have plans to extend KICS’ Terraform and TFPlan support, and improvements like this (handling multiple resources with the same name) are within that scope. Once that work is in place, the issue you’ve highlighted should be resolved.

I don’t have an exact timeline for when we’ll address these issues, but I’ll make sure to notify you once we start working on a fix.

Thanks again for helping us improve KICS!

[cx-andre-pereira]

Hi @ivangonzalezz,

This issue should be fixed by https://github.com/Checkmarx/kics/pull/7866, to guarantee uniqueness of resource instances the "address" field was used. The tfplan documentation explicitly states this value as a unique key per resource instance. With this all resources are now properly parsed.

Other issues persist since tfplan notation varies from the original tf file to the point were queries can over or under report vulnerabilities, but the problem you described should be completely fixed.

Thank you for your input.