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

template: disallow writeToFile by default #12312

Merged
merged 2 commits into from
Mar 29, 2022
Merged

template: disallow writeToFile by default #12312

merged 2 commits into from
Mar 29, 2022

Conversation

schmichael
Copy link
Member

@schmichael schmichael commented Mar 16, 2022

Resolves #12095 by WONTFIXing it.

This approach disables writeToFile as it allows arbitrary host
filesystem writes and is only a small quality of life improvement over
multiple template stanzas.

This approach has the significant downside of leaving people who have
altered their template.function_denylist still vulnerable! I added
an upgrade note, but we should have implemented the denylist as a
map[string]bool so that new funcs could be denied without overriding
custom configurations.

This PR also includes a bug fix that broke enabling all consul-template
funcs. We repeatedly failed to differentiate between a nil (unset)
denylist and an empty (allow all) one.

Copy link
Contributor

@lgfa29 lgfa29 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor tweaks, but LGTM 👍

website/content/docs/configuration/client.mdx Outdated Show resolved Hide resolved
client/allocrunner/taskrunner/template/template_test.go Outdated Show resolved Hide resolved
Resolves #12095 by WONTFIXing it.

This approach disables `writeToFile` as it allows arbitrary host
filesystem writes and is only a small quality of life improvement over
multiple `template` stanzas.

This approach has the significant downside of leaving people who have
altered their `template.function_denylist` *still vulnerable!* I added
an upgrade note, but we should have implemented the denylist as a
`map[string]bool` so that new funcs could be denied without overriding
custom configurations.

This PR also includes a bug fix that broke enabling all consul-template
funcs. We repeatedly failed to differentiate between a nil (unset)
denylist and an empty (allow all) one.
Review notes from @lgfa29

Co-authored-by: Luiz Aoqui <[email protected]>
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

implement function writeToFile in template stanza
2 participants