From 2f9092a49bdd26e69a5447c57f7d743a68154ae4 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Tue, 13 Aug 2024 16:52:06 -0700 Subject: [PATCH] Add support for minitar 1.x Update puppet to support minitar 1.0 which contains breaking changes. Note this important warning from https://github.com/halostatue/minitar Minitar does not perform validation of path names provided to the convenience classes Minitar::Output and Minitar::Input, which use Kernel.open for their underlying implementations when not given an IO-like object. As a result we always pass a reader/writer to the unpack/pack methods respectively. --- Gemfile | 2 +- lib/puppet/feature/base.rb | 2 +- lib/puppet/module_tool/tar/mini.rb | 26 +++++++++++++------------- puppet.gemspec | 2 +- spec/unit/module_tool/tar/mini_spec.rb | 6 +++--- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Gemfile b/Gemfile index 5d3caa47a35..2c1375348df 100644 --- a/Gemfile +++ b/Gemfile @@ -27,7 +27,7 @@ group(:features) do gem 'hocon', '~> 1.0', require: false # requires native libshadow headers/libs #gem 'ruby-shadow', '~> 2.5', require: false, platforms: [:ruby] - gem 'minitar', '~> 0.9', require: false + gem 'minitar', '~> 1.0', require: false gem 'msgpack', '~> 1.2', require: false gem 'rdoc', ['~> 6.0', '< 6.4.0'], require: false, platforms: [:ruby] # requires native augeas headers/libs diff --git a/lib/puppet/feature/base.rb b/lib/puppet/feature/base.rb index b7efedb291f..c4d538364c3 100644 --- a/lib/puppet/feature/base.rb +++ b/lib/puppet/feature/base.rb @@ -47,7 +47,7 @@ # We have Hiera Puppet.features.add(:hiera, :libs => ["hiera"]) -Puppet.features.add(:minitar, :libs => ["archive/tar/minitar"]) +Puppet.features.add(:minitar, :libs => ["minitar"]) # We can manage symlinks Puppet.features.add(:manages_symlinks) do diff --git a/lib/puppet/module_tool/tar/mini.rb b/lib/puppet/module_tool/tar/mini.rb index 8238bca7a14..00a992e3229 100644 --- a/lib/puppet/module_tool/tar/mini.rb +++ b/lib/puppet/module_tool/tar/mini.rb @@ -3,17 +3,12 @@ class Puppet::ModuleTool::Tar::Mini def unpack(sourcefile, destdir, _) Zlib::GzipReader.open(sourcefile) do |reader| - # puppet doesn't have a hard dependency on minitar, so we - # can't be certain which version is installed. If it's 0.9 - # or above then we can prevent minitar from fsync'ing each - # extracted file and directory, otherwise fallback to the - # old behavior - args = [reader, destdir, find_valid_files(reader)] - spec = Gem::Specification.find_by_name('minitar') - if spec && spec.version >= Gem::Version.new('0.9') - args << { :fsync => false } - end - Archive::Tar::Minitar.unpack(*args) do |action, name, stats| + files = find_valid_files(reader) + + # Never pass a source file as a string to unpack, otherwise minitar will + # call Kernel.open on it, which could invoke shell commands. Always pass a + # reader that responds to `:read` + Minitar.unpack(reader, destdir, files, fsync: false) do |action, name, stats| case action when :dir validate_entry(destdir, name) @@ -33,7 +28,10 @@ def unpack(sourcefile, destdir, _) def pack(sourcedir, destfile) Zlib::GzipWriter.open(destfile) do |writer| - Archive::Tar::Minitar.pack(sourcedir, writer) do |step, name, stats| + # Never pass the destination file as a string to pack, otherwise minitar + # will call Kernel.open on it, which could invoke shell commands. Always + # pass a writer that responds to `:write` + Minitar.pack(sourcedir, writer) do |step, name, stats| # TODO smcclellan 2017-10-31 Set permissions here when this yield block # executes before the header is written. As it stands, the `stats` # argument isn't mutable in a way that will effect the desired mode for @@ -93,7 +91,9 @@ def set_default_user_and_group!(stats) # tar format info: https://pic.dhe.ibm.com/infocenter/zos/v1r13/index.jsp?topic=%2Fcom.ibm.zos.r13.bpxa500%2Ftaf.htm # pax format info: https://pic.dhe.ibm.com/infocenter/zos/v1r13/index.jsp?topic=%2Fcom.ibm.zos.r13.bpxa500%2Fpxarchfm.htm def find_valid_files(tarfile) - Archive::Tar::Minitar.open(tarfile).collect do |entry| + raise ArgumentError, "Cannot list files from '#{tarfile}', because the object does not implement a 'read' method" unless tarfile.respond_to?(:read) + + Minitar.open(tarfile).collect do |entry| flag = entry.typeflag if flag.nil? || flag =~ /[[:digit:]]/ && (0..7).cover?(flag.to_i) entry.full_name diff --git a/puppet.gemspec b/puppet.gemspec index d784a9dbba4..9b2ee8a3fe3 100644 --- a/puppet.gemspec +++ b/puppet.gemspec @@ -39,6 +39,6 @@ Gem::Specification.new do |spec| if platform == 'x64-mingw32' || platform == 'x86-mingw32' # ffi 1.16.0 - 1.16.2 are broken on Windows spec.add_runtime_dependency('ffi', '>= 1.15.5', '< 1.17.0', '!= 1.16.0', '!= 1.16.1', '!= 1.16.2') - spec.add_runtime_dependency('minitar', '~> 0.9') + spec.add_runtime_dependency('minitar', '~> 1.0') end end diff --git a/spec/unit/module_tool/tar/mini_spec.rb b/spec/unit/module_tool/tar/mini_spec.rb index 807f3d495ce..2a2a00f910e 100644 --- a/spec/unit/module_tool/tar/mini_spec.rb +++ b/spec/unit/module_tool/tar/mini_spec.rb @@ -61,7 +61,7 @@ def initialize(mode = 0100) expect(Zlib::GzipWriter).to receive(:open).with(destfile).and_yield(writer) stats = {:mode => 0222} - expect(Archive::Tar::Minitar).to receive(:pack).with(sourcedir, writer).and_yield(:file_start, 'abc', stats) + expect(Minitar).to receive(:pack).with(sourcedir, writer).and_yield(:file_start, 'abc', stats) minitar.pack(sourcedir, destfile) end @@ -70,7 +70,7 @@ def initialize(mode = 0100) writer = double('GzipWriter') expect(Zlib::GzipWriter).to receive(:open).with(destfile).and_yield(writer) - expect(Archive::Tar::Minitar).to receive(:pack).with(sourcedir, writer). + expect(Minitar).to receive(:pack).with(sourcedir, writer). and_yield(:file_start, 'abc', {:entry => MockFileStatEntry.new(nil)}) minitar.pack(sourcedir, destfile) @@ -82,7 +82,7 @@ def unpacks_the_entry(type, name, mode = 0100) expect(Zlib::GzipReader).to receive(:open).with(sourcefile).and_yield(reader) expect(minitar).to receive(:find_valid_files).with(reader).and_return([name]) entry = MockFileStatEntry.new(mode) - expect(Archive::Tar::Minitar).to receive(:unpack).with(reader, destdir, [name], {:fsync => false}). + expect(Minitar).to receive(:unpack).with(reader, destdir, [name], {:fsync => false}). and_yield(type, name, {:entry => entry}) entry end