Auto merge of #5043 - bundler:aa-use-tmp, r=segiddins

use /tmp for mktmpdir

As we noticed in #4519, we need to use a temporary directory to hold
compact index downloads so that multiple processes don't write to the
same files at the same time and break everything.

The fix for that was #4561, which added temporary directories to hold
all files as they download, and then uses the (atomic) `FileUtils.cp` to
move the completed downloads into place, so there is never a point where
multiple processes are trying to write into the file at once.

Unfortunately, using `Dir.mktmpdir` requires that the parent directory
be _either_ world writable or sticky, but not both. Based on #4599, it
looks like it's common for home directories to be both world writable
and sticky. While that's a security problem by itself, it's not a big
concern for Bundler and the compact index. So we want to let users
continue to use Bundler, even with the compact index, without having to
change the permissions on their home directories.

This commit changes the `mktmpdir` call to create the temporary
directory inside the default OS tempdir, which is typically `/tmp` or
`/var/tmp` depending on distro. Since that directory is designed to hold
other temporary directories, that change should (theoretically) reduce
or eliminate the problem reported in #4599.

2 files changed, 9 insertions, 1 deletions

diff --git a/lib/bundler/vendor/compact_index_client/lib/compact_index_client/updater.rb b/lib/bundler/vendor/compact_index_client/lib/compact_index_client/updater.rb
index 6c5a4da57a..40c61644e3 100644
--- a/lib/bundler/vendor/compact_index_client/lib/compact_index_client/updater.rb
+++ b/lib/bundler/vendor/compact_index_client/lib/compact_index_client/updater.rb
@@ -26,7 +26,7 @@ class Bundler::CompactIndexClient
     def update(local_path, remote_path, retrying = nil)
       headers = {}
 
-      Dir.mktmpdir(local_path.basename.to_s, local_path.dirname) do |local_temp_dir|
+      Dir.mktmpdir("bundler-compact-index-") do |local_temp_dir|
         local_temp_path = Pathname.new(local_temp_dir).join(local_path.basename)
 
         # first try to fetch any new bytes on the existing file
diff --git a/spec/install/gems/compact_index_spec.rb b/spec/install/gems/compact_index_spec.rb
index 0edd1d20e7..b34d9e872d 100644
--- a/spec/install/gems/compact_index_spec.rb
+++ b/spec/install/gems/compact_index_spec.rb
@@ -695,4 +695,12 @@ The checksum of /versions does not match the checksum provided by the server! So
     expect(File.read(versions)).to start_with("created_at")
     expect(the_bundle).to include_gems "rack 1.0.0"
   end
+
+  it "works when cache dir is world-writable" do
+    install_gemfile! <<-G, :artifice => "compact_index"
+      File.umask(0000)
+      source "#{source_uri}"
+      gem "rack"
+    G
+  end
 end