Upgrade the gem to be rails 8.0 compatible#225
Conversation
There was a problem hiding this comment.
Pull request overview
Updates metasploit_data_models to be compatible with Rails 8.0 by relaxing Rails-related gem constraints and adjusting code paths that depend on ActiveRecord’s serialization API differences across versions.
Changes:
- Widen Rails/ActiveRecord/ActiveSupport/Railties dependency constraints to allow Rails 7.x through 8.0.
- Update
serializeversion-gating logic to treat Rails/ActiveRecord 8.0 as “>= 7.1” behavior. - Adjust the dummy Rails app configuration for Rails 8 connection handling.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| spec/dummy/config/application.rb | Enables new ActiveRecord connection handling in the dummy app. |
| metasploit_data_models.gemspec | Widens Rails-related runtime dependency constraints (and adjusts metasploit-model range). |
| Gemfile | Allows the test/dummy app to bundle with Rails 7.x through 8.0. |
| db/migrate/20110422000000_convert_binary.rb | Fixes ActiveRecord version checks so AR 8.0 uses the newer serialize API. |
| db/migrate/20110317144932_add_session_table.rb | Same AR version-check fix for migration-local models using serialize. |
| app/models/mdm/web_vuln.rb | Fixes AR version checks so AR 8.0 uses the newer serialize API. |
| app/models/mdm/web_site.rb | Same AR version-check fix for serialize. |
| app/models/mdm/web_page.rb | Same AR version-check fix for serialize on multiple attributes. |
| app/models/mdm/web_form.rb | Same AR version-check fix for serialize. |
| app/models/mdm/user.rb | Same AR version-check fix for serialize. |
| app/models/mdm/task.rb | Same AR version-check fix for serialize on multiple attributes. |
| app/models/mdm/session.rb | Same AR version-check fix for serialize. |
| app/models/mdm/profile.rb | Same AR version-check fix for serialize. |
| app/models/mdm/payload.rb | Same AR version-check fix for serialize on multiple attributes. |
| app/models/mdm/note.rb | Same AR version-check fix for serialize. |
| app/models/mdm/nexpose_console.rb | Same AR version-check fix for serialize. |
| app/models/mdm/macro.rb | Same AR version-check fix for serialize on multiple attributes. |
| app/models/mdm/loot.rb | Same AR version-check fix for serialize. |
| app/models/mdm/listener.rb | Same AR version-check fix for serialize. |
| app/models/mdm/event.rb | Same AR version-check fix for serialize. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| s.add_runtime_dependency 'metasploit-concern' | ||
| s.add_runtime_dependency 'metasploit-model', '~>5.0.4' | ||
| s.add_runtime_dependency 'railties', '~>7.0' | ||
| s.add_runtime_dependency 'metasploit-model', '>= 5.0', '< 7.0' |
There was a problem hiding this comment.
The metasploit-model runtime dependency was widened from ~> 5.0.4 to >= 5.0, < 7.0, which both lowers the minimum allowed version (to 5.0.0) and allows the entire 6.x series. If the gem relies on fixes introduced in 5.0.4+ (or if 6.x has breaking changes), this can cause unexpected resolution/runtime issues. Consider keeping the previous minimum (e.g., >= 5.0.4) and only widening the upper bound as needed, or document/test against the newly-allowed major range.
| s.add_runtime_dependency 'metasploit-model', '>= 5.0', '< 7.0' | |
| s.add_runtime_dependency 'metasploit-model', '>= 5.0.4', '< 7.0' |
9dfb38e to
f868742
Compare
f868742 to
1c03da0
Compare
| # | ||
| # @return [Hash] | ||
| if ActiveRecord::VERSION::MAJOR >= 7 && ActiveRecord::VERSION::MINOR >= 1 | ||
| if ActiveRecord::VERSION::MAJOR > 7 || (ActiveRecord::VERSION::MAJOR == 7 && ActiveRecord::VERSION::MINOR >= 1) |
There was a problem hiding this comment.
Is this code change needed? 🤔
yes, to handle 8.0 - which would have the minor as 0 👍
1506732 to
a145e62
Compare
c2a7817 to
3bc98a3
Compare
3bc98a3 to
a15bc62
Compare
Upgrade the gem to be rails 8.0 compatible