Skip to content

Commit 06b7e34

Browse files
Add active_record.config.validate_migration_timestamps config option.
When set, an `ActiveRecord::InvalidMigrationTimestampError` will be raised if the timestamp prefix for a migration is more than a day ahead of the timestamp associated with the current time. This is done to prevent forward-dating of migration files, which can impact migration generation and other migration commands. It is turned off by default, but will be turned on for applications starting in Rails 7.2.
1 parent 257c630 commit 06b7e34

File tree

11 files changed

+169
-8
lines changed

11 files changed

+169
-8
lines changed

activerecord/CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1+
* Add `active_record.config.validate_migration_timestamps` option for validating migration timestamps.
2+
3+
When set, validates that the timestamp prefix for a migration is no more than a day ahead of
4+
the timestamp associated with the current time. This is designed to prevent migrations prefixes
5+
from being hand-edited to future timestamps, which impacts migration generation and other
6+
migration commands.
7+
8+
*Adrianna Chang*
9+
110
* Properly synchronize `Mysql2Adapter#active?` and `TrilogyAdapter#active?`
211

312
As well as `disconnect!` and `verify!`.

activerecord/lib/active_record.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,14 @@ def self.global_executor_concurrency # :nodoc:
380380
singleton_class.attr_accessor :timestamped_migrations
381381
self.timestamped_migrations = true
382382

383+
##
384+
# :singleton-method: validate_migration_timestamps
385+
# Specify whether or not to validate migration timestamps. When set, an error
386+
# will be raised if a timestamp is more than a day ahead of the timestamp
387+
# associated with the current time. +timestamped_migrations+ must be set to true.
388+
singleton_class.attr_accessor :validate_migration_timestamps
389+
self.validate_migration_timestamps = false
390+
383391
##
384392
# :singleton-method: migration_strategy
385393
# Specify strategy to use for executing migrations.

activerecord/lib/active_record/migration.rb

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,22 @@ def initialize(name = nil)
131131
end
132132
end
133133

134+
class InvalidMigrationTimestampError < MigrationError # :nodoc:
135+
def initialize(version = nil, name = nil)
136+
if version && name
137+
super(<<~MSG)
138+
Invalid timestamp #{version} for migration file: #{name}.
139+
Timestamp must be in form YYYYMMDDHHMMSS, and less than #{(Time.now.utc + 1.day).strftime("%Y%m%d%H%M%S")}.
140+
MSG
141+
else
142+
super(<<~MSG)
143+
Invalid timestamp for migration.
144+
Timestamp must be in form YYYYMMDDHHMMSS, and less than #{(Time.now.utc + 1.day).strftime("%Y%m%d%H%M%S")}.
145+
MSG
146+
end
147+
end
148+
end
149+
134150
class PendingMigrationError < MigrationError # :nodoc:
135151
include ActiveSupport::ActionableError
136152

@@ -494,7 +510,11 @@ def initialize
494510
#
495511
# 20080717013526_your_migration_name.rb
496512
#
497-
# The prefix is a generation timestamp (in UTC).
513+
# The prefix is a generation timestamp (in UTC). Timestamps should not be
514+
# modified manually. To validate that migration timestamps adhere to the
515+
# format Active Record expects, you can use the following configuration option:
516+
#
517+
# config.active_record.validate_migration_timestamps = true
498518
#
499519
# If you'd prefer to use numeric prefixes, you can turn timestamped migrations
500520
# off by setting:
@@ -1316,6 +1336,9 @@ def migrations # :nodoc:
13161336
migrations = migration_files.map do |file|
13171337
version, name, scope = parse_migration_filename(file)
13181338
raise IllegalMigrationNameError.new(file) unless version
1339+
if validate_timestamp? && !valid_migration_timestamp?(version)
1340+
raise InvalidMigrationTimestampError.new(version, name)
1341+
end
13191342
version = version.to_i
13201343
name = name.camelize
13211344

@@ -1331,6 +1354,9 @@ def migrations_status # :nodoc:
13311354
file_list = migration_files.filter_map do |file|
13321355
version, name, scope = parse_migration_filename(file)
13331356
raise IllegalMigrationNameError.new(file) unless version
1357+
if validate_timestamp? && !valid_migration_timestamp?(version)
1358+
raise InvalidMigrationTimestampError.new(version, name)
1359+
end
13341360
version = schema_migration.normalize_migration_number(version)
13351361
status = db_list.delete(version) ? "up" : "down"
13361362
[status, version, (name + scope).humanize]
@@ -1375,6 +1401,14 @@ def parse_migration_filename(filename)
13751401
File.basename(filename).scan(Migration::MigrationFilenameRegexp).first
13761402
end
13771403

