Packer: shell provisioner truncates command-line at space in interpolated variable value

[philpennock]

Packer 1.2.5 on macOS (High Sierra) using amazon-ebs builder.

When using a shell provisioner with the first of these two settings:

  "execute_command": "chmod +x {{ .Path }}; sudo -E sh -c '{{.Vars}} {{ .Path }}'"
  "execute_command": "chmod -v +x {{ .Path }}; sudo -E sh -c 'set -x; {{.Vars}} {{ .Path }}'"

all the scripts failed to run; I switched to the second setting, which exposed the issue. One of the environment variables being set has a value which is a comma+space-separated list of URLs. The command-line generated is truncated after the first comma+space.

Providing the PACKER_LOG=1 output in a gist as requested is problematic (dealing with client processes), but below is the fragment for this provisioner, changing only hostnames.

The advice in https://www.packer.io/docs/provisioners/shell.html has single-quotes around the sudo command part, but with the generated shell fragment for setting the variables also using single-quotes, the content of each variable ends up being unquoted to the shell in the provisioner.

2018/07/20 19:39:35 ui: ==> amazon-ebs: Provisioning with shell script: scripts/10-users-packages-certs.bash
==> amazon-ebs: Provisioning with shell script: scripts/10-users-packages-certs.bash
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 Opening scripts/10-users-packages-certs.bash for reading
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [DEBUG] Opening new ssh session
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [INFO] 4691 bytes written for 'uploadData'
2018/07/20 19:39:35 [INFO] 4691 bytes written for 'uploadData'
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [DEBUG] Starting remote scp process:  scp -vt /tmp
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [DEBUG] Started SCP session, beginning transfers...
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [DEBUG] Copying input data into temporary file so we can read the length
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [DEBUG] scp: Uploading script_239.sh: perms=C0644 size=4691
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [DEBUG] SCP session complete, closing stdin pipe.
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [DEBUG] Waiting for SSH session to complete.
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [DEBUG] scp stderr (length 31): Sink: C0644 4691 script_239.sh
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [DEBUG] Opening new ssh session
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [DEBUG] starting remote command: chmod 0755 /tmp/script_239.sh
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [INFO] RPC endpoint: Communicator ended with: 0
2018/07/20 19:39:35 [INFO] RPC client: Communicator ended with: 0
2018/07/20 19:39:35 [INFO] RPC endpoint: Communicator ended with: 0
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [INFO] RPC client: Communicator ended with: 0
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [DEBUG] Opening new ssh session
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [DEBUG] starting remote command: chmod +x /tmp/script_239.sh; sudo -E sh -c 'PACKER_BUILDER_TYPE='amazon-ebs' PACKER_BUILD_NAME='amazon-ebs' RUNDECK_EMBED_HOSTNAME='rundeck.censored.tld' RUNDECK_OAUTH2_CALLBACK='https://rundeck-new.censored.tld/oauth2/callback' RUNDECK_SERVING_URLS='https://rundeck.censored.tld, https://rundeck-new.censored.tld' RUNDECK_SHA1='002037314382f8f7d0052ef4d4c961ae76e0ac6d' RUNDECK_VERSION='2.11.5-1-GA'  /tmp/script_239.sh'
2018/07/20 19:39:35 ui:     amazon-ebs: sudo: unable to resolve host ip-10-1-255-24: No such file or directory
    amazon-ebs: sudo: unable to resolve host ip-10-1-255-24: No such file or directory
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [INFO] RPC endpoint: Communicator ended with: 0
2018/07/20 19:39:35 [INFO] 0 bytes written for 'stdout'
2018/07/20 19:39:35 [INFO] 71 bytes written for 'stderr'
2018/07/20 19:39:35 [INFO] RPC client: Communicator ended with: 0
2018/07/20 19:39:35 [INFO] RPC endpoint: Communicator ended with: 0
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [INFO] 0 bytes written for 'stdout'
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [INFO] 71 bytes written for 'stderr'
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [INFO] RPC client: Communicator ended with: 0
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [DEBUG] Opening new ssh session
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [DEBUG] starting remote command: rm -f /tmp/script_239.sh
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [INFO] RPC endpoint: Communicator ended with: 0
2018/07/20 19:39:35 [INFO] RPC client: Communicator ended with: 0
2018/07/20 19:39:35 [INFO] RPC endpoint: Communicator ended with: 0
2018/07/20 19:39:35 packer: 2018/07/20 19:39:35 [INFO] RPC client: Communicator ended with: 0

[philpennock]

I'm currently using this, which is proof against all variable values except those which contain single-quotes; I haven't delved enough into Packer's interpolation logic to know if there's handling for those already or not.

"execute_command": "chmod +x {{ .Path }}; for x in {{.Vars}}; do printf \"export '%s'\\n\" \"$x\"; done > /tmp/packer.env.$$; sudo -E sh -c \". /tmp/packer.env.$$; {{ .Path }}\"",

[philpennock]

A better fix, based on code changes, will probably be to have Packer provide .Varfile as an interpolation variable, and have the code generate shell-safe directives in a text file, with all quoting correctly handled, and which exports the variables. Then the correct approach for configs is just to source that file, as I do here. You'd basically be doing the write-to-file from Go with proper escaping/quoting for all cases.

The execute_command then becomes something like this, in various forms:

"chmod +x {{ .Path }}; sudo -E sh -c 'source {{ .Varfile }}; {{ .Path }}'"

[rickard-von-essen]

the code generate shell-safe directives

Unfortunately there is no such thing.

