From 900bfd94a9c3c45484d88aa69071b7a52c5b04b4 Mon Sep 17 00:00:00 2001 From: schneems Date: Fri, 14 Aug 2015 11:31:33 -0500 Subject: Prevent destructive action on production database This PR introduces a key/value type store to Active Record that can be used for storing internal values. It is an alternative implementation to #21237 cc @sgrif @matthewd. It is possible to run your tests against your production database by accident right now. While infrequently, but as an anecdotal data point, Heroku receives a non-trivial number of requests for a database restore due to this happening. In these cases the loss can be large. To prevent against running tests against production we can store the "environment" version that was used when migrating the database in a new internal table. Before executing tests we can see if the database is a listed in `protected_environments` and abort. There is a manual escape valve to force this check from happening with environment variable `DISABLE_DATABASE_ENVIRONMENT_CHECK=1`. --- activerecord/test/cases/migration_test.rb | 44 ++++++++++++++++++++++ activerecord/test/cases/schema_dumper_test.rb | 4 ++ .../test/cases/tasks/database_tasks_test.rb | 28 ++++++++++++++ 3 files changed, 76 insertions(+) (limited to 'activerecord/test') diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 19be357b6e..2f7859a7be 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -354,6 +354,50 @@ class MigrationTest < ActiveRecord::TestCase Reminder.reset_table_name end + def test_internal_metadata_table_name + original_internal_metadata_table_name = ActiveRecord::Base.internal_metadata_table_name + + assert_equal "active_record_internal_metadatas", ActiveRecord::InternalMetadata.table_name + ActiveRecord::Base.table_name_prefix = "prefix_" + ActiveRecord::Base.table_name_suffix = "_suffix" + Reminder.reset_table_name + assert_equal "prefix_active_record_internal_metadatas_suffix", ActiveRecord::InternalMetadata.table_name + ActiveRecord::Base.internal_metadata_table_name = "changed" + Reminder.reset_table_name + assert_equal "prefix_changed_suffix", ActiveRecord::InternalMetadata.table_name + ActiveRecord::Base.table_name_prefix = "" + ActiveRecord::Base.table_name_suffix = "" + Reminder.reset_table_name + assert_equal "changed", ActiveRecord::InternalMetadata.table_name + ensure + ActiveRecord::Base.internal_metadata_table_name = original_internal_metadata_table_name + Reminder.reset_table_name + end + + def test_internal_metadata_stores_environment + current_env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call + migrations_path = MIGRATIONS_ROOT + "/valid" + old_path = ActiveRecord::Migrator.migrations_paths + ActiveRecord::Migrator.migrations_paths = migrations_path + + assert_equal current_env, ActiveRecord::InternalMetadata.value_for("environment") + + original_rails_env = ENV["RAILS_ENV"] + original_rack_env = ENV["RACK_ENV"] + ENV["RAILS_ENV"] = ENV["RACK_ENV"] = "foofoo" + new_env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call + + refute_equal current_env, new_env + + sleep 1 # mysql by default does not store fractional seconds in the database + ActiveRecord::Migrator.up(migrations_path) + assert_equal new_env, ActiveRecord::InternalMetadata.value_for("environment") + ensure + ActiveRecord::Migrator.migrations_paths = old_path + ENV["RAILS_ENV"] = original_rails_env + ENV["RACK_ENV"] = original_rack_env + end + def test_proper_table_name_on_migration reminder_class = new_isolated_reminder_class migration = ActiveRecord::Migration.new diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index 9a5e4313d8..97c3d664ba 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -38,6 +38,7 @@ class SchemaDumperTest < ActiveRecord::TestCase assert_match %r{create_table "accounts"}, output assert_match %r{create_table "authors"}, output assert_no_match %r{create_table "schema_migrations"}, output + assert_no_match %r{create_table "internal_metadatas"}, output end def test_schema_dump_uses_force_cascade_on_create_table @@ -158,6 +159,7 @@ class SchemaDumperTest < ActiveRecord::TestCase assert_no_match %r{create_table "accounts"}, output assert_match %r{create_table "authors"}, output assert_no_match %r{create_table "schema_migrations"}, output + assert_no_match %r{create_table "internal_metadatas"}, output end def test_schema_dump_with_regexp_ignored_table @@ -165,6 +167,7 @@ class SchemaDumperTest < ActiveRecord::TestCase assert_no_match %r{create_table "accounts"}, output assert_match %r{create_table "authors"}, output assert_no_match %r{create_table "schema_migrations"}, output + assert_no_match %r{create_table "internal_metadatas"}, output end def test_schema_dumps_index_columns_in_right_order @@ -342,6 +345,7 @@ class SchemaDumperTest < ActiveRecord::TestCase assert_no_match %r{create_table "foo_.+_bar"}, output assert_no_match %r{add_index "foo_.+_bar"}, output assert_no_match %r{create_table "schema_migrations"}, output + assert_no_match %r{create_table "internal_metadatas"}, output if ActiveRecord::Base.connection.supports_foreign_keys? assert_no_match %r{add_foreign_key "foo_.+_bar"}, output diff --git a/activerecord/test/cases/tasks/database_tasks_test.rb b/activerecord/test/cases/tasks/database_tasks_test.rb index c8f4179313..e91a222534 100644 --- a/activerecord/test/cases/tasks/database_tasks_test.rb +++ b/activerecord/test/cases/tasks/database_tasks_test.rb @@ -18,6 +18,34 @@ module ActiveRecord sqlite3: :sqlite_tasks } + class DatabaseTasksUtilsTask< ActiveRecord::TestCase + def test_raises_an_error_when_called_with_protected_environment + ActiveRecord::Migrator.stubs(:current_version).returns(1) + + protected_environments = ActiveRecord::Base.protected_environments.dup + current_env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call + assert !protected_environments.include?(current_env) + # Assert no error + ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! + + ActiveRecord::Base.protected_environments << current_env + assert_raise(ActiveRecord::ProtectedEnvironmentError) do + ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! + end + ensure + ActiveRecord::Base.protected_environments = protected_environments + end + + def test_raises_an_error_if_no_migrations_have_been_made + ActiveRecord::InternalMetadata.stubs(:table_exists?).returns(false) + ActiveRecord::Migrator.stubs(:current_version).returns(1) + + assert_raise(ActiveRecord::NoEnvironmentInSchemaError) do + ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! + end + end + end + class DatabaseTasksRegisterTask < ActiveRecord::TestCase def test_register_task klazz = Class.new do -- cgit v1.2.3 From de2cb20117af68cef4126d8998cc63e178c58187 Mon Sep 17 00:00:00 2001 From: schneems Date: Fri, 8 Jan 2016 09:27:25 -0600 Subject: Use hash like syntax for InternalMetadata Discussion: https://github.com/rails/rails/pull/22967#discussion_r49137035 --- activerecord/test/cases/migration_test.rb | 4 ++-- activerecord/test/cases/tasks/database_tasks_test.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'activerecord/test') diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 2f7859a7be..bd60b64ee2 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -380,7 +380,7 @@ class MigrationTest < ActiveRecord::TestCase old_path = ActiveRecord::Migrator.migrations_paths ActiveRecord::Migrator.migrations_paths = migrations_path - assert_equal current_env, ActiveRecord::InternalMetadata.value_for("environment") + assert_equal current_env, ActiveRecord::InternalMetadata[:environment] original_rails_env = ENV["RAILS_ENV"] original_rack_env = ENV["RACK_ENV"] @@ -391,7 +391,7 @@ class MigrationTest < ActiveRecord::TestCase sleep 1 # mysql by default does not store fractional seconds in the database ActiveRecord::Migrator.up(migrations_path) - assert_equal new_env, ActiveRecord::InternalMetadata.value_for("environment") + assert_equal new_env, ActiveRecord::InternalMetadata[:environment] ensure ActiveRecord::Migrator.migrations_paths = old_path ENV["RAILS_ENV"] = original_rails_env diff --git a/activerecord/test/cases/tasks/database_tasks_test.rb b/activerecord/test/cases/tasks/database_tasks_test.rb index e91a222534..326373791c 100644 --- a/activerecord/test/cases/tasks/database_tasks_test.rb +++ b/activerecord/test/cases/tasks/database_tasks_test.rb @@ -23,7 +23,7 @@ module ActiveRecord ActiveRecord::Migrator.stubs(:current_version).returns(1) protected_environments = ActiveRecord::Base.protected_environments.dup - current_env = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call + current_env = ActiveRecord::Migrator.current_environment assert !protected_environments.include?(current_env) # Assert no error ActiveRecord::Tasks::DatabaseTasks.check_protected_environments! -- cgit v1.2.3 From d70c68d76abcbc24ef0e56b7a7b580d0013255dd Mon Sep 17 00:00:00 2001 From: schneems Date: Fri, 8 Jan 2016 11:53:25 -0600 Subject: Fixing tests and re-locating error checking. --- activerecord/test/cases/schema_dumper_test.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'activerecord/test') diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index 97c3d664ba..2006f40b25 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -38,7 +38,7 @@ class SchemaDumperTest < ActiveRecord::TestCase assert_match %r{create_table "accounts"}, output assert_match %r{create_table "authors"}, output assert_no_match %r{create_table "schema_migrations"}, output - assert_no_match %r{create_table "internal_metadatas"}, output + assert_no_match %r{create_table "active_record_internal_metadatas"}, output end def test_schema_dump_uses_force_cascade_on_create_table @@ -159,7 +159,7 @@ class SchemaDumperTest < ActiveRecord::TestCase assert_no_match %r{create_table "accounts"}, output assert_match %r{create_table "authors"}, output assert_no_match %r{create_table "schema_migrations"}, output - assert_no_match %r{create_table "internal_metadatas"}, output + assert_no_match %r{create_table "active_record_internal_metadatas"}, output end def test_schema_dump_with_regexp_ignored_table @@ -167,7 +167,7 @@ class SchemaDumperTest < ActiveRecord::TestCase assert_no_match %r{create_table "accounts"}, output assert_match %r{create_table "authors"}, output assert_no_match %r{create_table "schema_migrations"}, output - assert_no_match %r{create_table "internal_metadatas"}, output + assert_no_match %r{create_table "active_record_internal_metadatas"}, output end def test_schema_dumps_index_columns_in_right_order @@ -345,7 +345,7 @@ class SchemaDumperTest < ActiveRecord::TestCase assert_no_match %r{create_table "foo_.+_bar"}, output assert_no_match %r{add_index "foo_.+_bar"}, output assert_no_match %r{create_table "schema_migrations"}, output - assert_no_match %r{create_table "internal_metadatas"}, output + assert_no_match %r{create_table "active_record_internal_metadatas"}, output if ActiveRecord::Base.connection.supports_foreign_keys? assert_no_match %r{add_foreign_key "foo_.+_bar"}, output -- cgit v1.2.3