Bug #15555
Updated by kbogtob (Karim Bogtob) about 5 years ago
The current implementation of the Dir.mktmpdir is the following: ```ruby def Dir.mktmpdir(prefix_suffix=nil, *rest) path = Tmpname.create(prefix_suffix || "d", *rest) {|n| mkdir(n, 0700)} if block_given? begin yield path ensure stat = File.stat(File.dirname(path)) if stat.world_writable? and !stat.sticky? raise ArgumentError, "parent directory is world writable but not sticky" end FileUtils.remove_entry path end else path end end ``` When used explicitly with a directory, it raises an ArgumentError when the directory is world writable (File::Stat#world_writable?) but not sticky (File::Stat#sticky?). This is a normal behavior. But with how the method is written, the behavior is inconsistent and raises an ArgumentError only when using it with the block form. Worse, it raises the error in the ensure block instead of the first thing done in the method. So it raises the ArgumentError **after** it yields to the block, so after the job is already done. Proof: ```ruby irb(main):001:0> puts `ls -la /tmp` total 4 drwxrwsrwx. 2 root 1000090000 6 Jan 22 10:02 . drwxr-xr-x. 18 root root 4096 Jan 22 09:58 .. => nil irb(main):002:0> # /tmp is non sticky but world writable irb(main):003:0* Dir.mktmpdir(nil, '/tmp') do |tmpdir| irb(main):004:1* puts "Hello, I'm using #{tmpdir}" irb(main):005:1> end Hello, I'm using /tmp/d20190122-26-5biuz2 ArgumentError: parent directory is world writable but not sticky from /opt/rh/rh-ruby24/root/usr/share/ruby/tmpdir.rb:93:in `mktmpdir' from (irb):3 from /opt/rh/rh-ruby24/root/usr/bin/irb:11:in `<top (required)>' from /opt/rh/rh-ruby24/root/usr/local/share/gems/gems/bundler-1.15.4/lib/bundler/cli/exec.rb:74:in `load' from /opt/rh/rh-ruby24/root/usr/local/share/gems/gems/bundler-1.15.4/lib/bundler/cli/exec.rb:74:in `kernel_load' from /opt/rh/rh-ruby24/root/usr/local/share/gems/gems/bundler-1.15.4/lib/bundler/cli/exec.rb:27:in `run' from /opt/rh/rh-ruby24/root/usr/local/share/gems/gems/bundler-1.15.4/lib/bundler/cli.rb:362:in `exec' from /opt/rh/rh-ruby24/root/usr/local/share/gems/gems/bundler-1.15.4/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run' from /opt/rh/rh-ruby24/root/usr/local/share/gems/gems/bundler-1.15.4/lib/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command' from /opt/rh/rh-ruby24/root/usr/local/share/gems/gems/bundler-1.15.4/lib/bundler/vendor/thor/lib/thor.rb:387:in `dispatch' from /opt/rh/rh-ruby24/root/usr/local/share/gems/gems/bundler-1.15.4/lib/bundler/cli.rb:22:in `dispatch' from /opt/rh/rh-ruby24/root/usr/local/share/gems/gems/bundler-1.15.4/lib/bundler/vendor/thor/lib/thor/base.rb:466:in `start' from /opt/rh/rh-ruby24/root/usr/local/share/gems/gems/bundler-1.15.4/lib/bundler/cli.rb:13:in `start' from /opt/rh/rh-ruby24/root/usr/local/share/gems/gems/bundler-1.15.4/exe/bundle:30:in `block in <top (required)>' from /opt/rh/rh-ruby24/root/usr/local/share/gems/gems/bundler-1.15.4/lib/bundler/friendly_errors.rb:121:in `with_friendly_errors' from /opt/rh/rh-ruby24/root/usr/local/share/gems/gems/bundler-1.15.4/exe/bundle:22:in `<top (required)>' from /opt/rh/rh-ruby24/root/usr/bin/bundle:23:in `load' from /opt/rh/rh-ruby24/root/usr/bin/bundle:23:in `<main>' irb(main):006:0> puts `ls -la /tmp` total 4 drwxrwsrwx. 3 root 1000090000 33 Jan 22 10:03 . drwxr-xr-x. 18 root root 4096 Jan 22 09:58 .. drwx--S---. 2 1000090000 1000090000 6 Jan 22 10:03 d20190122-26-5biuz2 => nil ``` We can also see that the allocated directory **remains** after the exception is raised as the implementation raised in the ensure block. An advisable implementation would be: ```ruby def Dir.mktmpdir(prefix_suffix=nil, *rest) path = Tmpname.create(prefix_suffix || "d", *rest) {|n| mkdir(n, 0700)} stat = File.stat(File.dirname(path)) if stat.world_writable? and !stat.sticky? FileUtils.remove_entry path raise ArgumentError, "parent directory is world writable but not sticky" end if block_given? begin yield path ensure FileUtils.remove_entry path end else path end end ``` This way, it will make the checks first and raise directly. And no temporary directory will be leaked with the block form. As there is no maintainer for this lib, I can make a pull request with the following changes (with unit tests) if I'm asked to.