From 9a4345220fa48a2d7cc748276ab8f03247404d05 Mon Sep 17 00:00:00 2001 From: Homu Date: Wed, 5 Oct 2016 07:10:45 +0900 Subject: [PATCH] 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. --- .../lib/compact_index_client/updater.rb | 2 +- spec/install/gems/compact_index_spec.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) 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 6c5a4da57ac..40c61644e36 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 @@ def initialize(fetcher) 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 0edd1d20e7a..b34d9e872d5 100644 --- a/spec/install/gems/compact_index_spec.rb +++ b/spec/install/gems/compact_index_spec.rb @@ -695,4 +695,12 @@ def start 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