Bump rails version to 7.2 #20109

dwelch-r7 · 2025-04-30T16:44:03Z

initial attempt for upgrading Rails to 7.2

adfoster-r7 · 2025-06-03T09:59:54Z

config/application.rb

+      if config.respond_to?(:active_record)
+        # Timecop in tests followed by db interaction causes a ActiveRecord::InvalidMigrationTimestampError
+        # due to the current time (set by timecop) being less than the migration timestamp.
+        config.active_record.validate_migration_timestamps = false


Coud we see what list of tests fail here? We might be able to update the tests instead here, and keep the option enabled

rspec './spec/lib/msf/db_manager_spec.rb[1:14:3:2]' # Msf::DBManager it should behave like Msf::DBManager::Migration #migrate should return an ActiveRecord::MigrationContext with known migrations rspec ./spec/lib/msf/ui/console/command_dispatcher/core_spec.rb:657 # Msf::Ui::Console::CommandDispatcher::Core#cmd_sessions Searches with sessions that have different checkins and types When the user specifies both type and checkin rspec ./spec/lib/msf/ui/console/command_dispatcher/core_spec.rb:669 # Msf::Ui::Console::CommandDispatcher::Core#cmd_sessions Searches with sessions that have different checkins and types When the user specifies both type and checkin but there are only partial matches rspec ./spec/lib/msf/ui/console/command_dispatcher/core_spec.rb:573 # Msf::Ui::Console::CommandDispatcher::Core#cmd_sessions searches with sessions with different checkin values When the user searches using fractions of a minute rspec ./spec/lib/msf/ui/console/command_dispatcher/core_spec.rb:586 # Msf::Ui::Console::CommandDispatcher::Core#cmd_sessions searches with sessions with different checkin values When the user searches using capital letters rspec ./spec/lib/msf/ui/console/command_dispatcher/core_spec.rb:560 # Msf::Ui::Console::CommandDispatcher::Core#cmd_sessions searches with sessions with different checkin values When the user searches using multiple units with fractional seconds rspec ./spec/lib/msf/ui/console/command_dispatcher/core_spec.rb:547 # Msf::Ui::Console::CommandDispatcher::Core#cmd_sessions searches with sessions with different checkin values When the user searches using fractions of a second rspec ./spec/lib/msf/ui/console/command_dispatcher/core_spec.rb:632 # Msf::Ui::Console::CommandDispatcher::Core#cmd_sessions searches with sessions with different checkin values When the user uses two before arguments with last checkin rspec ./spec/lib/msf/ui/console/command_dispatcher/core_spec.rb:613 # Msf::Ui::Console::CommandDispatcher::Core#cmd_sessions searches with sessions with different checkin values When the user properly specifies both less_than and greater_than checkin ranges rspec ./spec/lib/msf/ui/console/command_dispatcher/core_spec.rb:599 # Msf::Ui::Console::CommandDispatcher::Core#cmd_sessions searches with sessions with different checkin values When the user searches using an invalid checkin parameter rspec ./spec/lib/msf/ui/console/command_dispatcher/core_spec.rb:625 # Msf::Ui::Console::CommandDispatcher::Core#cmd_sessions searches with sessions with different checkin values When the user specifies a greater_than time that is larger than the less_than time rspec ./spec/lib/msf/ui/console/command_dispatcher/core_spec.rb:606 # Msf::Ui::Console::CommandDispatcher::Core#cmd_sessions searches with sessions with different checkin values When the user searches using duplicated time units rspec ./spec/modules/auxiliary/admin/kerberos/forge_ticket_spec.rb:28 # kerberos keytab #run when forging golden tickets generates a golden ticket

here ya go

13) kerberos keytab #run when forging golden tickets generates a golden ticket Failure/Error: return context.needs_migration? ActiveRecord::InvalidMigrationTimestampError: Invalid timestamp 20221209005658 for migration file: create_index_on_private_data_and_type_for_pkcs12. Timestamp must be in form YYYYMMDDHHMMSS, and less than 20220716123340. # /Users/dwelch/.rvm/gems/ruby-3.2.8@metasploit-framework/gems/activerecord-7.2.2.1/lib/active_record/migration.rb:1311:in `block in migrations' # /Users/dwelch/.rvm/gems/ruby-3.2.8@metasploit-framework/gems/activerecord-7.2.2.1/lib/active_record/migration.rb:1307:in `map' # /Users/dwelch/.rvm/gems/ruby-3.2.8@metasploit-framework/gems/activerecord-7.2.2.1/lib/active_record/migration.rb:1307:in `migrations' # /Users/dwelch/.rvm/gems/ruby-3.2.8@metasploit-framework/gems/activerecord-7.2.2.1/lib/active_record/migration.rb:1303:in `pending_migration_versions' # /Users/dwelch/.rvm/gems/ruby-3.2.8@metasploit-framework/gems/activerecord-7.2.2.1/lib/active_record/migration.rb:1299:in `needs_migration?' # ./lib/msf/core/db_manager/migration.rb:57:in `block in needs_migration?' # ./lib/msf/core/db_manager/migration.rb:69:in `with_migration_context' # ./lib/msf/core/db_manager/migration.rb:56:in `needs_migration?' # ./lib/msf/core/db_manager/connection.rb:7:in `active' # ./lib/metasploit/framework/data_service/proxy/core.rb:45:in `active' # ./lib/msf/core/auxiliary/report.rb:84:in `db' # ./lib/msf/core/auxiliary/report.rb:419:in `store_loot' # ./lib/msf/core/exploit/remote/kerberos/ticket/storage/write_mixin.rb:26:in `store_ccache' # ./lib/msf/core/exploit/remote/kerberos/ticket/storage.rb:11:in `store_ccache' # ./modules/auxiliary/admin/kerberos/forge_ticket.rb:121:in `forge_ccache' # ./modules/auxiliary/admin/kerberos/forge_ticket.rb:142:in `forge_golden' # ./modules/auxiliary/admin/kerberos/forge_ticket.rb:86:in `run' # ./spec/modules/auxiliary/admin/kerberos/forge_ticket_spec.rb:39:in `block (4 levels) in <top (required)>' ```

