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

artifact/template: make destination path absolute inside taskdir #9149

Merged
merged 2 commits into from
Oct 22, 2020

Conversation

tgross
Copy link
Member

@tgross tgross commented Oct 22, 2020

Fixes #9148

Prior to Nomad 0.12.5, you could use ${NOMAD_SECRETS_DIR}/mysecret.txt as
the artifact.destination and template.destination because we would always
append the destination to the task working directory. In the recent security
patch we treated the destination absolute path as valid if it didn't escape
the working directory, but this breaks backwards compatibility and
interpolation of destination fields.

This changeset partially reverts the behavior so that we always append the
destination, but we also perform the escape check on that new destination
after interpolation so the security hole is closed.

helper/funcs_test.go Outdated Show resolved Hide resolved
@tgross
Copy link
Member Author

tgross commented Oct 22, 2020

Relevant E2E tests against this PR:

$ go test -v . -suite VaultSecrets
=== RUN   TestE2E
...
=== RUN   TestE2E/VaultSecrets
=== RUN   TestE2E/VaultSecrets/*vaultsecrets.VaultSecretsTest
=== RUN   TestE2E/VaultSecrets/*vaultsecrets.VaultSecretsTest/TestVaultSecrets
...
--- PASS: TestE2E (75.05s)
    --- PASS: TestE2E/VaultSecrets (75.05s)
        --- PASS: TestE2E/VaultSecrets/*vaultsecrets.VaultSecretsTest (74.96s)
            --- PASS: TestE2E/VaultSecrets/*vaultsecrets.VaultSecretsTest/TestVaultSecrets (74.79s)
PASS
ok      github.com/hashicorp/nomad/e2e  75.178s
$ go test -v . -suite ConsulTemplate
=== RUN   TestE2E
...
=== RUN   TestE2E/ConsulTemplate
=== RUN   TestE2E/ConsulTemplate/*consultemplate.ConsulTemplateTest
=== RUN   TestE2E/ConsulTemplate/*consultemplate.ConsulTemplateTest/TestTemplateUpdateTriggers
...
--- PASS: TestE2E (48.58s)
    --- PASS: TestE2E/ConsulTemplate (48.58s)
        --- PASS: TestE2E/ConsulTemplate/*consultemplate.ConsulTemplateTest (48.49s)
            --- PASS: TestE2E/ConsulTemplate/*consultemplate.ConsulTemplateTest/TestTemplateUpdateTriggers (48.32s)

PASS
ok      github.com/hashicorp/nomad/e2e  48.702s

helper/funcs.go Outdated Show resolved Hide resolved
Prior to Nomad 0.12.5, you could use `${NOMAD_SECRETS_DIR}/mysecret.txt` as
the `artifact.destination` and `template.destination` because we would always
append the destination to the task working directory. In the recent security
patch we treated the `destination` absolute path as valid if it didn't escape
the working directory, but this breaks backwards compatibility and
interpolation of `destination` fields.

This changeset partially reverts the behavior so that we always append the
destination, but we also perform the escape check on that new destination
after interpolation so the security hole is closed.
@krishicks
Copy link
Contributor

I wanted to follow-up and say the updated tests are way more clear to me. Thanks!

@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 Dec 13, 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.

template destination backwards compatibility
3 participants