terraform-provider-helm icon indicating copy to clipboard operation
terraform-provider-helm copied to clipboard

Terraform destroy will reveal sensitive values via metadata field

Open timmjd opened this issue 3 years ago • 1 comments

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)

  • helm_release

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

  • Provide data to the helm_release that is marked as sensitive
  • This data will not be shown during planing or apply
  • Data will be revealed during destroy step

Expected Behavior

Sensitive data should not be revealed at any time

Actual Behavior

Sensitive data get display during terraform destroy call

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

timmjd avatar Oct 27 '21 20:10 timmjd

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:

  • Don't copy the values verbatim from an argument into an attribute. But I assume the provider is doing this in order to echo back a complete metadata object based on a manifest, and so this may be happening as a result of something the remote system is doing rather than the provider itself, and even if not it would make the result incomplete.
  • Mark the entire metadata attribute as being sensitive in the provider schema. This would avoid disclosing the sensitive value, here, but it would also hide all of the metadata when rendering in the plan UI like this. If all of metadata consists of data copied from other arguments then perhaps that's acceptable, but it would be very inconvenient if any of the other values in metadata are solely in that attribute, and so not visible in any other part of the plan.

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.

apparentlymart avatar Mar 07 '22 18:03 apparentlymart

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!

github-actions[bot] avatar Mar 08 '23 00:03 github-actions[bot]

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.

cdtzabra avatar Nov 08 '23 18:11 cdtzabra