-
Notifications
You must be signed in to change notification settings - Fork 616
Document YAML injection risks in chart templates #2158
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,42 @@ Template functions follow the syntax `functionName arg1 arg2...`. In the snippet | |
| above, `quote .Values.favorite.drink` calls the `quote` function and passes it a | ||
| single argument. | ||
|
|
||
| ## Avoid YAML injection | ||
|
|
||
| Values may come from users or other untrusted sources. Rendering an untrusted | ||
| string directly into a template can allow the string to change the structure of | ||
| the generated YAML. For example, this template renders a scalar value without | ||
| quoting it: | ||
|
|
||
| ```yaml | ||
| data: | ||
| drink: {{ .Values.favorite.drink }} | ||
| ``` | ||
|
|
||
| A value containing YAML syntax, such as a newline followed by another key, may | ||
| then add unexpected data to the rendered manifest. Quote string values so that | ||
| YAML treats the result as a scalar: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about other types of scalars (numbers, bools, null)? I forgot if kubectl or the k8s API do type coercion here? From a pure YAML perspective, Three alternatives to address this issue, if necessary: A. Use |
||
|
|
||
| ```yaml | ||
| data: | ||
| drink: {{ .Values.favorite.drink | quote }} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This works fine for values that map to a single YAML scalar. But it gets tricky, when you want construct a YAML string from multiple values. In my demo project, I've recommended the following approach: I guess there are many other alternatives, e.g. using |
||
| ``` | ||
|
|
||
| When a value is intentionally structured data, serialize the complete value | ||
| with `toYaml` and indent it for its location in the manifest: | ||
|
|
||
| ```yaml | ||
| metadata: | ||
| annotations: | ||
| {{- .Values.podAnnotations | toYaml | nindent 4 }} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think all this different whitespace handling is getting a little confusing here. First we remove a line-break through go template syntax I think this would be better: |
||
| ``` | ||
|
|
||
| Using `toYaml` with the correct indentation preserves the intended YAML | ||
| structure, but it does not make arbitrary content safe or validate that the | ||
| result is appropriate for a particular Kubernetes field. Chart authors should | ||
| validate and constrain accepted values when they may come from untrusted | ||
| sources. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's good to mention validation. But how to do it, exactly? I could not find anything about validation elsewhere in the Helm docs. For comparison, Terraform/OpenTofu have a dedicated mechanism for validating input variables. And they have a dedicated guide that covers all sorts of additional validation mechanisms. I'm not an active Helm user, but afaik, Helm does not have similar mechanisms. If I'm wrong here, how about linking to existing documentation? Of course, there's always the possibility of validating inside the template itself. Do go templates recommend a standard way to do validation? I couldn't find it in the package docs. In my demo project, I've used a rather crude validation approach: I added some validation logic in the beginning, and I simply emit an empty YAML file, if any of the validations fail. Could not find a more idiomatic approach, e.g. failing the template run through an exception or similar. I can do more research on this... I feel that template authors need more guidance here. |
||
|
|
||
| Helm has over 60 available functions. Some of them are defined by the [Go | ||
| template language](https://godoc.org/text/template) itself. Most of the others | ||
| are part of the [Sprig template library](https://masterminds.github.io/sprig/). | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be helpful to explain the risk more thoroughly. It may not be a risk at all in certain scenarios. E.g. when I'm writing a Helm chart that I'll only use myself, and I always know which values will go into it, I might get away without bullet-proof YAML encoding.
But in my experience, some values will ultimately come from somewhere unexpected. E.g. some orchestration tool. Which in turn may pull some values from a dynamic source, e.g. label names that someone has entered into some web UI. See the problem statement in my demo project.
On the other hand, it might be worthwhile mentioning the other benefits of consistent YAML encoding. Templates will be more robust against non-malicious input values that just contain YAML control characters. Proper encoding helps prevent such errors and can save a lot of time debugging template code.