fix: quote container paths interpolated into docker exec command lines

lib/kitchen/docker/container/linux.rb

@@ -84,7 +84,10 @@ def execute(command)
           debug("Executing temp script #{remote_path}/#{filename} on container")
           remote_path = replace_env_variables(@config, remote_path)
 
-          container_exec(@config, "/bin/bash #{remote_path}/#{filename}")
+          # Escaped because the exec command line is assembled as one string:
+          # a temp_dir with a space in it was split before docker saw it, and
+          # bash was handed the first half of the directory as the script.
+          container_exec(@config, "/bin/bash #{Shellwords.escape("#{remote_path}/#{filename}")}")
         rescue => e
           raise "Failed to execute command on Linux container. #{e}"
         ensure

lib/kitchen/docker/container/windows.rb

@@ -69,7 +69,11 @@ def execute(command)
           # Replace any environment variables used in the path and execute script file
           debug("Executing temp script #{remote_path}\\#{filename} on container")
           remote_path = replace_env_variables(@config, remote_path)
-          cmd = build_powershell_command("-File #{remote_path}\\#{filename}")
+          # Quoted because PowerShell's -File takes exactly one argument. An
+          # unquoted path with a space in it -- which $env:TEMP has whenever the
+          # user's name does -- left PowerShell looking for a script named
+          # after the first word of the directory.
+          cmd = build_powershell_command(%{-File "#{remote_path}\\#{filename}"})
 
           container_exec(@config, cmd)
         rescue => e

lib/kitchen/docker/helpers/container_helper.rb

@@ -155,14 +155,25 @@ def container_exec(state, command)
 
         # Creates a directory inside the container, on Linux or Windows.
         #
+        # The path is escaped for the shell, because it comes from the
+        # transport's +temp_dir+ and a `docker exec` command line is assembled
+        # as one string. A directory with a space in it was torn in two before
+        # docker ever saw it, and `mkdir -p` obligingly created both halves --
+        # neither of them the directory that was asked for. Every upload that
+        # followed then went to a path that did not exist.
+        #
+        # 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 as it is.
+        #
         # @param state [Hash] instance state naming the container
         # @param path [String] the directory to create; environment variable
         #   references are expanded first
         # @return [String] the command's combined output
         # @raise [RuntimeError] if the directory cannot be created
         def create_dir_on_container(state, path)
           path = replace_env_variables(state, path)
-          cmd = "mkdir -p #{path}"
+          cmd = "mkdir -p #{Shellwords.escape(path)}"
 
           if state[:platform].include?("windows")
             psh = "-Command if(-not (Test-Path '#{path}')) { New-Item -Path '#{path}' -Force }"

spec/container_helper_spec.rb

@@ -333,6 +333,89 @@ def helper_inspecting(output)
     end
   end
 
+  describe "#create_dir_on_container" do
+    def dir_maker(state)
+      helper.tap do |h|
+        @commands = []
+        allow(h).to receive(:replace_env_variables) { |_s, path| path }
+        allow(h).to receive(:docker_command) { |cmd, _opts = {}| @commands << cmd; "" }
+      end
+    end
+
+    it "creates the directory inside the container" do
+      dir_maker(nil).create_dir_on_container({ container_id: "abc", platform: "ubuntu-24.04" }, "/tmp")
+      expect(argv(@commands.first)).to include_consecutive("mkdir", "-p", "/tmp")
+    end
+
+    it "expands an environment variable reference before creating it" do
+      h = helper
+      allow(h).to receive(:container_env_variables).and_return("TEMP" => "/var/tmp")
+      allow(h).to receive(:docker_command) { |cmd, _opts = {}| @cmd = cmd; "" }
+      h.create_dir_on_container({ container_id: "abc", platform: "ubuntu-24.04" }, "$TEMP/kitchen")
+      expect(argv(@cmd)).to include_consecutive("mkdir", "-p", "/var/tmp/kitchen")
+    end
+
+    # A temp_dir with a space in it reached `mkdir -p` unquoted, so the shell
+    # tore it in two and mkdir made two directories -- neither of them the one
+    # asked for. Every later upload then landed somewhere that did not exist.
+    it "keeps a path containing a space as one argument" do
+      dir_maker(nil).create_dir_on_container({ container_id: "abc", platform: "ubuntu-24.04" },
+        "/var/tmp/kitchen docker")
+      expect(argv(@commands.first)).to include_consecutive("mkdir", "-p", "/var/tmp/kitchen docker")
+    end
+
+    it "uses PowerShell on a Windows container" do
+      dir_maker(nil).create_dir_on_container({ container_id: "abc", platform: "windows-2022" }, 'C:\\Temp')
+      expect(@commands.first).to include("powershell").and include("New-Item")
+    end
+
+    it "says which directory it could not create" do
+      h = helper
+      allow(h).to receive(:replace_env_variables) { |_s, path| path }
+      allow(h).to receive(:docker_command).and_raise("boom")
+      expect { h.create_dir_on_container({ container_id: "abc", platform: "ubuntu-24.04" }, "/tmp/kitchen") }
+        .to raise_error(RuntimeError, %r{Failed to create directory /tmp/kitchen})
+    end
+  end
+
+  describe "#container_exec" do
+    it "runs the command through docker exec" do
+      h = helper
+      allow(h).to receive(:docker_command) { |cmd, _opts = {}| @cmd = cmd; "output" }
+      expect(h.container_exec({ container_id: "abc" }, "echo hi")).to eq "output"
+      expect(argv(@cmd)).to include_consecutive("exec", "abc", "echo", "hi")
+    end
+
+    it "names the container operation when the command fails" do
+      h = helper
+      allow(h).to receive(:docker_command).and_raise("boom")
+      expect { h.container_exec({ container_id: "abc" }, "echo hi") }
+        .to raise_error(RuntimeError, /Failed to execute command on Docker container/)
+    end
+  end
+
+  describe "#run_container" do
+    it "returns the id docker printed for the container it started" do
+      h = helper(instance_name: "kitchen-test")
+      allow(h).to receive(:docker_command).and_return(DockerOutput::RUN_CLEAN)
+      expect(h.run_container({ image_id: "sha256:abc" }, 22)).to eq DockerOutput::RUN_CONTAINER_ID
+    end
+
+    it "publishes the port it was given" do
+      h = helper
+      allow(h).to receive(:docker_command) { |cmd, _opts = {}| @cmd = cmd; DockerOutput::RUN_CLEAN }
+      h.run_container({ image_id: "sha256:abc" }, 22)
+      expect(argv(@cmd)).to include_consecutive("-p", "22")
+    end
+
+    it "publishes no port when there is none, as for a Windows container" do
+      h = helper
+      allow(h).to receive(:docker_command) { |cmd, _opts = {}| @cmd = cmd; DockerOutput::RUN_CLEAN }
+      h.run_container(image_id: "sha256:abc")
+      expect(argv(@cmd)).not_to include("-p")
+    end
+  end
+
   describe "#copy_file_to_container" do
     let(:state) { { container_id: "abc", platform: "ubuntu-24.04" } }
 

spec/linux_container_spec.rb

@@ -211,6 +211,33 @@ def container_in(dir, &upload)
       c.execute("echo hi")
       expect(uploaded).to eq "echo hi"
     end
+
+    # The staged script is run by path, and that path is built from temp_dir,
+    # which the user sets.
+    def ran_in(temp_dir)
+      c = container(temp_dir: temp_dir)
+      allow(c).to receive(:create_dir_on_container)
+      allow(c).to receive(:replace_env_variables) { |_cfg, path| path }
+      allow(c).to receive(:upload)
+      ran = nil
+      allow(c).to receive(:container_exec) { |_cfg, cmd| ran = cmd }
+      c.execute("echo hi")
+      ran
+    end
+
+    it "runs the staged script with bash" do
+      ran = ran_in("/tmp")
+      expect(argv(ran).first).to eq "/bin/bash"
+      expect(argv(ran).last).to match(%r{\A/tmp/docker-[0-9a-f-]+\.sh\z})
+    end
+
+    # Interpolated unquoted, a temp_dir with a space in it reached `docker
+    # exec` as two arguments, and bash was handed the first half of the
+    # directory as the script to run.
+    it "keeps a temp_dir containing a space as one argument" do
+      expect(argv(ran_in("/var/tmp/kitchen docker")).last)
+        .to match(%r{\A/var/tmp/kitchen docker/docker-[0-9a-f-]+\.sh\z})
+    end
   end
 
   describe "#generate_keys" do

spec/windows_container_spec.rb

@@ -64,6 +64,60 @@ def container(config = {})
     end
   end
 
+  describe "#execute" do
+    def staged(config = {})
+      c = container({ temp_dir: 'C:\\Temp' }.merge(config))
+      allow(c).to receive(:create_dir_on_container)
+      allow(c).to receive(:replace_env_variables) { |_cfg, path| path }
+      allow(c).to receive(:upload)
+      allow(c).to receive(:container_exec) { |_cfg, cmd| @ran = cmd }
+      c
+    end
+
+    around do |example|
+      Dir.mktmpdir { |dir| Dir.chdir(dir) { example.run } }
+    end
+
+    it "runs the staged script with powershell" do
+      staged.execute("Write-Host hi")
+      expect(@ran).to start_with "powershell -ExecutionPolicy Bypass -NoLogo "
+      expect(@ran).to match(/-File .*docker-[0-9a-f-]+\.ps1/)
+    end
+
+    it "stages the script under the configured temp_dir" do
+      staged.execute("Write-Host hi")
+      expect(@ran).to include 'C:\\Temp\\docker-'
+    end
+
+    it "accepts a temp_dir written with forward slashes" do
+      # Kitchen configuration is YAML that is often shared with Linux suites,
+      # so the separator is normalised rather than rejected.
+      staged(temp_dir: "C:/Temp").execute("Write-Host hi")
+      expect(@ran).to include 'C:\\Temp\\docker-'
+    end
+
+    # A Windows temp_dir routinely contains a space -- $env:TEMP under a user
+    # whose name has one does. PowerShell's -File takes exactly one argument,
+    # so an unquoted path left it looking for a script named after the first
+    # word.
+    it "quotes a script path containing a space" do
+      staged(temp_dir: 'C:\\Users\\Foo Bar\\Temp').execute("Write-Host hi")
+      expect(@ran).to include('-File "C:\\Users\\Foo Bar\\Temp\\docker-').and end_with('.ps1"')
+    end
+
+    it "removes the staged script once the command has run" do
+      staged.execute("Write-Host hi")
+      expect(Dir.glob(".kitchen/temp/docker-*.ps1")).to be_empty
+    end
+
+    it "removes the staged script when the command fails" do
+      c = staged
+      allow(c).to receive(:container_exec).and_raise("command exploded")
+      expect { c.execute("Write-Host hi") }.to raise_error(/Failed to execute command/)
+      expect(Dir.glob(".kitchen/temp/docker-*.ps1")).to be_empty
+    end
+  end
+
   describe "the contract it shares with the Linux container" do
     # The two classes share almost no code but must agree on the state they
     # populate, because the driver and transport read the same keys whichever