Fix sudo usage in exports write within linux host cap

Only move new exports file to destination without sudo when the
file has write access and the directory has write access. Always
use sudo when changing file ownership.
This commit is contained in:
Chris Roberts 2018-08-02 15:13:27 -07:00
parent 88d2e25634
commit 051c7a7823
2 changed files with 69 additions and 12 deletions

View File

@ -173,16 +173,14 @@ module VagrantPlugins
def self.nfs_write_exports(new_exports_content)
if(nfs_exports_content != new_exports_content.strip)
begin
exports_path = Pathname.new(NFS_EXPORTS_PATH)
# Write contents out to temporary file
new_exports_file = Tempfile.create('vagrant')
new_exports_file.puts(new_exports_content)
new_exports_file.close
new_exports_path = new_exports_file.path
# Only use "sudo" if we can't write to /etc/exports directly
sudo_command = ""
sudo_command = "sudo " if !File.writable?(NFS_EXPORTS_PATH)
# Ensure new file mode and uid/gid match existing file to replace
existing_stat = File.stat(NFS_EXPORTS_PATH)
new_stat = File.stat(new_exports_path)
@ -190,7 +188,7 @@ module VagrantPlugins
File.chmod(existing_stat.mode, new_exports_path)
end
if existing_stat.uid != new_stat.uid || existing_stat.gid != new_stat.gid
chown_cmd = "#{sudo_command}chown #{existing_stat.uid}:#{existing_stat.gid} #{new_exports_path}"
chown_cmd = "sudo chown #{existing_stat.uid}:#{existing_stat.gid} #{new_exports_path}"
result = Vagrant::Util::Subprocess.execute(*Shellwords.split(chown_cmd))
if result.exit_code != 0
raise Vagrant::Errors::NFSExportsFailed,
@ -200,6 +198,7 @@ module VagrantPlugins
end
end
# Always force move the file to prevent overwrite prompting
sudo_command = "sudo " if !exports_path.writable? || !exports_path.dirname.writable?
mv_cmd = "#{sudo_command}mv -f #{new_exports_path} #{NFS_EXPORTS_PATH}"
result = Vagrant::Util::Subprocess.execute(*Shellwords.split(mv_cmd))
if result.exit_code != 0

View File

@ -269,14 +269,72 @@ EOH
expect{ described_class.nfs_write_exports("new content") }.to raise_error(Vagrant::Errors::NFSExportsFailed)
end
it "should retain existing file owner and group IDs" do
pending("investigate using a simulated FS to test")
test_with_simulated_fs
end
context "exports file modification" do
let(:tmp_stat) { double("tmp_stat", uid: 100, gid: 100, mode: tmp_mode) }
let(:tmp_mode) { 0 }
let(:exports_stat) { double("stat", uid: exports_uid, gid: exports_gid, mode: exports_mode) }
let(:exports_uid) { -1 }
let(:exports_gid) { -1 }
let(:exports_mode) { 0 }
let(:new_exports_file) { double("new_exports_file", path: "/dev/null/exports") }
it "should raise custom exception when chown fails" do
pending("investigate using a simulated FS to test")
test_with_simulated_fs
before do
allow(File).to receive(:stat).with(new_exports_file.path).and_return(tmp_stat)
allow(File).to receive(:stat).with(tmp_exports_path.to_s).and_return(exports_stat)
allow(new_exports_file).to receive(:puts)
allow(new_exports_file).to receive(:close)
allow(Vagrant::Util::Subprocess).to receive(:execute).and_return(Vagrant::Util::Subprocess::Result.new(0, "", ""))
allow(Tempfile).to receive(:create).with("vagrant").and_return(new_exports_file)
end
it "should retain existing file owner and group IDs" do
expect(Vagrant::Util::Subprocess).to receive(:execute) { |*args|
expect(args).to include("sudo")
expect(args).to include("chown")
}.and_return(Vagrant::Util::Subprocess::Result.new(0, "", ""))
described_class.nfs_write_exports("new content")
end
it "should raise custom exception when chown fails" do
expect(Vagrant::Util::Subprocess).to receive(:execute) { |*args|
expect(args).to include("sudo")
expect(args).to include("chown")
}.and_return(Vagrant::Util::Subprocess::Result.new(1, "", ""))
expect { described_class.nfs_write_exports("new content") }.to raise_error(Vagrant::Errors::NFSExportsFailed)
end
context "when user has write access to exports file" do
let(:file_writable?) { true }
let(:dir_writable?) { false }
let(:exports_pathname) { double("exports_pathname", writable?: file_writable?, dirname: exports_dir_pathname) }
let(:exports_dir_pathname) { double("exports_dir_pathname", writable?: dir_writable?) }
before do
allow(File).to receive(:stat).and_return(exports_stat)
allow(File).to receive(:exist?).and_return(false)
allow(Pathname).to receive(:new).with(tmp_exports_path.to_s).and_return(exports_pathname)
end
it "should use sudo when moving new file" do
expect(Vagrant::Util::Subprocess).to receive(:execute) { |*args|
expect(args).to include("sudo")
expect(args).to include("mv")
}.and_return(Vagrant::Util::Subprocess::Result.new(0, "", ""))
described_class.nfs_write_exports("new content")
end
context "and write access to exports parent directory" do
let(:dir_writable?) { true }
it "should not use sudo when moving new file" do
expect(Vagrant::Util::Subprocess).to receive(:execute) { |*args|
expect(args).not_to include("sudo")
expect(args).to include("mv")
}.and_return(Vagrant::Util::Subprocess::Result.new(0, "", ""))
described_class.nfs_write_exports("new content")
end
end
end
end
end
end