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.

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