(PUP-9997) Don't Dir.chdir when installing modules

When the Dir.chdir block ends we may not have permission to switch our cwd back
to where we started. So keep the cwd as is and pass the dest dir to the
tar/find/chown commands. Since we're passing arbitrary destination directories,
escape it.

Author: joshcooper

lib/puppet/module_tool/tar/gnu.rb

@@ -4,18 +4,20 @@
 
 class Puppet::ModuleTool::Tar::Gnu
   def unpack(sourcefile, destdir, owner)
-    sourcefile = File.expand_path(sourcefile)
+    safe_sourcefile = Shellwords.shellescape(File.expand_path(sourcefile))
     destdir = File.expand_path(destdir)
+    safe_destdir = Shellwords.shellescape(destdir)
 
-    Dir.chdir(destdir) do
-      Puppet::Util::Execution.execute("gzip -dc #{Shellwords.shellescape(sourcefile)} | tar xof -")
-      Puppet::Util::Execution.execute("find . -type d -exec chmod 755 {} +")
-      Puppet::Util::Execution.execute("find . -type f -exec chmod u+rw,g+r,a-st {} +")
-      Puppet::Util::Execution.execute("chown -R #{owner} .")
-    end
+    Puppet::Util::Execution.execute("gzip -dc #{safe_sourcefile} | tar --extract --no-same-owner --directory #{safe_destdir} --file -")
+    Puppet::Util::Execution.execute(['find', destdir, '-type', 'd', '-exec', 'chmod', '755', '{}', '+'])
+    Puppet::Util::Execution.execute(['find', destdir, '-type', 'f', '-exec', 'chmod', 'u+rw,g+r,a-st', '{}', '+'])
+    Puppet::Util::Execution.execute(['chown', '-R', owner, destdir])
   end
 
   def pack(sourcedir, destfile)
-    Puppet::Util::Execution.execute("tar cf - #{sourcedir} | gzip -c > #{File.basename(destfile)}")
+    safe_sourcedir = Shellwords.shellescape(sourcedir)
+    safe_destfile = Shellwords.shellescape(File.basename(destfile))
+
+    Puppet::Util::Execution.execute("tar cf - #{safe_sourcedir} | gzip -c > #{safe_destfile}")
   end
 end

spec/unit/module_tool/tar/gnu_spec.rb

@@ -1,23 +1,27 @@
 require 'spec_helper'
 require 'puppet/module_tool'
 
-describe Puppet::ModuleTool::Tar::Gnu do
+describe Puppet::ModuleTool::Tar::Gnu, unless: Puppet::Util::Platform.windows? do
+  let(:sourcedir)  { '/space path/the/src/dir' }
   let(:sourcefile) { '/space path/the/module.tar.gz' }
   let(:destdir)    { '/space path/the/dest/dir' }
-  let(:sourcedir)  { '/space path/the/src/dir' }
-  let(:destfile)   { '/space path/the/dest/file.tar.gz' }
+  let(:destfile)   { '/space path/the/dest/fi le.tar.gz' }
+
+  let(:safe_sourcedir)  { '/space\ path/the/src/dir' }
+  let(:safe_sourcefile) { '/space\ path/the/module.tar.gz' }
+  let(:safe_destdir)    { '/space\ path/the/dest/dir' }
+  let(:safe_destfile)   { 'fi\ le.tar.gz' }
 
   it "unpacks a tar file" do
-    expect(Dir).to receive(:chdir).with(File.expand_path(destdir)).and_yield
-    expect(Puppet::Util::Execution).to receive(:execute).with("gzip -dc #{Shellwords.shellescape(File.expand_path(sourcefile))} | tar xof -")
-    expect(Puppet::Util::Execution).to receive(:execute).with("find . -type d -exec chmod 755 {} +")
-    expect(Puppet::Util::Execution).to receive(:execute).with("find . -type f -exec chmod u+rw,g+r,a-st {} +")
-    expect(Puppet::Util::Execution).to receive(:execute).with("chown -R <owner:group> .")
+    expect(Puppet::Util::Execution).to receive(:execute).with("gzip -dc #{safe_sourcefile} | tar --extract --no-same-owner --directory #{safe_destdir} --file -")
+    expect(Puppet::Util::Execution).to receive(:execute).with(['find', destdir, '-type', 'd', '-exec', 'chmod', '755', '{}', '+'])
+    expect(Puppet::Util::Execution).to receive(:execute).with(['find', destdir, '-type', 'f', '-exec', 'chmod', 'u+rw,g+r,a-st', '{}', '+'])
+    expect(Puppet::Util::Execution).to receive(:execute).with(['chown', '-R', '<owner:group>', destdir])
     subject.unpack(sourcefile, destdir, '<owner:group>')
   end
 
   it "packs a tar file" do
-    expect(Puppet::Util::Execution).to receive(:execute).with("tar cf - #{sourcedir} | gzip -c > #{File.basename(destfile)}")
+    expect(Puppet::Util::Execution).to receive(:execute).with("tar cf - #{safe_sourcedir} | gzip -c > #{safe_destfile}")
     subject.pack(sourcedir, destfile)
   end
 end