Information disclosure via ENV leakage across test boundaries (test isolation weakness)

Proof-of-concept to illustrate a potential ENV leakage scenario prior to this fix: 1) Create two tests in the same Minitest suite where the first test intentionally leaks an environment variable and does not revert it, and the second test relies on a clean ENV state. File: test/env_leak_poc_test.rb ```ruby require 'test_helper' class EnvLeakPOCTest < ActiveSupport::TestCase # Test 1: modifies ENV and does not revert def test_sets_env_var_without_reverting ENV["SECRET_TOKEN"] = "super_secret_value" # perform some assertions or operations... end # Test 2: runs after Test 1 (order may vary in real runs unless fixed) def test_env_is_clean_for_next_test # If ENV leaks persisted from Test 1, this would observe the value leaked = ENV["SECRET_TOKEN"] assert_nil leaked, "ENV SECRET_TOKEN leaked across tests: #{leaked.inspect}" end end ``` How this would behave without the LeakChecker - If test ordering happens to run test_sets_env_var_without_reverting before test_env_is_clean_for_next_test, ENV["SECRET_TOKEN"] would be set to "super_secret_value" in the second test, causing leakage into a subsequent test and potentially exposing secrets through logs or assertions. - This demonstrates how environment state can escape test boundaries and be observed by later tests or in test artifacts. How the LeakChecker fixes this - After this commit, LeakChecker records ENV before setup and compares ENV after teardown (excluding allowed keys). If any change is detected, it fails the test with a descriptive message, preventing test isolation leakage and surfacing the issue during CI. Notes and prerequisites - In practice, test order is often randomized; the PoC will reliably reveal leakage when a test that mutates ENV does not revert, and LeakChecker will catch the divergence after teardown. - The ALLOWED_KEYS list includes keys that are safe to ignore (e.g., VIPSHOME); any other ENV mutations will trigger a failure. - The PoC demonstrates a minimal path for leakage; in real Rails apps, the leak could occur with any sensitive env var (e.g., API keys, credentials) leaking into other tests, logs, or error messages.

diff --git a/actionview/lib/action_view/test_case.rb b/actionview/lib/action_view/test_case.rb index c0cc707845f9a..733c904822219 100644 --- a/actionview/lib/action_view/test_case.rb +++ b/actionview/lib/action_view/test_case.rb @@ -266,6 +266,10 @@ def _test_case end end + def after_setup + @__setup_ivars = instance_variables << :@__setup_ivars + end + def setup_with_controller controller_class = Class.new(ActionView::TestCase::TestController) @controller = controller_class.new @@ -399,7 +403,7 @@ def view ] def _user_defined_ivars - instance_variables - INTERNAL_IVARS + instance_variables - INTERNAL_IVARS - @__setup_ivars end # Returns a Hash of instance variables and their values, as defined by diff --git a/railties/test/application/zeitwerk_checker_test.rb b/railties/test/application/zeitwerk_checker_test.rb index b0606248161af..136cd6d4e1531 100644 --- a/railties/test/application/zeitwerk_checker_test.rb +++ b/railties/test/application/zeitwerk_checker_test.rb @@ -5,6 +5,7 @@ class ZeitwerkCheckerTest < ActiveSupport::TestCase include ActiveSupport::Testing::Isolation + teardown :teardown_app def setup build_app diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index a8824bfc3e26b..98ca0f066d18d 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -104,11 +104,11 @@ def assert_welcome(resp) module Generation # Build an application by invoking the generator and going through the whole stack. def build_app(options = {}) - @prev_rails_app_class = Rails.app_class - @prev_rails_application = Rails.application + @prev_rails_app_class ||= Rails.app_class + @prev_rails_application ||= Rails.application Rails.app_class = Rails.application = nil - @prev_rails_env = ENV["RAILS_ENV"] + @prev_rails_env ||= ENV["RAILS_ENV"] ENV["RAILS_ENV"] = "development" FileUtils.rm_rf(app_path) diff --git a/railties/test/isolation/test_helpers_test.rb b/railties/test/isolation/test_helpers_test.rb index fd9dea64b6152..645c1570699f5 100644 --- a/railties/test/isolation/test_helpers_test.rb +++ b/railties/test/isolation/test_helpers_test.rb @@ -5,6 +5,7 @@ module TestHelpersTests class GenerationTest < ActiveSupport::TestCase include ActiveSupport::Testing::Isolation + teardown :teardown_app def test_build_app build_app diff --git a/tools/support/leak_checker.rb b/tools/support/leak_checker.rb new file mode 100644 index 0000000000000..84efd93312f63 --- /dev/null +++ b/tools/support/leak_checker.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module LeakChecker + # These keys are out of our control. + # Either added by the system or by other libraries. + ALLOWED_KEYS = %w[ + VIPSHOME + ] + + def before_setup + @__leak_checker_before_env = ENV.to_h + super + end + + def after_teardown + super + + after = ENV.to_h + before = @__leak_checker_before_env + + ALLOWED_KEYS.each do |k| + after.delete(k) + before.delete(k) + end + + if after != before + message = +"Environment leak detected:\n" + added_keys = after.keys - before.keys + unless added_keys.empty? + message << " - Added variables:\n" + added_keys.each do |k| + message << " - #{k}" + end + end + + removed_keys = before.keys - after.keys + unless removed_keys.empty? + message << " - Removed variables:\n" + removed_keys.each do |k| + message << " - #{k}" + end + end + + changed_keys = (before.keys & after.keys).select { |k| before[k] != after[k] } + unless changed_keys.empty? + message << " - Changed variables:\n" + changed_keys.each do |k| + message << " - #{k} from #{before[k].inspect} to #{after[k].inspect}" + end + end + + flunk message + end + end +end diff --git a/tools/test_common.rb b/tools/test_common.rb index 9866241b6dca0..425cc19529d6e 100644 --- a/tools/test_common.rb +++ b/tools/test_common.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative "support/leak_checker" + ActiveSupport::TestCase.alias_method :force_skip, :skip ENV["RAILS_TEST_EXECUTABLE"] = "bin/test" @@ -20,3 +22,5 @@ def skip(message = nil, *) end ActiveSupport::TestCase.include(DisableSkipping) end + +ActiveSupport::TestCase.prepend(LeakChecker)