1404+
def validate_timestamp?
1405+
ActiveRecord.timestamped_migrations && ActiveRecord.validate_migration_timestamps
1406+
end
1407+
1408+
def valid_migration_timestamp?(version)
1409+
version.to_i < (Time.now.utc + 1.day).strftime("%Y%m%d%H%M%S").to_i
1410+
end
1411+
13781412
def move(direction, steps)
13791413
migrator = Migrator.new(direction, migrations, schema_migration, internal_metadata)
13801414

activerecord/test/cases/migration_test.rb

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1807,6 +1807,8 @@ def setup
18071807
@verbose_was, ActiveRecord::Migration.verbose = ActiveRecord::Migration.verbose, false
18081808
@schema_migration = ActiveRecord::Base.connection.schema_migration
18091809
@internal_metadata = ActiveRecord::Base.connection.internal_metadata
1810+
@active_record_validate_timestamps_was = ActiveRecord.validate_migration_timestamps
1811+
ActiveRecord.validate_migration_timestamps = true
18101812
ActiveRecord::Base.connection.schema_cache.clear!
18111813

18121814
@migrations_path = MIGRATIONS_ROOT + "/temp"
@@ -1816,15 +1818,69 @@ def setup
18161818
def teardown
18171819
@schema_migration.create_table
18181820
@schema_migration.delete_all_versions
1821+
ActiveRecord.validate_migration_timestamps = @active_record_validate_timestamps_was
18191822
ActiveRecord::Migration.verbose = @verbose_was
18201823
end
18211824

