[rubygems/rubygems] Better error when having an insecure install folder

https://github.com/rubygems/rubygems/commit/e41156e272

6 files changed, 61 insertions, 31 deletions

diff --git a/lib/bundler.rb b/lib/bundler.rb
index cc6c2c075c..bc28b24e2e 100644
--- a/lib/bundler.rb
+++ b/lib/bundler.rb
@@ -331,14 +331,6 @@ module Bundler
 
     def rm_rf(path)
       FileUtils.remove_entry_secure(path) if path && File.exist?(path)
-    rescue ArgumentError
-      message = <<EOF
-It is a security vulnerability to allow your home directory to be world-writable, and bundler cannot continue.
-You should probably consider fixing this issue by running `chmod o-w ~` on *nix.
-Please refer to https://ruby-doc.org/stdlib-3.1.2/libdoc/fileutils/rdoc/FileUtils.html#method-c-remove_entry_secure for details.
-EOF
-      File.world_writable?(path) ? Bundler.ui.warn(message) : raise
-      raise PathError, "Please fix the world-writable issue with your #{path} directory"
     end
 
     def settings
diff --git a/lib/bundler/errors.rb b/lib/bundler/errors.rb
index c6b3cec4dc..eec72b1692 100644
--- a/lib/bundler/errors.rb
+++ b/lib/bundler/errors.rb
@@ -215,4 +215,19 @@ module Bundler
 
     status_code(36)
   end
+
+  class InsecureInstallPathError < BundlerError
+    def initialize(path)
+      @path = path
+    end
+
+    def message
+      "The installation path is insecure. Bundler cannot continue.\n" \
+      "#{@path} is world-writable (without sticky bit).\n" \
+      "Bundler cannot safely replace gems in world-writeable directories due to potential vulnerabilities.\n" \
+      "Please change the permissions of this directory or choose a different install path."
+    end
+
+    status_code(38)
+  end
 end
diff --git a/lib/bundler/installer/gem_installer.rb b/lib/bundler/installer/gem_installer.rb
index 1c8a24975d..b6065c24d0 100644
--- a/lib/bundler/installer/gem_installer.rb
+++ b/lib/bundler/installer/gem_installer.rb
@@ -17,7 +17,7 @@ module Bundler
       Bundler.ui.debug "#{worker}:  #{spec.name} (#{spec.version}) from #{spec.loaded_from}"
       generate_executable_stubs
       [true, post_install_message]
-    rescue Bundler::InstallHookError, Bundler::SecurityError, Bundler::APIResponseMismatchError
+    rescue Bundler::InstallHookError, Bundler::SecurityError, Bundler::APIResponseMismatchError, Bundler::InsecureInstallPathError
       raise
     rescue Errno::ENOSPC
       [false, out_of_space_message]
diff --git a/lib/bundler/rubygems_gem_installer.rb b/lib/bundler/rubygems_gem_installer.rb
index 8463b24bfa..d04ef62e8e 100644
--- a/lib/bundler/rubygems_gem_installer.rb
+++ b/lib/bundler/rubygems_gem_installer.rb
@@ -124,11 +124,22 @@ module Bundler
     end
 
     def strict_rm_rf(dir)
-      Bundler.rm_rf dir
-    rescue StandardError => e
-      raise unless File.exist?(dir)
+      return unless File.exist?(dir)
 
-      raise DirectoryRemovalError.new(e, "Could not delete previous installation of `#{dir}`")
+      parent = File.dirname(dir)
+      parent_st = File.stat(parent)
+
+      if parent_st.world_writable? && !parent_st.sticky?
+        raise InsecureInstallPathError.new(parent)
+      end
+
+      begin
+        FileUtils.remove_entry_secure(dir)
+      rescue StandardError => e
+        raise unless File.exist?(dir)
+
+        raise DirectoryRemovalError.new(e, "Could not delete previous installation of `#{dir}`")
+      end
     end
   end
 end
diff --git a/spec/bundler/bundler/bundler_spec.rb b/spec/bundler/bundler/bundler_spec.rb
index 132ce5c8ad..f870007f5a 100644
--- a/spec/bundler/bundler/bundler_spec.rb
+++ b/spec/bundler/bundler/bundler_spec.rb
@@ -224,24 +224,6 @@ RSpec.describe Bundler do
     end
   end
 
-  describe "#rm_rf" do
-    context "the directory is world writable" do
-      let(:bundler_ui) { Bundler.ui }
-      it "should raise a friendly error" do
-        allow(File).to receive(:exist?).and_return(true)
-        allow(::Bundler::FileUtils).to receive(:remove_entry_secure).and_raise(ArgumentError)
-        allow(File).to receive(:world_writable?).and_return(true)
-        message = <<EOF
-It is a security vulnerability to allow your home directory to be world-writable, and bundler cannot continue.
-You should probably consider fixing this issue by running `chmod o-w ~` on *nix.
-Please refer to https://ruby-doc.org/stdlib-3.1.2/libdoc/fileutils/rdoc/FileUtils.html#method-c-remove_entry_secure for details.
-EOF
-        expect(bundler_ui).to receive(:warn).with(message)
-        expect { Bundler.send(:rm_rf, bundled_app) }.to raise_error(Bundler::PathError)
-      end
-    end
-  end
-
   describe "#mkdir_p" do
     it "creates a folder at the given path" do
       install_gemfile <<-G
diff --git a/spec/bundler/commands/install_spec.rb b/spec/bundler/commands/install_spec.rb
index e333e04108..c2f55befc3 100644
--- a/spec/bundler/commands/install_spec.rb
+++ b/spec/bundler/commands/install_spec.rb
@@ -827,6 +827,36 @@ RSpec.describe "bundle install with gem sources" do
     end
   end
 
+  describe "when gems path is world writable (no sticky bit set)", :permissions do
+    let(:gems_path) { bundled_app("vendor/#{Bundler.ruby_scope}/gems") }
+
+    before do
+      build_repo4 do
+        build_gem "foo", "1.0.0" do |s|
+          s.write "CHANGELOG.md", "foo"
+        end
+      end
+
+      gemfile <<-G
+        source "#{file_uri_for(gem_repo4)}"
+        gem 'foo'
+      G
+    end
+
+    it "should display a proper message to explain the problem" do
+      bundle "config set --local path vendor"
+      bundle :install
+      expect(out).to include("Bundle complete!")
+      expect(err).to be_empty
+
+      FileUtils.chmod(0o777, gems_path)
+
+      bundle "install --redownload", :raise_on_error => false
+
+      expect(err).to include("The installation path is insecure. Bundler cannot continue.")
+    end
+  end
+
   describe "when bundle cache path does not have write access", :permissions do
     let(:cache_path) { bundled_app("vendor/#{Bundler.ruby_scope}/cache") }