fix: quote container paths interpolated into docker exec command lines - #496
Merged
Conversation
The transport stages every provisioner command as a script under the configured `temp_dir` and then runs it inside the container. Both halves of that -- creating the directory and running the script -- built their `docker exec` command line by interpolating the path into a string, with no quoting, so a `temp_dir` containing a space was torn apart by the shell before docker ever saw it. On Linux that meant `mkdir -p /var/tmp/kitchen docker` created two directories, neither of them the one asked for, and bash was then handed `/var/tmp/kitchen` as the script to run. On Windows it meant PowerShell's `-File`, which takes exactly one argument, looked for a script named after the first word of the directory -- and `$env:TEMP` contains a space whenever the logged-in user's name does. The copy that puts the script there already escaped both of its paths, so this was the one step in the sequence that did not. Escaping is a no-op for a path without a space, so nothing changes for a command line that already worked. Signed-off-by: Tim Smith <tim@mondoo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The transport stages every provisioner command as a script under the configured
temp_dirand then runs it inside the container. Both halves of that — creating the directory, and running the script — built theirdocker execcommand line by interpolating the path into a string with no quoting, so atemp_dircontaining a space was torn apart by the shell before docker ever saw it.On Linux,
create_dir_on_containerproducedmkdir -p /var/tmp/kitchen docker, which created two directories, neither of them the one asked for;Container::Linux#executethen handed bash/var/tmp/kitchenas the script to run.On Windows,
Container::Windows#executeproducedpowershell ... -File C:\Users\Foo Bar\Temp\docker-<uuid>.ps1.-Filetakes exactly one argument, so PowerShell looked for a script named after the first word of the directory.$env:TEMP— the default — contains a space whenever the logged-in user's name does.The
docker cpthat puts the script there already escapes both of its paths, and so does the probe that checks the copy landed, so these were the only steps in the sequence that did not.What changed
create_dir_on_containerescapes the path it passes tomkdir -p. The PowerShell branch already quotes the path itself and its argument is reassembled by PowerShell rather than split by a shell, so it is left alone.Container::Linux#executeescapes the script path it passes to/bin/bash.Container::Windows#executequotes the script path it passes to-File.Escaping is a no-op for a path with no space in it, so nothing changes for a command line that already worked.
Tests
Three regression tests, one per site, asserting on the argument vector the command splits into rather than on the string. All three fail on
main:Container::Windows#executehad no tests at all before this, and neither didcreate_dir_on_container,container_execorrun_container; they are covered here alongside the fix.Verification