managed to sort these out

adfoster-r7 · 2025-06-03T10:01:55Z

lib/msf/core/db_manager/cred.rb


      if opts[:type].present?
-        query = query.where('"metasploit_credential_privates"."type" = ?', opts[:type])
+        query = query.where('"metasploit_credential_privates"."type" = ?', opts[:type].to_s)


Fixing a type coercion, where opts[:type] was a real Ruby object when it was expecting a string or primitive value - which broke Rails. Forcing .to_s ensures the string value is used

adfoster-r7 · 2025-06-12T09:35:53Z

spec/modules/auxiliary/admin/kerberos/forge_ticket_spec.rb

+        expect(framework.db.active).to be(true)
+        Timecop.freeze(Time.parse('Jul 15, 2022 12:33:40.000000000 GMT'))
+
        subject.run


Suggested change

subject.run

Timecop.freeze(Time.parse('Jul 15, 2022 12:33:40.000000000 GMT')) do

subject.run

end

adfoster-r7 · 2025-06-12T09:36:07Z

spec/modules/auxiliary/admin/kerberos/forge_ticket_spec.rb

        subject.datastore['EXTRA_SIDS'] = ' S-1-18-1,  S-1-5-21-1266190811-2419310613-1856291569-519, '
        subject.datastore['SessionKey'] = 'A' * 16

+        expect(framework.db.active).to be(true)


Why is this needed? 👀

this one was a bit hacky, but if we trigger the check for the db being migrated before we set timecop then we don't need to re-check if the db has been migrated and so there's no issue anymore

adfoster-r7 · 2025-06-12T09:36:49Z

spec/support/shared/contexts/msf/db_manager.rb

    # already connected due to use_transactional_fixtures, but need some of the side-effects of #connect
-    db_manager.workspace = db_manager.default_workspace
    allow(db_manager).to receive(:active).and_return(active)
+    db_manager.workspace = db_manager.default_workspace


Any context on why these lines needed to be switched around?

calling off to default_workspace was triggering the migration check before the db_manager was mocked out which led to the migration date being in the future for timecopped tests, so I just swapped them round

adfoster-r7 · 2025-06-12T09:38:33Z

spec/file_fixtures/migrate/20250612131915_test_db_migration.rb

I'm assuming renaming this file will break assumptions in the test? 👀

Nope it gets the migration form the folder path and before we were getting this error

Invalid timestamp 99999999999999 for migration file: test_db_migration. Timestamp must be in form YYYYMMDDHHMMSS, and less than 20250612131915.

Do we need to use timecop here to get it to work as it did previously? Or I might be missing a nuance still 😄

uhhhh I actually don't know if that would work, the current date/time in the file format is invalid even if we set timecop to the year 10,000 I think it'd still complain since there is no 99th day of the 99th month 😅

dwelch-r7 force-pushed the rails-7.2-upgrade branch 3 times, most recently from b44b947 to 690c6a7 Compare May 6, 2025 16:12

dwelch-r7 force-pushed the rails-7.2-upgrade branch 3 times, most recently from f971b6b to 3883404 Compare May 20, 2025 11:31

dwelch-r7 force-pushed the rails-7.2-upgrade branch 5 times, most recently from 85fcc2b to 924fbef Compare May 28, 2025 12:30

dwelch-r7 force-pushed the rails-7.2-upgrade branch from 924fbef to c2fdef7 Compare June 2, 2025 10:28

adfoster-r7 reviewed Jun 3, 2025

View reviewed changes

dwelch-r7 force-pushed the rails-7.2-upgrade branch 3 times, most recently from aa76c01 to f85f661 Compare June 10, 2025 11:15

adfoster-r7 reviewed Jun 12, 2025

View reviewed changes

dwelch-r7 force-pushed the rails-7.2-upgrade branch from 969d08c to be51583 Compare June 12, 2025 09:48

dwelch-r7 added 5 commits June 12, 2025 16:41

Bump rails version to 7.2

b87ef99

Add rpc debugging

640d992

Minor bug fixes

62f3572

Remove debug output

4c2eeee

Fix migration date issues when running tests

39356d5

dwelch-r7 force-pushed the rails-7.2-upgrade branch from be51583 to 39356d5 Compare June 12, 2025 15:41

dwelch-r7 marked this pull request as ready for review June 17, 2025 09:04

adfoster-r7 approved these changes Jun 17, 2025

View reviewed changes

adfoster-r7 merged commit 04c368f into rapid7:master Jun 17, 2025
61 checks passed

dwelch-r7 changed the title ~~Bump rails version to 7.2 [WIP]~~ Bump rails version to 7.2 Jun 17, 2025

adfoster-r7 mentioned this pull request Jun 17, 2025

Revert "Bump rails version to 7.2" #20320

Merged

adfoster-r7 added the rn-no-release-notes no release notes label Jun 17, 2025

dwelch-r7 mentioned this pull request Jul 28, 2025

Bump rails to 7.2 #20420

Merged

Bump rails version to 7.2 #20109

Bump rails version to 7.2 #20109

Uh oh!

Conversation

dwelch-r7 commented Apr 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants