From b45ee4f4551b006a71b4d85e0b03b28eac1bd490 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 22 Aug 2017 16:43:20 -0700 Subject: [PATCH] (#6656) Format windows paths for ssh_config command Prior to this commit, if the ssh-config command was invoked within cygwin or msys2, it would show a regular windows style path for private keys rather than a path that could be used within msys2 or cygwin. This commit updates that behavior by converting all of the private key paths to the proper msys2 or cygwin path if the platform is windows and the command was invoked from one of those two shells. --- lib/vagrant/util/platform.rb | 70 ++++++++++++++----- plugins/commands/ssh_config/command.rb | 9 +++ .../commands/ssh_config/command_test.rb | 17 ++++- test/unit/vagrant/util/platform_test.rb | 63 +++++++++++++++++ 4 files changed, 142 insertions(+), 17 deletions(-) diff --git a/lib/vagrant/util/platform.rb b/lib/vagrant/util/platform.rb index b9d4df6d9..2fbcac838 100644 --- a/lib/vagrant/util/platform.rb +++ b/lib/vagrant/util/platform.rb @@ -4,6 +4,7 @@ require "tmpdir" require "vagrant/util/subprocess" require "vagrant/util/powershell" +require "vagrant/util/which" module Vagrant module Util @@ -19,6 +20,15 @@ module Vagrant @_cygwin end + def msys? + if !defined?(@_msys) + @_msys = ENV["VAGRANT_DETECTED_OS"].to_s.downcase.include?("msys") || + platform.include?("msys") || + ENV["OSTYPE"].to_s.downcase.include?("msys") + end + @_msys + end + def wsl? if !defined?(@_wsl) @_wsl = false @@ -93,25 +103,36 @@ module Vagrant # @param [String] path # @return [String] def cygwin_path(path) - if cygwin? - begin - # First try the real cygpath - process = Subprocess.execute("cygpath", "-u", "-a", path.to_s) - return process.stdout.chomp - rescue Errors::CommandUnavailableWindows - end + begin + # We have to revert to the old env + # path here, otherwise it looks like + # msys2 ends up using the wrong cygpath + # binary and ends up with a `/cygdrive` + # when it doesn't exist in msys2 + original_path_env = ENV['PATH'] + ENV['PATH'] = ENV['VAGRANT_OLD_ENV_PATH'] + cygpath = Vagrant::Util::Which.which("cygpath") + cygpath.gsub!("/", '\\') + process = Subprocess.execute( + cygpath, "-u", "-a", path.to_s) + return process.stdout.chomp + rescue Errors::CommandUnavailableWindows => e + # Sometimes cygpath isn't available (msys). Instead, do what we + # can with bash tricks. + process = Subprocess.execute( + "bash", + "--noprofile", + "--norc", + "-c", "cd #{Shellwords.escape(path)} && pwd") + return process.stdout.chomp + ensure + ENV['PATH'] = original_path_env end - - # Sometimes cygpath isn't available (msys). Instead, do what we - # can with bash tricks. - process = Subprocess.execute( - "bash", - "--noprofile", - "--norc", - "-c", "cd #{Shellwords.escape(path)} && pwd") - return process.stdout.chomp end + # Identical to cygwin_path for now + alias_method :msys_path, :cygwin_path + # This takes any path and converts it to a full-length Windows # path on Windows machines in Cygwin. # @@ -264,6 +285,23 @@ module Vagrant end end + # Takes a windows path and formats it to the + # 'unix' style (i.e. `/cygdrive/c` or `/c/`) + # + # @param [Pathname, String] path Path to convert + # @param [Hash] hash of arguments + # @return [String] + def format_windows_path(path, *args) + path = cygwin_path(path) if cygwin? + path = msys_path(path) if msys? + path = wsl_to_windows_path(path) if wsl? + if windows? || wsl? + path = windows_unc_path(path) if !args.include?(:disable_unc) + end + + path + end + # Automatically convert a given path to a Windows path. Will only # be applied if running on a Windows host. If running on Windows # host within the WSL, the actual Windows path will be returned. diff --git a/plugins/commands/ssh_config/command.rb b/plugins/commands/ssh_config/command.rb index 7df07ff6e..9ec356e66 100644 --- a/plugins/commands/ssh_config/command.rb +++ b/plugins/commands/ssh_config/command.rb @@ -1,6 +1,7 @@ require 'optparse' require "vagrant/util/safe_puts" +require "vagrant/util/platform" module VagrantPlugins module CommandSSHConfig @@ -11,6 +12,10 @@ module VagrantPlugins "outputs OpenSSH valid configuration to connect to the machine" end + def convert_win_paths(paths) + paths.map! { |path| Vagrant::Util::Platform.format_windows_path(path, :disable_unc) } + end + def execute options = {} @@ -32,6 +37,10 @@ module VagrantPlugins ssh_info = machine.ssh_info raise Vagrant::Errors::SSHNotReady if ssh_info.nil? + if Vagrant::Util::Platform.windows? + ssh_info[:private_key_path] = convert_win_paths(ssh_info[:private_key_path]) + end + variables = { host_key: options[:host] || machine.name || "vagrant", ssh_host: ssh_info[:host], diff --git a/test/unit/plugins/commands/ssh_config/command_test.rb b/test/unit/plugins/commands/ssh_config/command_test.rb index 27f0844b4..25bb43676 100644 --- a/test/unit/plugins/commands/ssh_config/command_test.rb +++ b/test/unit/plugins/commands/ssh_config/command_test.rb @@ -24,7 +24,7 @@ describe VagrantPlugins::CommandSSHConfig::Command do username: "testuser", keys_only: true, paranoid: false, - private_key_path: [], + private_key_path: ["/home/vagrant/.private/keys.key"], forward_agent: false, forward_x11: false }} @@ -53,6 +53,7 @@ Host #{machine.name} UserKnownHostsFile /dev/null StrictHostKeyChecking no PasswordAuthentication no + IdentityFile /home/vagrant/.private/keys.key IdentitiesOnly yes LogLevel FATAL SSHCONFIG @@ -136,5 +137,19 @@ Host #{machine.name} expect(output).not_to include('StrictHostKeyChecking ') expect(output).not_to include('UserKnownHostsFile ') end + + it "formats windows paths if windows" do + allow(machine).to receive(:ssh_info) { ssh_info.merge(private_key_path: ["C:\\path\\to\\vagrant\\home.key"]) } + allow(Vagrant::Util::Platform).to receive(:format_windows_path).and_return("/home/vagrant/home.key") + allow(Vagrant::Util::Platform).to receive(:windows?).and_return(true) + + output = "" + allow(subject).to receive(:safe_puts) do |data| + output += data if data + end + + subject.execute + expect(output).to include('IdentityFile /home/vagrant/home.key') + end end end diff --git a/test/unit/vagrant/util/platform_test.rb b/test/unit/vagrant/util/platform_test.rb index 974d8a706..28511b2a1 100644 --- a/test/unit/vagrant/util/platform_test.rb +++ b/test/unit/vagrant/util/platform_test.rb @@ -5,8 +5,27 @@ require "vagrant/util/platform" describe Vagrant::Util::Platform do include_context "unit" + subject { described_class } + describe "#cygwin_path" do + let(:path) { "C:\\msys2\\home\\vagrant" } + let(:updated_path) { "/home/vagrant" } + let(:subprocess_result) do + double("subprocess_result").tap do |result| + allow(result).to receive(:exit_code).and_return(0) + allow(result).to receive(:stdout).and_return(updated_path) + end + end + + it "takes a windows path and returns a formatted path" do + allow(Vagrant::Util::Which).to receive(:which).and_return("C:/msys2/cygpath") + allow(Vagrant::Util::Subprocess).to receive(:execute).and_return(subprocess_result) + + expect(subject.cygwin_path(path)).to eq("/home/vagrant") + end + end + describe "#cygwin?" do before do allow(subject).to receive(:platform).and_return("test") @@ -51,6 +70,50 @@ describe Vagrant::Util::Platform do end end + describe "#msys?" do + before do + allow(subject).to receive(:platform).and_return("test") + described_class.reset! + end + + after do + described_class.reset! + end + + around do |example| + with_temp_env(VAGRANT_DETECTED_OS: "nope", PATH: "") do + example.run + end + end + + it "returns true if VAGRANT_DETECTED_OS includes msys" do + with_temp_env(VAGRANT_DETECTED_OS: "msys") do + expect(subject).to be_msys + end + end + + it "returns true if OSTYPE includes msys" do + with_temp_env(OSTYPE: "msys") do + expect(subject).to be_msys + end + end + + it "returns true if platform has msys" do + allow(subject).to receive(:platform).and_return("msys") + expect(subject).to be_msys + end + + it "returns false if the PATH contains msys" do + with_temp_env(PATH: "C:/msys") do + expect(subject).to_not be_msys + end + end + + it "returns false if nothing is available" do + expect(subject).to_not be_msys + end + end + describe "#fs_real_path" do it "fixes drive letters on Windows", :windows do expect(described_class.fs_real_path("c:/foo").to_s).to eql("C:/foo")