helm 3 shows secrets with --dry-run option in clear text

[darkstarmv]

Output of helm version:

helm version version.BuildInfo{Version:"v3.0.2", GitCommit:"19e47ee3283ae98139d98460de796c1be1e3975f", GitTreeState:"clean", GoVersion:"go1.13.5"}
Output of kubectl version:

kubectl version Client Version: version.Info{Major:"1", Minor:"15", GitVersion:"v1.15.3", GitCommit:"2d3c76f9091b6bec110a5e63777c332469e0cba2", GitTreeState:"clean", BuildDate:"2019-08-19T11:13:54Z", GoVersion:"go1.12.9", Compiler:"gc", Platform:"darwin/amd64"}

Cloud Provider/Platform (AKS, GKE, Minikube etc.):

Issue

helm3 --dry-run command prints content of the secrets where helm2 just the fact that secret has been created.
This creates issue running helm --dry-run in CI/CD tools as it exposes secrets.

 helm upgrade --install myrelease  mychart  --set thanos.sayamlbase64=TESTSECRET  --dry-run
Release "myrelease" has been upgraded. Happy Helming!
NAME: myrelease
LAST DEPLOYED: Thu Dec 19 15:11:43 2019
NAMESPACE: kube-system
STATUS: pending-upgrade
REVISION: 400
TEST SUITE: None
HOOKS:
MANIFEST:
---
# Source: mychart/templates/thanos-objstore-config-secret.yaml
apiVersion: v1
data:
  thanos.yaml: TESTSECRET
kind: Secret
metadata:
  name: thanos-objstore-config
  namespace: kube-system
type: Opaque
---

helm2 upgrade --install release-name chart  --set thanos.sayamlbase64=TESTSECRET  --dry-run
Release "release-name" has been upgraded. Happy Helming!
LAST DEPLOYED: Thu Dec 19 14:51:59 2019
NAMESPACE: kube-system
STATUS: DEPLOYED

RESOURCES:
==> v1/Secret
NAME                    TYPE    DATA  AGE
thanos-objstore-config  Opaque  1     80d

[bacongobbler]

Hi @darkstarmv. It looks as though helm install --dry-run intentionally displays this output:

helm/cmd/helm/status.go

Line 141 in 0eaa881

if strings.EqualFold(s.release.Info.Description, "Dry run complete") || s.debug {

What is your recommended course of action here?

We could relax this constraint to only display this output when the --debug flag is applied. However, I am concerned about this breaking backwards compatibility, and that there may be users who are relying on this behaviour since this was intentionally introduced.

@thomastaylor312 any thoughts here?

[darkstarmv]

i think by default --dry-run should not print debug output. Each user has to provide additional flags if they would like to see more information.

[thomastaylor312]

I can see the argument, but at this point we can't change that output because of our backwards compatibility guarantees. So anything that we'd want to implement shouldn't break that

[darkstarmv]

so, helm2 was not printing secrets and now helm3 does, should it be changed not to print?

helm2 upgrade --install release-name chart  --set thanos.sayamlbase64=TESTSECRET  --dry-run
Release "release-name" has been upgraded. Happy Helming!
LAST DEPLOYED: Thu Dec 19 14:51:59 2019
NAMESPACE: kube-system
STATUS: DEPLOYED

RESOURCES:
==> v1/Secret
NAME                    TYPE    DATA  AGE
thanos-objstore-config  Opaque  1     80d

[thomastaylor312]

So given our backwards compatibility guarantees, we cannot remove the secret output by default. However, we would be more than willing to review and accept any implementation that uses a feature flag (something like --sanitize) to prevent those values from printing. I'll relabel this as a feature and anyone can implement it

[vaivanov]

any updates on this?

[bobbywatson3]

We'd rather not have secrets splashed across each jenkins job's console output. Any update on this? A --sanitize flag would be excellent.

[bacongobbler]

No updates. Feel free to contribute!

[EssentialMix]

@thomastaylor312 i am not quite understand your comment about backwards compatibility. Helm 2 with dry-run did not exposed secrets, and Helm 3 expose them. So actually there is breaking compatibility changes introduced in Helm 3. Also if regular update/run do not expose secrets, i would expect from dry-run same. Because dry-run means: run as regular, but don't apply changes. We see different behavior here. It would be confusing if we would need to use extra flag for getting same behavior for dry-run and regular run.

[EssentialMix]

I guess this is really depends how we are treating it, as feature or bug? For me this is definitely a bug in dry-run mode.

[jkroepke]

I would say "it works as expected".

The helm diff plugin have a similar functionality. But there is available flag to explicit mask secrets.

[myuseridbws]

@EssentialMix has it right. Helm 3 broke the backward compatibility of the Helm 2 --dry-run behavior.

Helm 2 --dry-run did not print any output (let alone secrets) as you can see here:

$ helm version
Client: &version.Version{SemVer:"v2.16.3", GitCommit:"1ee0254c86d4ed6887327dabed7aa7da29d7eb0d", GitTreeState:"clean"}
Server: &version.Version{SemVer:"v2.16.3", GitCommit:"1ee0254c86d4ed6887327dabed7aa7da29d7eb0d", GitTreeState:"clean"}

$ helm install . --dry-run                                    
NAME:   anxious-marmot
<end-of-output>

[thomastaylor312]

By backwards compatibility we don't mean with Helm 2. The reason for a new major version of Helm was to introduce breaking changes like this one. Now that the feature is in place, our backwards compatibility guarantees have to be honored for all of v3. However, this statement is interesting and deserves conversation:

We see different behavior here. It would be confusing if we would need to use extra flag for getting same behavior for dry-run and regular run.

This is something we'd have to run by other maintainers if we choose to do it because it would be a breaking change in Helm 3, something we do very very rarely and very carefully

[github-actions]

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

[erzz]

Just adding a me-too to keep it alive and add my input.

My biggest issue with this is that when wrapping a helm release into a pipeline using things like Gitlab or Actions etc - then secrets get dumped all over job logs when we are executing a dryrun as one of the tests. This requires then a team to a) be aware its even happening and b) go and manually delete jobs or even entire pipelines in order to hide these credentials.

