hashicorp / terraform-provider-helm

Terraform Helm provider
https://www.terraform.io/docs/providers/helm/
Mozilla Public License 2.0
1k stars 371 forks source link

Terraform destroy will reveal sensitive values via metadata field #793

Closed timmjd closed 1 year ago

timmjd commented 3 years ago

Data provided via the values that was marked as sensitive will be revealed on terraform destroy via the metadata field.

Terraform, Provider, Kubernetes and Helm Versions

Terraform version: 1.0.9
Provider version: 2.3.0
Kubernetes version: 20.x

Affected Resource(s)

Terraform Configuration Files

locals {
  values = {
    foo = "SECRET"
  }
}

resource "helm_release" "this" {
  values            = [ sensitive( jsonencode( local.values ) ) ]
}

Debug Output

terraform destroy

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  - destroy

Terraform will perform the following actions:

  # helm_release.this will be destroyed
  - resource "helm_release" "this" {
      - metadata                   = [
          - {
              - values      = jsonencode(
                    {
                      foo = "SECRET"
                    }
            }
      ]
  }

Steps to Reproduce

Expected Behavior

Sensitive data should not be revealed at any time

Actual Behavior

Sensitive data get display during terraform destroy call

Community Note

apparentlymart commented 2 years ago

Hi! I work on Terraform Core and saw this issue and wanted to add some context about what's going on here in case it's useful to provider maintainers when reviewing this issue.

It seems that what happened here is that the provider or remote API internally copy values from the values argument into a provider-populated attribute metadata, which has its own attribute values. Although Terraform can see that values is sensitive, there's nothing here to tell Terraform that metadata.values is derived from values and therefore Terraform can't trace the sensitivity through the provider's "black box" and automatically understand that metadata.values ought to be sensitive too.

Currently Terraform's modelling of sensitive values is entirely within the language itself and so there's no way for a provider to help trace through sensitive values in this way. In principle there could be a mechanism for a provider to dynamically report which parts of the response are sensitive due to being derived from sensitive values, which is tracked by hashicorp/terraform-plugin-sdk#736, but still has some significant unanswered design questions.

Therefore unfortunately I don't think there are any great answers for what this provider could've done differently here. The following options come to my mind, but all of them have significant drawbacks:

From the provider docs I see that the provider supports a set_sensitive nested block type for adding additional values to the values, but I don't know enough about the behavior of Helm or this provider to know if that would be sufficient to prevent those values from being included in metadata.values. If not being done already, perhaps filtering out anything set using set_sensitive from metadata.values before writing the state would be a plausible mitigation.

github-actions[bot] commented 1 year ago

Marking this issue as stale due to inactivity. If this issue receives no comments in the next 30 days it will automatically be closed. If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. This helps our maintainers find and focus on the active issues. Maintainers may also remove the stale label at their discretion. Thank you!

cdtzabra commented 1 year ago

I still got the same issue with helm provider version 2.11.0 and terraform version v1.6.0

Eveything is fine during plan and apply until there is some changes like: update in-place or destroy For theses cases, sentive values are revelead in metea field as reported by @timmjd

 ~ resource "helm_release" "vsphere-csi" {
        id                         = "csi"
      ~ metadata                   = [
          - {
              - app_version = "3.0.2"
              - chart       = "vsphere-csi"
              - name        = "csi"
              - namespace   = "infra-vmware-system-csi"
              - revision    = 2
              - values      = jsonencode(
                    {
                      - controller = {
                          - config       = {
                              - async-query-volume                = true
                              - block-volume-snapshot             = false
                              - cnsmgr-suspend-create-volume      = true
                              - csi-auth-check                    = true
                              - csi-internal-generated-cluster-id = true
                              - csi-migration                     = true
                              - csi-windows-support               = false
                              - list-volumes                      = true
                              - listview-tasks                    = false
                              - max-pvscsi-targets-per-vm         = true
                              - multi-vcenter-csi-topology        = false
                              - online-volume-extend              = true
                              - pv-to-backingdiskobjectid-mapping = false
                              - topology-preferential-datastores  = false
                              - trigger-csi-fullsync              = false
                              - use-csinode-id                    = true
                            }
                          - nodeSelector = {
                              - "node-role.kubernetes.io/control-plane" = "true"
                            }
                          - tolerations  = {}
                        }
                      - global     = {
                          - config = {
                              - storageClass = "thin-csi"
                              - storageclass = {
                                  - default   = false
                                  - enabled   = false
                                  - expansion = true
                                }
                              - vcenter      = {
                                  - "xxxxxx" = {
                                      - datacenters = [
                                          - "xxxxxxxx",
                                        ]
                                      - password    = SENSITIVE_VALUE_REVELEAD
                                      - server      = "xxxxx"
                                      - user        = SENSITIVE_VALUE_REVELEAD
                                    }
                                }
                            }
                        }
                    }
                )
              - version     = "3.2.3"
            },
        ] -> (known after apply)
        name                       = "csi"
      ~ values                     = [
          - (sensitive value),
          + (sensitive value),
        ]
        # (26 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.