web: specs leave no temp dirs; a leaking spec fails the run (#1053)

web/spec/meshtastic_payload_decoder_spec.rb

@@ -94,9 +94,11 @@ RSpec.describe PotatoMesh::App::Meshtastic::PayloadDecoder do
       FileUtils.mkdir_p(File.dirname(script_path))
       File.write(script_path, "")
 
-      with_repo_root(Dir.mktmpdir) do
-        allow(PotatoMesh::Config).to receive(:web_root).and_return(dir)
-        expect(described_class.decoder_script_path).to eq(script_path)
+      Dir.mktmpdir do |empty_root|
+        with_repo_root(empty_root) do
+          allow(PotatoMesh::Config).to receive(:web_root).and_return(dir)
+          expect(described_class.decoder_script_path).to eq(script_path)
+        end
       end
     end
   end
@@ -200,8 +202,10 @@ RSpec.describe PotatoMesh::App::Meshtastic::PayloadDecoder do
   it "returns nil when no python executable can be found" do
     with_env("MESHTASTIC_PYTHON", nil) do
       with_env("PATH", "") do
-        with_repo_root(Dir.mktmpdir) do
-          expect(described_class.python_executable_path).to be_nil
+        Dir.mktmpdir do |empty_root|
+          with_repo_root(empty_root) do
+            expect(described_class.python_executable_path).to be_nil
+          end
         end
       end
     end

web/spec/spec_helper.rb

@@ -32,11 +32,15 @@ end
 
 require "tmpdir"
 require "fileutils"
+require_relative "support/suite_tmpdir"
 
 ENV["RACK_ENV"] = "test"
 ENV["INSTANCE_DOMAIN"] ||= "spec.mesh.test"
 
-SPEC_TMPDIR = Dir.mktmpdir("potato-mesh-spec-")
+# The run's one temporary directory: TMPDIR points at its tmp directory, and
+# an at_exit hook removes it; a SIGKILL or a second INT leaves it behind
+# (SPEC TD1).
+SPEC_TMPDIR = SuiteTmpdir.create!
 ENV["XDG_DATA_HOME"] = File.join(SPEC_TMPDIR, "xdg-data")
 ENV["XDG_CONFIG_HOME"] = File.join(SPEC_TMPDIR, "xdg-config")
 
@@ -70,7 +74,9 @@ RSpec.configure do |config|
     PotatoMesh::App::DataProcessing.collision_warning_limiter.reset!
   end
 
+  # A spec that leaves anything in the suite's TMPDIR fails the run; the
+  # at_exit hook removes SPEC_TMPDIR either way (SPEC TD2).
   config.after(:suite) do
-    FileUtils.remove_entry(SPEC_TMPDIR) if File.directory?(SPEC_TMPDIR)
+    SuiteTmpdir.check!(SuiteTmpdir.tmp_path(SPEC_TMPDIR))
   end
 end

web/spec/suite_tmpdir_spec.rb

@@ -0,0 +1,109 @@
+# Copyright © 2025-26 l5yth & contributors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# frozen_string_literal: true
+
+require "spec_helper"
+require "tmpdir"
+require_relative "support/suite_tmpdir"
+
+# The suite's temporary directory (SPEC TD1, TD2): where it is made, where
+# TMPDIR points, who removes it, and the check that fails a leaking run.
+RSpec.describe SuiteTmpdir do
+  it "points this run's TMPDIR inside SPEC_TMPDIR (TD1)" do
+    expect(ENV["TMPDIR"]).to eq(File.join(SPEC_TMPDIR, "tmp"))
+    expect(Dir.tmpdir).to eq(File.join(SPEC_TMPDIR, "tmp"))
+    expect(File.basename(SPEC_TMPDIR)).to start_with("potato-mesh-spec-")
+  end
+
+  describe ".create!" do
+    let(:env) { {} }
+    let(:hooks) { [] }
+    let(:exit_hook) { ->(&block) { hooks << block } }
+    let(:failures) { [] }
+    let(:on_failure) { ->(message) { failures << message } }
+
+    it "makes the directory and its tmp, points TMPDIR there, and removes it at exit (TD1)" do
+      root = described_class.create!(env: env, exit_hook: exit_hook, on_failure: on_failure)
+      expect(File.dirname(root)).to eq(Dir.tmpdir)
+      expect(File.basename(root)).to start_with("potato-mesh-spec-")
+      expect(env["TMPDIR"]).to eq(File.join(root, "tmp"))
+      expect(File.directory?(env["TMPDIR"])).to be(true)
+      expect(hooks.size).to eq(1)
+
+      File.write(File.join(env["TMPDIR"], "left-behind"), "")
+      hooks.first.call
+      expect(File.exist?(root)).to be(false)
+      expect(failures).to be_empty
+    end
+
+    it "fails the run when the directory outlives its removal (TD1)" do
+      root = described_class.create!(env: env, exit_hook: exit_hook, on_failure: on_failure)
+      # rm_rf raises nothing when it cannot remove an entry; here it removes
+      # nothing at all.
+      allow(FileUtils).to receive(:rm_rf)
+      hooks.first.call
+      expect(failures).to eq(["SuiteTmpdir: could not remove the suite directory #{root}"])
+    ensure
+      FileUtils.remove_entry(root) if root
+    end
+
+    it "makes tmp owner-only, so Dir.tmpdir takes it under umask 0 (TD1)" do
+      previous_umask = File.umask(0)
+      root = described_class.create!(env: env, exit_hook: exit_hook)
+      File.umask(previous_umask)
+      expect(File.stat(env["TMPDIR"]).mode & 0o777).to eq(0o700)
+      previous_tmpdir = ENV["TMPDIR"]
+      begin
+        ENV["TMPDIR"] = env["TMPDIR"]
+        expect(Dir.tmpdir).to eq(env["TMPDIR"])
+      ensure
+        ENV["TMPDIR"] = previous_tmpdir
+      end
+    ensure
+      File.umask(previous_umask) if previous_umask
+      FileUtils.rm_rf(root) if root
+    end
+
+    it "leaves the directory to its owner when a forked child exits (TD1)" do
+      root = described_class.create!(env: env, exit_hook: exit_hook)
+      owner = Process.pid
+      allow(Process).to receive(:pid).and_return(owner + 1)
+      hooks.first.call
+      expect(File.directory?(root)).to be(true)
+    ensure
+      FileUtils.rm_rf(root) if root
+    end
+  end
+
+  describe ".check!" do
+    it "passes an empty or missing TMPDIR (TD2)" do
+      Dir.mktmpdir("suite-tmpdir-spec-") do |dir|
+        expect { described_class.check!(dir) }.not_to raise_error
+        expect { described_class.check!(File.join(dir, "gone")) }.not_to raise_error
+      end
+    end
+
+    it "names every entry a spec left behind (TD2)" do
+      Dir.mktmpdir("suite-tmpdir-spec-") do |dir|
+        FileUtils.mkdir(File.join(dir, "d20261016-1-abc"))
+        File.write(File.join(dir, "RackMultipart20261016-1-xyz.bin"), "")
+        expect { described_class.check!(dir) }.to raise_error(
+          SuiteTmpdir::LeakError,
+          "specs left these in the suite TMPDIR #{dir}: RackMultipart20261016-1-xyz.bin, d20261016-1-abc",
+        )
+      end
+    end
+  end
+end

web/spec/support/suite_tmpdir.rb

@@ -0,0 +1,97 @@
+# Copyright © 2025-26 l5yth & contributors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# frozen_string_literal: true
+
+require "fileutils"
+require "tmpdir"
+
+# The suite's one temporary directory (SPEC TD1, TD2).
+#
+# +spec_helper.rb+ makes it under the TMPDIR the run starts with and points
+# TMPDIR at its +tmp+ directory, so every temporary file or directory a spec,
+# a gem or a child process makes lands inside it.  An +at_exit+ hook removes
+# it however the run ends, short of SIGKILL or a second INT (RSpec's force
+# quit, +exit!+): passing, failing, +--dry-run+, a spec file that fails to
+# load, one INT or TERM.  Either of those runs no hook and leaves this one
+# directory behind, with the test database and keyfile under +xdg-data+ and
+# +xdg-config+ and whatever its +tmp+ held.  When the hook cannot remove the
+# directory it fails the run ({remove!}), and a spec that leaves anything in
+# +tmp+ fails the run ({check!}).
+module SuiteTmpdir
+  # Name prefix of the suite directory.
+  PREFIX = "potato-mesh-spec-"
+
+  # Raised by {check!} when a spec left files or directories in the suite's
+  # TMPDIR.
+  class LeakError < StandardError; end
+
+  module_function
+
+  # Make the suite directory and its +tmp+ directory, point TMPDIR at +tmp+,
+  # and register the directory's removal at exit.
+  #
+  # @param env [#[]=] environment whose +TMPDIR+ is set; +ENV+ in the suite.
+  # @param exit_hook [#call] takes the removal as a block; +Kernel#at_exit+
+  #   in the suite.
+  # @param on_failure [#call] takes the message when the removal leaves the
+  #   directory; +Kernel#abort+ in the suite, which fails the run.
+  # @return [String] the suite directory.
+  def create!(env: ENV, exit_hook: method(:at_exit), on_failure: method(:abort))
+    root = Dir.mktmpdir(PREFIX)
+    # Owner-only, as Dir.mktmpdir makes +root+: under umask 0 a world-writable
+    # +tmp+ would be refused by Dir.tmpdir, which would then pick another.
+    FileUtils.mkdir_p(tmp_path(root), mode: 0o700)
+    env["TMPDIR"] = tmp_path(root)
+    owner = Process.pid
+    # A forked child that exits normally runs the hooks it inherited; only
+    # the process that made the directory removes it.
+    exit_hook.call { remove!(root, on_failure) if Process.pid == owner }
+    root
+  end
+
+  # Remove the suite directory, and report through +on_failure+ when any of
+  # it is left: +FileUtils.rm_rf+ removes what it can and raises nothing.
+  #
+  # @param root [String] the suite directory.
+  # @param on_failure [#call] takes the message when +root+ is left.
+  # @return [void]
+  def remove!(root, on_failure)
+    FileUtils.rm_rf(root)
+    return unless File.exist?(root)
+
+    on_failure.call("SuiteTmpdir: could not remove the suite directory #{root}")
+  end
+
+  # The suite's TMPDIR inside the suite directory.
+  #
+  # @param root [String] the suite directory.
+  # @return [String] its +tmp+ directory.
+  def tmp_path(root)
+    File.join(root, "tmp")
+  end
+
+  # Raise {LeakError} naming every entry left in +dir+; an empty or missing
+  # directory passes.
+  #
+  # @param dir [String] the suite's TMPDIR.
+  # @return [void]
+  # @raise [LeakError] when +dir+ holds anything.
+  def check!(dir)
+    left = Dir.exist?(dir) ? Dir.children(dir).sort : []
+    return if left.empty?
+
+    raise LeakError, "specs left these in the suite TMPDIR #{dir}: #{left.join(", ")}"
+  end
+end