1825+
def test_migration_raises_if_timestamp_greater_than_14_digits
1826+
with_temp_migration_files(["201801010101010000_test_migration.rb"]) do
1827+
error = assert_raises(ActiveRecord::InvalidMigrationTimestampError) do
1828+
@migrator.up(201801010101010000)
1829+
end
1830+
assert_match(/Invalid timestamp 201801010101010000 for migration file: test_migration/, error.message)
1831+
end
1832+
end
1833+
1834+
def test_migration_raises_if_timestamp_is_future_date
1835+
timestamp = (Time.now.utc + 1.month).strftime("%Y%m%d%H%M%S").to_i
1836+
with_temp_migration_files(["#{timestamp}_test_migration.rb"]) do
1837+
error = assert_raises(ActiveRecord::InvalidMigrationTimestampError) do
1838+
@migrator.up(timestamp)
1839+
end
1840+
assert_match(/Invalid timestamp #{timestamp} for migration file: test_migration/, error.message)
1841+
end
1842+
end
1843+
1844+
def test_migration_succeeds_if_timestamp_is_less_than_one_day_in_the_future
1845+
timestamp = (Time.now.utc + 1.minute).strftime("%Y%m%d%H%M%S").to_i
1846+
with_temp_migration_files(["#{timestamp}_test_migration.rb"]) do
1847+
@migrator.up(timestamp)
1848+
assert_equal timestamp, @migrator.current_version
1849+
end
1850+
end
1851+
1852+
def test_migration_succeeds_despite_future_timestamp_if_validate_timestamps_is_false
1853+
validate_migration_timestamps_was = ActiveRecord.validate_migration_timestamps
1854+
ActiveRecord.validate_migration_timestamps = false
1855+
1856+
timestamp = (Time.now.utc + 1.month).strftime("%Y%m%d%H%M%S").to_i
1857+
with_temp_migration_files(["#{timestamp}_test_migration.rb"]) do
1858+
@migrator.up(timestamp)
1859+
assert_equal timestamp, @migrator.current_version
1860+
end
1861+
ensure
1862+
ActiveRecord.validate_migration_timestamps = validate_migration_timestamps_was
1863+
end
1864+
1865+
def test_migration_succeeds_despite_future_timestamp_if_timestamped_migrations_is_false
1866+
timestamped_migrations_was = ActiveRecord.timestamped_migrations
1867+
ActiveRecord.timestamped_migrations = false
1868+
1869+
timestamp = (Time.now.utc + 1.month).strftime("%Y%m%d%H%M%S").to_i
1870+
with_temp_migration_files(["#{timestamp}_test_migration.rb"]) do
1871+
@migrator.up(timestamp)
1872+
assert_equal timestamp, @migrator.current_version
1873+
end
1874+
ensure
1875+
ActiveRecord.timestamped_migrations = timestamped_migrations_was
1876+
end
1877+
18221878
def test_copied_migrations_at_timestamp_boundary_are_valid
18231879
migrations_path_source = MIGRATIONS_ROOT + "/temp_source"
18241880
migrations_path_dest = MIGRATIONS_ROOT + "/temp_dest"
18251881
migrations = ["20180101010101_test_migration.rb", "20180101010102_test_migration_two.rb", "20180101010103_test_migration_three.rb"]
18261882

1827-
with_temp_migration_files(migrations_path_source, migrations) do
1883+
with_temp_migration_files(migrations, migrations_path_source) do
18281884
travel_to(Time.utc(2023, 12, 1, 10, 10, 59)) do
18291885
ActiveRecord::Migration.copy(migrations_path_dest, temp: migrations_path_source)
18301886

@@ -1847,7 +1903,7 @@ def test_copied_migrations_at_timestamp_boundary_are_valid
18471903
end
18481904

18491905
private
1850-
def with_temp_migration_files(migrations_dir, filenames)
1906+
def with_temp_migration_files(filenames, migrations_dir = @migrations_path)
18511907
Dir.mkdir(migrations_dir) unless Dir.exist?(migrations_dir)
18521908

18531909
paths = []

guides/source/configuring.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ NOTE: If you need to apply configuration directly to a class, use a [lazy load h
5858

5959
Below are the default values associated with each target version. In cases of conflicting values, newer versions take precedence over older versions.
6060

61+
#### Default Values for Target Version 7.2
62+
63+
- [`config.active_record.validate_migration_timestamps`](#config-active-record-validate-migration-timestamps): `true`
64+
6165
#### Default Values for Target Version 7.1
6266

6367
- [`config.action_dispatch.debug_exception_log_level`](#config-action-dispatch-debug-exception-log-level): `:error`
@@ -1048,6 +1052,20 @@ Specifies if an error should be raised if the order of a query is ignored during
10481052
10491053
Controls whether migrations are numbered with serial integers or with timestamps. The default is `true`, to use timestamps, which are preferred if there are multiple developers working on the same application.
10501054
1055+
#### `config.active_record.validate_migration_timestamps`
1056+
1057+
Controls whether to validate migration timestamps. When set, an error will be raised if the
1058+
timestamp prefix for a migration is more than a day ahead of the timestamp associated with the
1059+
current time. This is done to prevent forward-dating of migration files, which can impact migration
1060+
generation and other migration commands. `config.active_record.timestamped_migrations` must be set to `true`.
1061+
1062+
The default value depends on the `config.load_defaults` target version:
1063+
1064+
| Starting with version | The default value is |
1065+
| --------------------- | -------------------- |
1066+
| (original) | `false` |
1067+
| 7.2 | `true` |
1068+
10511069
#### `config.active_record.db_warnings_action`
10521070
10531071
Controls the action to be taken when an SQL query produces a warning. The following options are available:

railties/lib/rails/application/configuration.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,10 @@ def load_defaults(target_version)
323323
end
324324
when "7.2"
325325
load_defaults "7.1"
326+
327+
if respond_to?(:active_record)
328+
active_record.validate_migration_timestamps = true
329+
end
326330
else
327331
raise "Unknown version #{target_version.to_s.inspect}"
328332
end

railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_2.rb.tt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,14 @@
88
#
99
# Read the Guide for Upgrading Ruby on Rails for more info on each option.
1010
# https://guides.rubyonrails.org/upgrading_ruby_on_rails.html
11+
12+
###
13+
# Enable validation of migration timestamps. When set, an ActiveRecord::InvalidMigrationTimestampError
14+
# will be raised if the timestamp prefix for a migration is more than a day ahead of the timestamp
15+
# associated with the current time. This is done to prevent forward-dating of migration files, which can
16+
# impact migration generation and other migration commands.
17+
#
18+
# Applications with existing timestamped migrations that do not adhere to the
19+
# expected format can disable validation by setting this config to `false`.
20+
#++
21+
# Rails.application.config.active_record.validate_migration_timestamps = true

railties/test/application/loading_test.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@ class User
389389
test "columns migrations also trigger reloading" do
390390
add_to_config <<-RUBY
391391
config.enable_reloading = true
392+
config.active_record.timestamped_migrations = false
392393
RUBY
393394

394395
app_file "config/routes.rb", <<-RUBY

railties/test/application/rake/migrations_test.rb

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ class RakeMigrationsTest < ActiveSupport::TestCase
88
def setup
99
build_app
1010
FileUtils.rm_rf("#{app_path}/config/environments")
11+
add_to_config("config.active_record.timestamped_migrations = false")
1112
end
1213

1314
def teardown
@@ -17,7 +18,7 @@ def teardown
1718
test "running migrations with given scope" do
1819
rails "generate", "model", "user", "username:string", "password:string"
1920

20-
app_file "db/migrate/01_a_migration.bukkits.rb", <<-MIGRATION
21+
app_file "db/migrate/02_a_migration.bukkits.rb", <<-MIGRATION
2122
class AMigration < ActiveRecord::Migration::Current
2223
end
2324
MIGRATION
@@ -200,6 +201,8 @@ class TwoMigration < ActiveRecord::Migration::Current
200201
end
201202

202203
test "migration status" do
204+
remove_from_config("config.active_record.timestamped_migrations = false")
205+
203206
rails "generate", "model", "user", "username:string", "password:string"
204207
rails "generate", "migration", "add_email_to_users", "email:string"
205208
rails "db:migrate"
@@ -217,7 +220,7 @@ class TwoMigration < ActiveRecord::Migration::Current
217220
end
218221

219222
test "migration status without timestamps" do
220-
add_to_config("config.active_record.timestamped_migrations = false")
223+
remove_from_config("config.active_record.timestamped_migrations = false")
221224

222225
rails "generate", "model", "user", "username:string", "password:string"
223226
rails "generate", "migration", "add_email_to_users", "email:string"
@@ -236,6 +239,8 @@ class TwoMigration < ActiveRecord::Migration::Current
236239
end
237240

238241
test "migration status after rollback and redo" do
242+
remove_from_config("config.active_record.timestamped_migrations = false")
243+
239244
rails "generate", "model", "user", "username:string", "password:string"
240245
rails "generate", "migration", "add_email_to_users", "email:string"
241246
rails "db:migrate"
@@ -259,6 +264,8 @@ class TwoMigration < ActiveRecord::Migration::Current
259264
end
260265

261266
test "migration status after rollback and forward" do
267+
remove_from_config("config.active_record.timestamped_migrations = false")
268+
262269
rails "generate", "model", "user", "username:string", "password:string"
263270
rails "generate", "migration", "add_email_to_users", "email:string"
264271
rails "db:migrate"
@@ -283,6 +290,8 @@ class TwoMigration < ActiveRecord::Migration::Current
283290

284291
test "raise error on any move when current migration does not exist" do
285292
Dir.chdir(app_path) do
293+
remove_from_config("config.active_record.timestamped_migrations = false")
294+
286295
rails "generate", "model", "user", "username:string", "password:string"
287296
rails "generate", "migration", "add_email_to_users", "email:string"
288297
rails "db:migrate"
@@ -382,8 +391,6 @@ class TwoMigration < ActiveRecord::Migration::Current
382391
end
383392

384393
test "migration status after rollback and redo without timestamps" do
385-
add_to_config("config.active_record.timestamped_migrations = false")
386-
387394
rails "generate", "model", "user", "username:string", "password:string"
388395
rails "generate", "migration", "add_email_to_users", "email:string"
389396
rails "db:migrate"
@@ -497,6 +504,8 @@ class TwoMigration < ActiveRecord::Migration::Current
497504

498505
test "migration status migrated file is deleted" do
499506
Dir.chdir(app_path) do
507+
remove_from_config("config.active_record.timestamped_migrations = false")
508+
500509
rails "generate", "model", "user", "username:string", "password:string"
501510
rails "generate", "migration", "add_email_to_users", "email:string"
502511
rails "db:migrate"
@@ -523,7 +532,7 @@ class ApplicationRecord < ActiveRecord::Base
523532

524533
rails("db:migrate")
525534

526-
app_file "db/migrate/01_a_migration.bukkits.rb", <<-MIGRATION
535+
app_file "db/migrate/02_a_migration.bukkits.rb", <<-MIGRATION
527536
class AMigration < ActiveRecord::Migration::Current
528537
def change
529538
User.first

railties/test/application/rake/multi_dbs_test.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ class RakeMultiDbsTest < ActiveSupport::TestCase
1010
def setup
1111
build_app(multi_db: true)
1212
FileUtils.rm_rf("#{app_path}/config/environments")
13+
add_to_config("config.active_record.timestamped_migrations = false")
1314
end
1415

1516
def teardown
@@ -779,11 +780,13 @@ class TwoMigration < ActiveRecord::Migration::Current
779780
end
780781

781782
test "db:migrate:status works on all databases" do
783+
remove_from_config("config.active_record.timestamped_migrations = false")
782784
require "#{app_path}/config/environment"
783785
db_migrate_and_migrate_status
784786
end
785787

786788
test "db:migrate:status:namespace works" do
789+
remove_from_config("config.active_record.timestamped_migrations = false")
787790
require "#{app_path}/config/environment"
788791
ActiveRecord::Base.configurations.configs_for(env_name: Rails.env).each do |db_config|
789792
db_migrate_namespaced db_config.name

0 commit comments

Comments
 (0)