Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Secrets in Helm chart resource are plaintext in state file #999

Closed
clstokes opened this issue Feb 18, 2020 · 4 comments · Fixed by #1577
Closed

Secrets in Helm chart resource are plaintext in state file #999

clstokes opened this issue Feb 18, 2020 · 4 comments · Fixed by #1577
Assignees
Labels
impact/security kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed

Comments

@clstokes
Copy link

Problem description

When a Pulumi secret is used in a Helm chart resource, its plaintext value ends up in the Pulumi state file where in other resource types its encrypted value is used as expected.

Affected product version(s)

Reproducing the issue

Code to reproduce - https://gist.github.com/clstokes/a9192849a157307438a433162c16dfc1.

  1. pulumi stack init gh-issue
  2. pulumi config set mySecret asdf1234
  3. pulumi up
  4. pulumi stack export
    1. observe the value asdf1234 appears multiple times including in kubectl.kubernetes.io/last-applied-configuration annotations
@lblackstone lblackstone added impact/security kind/bug Some behavior is incorrect or out of spec labels Feb 18, 2020
@lblackstone lblackstone self-assigned this Feb 18, 2020
@lblackstone
Copy link
Member

This happens because we use helm template to render YAML from the provided Chart, and then parse the YAML in a separate step. Information about the "secretness" of resources is currently lost during this process since it happens out of band from Pulumi.

Since we don't have insight into exactly where secret input values appear in the YAML output, this isn't trivial to fix.

A naive solution to this problem would be marking any resulting resources as Secret. It might also be possible to make this more targeted by post-processing the YAML resources and marking fields as secret if they contain a string that was marked secret on input.

@lblackstone
Copy link
Member

The following code should be close to what we need to search the unmarshaled object and mark additionalSecretOutputs prior to creation.

    // obj is the unmarshaled object
    // {"apiVersion":"v1","kind":"Secret","metadata":{"annotations":[{"secret":"supersecret"}],"name":"mysecret"},"stringData":{"password":"supersecret","username":"produser"},"type":"Opaque"}
    const secrets = [];
    function checkForValue(value) {
        return function check(v, path) {
            if (typeof v === "string" && v === value) {
                secrets.push(path)
            } else if (typeof v === "object") {
                for (const [key, val] of Object.entries(v)) {
                    const newPath = path !== undefined ? `${path}.${key}` : `.${key}`
                    check(val, newPath)
                }
            }
        }
    }
    checkForValue("supersecret")(obj)
    console.log(secrets)
    // [ '.metadata.annotations.0.secret', '.stringData.password' ]
    console.log(`hasSecret = ${secrets.length > 0}`)
    // hasSecret = true

@ksvladimir
Copy link

I encountered the same issue, this is unfortunately a blocker for our company adopting pulumi. Is there a way to perhaps mark the entire rendered helm template as additionalSecretOutputs?

@ksvladimir
Copy link

Here's a temporary workaround (see the transformations function):

const redis = new k8s.helm.v3.Chart("redis", {
  chart: "redis",
  version: "10.7.16",
  fetchOpts: {
    repo: "https://charts.bitnami.com/bitnami"
  },
  values: {
    password: config.requireSecret("testsecret")
  },
  transformations: [
    (obj: any, opts: pulumi.CustomResourceOptions) => {
      if (obj.kind === "Secret") {
        for (const key of ['data', 'stringData']) {
          if (key in obj) obj[key] = pulumi.secret(obj[key]);
        }
      }
    }
  ]
}, {
  provider: clusterProvider,
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/security kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants