diff --git a/web/spec/meshtastic_payload_decoder_spec.rb b/web/spec/meshtastic_payload_decoder_spec.rb index 8ff26ade..16bac34b 100644 --- a/web/spec/meshtastic_payload_decoder_spec.rb +++ b/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 diff --git a/web/spec/spec_helper.rb b/web/spec/spec_helper.rb index f8893e72..5c26aab8 100644 --- a/web/spec/spec_helper.rb +++ b/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 diff --git a/web/spec/suite_tmpdir_spec.rb b/web/spec/suite_tmpdir_spec.rb new file mode 100644 index 00000000..a1014711 --- /dev/null +++ b/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 diff --git a/web/spec/support/suite_tmpdir.rb b/web/spec/support/suite_tmpdir.rb new file mode 100644 index 00000000..4100ea8b --- /dev/null +++ b/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