From 1791d8067d00dc1381b9cdc3cc4ad44c1f6c7197 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Tue, 21 Jul 2026 21:01:30 +0200 Subject: Add storage isolation for tests, preventing live data destruction --- app/models/concerns/file_attachment.rb | 20 ++++++++++++++------ test/controllers/assets_controller_test.rb | 16 +++++++--------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/app/models/concerns/file_attachment.rb b/app/models/concerns/file_attachment.rb index e7a33c5..548531f 100644 --- a/app/models/concerns/file_attachment.rb +++ b/app/models/concerns/file_attachment.rb @@ -66,12 +66,23 @@ module FileAttachment @upload = UploadProxy.new(self) end + class_methods do + def upload_root + Rails.env.test? ? Rails.root.join("tmp", "test_uploads") : Rails.root.join("public", "system", "uploads") + end + end + + def upload_root + self.class.upload_root + end + def process_upload return unless @pending_upload uploaded_file = @pending_upload @pending_upload = nil - old_dir = Rails.root.join("public", "system", "uploads", id.to_s) + old_dir = upload_root.join(id.to_s) + FileUtils.rm_rf(old_dir) if Dir.exist?(old_dir) original_path = file_path(:original) @@ -102,15 +113,12 @@ module FileAttachment end def delete_upload_files - dir = Rails.root.join("public", "system", "uploads", id.to_s) + dir = upload_root.join(id.to_s) FileUtils.rm_rf(dir) if Dir.exist?(dir) end def file_path(style) - Rails.root.join( - "public", "system", "uploads", - id.to_s, style.to_s, upload_file_name - ).to_s + upload_root.join(id.to_s, style.to_s, variant_filename(style)).to_s end def sanitize_filename(filename) diff --git a/test/controllers/assets_controller_test.rb b/test/controllers/assets_controller_test.rb index 5f5f6e5..59ebab5 100644 --- a/test/controllers/assets_controller_test.rb +++ b/test/controllers/assets_controller_test.rb @@ -4,17 +4,16 @@ class AssetsControllerTest < ActionController::TestCase def setup login_as :quentin + @existing_asset_ids = Asset.pluck(:id) end def teardown - # Clean up any files written to disk during tests - Dir.glob(Rails.root.join('public', 'system', 'uploads', 'test_*')).each do |dir| + (Asset.pluck(:id) - @existing_asset_ids).each do |id| + dir = Asset.upload_root.join(id.to_s) + raise "Refusing to delete #{dir} -- outside tmp/, Rails.env.test? may be false" unless + dir.to_s.start_with?(Rails.root.join("tmp").to_s) FileUtils.rm_rf(dir) end - # Remove uploads created for assets created during tests - Asset.where("upload_file_name IS NOT NULL").where("id > 1000000").each do |a| - FileUtils.rm_rf(Rails.root.join('public', 'system', 'uploads', a.id.to_s)) - end end # --- index --- @@ -64,8 +63,7 @@ class AssetsControllerTest < ActionController::TestCase # original and all four variants should exist on disk %w[original medium thumb headline large].each do |style| - path = Rails.root.join('public', 'system', 'uploads', - asset.id.to_s, style, 'test_image.png') + path = asset.send(:file_path, style) assert File.exist?(path), "Expected #{style} variant at #{path}" end end @@ -137,7 +135,7 @@ class AssetsControllerTest < ActionController::TestCase ) post :create, params: { asset: { name: 'To be deleted', upload: uploaded } } asset = Asset.last - upload_dir = Rails.root.join('public', 'system', 'uploads', asset.id.to_s) + upload_dir = asset.send(:upload_root).join(asset.id.to_s) assert Dir.exist?(upload_dir), "Upload directory should exist before destroy" assert_difference 'Asset.count', -1 do -- cgit v1.3