diff options
author | The Bundler Bot <bot@bundler.io> | 2018-01-31 09:45:24 +0000 |
---|---|---|
committer | The Bundler Bot <bot@bundler.io> | 2018-01-31 09:45:24 +0000 |
commit | fe9d6989a11443dbfe1840cc07f89918e3d7b37d (patch) | |
tree | 6c601998906b0529a14e5909ab6ec96625219ee6 | |
parent | a0592e5d8d4b5f21df61b554b4622f0121730d7b (diff) | |
parent | d82a9ef14af86048ecf79826a55b633d5122d2c7 (diff) | |
download | bundler-fe9d6989a11443dbfe1840cc07f89918e3d7b37d.tar.gz |
Auto merge of #6129 - ajwann:check-permissions-in-doctor-command, r=colby-swandale
check permissions in doctor command
Thanks so much for the contribution!
To make reviewing this PR a bit easier, please fill out answers to the following questions.
### What was the end-user problem that led to this PR?
The problem was...
#5786
> We should have a check in bundle doctor for the file/folder permissions in the Bundler home directory.
>We should print a warning if there are any files/folders that is owned by another user but is readable/writable but prints an error when a file cannot be read or written to.
### What is your fix for the problem, implemented in this PR?
Created private method ```check_home_permissions``` that will print a warning if there are any files/folders that are owned by another user but are readable/writable, and print an error when the
bundler home dir contains a file cannot be read or written to
### Why did you choose this fix out of the possible options?
I chose this fix because it's what was requested in the open issue.
-rw-r--r-- | lib/bundler/cli/doctor.rb | 48 | ||||
-rw-r--r-- | spec/commands/doctor_spec.rb | 119 |
2 files changed, 131 insertions, 36 deletions
diff --git a/lib/bundler/cli/doctor.rb b/lib/bundler/cli/doctor.rb index 7f28a5eb13..3e0898ff8a 100644 --- a/lib/bundler/cli/doctor.rb +++ b/lib/bundler/cli/doctor.rb @@ -78,6 +78,8 @@ module Bundler end end + permissions_valid = check_home_permissions + if broken_links.any? message = "The following gems are missing OS dependencies:" broken_links.map do |spec, paths| @@ -86,9 +88,53 @@ module Bundler end end.flatten.sort.each {|m| message += m } raise ProductionError, message - else + elsif !permissions_valid Bundler.ui.info "No issues found with the installed bundle" end end + + private + + def check_home_permissions + require "find" + files_not_readable_or_writable = [] + files_not_rw_and_owned_by_different_user = [] + files_not_owned_by_current_user_but_still_rw = [] + Find.find(Bundler.home.to_s).each do |f| + if !File.writable?(f) || !File.readable?(f) + if File.stat(f).uid != Process.uid + files_not_rw_and_owned_by_different_user << f + else + files_not_readable_or_writable << f + end + elsif File.stat(f).uid != Process.uid + files_not_owned_by_current_user_but_still_rw << f + end + end + + ok = true + if files_not_owned_by_current_user_but_still_rw.any? + Bundler.ui.warn "Files exist in the Bundler home that are owned by another " \ + "user, but are still readable/writable. These files are:\n - #{files_not_owned_by_current_user_but_still_rw.join("\n - ")}" + + ok = false + end + + if files_not_rw_and_owned_by_different_user.any? + Bundler.ui.warn "Files exist in the Bundler home that are owned by another " \ + "user, and are not readable/writable. These files are:\n - #{files_not_rw_and_owned_by_different_user.join("\n - ")}" + + ok = false + end + + if files_not_readable_or_writable.any? + Bundler.ui.warn "Files exist in the Bundler home that are not " \ + "readable/writable by the current user. These files are:\n - #{files_not_readable_or_writable.join("\n - ")}" + + ok = false + end + + ok + end end end diff --git a/spec/commands/doctor_spec.rb b/spec/commands/doctor_spec.rb index 2572d4ff4d..5260e6cb36 100644 --- a/spec/commands/doctor_spec.rb +++ b/spec/commands/doctor_spec.rb @@ -1,11 +1,17 @@ # frozen_string_literal: true +require "find" require "stringio" require "bundler/cli" require "bundler/cli/doctor" RSpec.describe "bundle doctor" do before(:each) do + install_gemfile! <<-G + source "file://#{gem_repo1}" + gem "rack" + G + @stdout = StringIO.new [:error, :warn].each do |method| @@ -16,46 +22,89 @@ RSpec.describe "bundle doctor" do end end - it "exits with no message if the installed gem has no C extensions" do - install_gemfile! <<-G - source "file://#{gem_repo1}" - gem "rack" - G + context "when all files in home are readable/writable" do + before(:each) do + stat = double("stat") + unwritable_file = double("file") + allow(Find).to receive(:find).with(Bundler.home.to_s) { [unwritable_file] } + allow(File).to receive(:stat).with(unwritable_file) { stat } + allow(stat).to receive(:uid) { Process.uid } + allow(File).to receive(:writable?).with(unwritable_file) { true } + allow(File).to receive(:readable?).with(unwritable_file) { true } + end - expect { Bundler::CLI::Doctor.new({}).run }.not_to raise_error - expect(@stdout.string).to be_empty - end + it "exits with no message if the installed gem has no C extensions" do + expect { Bundler::CLI::Doctor.new({}).run }.not_to raise_error + expect(@stdout.string).to be_empty + end - it "exits with no message if the installed gem's C extension dylib breakage is fine" do - install_gemfile! <<-G - source "file://#{gem_repo1}" - gem "rack" - G + it "exits with no message if the installed gem's C extension dylib breakage is fine" do + doctor = Bundler::CLI::Doctor.new({}) + expect(doctor).to receive(:bundles_for_gem).exactly(2).times.and_return ["/path/to/rack/rack.bundle"] + expect(doctor).to receive(:dylibs).exactly(2).times.and_return ["/usr/lib/libSystem.dylib"] + allow(File).to receive(:exist?).and_call_original + allow(File).to receive(:exist?).with("/usr/lib/libSystem.dylib").and_return(true) + expect { doctor.run }.not_to(raise_error, @stdout.string) + expect(@stdout.string).to be_empty + end - doctor = Bundler::CLI::Doctor.new({}) - expect(doctor).to receive(:bundles_for_gem).exactly(2).times.and_return ["/path/to/rack/rack.bundle"] - expect(doctor).to receive(:dylibs).exactly(2).times.and_return ["/usr/lib/libSystem.dylib"] - allow(File).to receive(:exist?).and_call_original - allow(File).to receive(:exist?).with("/usr/lib/libSystem.dylib").and_return(true) - expect { doctor.run }.not_to(raise_error, @stdout.string) - expect(@stdout.string).to be_empty + it "exits with a message if one of the linked libraries is missing" do + doctor = Bundler::CLI::Doctor.new({}) + expect(doctor).to receive(:bundles_for_gem).exactly(2).times.and_return ["/path/to/rack/rack.bundle"] + expect(doctor).to receive(:dylibs).exactly(2).times.and_return ["/usr/local/opt/icu4c/lib/libicui18n.57.1.dylib"] + allow(File).to receive(:exist?).and_call_original + allow(File).to receive(:exist?).with("/usr/local/opt/icu4c/lib/libicui18n.57.1.dylib").and_return(false) + expect { doctor.run }.to raise_error(Bundler::ProductionError, strip_whitespace(<<-E).strip), @stdout.string + The following gems are missing OS dependencies: + * bundler: /usr/local/opt/icu4c/lib/libicui18n.57.1.dylib + * rack: /usr/local/opt/icu4c/lib/libicui18n.57.1.dylib + E + end end - it "exits with a message if one of the linked libraries is missing" do - install_gemfile! <<-G - source "file://#{gem_repo1}" - gem "rack" - G + context "when home contains files that are not readable/writable" do + before(:each) do + @stat = double("stat") + @unwritable_file = double("file") + allow(Find).to receive(:find).with(Bundler.home.to_s) { [@unwritable_file] } + allow(File).to receive(:stat).with(@unwritable_file) { @stat } + end - doctor = Bundler::CLI::Doctor.new({}) - expect(doctor).to receive(:bundles_for_gem).exactly(2).times.and_return ["/path/to/rack/rack.bundle"] - expect(doctor).to receive(:dylibs).exactly(2).times.and_return ["/usr/local/opt/icu4c/lib/libicui18n.57.1.dylib"] - allow(File).to receive(:exist?).and_call_original - allow(File).to receive(:exist?).with("/usr/local/opt/icu4c/lib/libicui18n.57.1.dylib").and_return(false) - expect { doctor.run }.to raise_error(Bundler::ProductionError, strip_whitespace(<<-E).strip), @stdout.string - The following gems are missing OS dependencies: - * bundler: /usr/local/opt/icu4c/lib/libicui18n.57.1.dylib - * rack: /usr/local/opt/icu4c/lib/libicui18n.57.1.dylib - E + it "exits with an error if home contains files that are not readable/writable" do + allow(@stat).to receive(:uid) { Process.uid } + allow(File).to receive(:writable?).with(@unwritable_file) { false } + allow(File).to receive(:readable?).with(@unwritable_file) { false } + expect { Bundler::CLI::Doctor.new({}).run }.not_to raise_error + expect(@stdout.string).to include( + "Files exist in the Bundler home that are not readable/writable by the current user. These files are:\n - #{@unwritable_file}" + ) + expect(@stdout.string).not_to include("No issues") + end + + context "when home contains files that are not owned by the current process" do + before(:each) do + allow(@stat).to receive(:uid) { 0o0000 } + end + + it "exits with an error if home contains files that are not readable/writable and are not owned by the current user" do + allow(File).to receive(:writable?).with(@unwritable_file) { false } + allow(File).to receive(:readable?).with(@unwritable_file) { false } + expect { Bundler::CLI::Doctor.new({}).run }.not_to raise_error + expect(@stdout.string).to include( + "Files exist in the Bundler home that are owned by another user, and are not readable/writable. These files are:\n - #{@unwritable_file}" + ) + expect(@stdout.string).not_to include("No issues") + end + + it "exits with a warning if home contains files that are read/write but not owned by current user" do + allow(File).to receive(:writable?).with(@unwritable_file) { true } + allow(File).to receive(:readable?).with(@unwritable_file) { true } + expect { Bundler::CLI::Doctor.new({}).run }.not_to raise_error + expect(@stdout.string).to include( + "Files exist in the Bundler home that are owned by another user, but are still readable/writable. These files are:\n - #{@unwritable_file}" + ) + expect(@stdout.string).not_to include("No issues") + end + end end end |