-
Notifications
You must be signed in to change notification settings - Fork 14.4k
Bump rails version to 7.2 [WIP] #20109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
b44b947
to
690c6a7
Compare
f971b6b
to
3883404
Compare
85fcc2b
to
924fbef
Compare
924fbef
to
c2fdef7
Compare
config/application.rb
Outdated
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coud we see what list of tests fail here? We might be able to update the tests instead here, and keep the option enabled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)>'
```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
managed to sort these out
@@ -16,7 +16,7 @@ def creds(opts) | |||
query = query.includes(logins: [ :service, { service: :host } ]) | |||
|
|||
if opts[:type].present? | |||
query = query.where('"metasploit_credential_privates"."type" = ?', opts[:type]) | |||
query = query.where('"metasploit_credential_privates"."type" = ?', opts[:type].to_s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
40a3583
to
aa76c01
Compare
aa76c01
to
f85f661
Compare
@@ -36,6 +35,9 @@ | |||
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) | |||
Timecop.freeze(Time.parse('Jul 15, 2022 12:33:40.000000000 GMT')) | |||
|
|||
subject.run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
subject.run | |
Timecop.freeze(Time.parse('Jul 15, 2022 12:33:40.000000000 GMT')) do | |
subject.run | |
end |
@@ -36,6 +35,9 @@ | |||
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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed? 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
allow(db_manager).to receive(:active).and_return(active) | ||
db_manager.workspace = db_manager.default_workspace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any context on why these lines needed to be switched around?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming renaming this file will break assumptions in the test? 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to use timecop here to get it to work as it did previously? Or I might be missing a nuance still 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 😅
969d08c
to
be51583
Compare
initial attempt for upgrading Rails to 7.2