From 570d5abb9581bccc1e105f452692a047f81c74ad Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 17 Jan 2018 13:14:48 -0800 Subject: [PATCH] Prefer ssh executable found on original path --- lib/vagrant/util/ssh.rb | 29 ++++++---------------- test/unit/vagrant/util/ssh_test.rb | 39 ++++++++++-------------------- 2 files changed, 21 insertions(+), 47 deletions(-) diff --git a/lib/vagrant/util/ssh.rb b/lib/vagrant/util/ssh.rb index b80c515fc..8d120aab0 100644 --- a/lib/vagrant/util/ssh.rb +++ b/lib/vagrant/util/ssh.rb @@ -54,23 +54,6 @@ module Vagrant raise Errors::SSHKeyBadPermissions, key_path: key_path end - def self.determine_ssh_bin(original_ssh_path) - begin - original_path_env = ENV['PATH'] - ENV['PATH'] = ENV['VAGRANT_OLD_ENV_PATH'] - ssh_path = Which.which("ssh") - puts "the path: #{ssh_path}" - if !ssh_path.nil? - LOGGER.debug("Found default ssh binary. Using that instead...") - original_ssh_path = ssh_path - end - ensure - ENV['PATH'] = original_path_env - end - - return original_ssh_path - end - # Halts the running of this process and replaces it with a full-fledged # SSH shell into a remote machine. # @@ -83,7 +66,14 @@ module Vagrant def self.exec(ssh_info, opts={}) # Ensure the platform supports ssh. On Windows there are several programs which # include ssh, notably git, mingw and cygwin, but make sure ssh is in the path! - ssh_path = Which.which("ssh") + + # First try using the original path provided + ssh_path = Which.which("ssh", original_path: true) + # If we didn't find an ssh executable, see if we shipped one + if !ssh_path + ssh_path = Which.which("ssh") + end + if !ssh_path if Platform.windows? raise Errors::SSHUnavailableWindows, @@ -107,9 +97,6 @@ module Vagrant username: ssh_info[:username], key_path: ssh_info[:private_key_path].join(", ") end - - # use system ssh if available - ssh_path = determine_ssh_bin(ssh_path) end # If plain mode is enabled then we don't do any authentication (we don't diff --git a/test/unit/vagrant/util/ssh_test.rb b/test/unit/vagrant/util/ssh_test.rb index 351c393dd..7f9b40fdc 100644 --- a/test/unit/vagrant/util/ssh_test.rb +++ b/test/unit/vagrant/util/ssh_test.rb @@ -28,32 +28,6 @@ describe Vagrant::Util::SSH do end end - describe "#determine_ssh_bin" do - let (:original_ssh_path) { "/system/dir/bin/ssh" } - let (:new_ssh_path) { "/new/ssh/path/bin/ssh" } - let (:old_path) { "/old/path/bin:/usr/local/bin:/usr/bin" } - - it "returns passed in ssh path if not found on original path" do - path = ENV["PATH"] - allow(ENV).to receive(:[]).with("PATH").and_return(path) - allow(ENV).to receive(:[]).with("VAGRANT_OLD_ENV_PATH").and_return(old_path) - allow(Vagrant::Util::Which).to receive(:which).and_return(nil) - - expect(described_class.determine_ssh_bin(original_ssh_path)).to eq(original_ssh_path) - expect(ENV["PATH"]).to eq(path) - end - - it "returns system ssh path if found on original path" do - path = ENV["PATH"] - allow(ENV).to receive(:[]).with("PATH").and_return(path) - allow(ENV).to receive(:[]).with("VAGRANT_OLD_ENV_PATH").and_return(old_path) - allow(Vagrant::Util::Which).to receive(:which).and_return(new_ssh_path) - - expect(described_class.determine_ssh_bin(original_ssh_path)).to eq(new_ssh_path) - expect(ENV["PATH"]).to eq(path) - end - end - describe "#exec" do let(:ssh_info) {{ host: "localhost", @@ -66,6 +40,19 @@ describe Vagrant::Util::SSH do let(:ssh_path) { "/usr/bin/ssh" } + it "searches original PATH for exectuable" do + expect(Vagrant::Util::Which).to receive(:which).with("ssh", original_path: true).and_return("valid-return") + allow(Vagrant::Util::SafeExec).to receive(:exec).and_return(nil) + described_class.exec(ssh_info) + end + + it "searches current PATH if original PATH did not result in valid executable" do + expect(Vagrant::Util::Which).to receive(:which).with("ssh", original_path: true).and_return(nil) + expect(Vagrant::Util::Which).to receive(:which).with("ssh").and_return("valid-return") + allow(Vagrant::Util::SafeExec).to receive(:exec).and_return(nil) + described_class.exec(ssh_info) + end + it "raises an exception if there is no ssh" do allow(Vagrant::Util::Which).to receive(:which).and_return(nil)