Given that a CI pipeline is a very prominent case for the existence of the dry-run flag in the first place, I feel it should be handled better

Its a giant hole - it would be bad enough should a developers SCM account get exposed, but to also gift an attacker then access to secrets / infra credentials is like a nuke :)

Can I suggest an opt-in --hide-secrets flag that perhaps would not then be a breaking change?

[tommyreilly]

I would like to vote a +1 for @sean-ersw suggestion of a --hide-secrets flag.

In our pipelines, the quandry is to allow the use of the --dry-run or --debug flag to help our product teams debug their deployments but also at the same time, the concern that @sean-ersw articulates that secrets are printed to the CD (Jenkins) console.

For deployments to production environments we may consider not allowing the use of a --debug or --dry-run flag and we may even have to go there for our staging/verify environments. I'd prefer we didn't have to.

The ability to enforce in our pipeline code the use of a --hide-secrets flag whenever a --debug or --dry-run flag is provided in a helm command line would a) not restrict our end users current capabilities to use --dry-run or --debug flags and b) tighten this security hole when automating the use of helm in a CD pipeline.

I think this is a serious enough consideration for the community to consider a solution, given that the use of helm in automation is common.

[bacongobbler]

Cool - feel free to contribute a fix.

[tommyreilly]

Understood @bacongobbler , fair enough reply :)

[azhurbilo]

Could someone explain why this issue labeled as "feature" but not a "bug" ?!
It's a CRITICAL security breach which blocks migration from Helm V2 to V3

@bacongobbler do you have any ETA of your MR? 🙏🏼

[eechau]

Any updates regarding the issue? This is a CRITICAL security breach, but nobody cares.

[wirwolf]

I planing implement this feature in my project helm-assistant.
This project proxy command into the helm and apply some changes to the output
https://github.com/SomeBlackMagic/helm-assistant

[bstrdsmkr]

Same story here, secrets in logs aren't secrets at all.

I also think this an issue and should be fixed. @NBardelot provided a one-line solution in perl; similar solution should be available in go.

| perl -0777 -pe "s/---.*?type: Opaque.*?---/---/sg"

One more option for mitigating this using yq:

helm template helm/ | yq --yaml-roundtrip '( select(.kind == "Secret")  | (.data[]?, .stringData[]?) ) = "[Masked]"'

[skimwpi]

For those who run CI pipeline w/o perl, the following is awk cmd solution:

