Auto merge of #6318 - jhawthorn:fix_incorrect_test_in_requires_sudo, …

…r=segiddins

Check correct paths are writable in requires_sudo?

### What was the end-user problem that led to this PR?

Bundler was attempting to use `sudo`, despite having permissions to create `bundle_path` (which didn't yet exist).

For some reason this became an issue in recent versions of bundler, where it wasn't one before. I'm not sure what change caused this problem to be exposed since `requires_sudo?` has not changed in a while.

### What was your diagnosis of the problem?

`requires_sudo?` checks that bundle_path, and directories that bundler may need to write within it are writable. If bundle_path itself does not exist, we instead find the nearest existent parent directory, and check that that exists.

Unfortunately, when these two rules were applied together, we got the wrong result. If `bundle_path` did not exist, bundler would check that the nearest parent directory **and everything within that parent directory** were writable. This could lead to false positives for `requires_sudo?` when `bundle_path` did not yet exist.

### What is your fix for the problem, implemented in this PR?

This commit fixes the issue by always checking the writability of `$bundle_path/*` instead of `$parent_path/*`.

### Why did you choose this fix out of the possible options?

If we are able to create `bundle_path`, we know that we can write to anything within it. Changing the test to use `Dir["$bundle_path/*"]` is a succinct way to implement this since it will return `[]` if `bundle_path` does not yet exist.

lib/bundler.rb

@@ -365,7 +365,7 @@ def requires_sudo?
         bin_dir = bin_dir.parent until bin_dir.exist?
 
         # if any directory is not writable, we need sudo
-        files = [path, bin_dir] | Dir[path.join("build_info/*").to_s] | Dir[path.join("*").to_s]
+        files = [path, bin_dir] | Dir[bundle_path.join("build_info/*").to_s] | Dir[bundle_path.join("*").to_s]
         sudo_needed = files.any? {|f| !File.writable?(f) }
       end
 

spec/bundler/bundler_spec.rb

@@ -2,6 +2,7 @@
 # frozen_string_literal: true
 
 require "bundler"
+require "tmpdir"
 
 RSpec.describe Bundler do
   describe "#load_gemspec_uncached" do
@@ -228,6 +229,72 @@
     end
   end
 
+  describe "#requires_sudo?" do
+    let!(:tmpdir) { Dir.mktmpdir }
+    let(:bundle_path) { Pathname("#{tmpdir}/bundle") }
+
+    def clear_cached_requires_sudo
+      # Private in ruby 1.8.7
+      return unless Bundler.instance_variable_defined?(:@requires_sudo_ran)
+      Bundler.send(:remove_instance_variable, :@requires_sudo_ran)
+      Bundler.send(:remove_instance_variable, :@requires_sudo)
+    end
+
+    before do
+      clear_cached_requires_sudo
+      allow(Bundler).to receive(:which).with("sudo").and_return("/usr/bin/sudo")
+      allow(Bundler).to receive(:bundle_path).and_return(bundle_path)
+    end
+
+    after do
+      FileUtils.rm_rf(tmpdir)
+      clear_cached_requires_sudo
+    end
+
+    subject { Bundler.requires_sudo? }
+
+    context "bundle_path doesn't exist" do
+      it { should be false }
+
+      context "and parent dir can't be written" do
+        before do
+          FileUtils.chmod(0o500, tmpdir)
+        end
+
+        it { should be true }
+      end
+
+      context "with unwritable files in a parent dir" do
+        # Regression test for https://github.com/bundler/bundler/pull/6316
+        # It doesn't matter if there are other unwritable files so long as
+        # bundle_path can be created
+        before do
+          file = File.join(tmpdir, "unrelated_file")
+          FileUtils.touch(file)
+          FileUtils.chmod(0o400, file)
+        end
+
+        it { should be false }
+      end
+    end
+
+    context "bundle_path exists" do
+      before do
+        FileUtils.mkdir_p(bundle_path)
+      end
+
+      it { should be false }
+
+      context "and is unwritable" do
+        before do
+          FileUtils.chmod(0o500, bundle_path)
+        end
+
+        it { should be true }
+      end
+    end
+  end
+
   context "user cache dir" do
     let(:home_path)                  { Pathname.new(ENV["HOME"]) }