diff --git a/Library/Homebrew/test/.rubocop.yml b/Library/Homebrew/test/.rubocop.yml index 93f0b48920..85cbaa2444 100644 --- a/Library/Homebrew/test/.rubocop.yml +++ b/Library/Homebrew/test/.rubocop.yml @@ -4,3 +4,13 @@ inherit_from: RSpec: Include: - ./* + +RSpec/ContextWording: + Prefixes: + - when + - with + - without + - if + - unless + - for + - which diff --git a/Library/Homebrew/test/unpack_strategy_spec.rb b/Library/Homebrew/test/unpack_strategy_spec.rb index 279bf48f37..6619360de3 100644 --- a/Library/Homebrew/test/unpack_strategy_spec.rb +++ b/Library/Homebrew/test/unpack_strategy_spec.rb @@ -28,11 +28,22 @@ describe UnpackStrategy do context "when extracting a directory with nested directories" do let(:directories) { "A/B/C" } + let(:executable) { "#{directories}/executable" } + let(:writable) { true } let(:path) { (mktmpdir/"file.tar").tap do |path| - mktmpdir do |dir| + Dir.mktmpdir do |dir| + dir = Pathname(dir) (dir/directories).mkpath - system "tar", "--create", "--file", path, "--directory", dir, "A/" + FileUtils.touch dir/executable + FileUtils.chmod 0555, dir/executable + + FileUtils.chmod "-w", dir/directories unless writable + begin + system "tar", "--create", "--file", path, "--directory", dir, "A/" + ensure + FileUtils.chmod "+w", dir/directories unless writable + end end end } @@ -41,6 +52,23 @@ describe UnpackStrategy do strategy.extract_nestedly(to: unpack_dir) expect(Pathname.glob(unpack_dir/"**/*")).to include unpack_dir/directories end + + context "which are not writable" do + let(:writable) { false } + + it "makes them writable but not world-writable" do + strategy.extract_nestedly(to: unpack_dir) + + expect(unpack_dir/directories).to be_writable + expect(unpack_dir/directories).not_to be_world_writable + end + + it "does not make other files writable" do + strategy.extract_nestedly(to: unpack_dir) + + expect(unpack_dir/executable).not_to be_writable + end + end end context "when extracting a nested archive" do diff --git a/Library/Homebrew/unpack_strategy.rb b/Library/Homebrew/unpack_strategy.rb index 25f02de187..b7572d18e7 100644 --- a/Library/Homebrew/unpack_strategy.rb +++ b/Library/Homebrew/unpack_strategy.rb @@ -3,6 +3,27 @@ require "system_command" +# Helper module for iterating over directory trees. +# +# @api private +module PathnameEachDirectory + refine Pathname do + extend T::Sig + + sig { + type_parameters(:T) + .params( + _block: T.proc.params(path: Pathname).returns(T.type_parameter(:T)), + ).returns(T.type_parameter(:T)) + } + def each_directory(&_block) + find do |path| + yield path if path.directory? + end + end + end +end + # Module containing all available strategies for unpacking archives. # # @api private @@ -12,6 +33,8 @@ module UnpackStrategy include SystemCommand::Mixin + using PathnameEachDirectory + # Helper module for identifying the file type. module Magic # Length of the longest regex (currently Tar). @@ -164,17 +187,21 @@ module UnpackStrategy children = tmp_unpack_dir.children if children.count == 1 && !children.first.directory? - FileUtils.chmod "+rw", children.first, verbose: verbose - s = UnpackStrategy.detect(children.first, prioritize_extension: prioritize_extension) s.extract_nestedly(to: to, verbose: verbose, prioritize_extension: prioritize_extension) + next end - Directory.new(tmp_unpack_dir).extract(to: to, verbose: verbose) + # Ensure all extracted directories are writable. + tmp_unpack_dir.each_directory do |path| + next if path.writable? - FileUtils.chmod_R "+w", tmp_unpack_dir, force: true, verbose: verbose + FileUtils.chmod "u+w", path, verbose: verbose + end + + Directory.new(tmp_unpack_dir).extract(to: to, verbose: verbose) end end