I think this is something we should document better, the current docs are confusing and I'm not even sure it's correct. You should quote your env vars in environment_vars.

[philpennock]

How strange. I don't quote the env vars, but the PACKER_LOG fragment above shows clearly that packer has automatically put single-quotes around each value itself. Thus any quoting manually applied in environment_vars will be fighting against the quoting applied by Packer.

The shell-safe way to quote variable names, which works with any even vaguely POSIX-like shell, is to replace any single quotes ' with the five-character sequence '"'"' and then wrap the result in single quotes ', such that foo'bar becomes 'foo'"'"'bar'. This is well-established, in use for decades.

This shell-safe way breaks down when you deal with ssh which joins parameters together without applying quoting, thus adding a lot of complexity for workarounds. Thus my suggestion, to upload as a file and avoid letting the variables themselves be subject to ssh mangling. You can use .VarfileSH if you want to also have .VarfilePSH or whatever Windows would require.

So yes, the current docs are wrong, and I think I've provided a work-around which is ugly as hell but will break for certain values, and suggested a proper solution (.Varfile) which can be completely correct.

[rickard-von-essen]

But environment variables can reference other variables. And all shells are even trying to be posix. So we shouldn't assume that.

[rickard-von-essen]

So your suggested quoting would brake aaa=$bbb. There is no why to know if this have to be '$bbb' or "$bbb".

[philpennock]

Any shell which breaks single-quoting escaping as I describe it will introduce security vulnerabilities in countless other applications. Single quoting has to work like that for a shell to be usable on Unix-like systems. Not all shells are POSIX, I agree, but this is broader.

As to variables referencing other variables: in the context of Packer, providing fixed strings to be used, that does not appear to be documented intended behavior. It looks like it works right now, _because_ the quoting is broken. If you want that to continue to work, then keep using .Vars exactly as it is now. That does not preclude adding a new mechanism which is safe for arbitrary string values (everything except embedded NULs) and encouraging the use of the safe mechanism.

[SwampDragons]

We already create and source a var file for powershell, so implementing it for the shell provisioner probably makes sense too.

[SwampDragons]

Just so I can be sure about a repro case, the vars you're finding break this look something like brokenvar="www.myurl.com, www.anotherurl.com, www.moreurl.com" yeah?

[philpennock]

I'm sorry, I thought I'd called out the inputs as part of the report, but I only left them buried in the debug output where you have to scroll a lot to see them. I intended to do better than that.

RUNDECK_SERVING_URLS='https://rundeck.censored.tld, https://rundeck-new.censored.tld' 

So yes, your repro case looks sufficient. Thanks.

[rickard-von-essen]

This line looks like the culprit. Changing it to also use '"'"' and fixing the docs is probably good enough to fix this.

But I think it's wrong that we quote at all. That should be left to the user.

[SwampDragons]

I just picked this ticket up and I can't reproduce. Can you provide me with the actual contents of your "shell" provisioner template?

[SwampDragons]

ah, I found it. You weren't quoting at all except to form the json string.

[SwampDragons]

I've tinkered with this and read through the code a bit, and have a few notes:
first:

The shell-safe way to quote variable names, which works with any even vaguely POSIX-like shell, is to replace any single quotes ' with the five-character sequence '"'"' and then wrap the result in single quotes ', such that foo'bar becomes 'foo'"'"'bar'. This is well-established, in use for decades.

This is precisely what we currently do inside the shell provisioner.

Second:
Rickard's solution doesn't work for the special execute_command in the issue description.

Third:
Any change we make here is going to be rough; no matter what we do we're going to break backwards compatibility. For now, I'm going to make a detailed example in the docs describing current behavior for template input and output for various environment variables so that users know what to expect.

[SwampDragons]

Currently I think the best way to address the underlying problem here (our inline variable quoting breaks complex execute_commands) without also breaking backwards compatibility is to add a new template option that allows users to decide for themselves whether to write their env vars to a varfile on the remote system or have us set them inline; we keep the inline behavior the same, but if they chose to source env vars from a tempfile then we can do so. I'm going to sleep on this and think over the best UX.

[philpennock]

FWIW, the quoting I used was exactly that from the documentation; I know a few organizations using exactly that, because it comes from the docs.

100% agreed on the option to use a varfile, I'm sorry that I wasn't clear above that it was supposed to be an option instead of the only way. Oops!

Thanks for picking this up!

[SwampDragons]

I've implemented this option and tested that the sourcing works. Take a look at PR #6636 -- I've uploaded an OSX build of that branch. I'd love it if you could test it and report back.

[philpennock]

Please forgive my ignorance, but ... uploaded to where?

[SwampDragons]

https://github.com/hashicorp/packer/pull/6636#issuecomment-417067900

[philpennock]

Thanks, no wonder I thought I'd seen it and I was being stupid to ask. I had and I was.

Blindly running random executables given to me over The Internet, I can confirm that a Packer build is in progress now and appears to have successfully used this new feature to get as far as it has, so thank you!

SHA256(Downloads/packer.zip)= ea58059e1d365b3b07d41b130b6cbace0693b82a51286b77df41e23e92915db2
SHA256(packer)= e236d392c5cda6507816b8906b8b32349be2c3ba9301827c460c3119e2091ea5

[SwampDragons]

😂 Glad to be your source of Definitely Safe Internet Executables (though for what it's worth, people with the "Member" flair on here are paid by HashiCorp and you can probably trust us when it comes to HashiCorp tools)

[hashibot[bot]]

I'm going to lock this issue because it has been closed for _30 days_ ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.