Skip to content

Commit f0c9995

Browse files
Daniel Pittmanhaus
Daniel Pittman
authored andcommitted
(#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 <[email protected]>
1 parent 0c96703 commit f0c9995

File tree

5 files changed

+12
-33
lines changed

5 files changed

+12
-33
lines changed

lib/puppet/daemon.rb

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ def daemonize
3333
Puppet::Util::Log.reopen
3434
rescue => detail
3535
Puppet.err "Could not start #{Puppet[:name]}: #{detail}"
36-
Puppet::Util::secure_open("/tmp/daemonout", "w") { |f|
36+
Puppet::Util::replace_file("/tmp/daemonout", 0644) do |f|
3737
f.puts "Could not start #{Puppet[:name]}: #{detail}"
38-
}
38+
end
3939
exit(12)
4040
end
4141
end

lib/puppet/network/server.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ def daemonize
2222
$stderr.reopen $stdout
2323
Puppet::Util::Log.reopen
2424
rescue => detail
25-
Puppet::Util.secure_open("/tmp/daemonout", "w") { |f|
25+
Puppet::Util.replace_file("/tmp/daemonout", 0644) { |f|
2626
f.puts "Could not start #{Puppet[:name]}: #{detail}"
2727
}
2828
raise "Could not start #{Puppet[:name]}: #{detail}"

lib/puppet/rails/benchmark.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,6 @@ def write_benchmarks
5858
data = {}
5959
end
6060
data[branch] = $benchmarks
61-
Puppet::Util.secure_open(file, "w") { |f| f.print YAML.dump(data) }
61+
Puppet::Util.replace_file(file, 0644) { |f| f.print YAML.dump(data) }
6262
end
6363
end

lib/puppet/util.rb

-22
Original file line numberDiff line numberDiff line change
@@ -481,28 +481,6 @@ def thinmark
481481

482482
module_function :memory, :thinmark
483483

484-
def secure_open(file,must_be_w,&block)
485-
raise Puppet::DevError,"secure_open only works with mode 'w'" unless must_be_w == 'w'
486-
raise Puppet::DevError,"secure_open only requires a block" unless block_given?
487-
Puppet.warning "#{file} was a symlink to #{File.readlink(file)}" if File.symlink?(file)
488-
if File.exists?(file) or File.symlink?(file)
489-
wait = File.symlink?(file) ? 5.0 : 0.1
490-
File.delete(file)
491-
sleep wait # give it a chance to reappear, just in case someone is actively trying something.
492-
end
493-
begin
494-
File.open(file,File::CREAT|File::EXCL|File::TRUNC|File::WRONLY,&block)
495-
rescue Errno::EEXIST
496-
desc = File.symlink?(file) ? "symlink to #{File.readlink(file)}" : File.stat(file).ftype
497-
puts "Warning: #{file} was apparently created by another process (as"
498-
puts "a #{desc}) as soon as it was deleted by this process. Someone may be trying"
499-
puts "to do something objectionable (such as tricking you into overwriting system"
500-
puts "files if you are running as root)."
501-
raise
502-
end
503-
end
504-
module_function :secure_open
505-
506484
# Because IO#binread is only available in 1.9
507485
def binread(file)
508486
File.open(file, 'rb') { |f| f.read }

lib/puppet/util/reference.rb

+8-7
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,15 @@ def self.page(*sections)
3636

3737
def self.pdf(text)
3838
puts "creating pdf"
39-
Puppet::Util.secure_open("/tmp/puppetdoc.txt", "w") do |f|
40-
f.puts text
41-
end
42-
rst2latex = which('rst2latex') || which('rst2latex.py') || raise("Could not find rst2latex")
39+
rst2latex = which('rst2latex') || which('rst2latex.py') ||
40+
raise("Could not find rst2latex")
41+
4342
cmd = %{#{rst2latex} /tmp/puppetdoc.txt > /tmp/puppetdoc.tex}
44-
Puppet::Util.secure_open("/tmp/puppetdoc.tex","w") do |f|
45-
# If we get here without an error, /tmp/puppetdoc.tex isn't a tricky cracker's symlink
46-
end
43+
Puppet::Util.replace_file("/tmp/puppetdoc.txt") {|f| f.puts text }
44+
# There used to be an attempt to use secure_open / replace_file to secure
45+
# the target, too, but that did nothing: the race was still here. We can
46+
# get exactly the same benefit from running this effort:
47+
File.unlink('/tmp/puppetdoc.tex') rescue nil
4748
output = %x{#{cmd}}
4849
unless $CHILD_STATUS == 0
4950
$stderr.puts "rst2latex failed"

0 commit comments

Comments
 (0)