Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Commit

Permalink
Auto merge of #6129 - ajwann:check-permissions-in-doctor-command, r=c…
Browse files Browse the repository at this point in the history 
…olby-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.
  • Loading branch information
@bundlerbot
bundlerbot committed Jan 31, 2018
2 parents a0592e5 + d82a9ef commit fe9d698
Show file tree
Hide file tree
Showing 2 changed files with 131 additions and 36 deletions.
48 changes: 47 additions & 1 deletion lib/bundler/cli/doctor.rb
View file
Expand Up @@ -78,6 +78,8 @@ def run
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|
Expand All @@ -86,9 +88,53 @@ def run
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
119 changes: 84 additions & 35 deletions spec/commands/doctor_spec.rb
View file
@@ -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|
Expand All @@ -16,46 +22,89 @@
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

0 comments on commit fe9d698

Please sign in to comment.