Merge pull request #10389 from briancain/ssh-keyfile-paths-spaces-windows

FIXES #9870: Allow for windows path spaces with ssh utility

lib/vagrant/util/ssh.rb

@@ -150,11 +150,24 @@ module Vagrant
         if !plain_mode && options[:private_key_path]
           options[:private_key_path].each do |path|
 
-            # Use '-o' instead of '-i' because '-i' does not call
+            private_key_arr = []
-            # percent_expand in misc.c, but '-o' does. when passing the path,
+
-            # replace '%' in the path with '%%' to escape the '%'
+            if path.include?('%')
-            path = path.to_s.gsub('%', '%%')
+              if path.include?(' ') && Platform.windows?
-            command_options += ["-o", "IdentityFile=\"#{path}\""]
+                LOGGER.warn("Paths with spaces and % on windows is not supported and will fail to read the file")
+              end
+              # Use '-o' instead of '-i' because '-i' does not call
+              # percent_expand in misc.c, but '-o' does. when passing the path,
+              # replace '%' in the path with '%%' to escape the '%'
+              path = path.to_s.gsub('%', '%%')
+              private_key_arr = ["-o", "IdentityFile=\"#{path}\""]
+            else
+              # Pass private key file directly with '-i', which properly supports
+              # paths with spaces on Windows guests
+              private_key_arr = ["-i", path]
+            end
+
+            command_options += private_key_arr
           end
         end
 

test/unit/vagrant/util/ssh_test.rb

@@ -5,6 +5,7 @@ require "vagrant/util/ssh"
 
 describe Vagrant::Util::SSH do
   include_context "unit"
+  let(:private_key_path) { temporary_file.to_s }
 
   describe "checking key permissions" do
     let(:key_path) { temporary_file }
@@ -33,7 +34,7 @@ describe Vagrant::Util::SSH do
       host: "localhost",
       port: 2222,
       username: "vagrant",
-      private_key_path: [temporary_file],
+      private_key_path: [private_key_path],
       compression: true,
       dsa_authentication: true
     }}
@@ -82,7 +83,29 @@ describe Vagrant::Util::SSH do
 
       expect(described_class.exec(ssh_info)).to eq(nil)
       expect(Vagrant::Util::SafeExec).to have_received(:exec)
-        .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL","-o", "Compression=yes", "-o", "DSAAuthentication=yes", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"")
+        .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL","-o", "Compression=yes", "-o", "DSAAuthentication=yes", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-i", ssh_info[:private_key_path][0])
+    end
+
+    context "when using '%' in a private_key_path" do
+      let(:private_key_path) { "/tmp/percent%folder" }
+      let(:ssh_info) {{
+        host: "localhost",
+        port: 2222,
+        username: "vagrant",
+        private_key_path: [private_key_path],
+        compression: true,
+        dsa_authentication: true
+      }}
+
+      let(:ssh_path) { "/usr/bin/ssh" }
+
+      it "uses the IdentityFile argument and escapes the '%' character" do
+        allow(Vagrant::Util::SafeExec).to receive(:exec).and_return(nil)
+        described_class.exec(ssh_info)
+
+        expect(Vagrant::Util::SafeExec).to have_received(:exec)
+          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL","-o", "Compression=yes", "-o", "DSAAuthentication=yes", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"/tmp/percent%%folder\"")
+      end
     end
 
     context "when disabling compression or dsa_authentication flags" do
@@ -90,7 +113,7 @@ describe Vagrant::Util::SSH do
         host: "localhost",
         port: 2222,
         username: "vagrant",
-        private_key_path: [temporary_file],
+        private_key_path: [private_key_path],
         compression: false,
         dsa_authentication: false
       }}
@@ -100,7 +123,7 @@ describe Vagrant::Util::SSH do
 
         expect(described_class.exec(ssh_info)).to eq(nil)
         expect(Vagrant::Util::SafeExec).to have_received(:exec)
-          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"")
+          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-i", ssh_info[:private_key_path][0])
       end
     end
 
