#357: Fix TOCTOU race in durable_place by opening file before checking

lib/baza-rb.rb

@@ -315,26 +315,31 @@ def durable_place(pname, file)
     raise(RuntimeError, "The name #{pname.inspect} is not valid") unless pname.match?(/\A[a-z0-9-]+\z/)
     raise(RuntimeError, "The name #{pname.inspect} is too long") if pname.length > 32
     raise(RuntimeError, 'The "file" of the durable is nil') if file.nil?
-    raise(RuntimeError, "The file '#{file}' is absent") unless File.exist?(file)
-    if File.size(file) > 1024
-      raise(
-        RuntimeError,
-        "The file '#{file}' is too big (#{File.size(file)} bytes) for durable_place(), use durable_save() instead"
-      )
-    end
+    raise(RuntimeError, "The file '#{file}' is a symlink") if File.lstat(file).symlink?
     id = nil
-    elapsed(@loog, level: Logger::INFO) do
-      id = post(
-        home.append('durable-place'),
-        {
-          'pname' => pname,
-          'file' => File.basename(file),
-          'zip' => File.open(file, 'rb')
-        }
-      ).headers['X-Zerocracy-DurableId'].to_i
-      throw(:"Durable ##{id} (#{file}, #{File.size(file)} bytes) placed for job \"#{pname}\" at #{@host}")
+    File.open(file, 'rb') do |f|
+      raise(RuntimeError, "The file '#{file}' is absent") unless f.stat.file?
+      if f.size > 1024
+        raise(
+          RuntimeError,
+          "The file '#{file}' is too big (#{f.size} bytes) for durable_place(), use durable_save() instead"
+        )
+      end
+      elapsed(@loog, level: Logger::INFO) do
+        id = post(
+          home.append('durable-place'),
+          {
+            'pname' => pname,
+            'file' => File.basename(file),
+            'zip' => f
+          }
+        ).headers['X-Zerocracy-DurableId'].to_i
+        throw(:"Durable ##{id} (#{file}, #{f.size} bytes) placed for job \"#{pname}\" at #{@host}")
+      end
     end
     id
+  rescue Errno::ENOENT
+    raise(RuntimeError, "The file '#{file}' is absent")
   end
 
   # Save a single durable from local file to server.

test/test_baza_edge.rb

@@ -49,22 +49,20 @@ def test_durable_place
     end
   end
 
-  def test_durable_place_raises_when_pname_is_invalid
-    assert_includes(
-      assert_raises(RuntimeError) { fake_baza.durable_place('INVALID', '/tmp/x') }.message,
-      'is not valid'
-    )
-  end
-
-  def test_durable_place_raises_when_pname_is_too_long
-    assert_includes(
-      assert_raises(RuntimeError) { fake_baza.durable_place('a' * 33, '/tmp/x') }.message,
-      'is too long'
-    )
-  end
-
-  def test_durable_find_raises_when_pname_is_invalid
-    assert_includes(assert_raises(RuntimeError) { fake_baza.durable_find('BAD!', 'file') }.message, 'is not valid')
+  def test_durable_place_rejects_symlink
+    WebMock.disable_net_connect!
+    Dir.mktmpdir do |dir|
+      real = File.join(dir, 'real.bin')
+      File.binwrite(real, 'content')
+      link = File.join(dir, 'link.bin')
+      File.symlink(real, link)
+      assert_includes(
+        assert_raises(RuntimeError) do
+          fake_baza(compress: false).durable_place('simple', link)
+        end.message,
+        'symlink'
+      )
+    end
   end
 
   def test_real_http