helm upgrade --dry-run ... | awk 'BEGIN{RS="---"; ORS="***"}{ gsub("type: Opaque.*", "[HIDDEN]\n"); print; }'

This will replace the secret object output such that all lines after and including "type: Opaque" in the secret output is replaced with [HIDDEN].

I hope this helps those who don't have perl.

[Shivam0609]

Using the workaround suggested by @bstrdsmkr 👉 #7275 (comment)

Created a helm plugin to mask Secret data.

https://github.com/Shivam0609/helm-mask-secrets

[github-actions]

This issue has been marked as stale because it has been open for 90 days with no activity. This thread will be automatically closed in 30 days if no further activity occurs.

[joejulian]

This looks like there are several suggestions and tools to address the issue. Closing.

[pipeworks-asmith]

Not sure I agree that this should be closed, as it feels like a pretty flagrant miss from a security perspective. Just because duct tape exists doesn't mean we shouldn't expect our tools to work without them.

[wirwolf]

Please reopen the issue. Ith an important problem and I wait to fix it in helm

[OhZedTee]

I'm curious why there's no open CVE against a blatant security vulnerability that's been open for 3 years. It almost seems like a callout is needed here.

[m1rm]

For others bumping into this.

helm template --debug --values also exposes the secrets.

A way to avoid the secrets showing up in the CLI output is to pipe the result of the command to sed and mutate it:

sed '/^kind: Secret/,/^---/{//!d;};'

For us this resulted in the section looking like this in the output:

[...]
apiVersion: v1
kind: Secret
---
[...]

yet I fully agree with all others here stating, that this should be solved in helm directly

[rafaelbattesti]

I know a lot of folks already came here to say how much of a security breach this is, but I'll say it again because it shocks me that Helm as a graduated CNCF project closes this issue like it's nothing after 3.5 years.
And just like @OhZedTee I'm curious as well.

[codersasha]

For anyone still experiencing this issue -- this is a sed command that deletes everything from USER-SUPPLIED VALUES: down:

helm install --debug ... | sed '/USER-SUPPLIED VALUES:/,$d'

We now use this in our deploys, so we can debug when a deploy fails, but not print any secrets/configuration that may reveal sensitive data.

[molnare]

We used yq, tail and head to redact any element's value in any depth with any name. (Raspberry PI 4, Ubuntu 23.10, helm vesion 3.10.1)
Say for example the field name has to be redacted everywhere then with yq 4.40.4, tail and head I can do the following:

helm upgrade --install --dry-run --debug my-release ./path/to/chart/ | tail -n +11 | head -n -4|yq eval '(.. | select(has("name")) | .name) |= "REDACTED"'

What is happening:

• tail removes the first 11 lines. That part is metadata like NAME, LAST DEPLOYED, NAMESPACE, etc. and it isn't for yq consumption
• head removes the 4 lines from the end because that is also metadata. For now I only saw NOTES: as metadata.
• yq will evaluate every element which has a subelement name. If it finds one then the name's value will be replaced with REDACTED.

If we want to redact more than one element value then multiple yq statements can be chained one after another:

helm upgrade --install --dry-run --debug my-release ./path/to/chart/ | tail -n +11 | head -n -4 | yq eval '(.. | select(has("name")) | .name) |= "REDACTED"'  | yq eval '(.. | select(has("securityContext")) | .securityContext) |= "REDACTED"'

This will replace name and securityContext values everywhere with REDACTED. There is probably a better shorter way of writing this, but I found it more readable.

Note: Omitting --debug from helm chart will print less metadata. In that case tail should be reduced to +9 from +11.

[github-usr-name]

This looks like there are several suggestions and tools to address the issue. Closing.

There are zero suggestions or tools to address the critical security issue. Rather, there are a several kludges that can be used to workaround it as long as every single team member remembers to use a non-obvious pipe consistently, all the time. That does not address the issue. Adding a --sanitize flag or similar that natively performs the operation without breaking backwards compatibility would be a good resolution.

[oscerd]

Do we know the organization or individual who allocated the CVE? This could be useful for tracking. If someone know about it, please let me know. Thanks.

[TobyTheHutt]

I opened the CVE. My apologies for not notifying you sooner.
If you need information on use cases or other information, feel free to get in touch.

[oscerd]

I opened the CVE. My apologies for not notifying you sooner. If you need information on use cases or other information, feel free to get in touch.

Thanks for letting me know.