From f0c99956e7e3dcd68bdec570cbc32adf88b53163 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Mon, 6 Feb 2012 14:37:06 -0800
Subject: [PATCH] (#12463) eliminate `secure_open` in favour of `replace_file`

None of the users of `secure_open` in the codebase actually want to achieve a
result different from `replace_file`, so we might as well eliminate the older
API in favour of one that is easier to use and clearer in intent.

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
---
 lib/puppet/daemon.rb          |  4 ++--
 lib/puppet/network/server.rb  |  2 +-
 lib/puppet/rails/benchmark.rb |  2 +-
 lib/puppet/util.rb            | 22 ----------------------
 lib/puppet/util/reference.rb  | 15 ++++++++-------
 5 files changed, 12 insertions(+), 33 deletions(-)

diff --git a/lib/puppet/daemon.rb b/lib/puppet/daemon.rb
index 22630ffb8ac..2632bc2a8df 100755
--- a/lib/puppet/daemon.rb
+++ b/lib/puppet/daemon.rb
@@ -33,9 +33,9 @@ def daemonize
       Puppet::Util::Log.reopen
     rescue => detail
       Puppet.err "Could not start #{Puppet[:name]}: #{detail}"
-      Puppet::Util::secure_open("/tmp/daemonout", "w") { |f|
+      Puppet::Util::replace_file("/tmp/daemonout", 0644) do |f|
         f.puts "Could not start #{Puppet[:name]}: #{detail}"
-      }
+      end
       exit(12)
     end
   end
diff --git a/lib/puppet/network/server.rb b/lib/puppet/network/server.rb
index e4de07deac7..80987e4b092 100644
--- a/lib/puppet/network/server.rb
+++ b/lib/puppet/network/server.rb
@@ -22,7 +22,7 @@ def daemonize
       $stderr.reopen $stdout
       Puppet::Util::Log.reopen
     rescue => detail
-      Puppet::Util.secure_open("/tmp/daemonout", "w") { |f|
+      Puppet::Util.replace_file("/tmp/daemonout", 0644) { |f|
         f.puts "Could not start #{Puppet[:name]}: #{detail}"
       }
       raise "Could not start #{Puppet[:name]}: #{detail}"
diff --git a/lib/puppet/rails/benchmark.rb b/lib/puppet/rails/benchmark.rb
index 375674e194e..e1e92bb7966 100644
--- a/lib/puppet/rails/benchmark.rb
+++ b/lib/puppet/rails/benchmark.rb
@@ -58,6 +58,6 @@ def write_benchmarks
       data = {}
     end
     data[branch] = $benchmarks
-    Puppet::Util.secure_open(file, "w") { |f| f.print YAML.dump(data) }
+    Puppet::Util.replace_file(file, 0644) { |f| f.print YAML.dump(data) }
   end
 end
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index d58e45dcf9c..ff8eb6d1575 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -481,28 +481,6 @@ def thinmark
 
   module_function :memory, :thinmark
 
-  def secure_open(file,must_be_w,&block)
-    raise Puppet::DevError,"secure_open only works with mode 'w'" unless must_be_w == 'w'
-    raise Puppet::DevError,"secure_open only requires a block"    unless block_given?
-    Puppet.warning "#{file} was a symlink to #{File.readlink(file)}" if File.symlink?(file)
-    if File.exists?(file) or File.symlink?(file)
-      wait = File.symlink?(file) ? 5.0 : 0.1
-      File.delete(file)
-      sleep wait # give it a chance to reappear, just in case someone is actively trying something.
-    end
-    begin
-      File.open(file,File::CREAT|File::EXCL|File::TRUNC|File::WRONLY,&block)
-    rescue Errno::EEXIST
-      desc = File.symlink?(file) ? "symlink to #{File.readlink(file)}" : File.stat(file).ftype
-      puts "Warning: #{file} was apparently created by another process (as"
-      puts "a #{desc}) as soon as it was deleted by this process.  Someone may be trying"
-      puts "to do something objectionable (such as tricking you into overwriting system"
-      puts "files if you are running as root)."
-      raise
-    end
-  end
-  module_function :secure_open
-
   # Because IO#binread is only available in 1.9
   def binread(file)
     File.open(file, 'rb') { |f| f.read }
diff --git a/lib/puppet/util/reference.rb b/lib/puppet/util/reference.rb
index 60032510979..bb0ead7dffd 100644
--- a/lib/puppet/util/reference.rb
+++ b/lib/puppet/util/reference.rb
@@ -36,14 +36,15 @@ def self.page(*sections)
 
   def self.pdf(text)
     puts "creating pdf"
-    Puppet::Util.secure_open("/tmp/puppetdoc.txt", "w") do |f|
-      f.puts text
-    end
-    rst2latex = which('rst2latex') || which('rst2latex.py') || raise("Could not find rst2latex")
+    rst2latex = which('rst2latex') || which('rst2latex.py') ||
+      raise("Could not find rst2latex")
+
     cmd = %{#{rst2latex} /tmp/puppetdoc.txt > /tmp/puppetdoc.tex}
-    Puppet::Util.secure_open("/tmp/puppetdoc.tex","w") do |f|
-      # If we get here without an error, /tmp/puppetdoc.tex isn't a tricky cracker's symlink
-    end
+    Puppet::Util.replace_file("/tmp/puppetdoc.txt") {|f| f.puts text }
+    # There used to be an attempt to use secure_open / replace_file to secure
+    # the target, too, but that did nothing: the race was still here.  We can
+    # get exactly the same benefit from running this effort:
+    File.unlink('/tmp/puppetdoc.tex') rescue nil
     output = %x{#{cmd}}
     unless $CHILD_STATUS == 0
       $stderr.puts "rst2latex failed"