@@ -109,7 +132,7 @@ describe Vagrant::Util::SSH do
         host: "localhost",
         port: 2222,
         username: "vagrant",
-        private_key_path: [temporary_file],
+        private_key_path: [private_key_path],
         verify_host_key: true
       }}
 
@@ -118,7 +141,7 @@ describe Vagrant::Util::SSH do
 
         expect(described_class.exec(ssh_info)).to eq(nil)
         expect(Vagrant::Util::SafeExec).to have_received(:exec)
-          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"")
+          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-i", ssh_info[:private_key_path][0])
       end
     end
 
@@ -127,7 +150,7 @@ describe Vagrant::Util::SSH do
         host: "localhost",
         port: 2222,
         username: "vagrant",
-        private_key_path: [temporary_file],
+        private_key_path: [private_key_path],
         keys_only: true
       }}
 
@@ -146,7 +169,7 @@ describe Vagrant::Util::SSH do
         host: "localhost",
         port: 2222,
         username: "vagrant",
-        private_key_path: [temporary_file],
+        private_key_path: [private_key_path],
         forward_x11: true
       }}
 
@@ -155,7 +178,7 @@ describe Vagrant::Util::SSH do
 
         expect(described_class.exec(ssh_info)).to eq(nil)
         expect(Vagrant::Util::SafeExec).to have_received(:exec)
-          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"","-o", "ForwardX11=yes", "-o", "ForwardX11Trusted=yes")
+          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-i", ssh_info[:private_key_path][0],"-o", "ForwardX11=yes", "-o", "ForwardX11Trusted=yes")
       end
     end
 
@@ -164,7 +187,7 @@ describe Vagrant::Util::SSH do
         host: "localhost",
         port: 2222,
         username: "vagrant",
-        private_key_path: [temporary_file],
+        private_key_path: [private_key_path],
         forward_agent: true
       }}
 
@@ -173,7 +196,7 @@ describe Vagrant::Util::SSH do
 
         expect(described_class.exec(ssh_info)).to eq(nil)
         expect(Vagrant::Util::SafeExec).to have_received(:exec)
-          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"","-o", "ForwardAgent=yes")
+          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-i", ssh_info[:private_key_path][0],"-o", "ForwardAgent=yes")
       end
     end
 
@@ -182,7 +205,7 @@ describe Vagrant::Util::SSH do
         host: "localhost",
         port: 2222,
         username: "vagrant",
-        private_key_path: [temporary_file],
+        private_key_path: [private_key_path],
         extra_args: ["-L", "8008:localhost:80"]
       }}
 
@@ -191,7 +214,7 @@ describe Vagrant::Util::SSH do
 
         expect(described_class.exec(ssh_info)).to eq(nil)
         expect(Vagrant::Util::SafeExec).to have_received(:exec)
-          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"", "-L", "8008:localhost:80")
+          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-i", ssh_info[:private_key_path][0], "-L", "8008:localhost:80")
       end
     end
 
@@ -200,7 +223,7 @@ describe Vagrant::Util::SSH do
         host: "localhost",
         port: 2222,
         username: "vagrant",
-        private_key_path: [temporary_file],
+        private_key_path: [private_key_path],
         extra_args: "-6"
       }}
 
@@ -209,7 +232,7 @@ describe Vagrant::Util::SSH do
 
         expect(described_class.exec(ssh_info)).to eq(nil)
         expect(Vagrant::Util::SafeExec).to have_received(:exec)
-          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-o", "IdentityFile=\"#{ssh_info[:private_key_path][0]}\"", "-6")
+          .with(ssh_path, "vagrant@localhost", "-p", "2222", "-o", "LogLevel=FATAL", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "-i", ssh_info[:private_key_path][0], "-6")
       end
     end
 
@@ -218,7 +241,7 @@ describe Vagrant::Util::SSH do
         host: "localhost",
         port: 2222,
         username: "vagrant",
-        private_key_path: [temporary_file],
+        private_key_path: [private_key_path],
       }}
 
       it "executes SSH in a subprocess with options and returns an exit code Fixnum" do