Skip to content

Commit c3de25d

Browse files
committed
(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.
1 parent ad07a6f commit c3de25d

File tree

2 files changed

+12
-12
lines changed

2 files changed

+12
-12
lines changed

lib/puppet/module_tool/tar/gnu.rb

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,14 @@
44

55
class Puppet::ModuleTool::Tar::Gnu
66
def unpack(sourcefile, destdir, owner)
7-
sourcefile = File.expand_path(sourcefile)
7+
safe_sourcefile = Shellwords.shellescape(File.expand_path(sourcefile))
88
destdir = File.expand_path(destdir)
9+
safe_destdir = Shellwords.shellescape(destdir)
910

10-
Dir.chdir(destdir) do
11-
Puppet::Util::Execution.execute("gzip -dc #{Shellwords.shellescape(sourcefile)} | tar xof -")
12-
Puppet::Util::Execution.execute("find . -type d -exec chmod 755 {} +")
13-
Puppet::Util::Execution.execute("find . -type f -exec chmod u+rw,g+r,a-st {} +")
14-
Puppet::Util::Execution.execute("chown -R #{owner} .")
15-
end
11+
Puppet::Util::Execution.execute("gzip -dc #{safe_sourcefile} | tar --extract --no-same-owner --directory #{safe_destdir} --file -")
12+
Puppet::Util::Execution.execute("find #{safe_destdir} -type d -exec chmod 755 {} +")
13+
Puppet::Util::Execution.execute("find #{safe_destdir} -type f -exec chmod u+rw,g+r,a-st {} +")
14+
Puppet::Util::Execution.execute("chown -R #{owner} #{safe_destdir}")
1615
end
1716

1817
def pack(sourcedir, destfile)

spec/unit/module_tool/tar/gnu_spec.rb

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,13 @@
77
let(:sourcedir) { '/space path/the/src/dir' }
88
let(:destfile) { '/space path/the/dest/file.tar.gz' }
99

10+
let(:safe_sourcefile) { '/space\ path/the/module.tar.gz' }
11+
let(:safe_destdir) { '/space\ path/the/dest/dir' }
1012
it "unpacks a tar file" do
11-
expect(Dir).to receive(:chdir).with(File.expand_path(destdir)).and_yield
12-
expect(Puppet::Util::Execution).to receive(:execute).with("gzip -dc #{Shellwords.shellescape(File.expand_path(sourcefile))} | tar xof -")
13-
expect(Puppet::Util::Execution).to receive(:execute).with("find . -type d -exec chmod 755 {} +")
14-
expect(Puppet::Util::Execution).to receive(:execute).with("find . -type f -exec chmod u+rw,g+r,a-st {} +")
15-
expect(Puppet::Util::Execution).to receive(:execute).with("chown -R <owner:group> .")
13+
expect(Puppet::Util::Execution).to receive(:execute).with("gzip -dc #{safe_sourcefile} | tar --extract --no-same-owner --directory #{safe_destdir} --file -")
14+
expect(Puppet::Util::Execution).to receive(:execute).with("find #{safe_destdir} -type d -exec chmod 755 {} +")
15+
expect(Puppet::Util::Execution).to receive(:execute).with("find #{safe_destdir} -type f -exec chmod u+rw,g+r,a-st {} +")
16+
expect(Puppet::Util::Execution).to receive(:execute).with("chown -R <owner:group> #{safe_destdir}")
1617
subject.unpack(sourcefile, destdir, '<owner:group>')
1718
end
1819

0 commit comments

Comments
 (0)