Information disclosure / cross-test environment variable leakage

Proof-of-Concept to illustrate the leak-detection behavior (before this fix, a test that mutates ENV permanently could leak values to subsequent tests): # File: test/env_leak_poc_a.rb class EnvLeakPocA < ActiveSupport::TestCase test "leaks environment variable after test" do # Simulate leaking a sensitive value into the process environment ENV["SENSITIVE_TOKEN"] = "super-secret-value" # Note: Intentionally not reverting ENV["SENSITIVE_TOKEN"] here end end # File: test/env_leak_poc_b.rb class EnvLeakPocB < ActiveSupport::TestCase test "environment should be clean between tests" do # In a vulnerable run, the previous test left SENSITIVE_TOKEN set # LeakChecker should have raised an error after the first test teardown refute ENV.key?("SENSITIVE_TOKEN") end end # How to reproduce (without the fix): # 1) Run the Rails test suite (e.g., bin/rails test). # 2) Observe that the first test mutates ENV and, since it isn't reverted, the LeakChecker detects the change during after_teardown and fails the run with a message like: # "Environment leak detected:..." including added/removed/changed variables. # With the fix in place (as of this commit), the test suite will fail fast with a clear error indicating ENV leakage, guiding removal or restoration of env vars in tests.

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)