🚨 [security] Update rails 7.1.6 → 8.0.2.1 (major) #560
Conversation
depfu
bot
force-pushed
the
depfu/update/group/rails-8.0.2.1
branch
from
615e103 to
9cbb15f
Compare
mayorova
force-pushed
the
depfu/update/group/rails-8.0.2.1
branch
from
b3589df to
53c5665
Compare
depfu
bot
force-pushed
the
depfu/update/group/rails-8.0.2.1
branch
from
53c5665 to
1f70b27
Compare
depfu
bot
force-pushed
the
depfu/update/group/rails-8.0.2.1
branch
from
1f70b27 to
b58c09a
Compare
depfu
bot
changed the title
depfu
bot
force-pushed
the
depfu/update/group/rails-8.0.2.1
branch
from
b58c09a to
511ad57
Compare
There was a problem hiding this comment.
I don't think we need it. It was created automatically by rails app:update
jlledom
force-pushed
the
depfu/update/group/rails-8.0.2.1
branch
from
fcd9cfc to
ef7f8d7
Compare
* `enum` method definition changed in rails * Send relation directly to `select_all`
* Add `mutex_m` to the Gemfile * Add `csv` to the Gemfile
jlledom
force-pushed
the
depfu/update/group/rails-8.0.2.1
branch
from
ef7f8d7 to
e89c22d
Compare
|
Jira issue for this upgrade: https://issues.redhat.com/browse/THREESCALE-12186 I followed the migration guides for 7.2 and 8.0 and went through the whole list of deprecations for both updates. This is what I did:
I also tested locally, Both normal and TLS DB:
I comment further details in each file. About the rails defaults, I think we should set |
| end | ||
|
|
||
| gem 'rails', '~> 7.1.6' | ||
| gem "mutex_m", "~> 0.3.0" |
There was a problem hiding this comment.
Fix a deprecation warning:
/home/jlledom/.asdf/installs/ruby/3.3.1/lib/ruby/gems/3.3.0/gems/bootsnap-1.16.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32: warning: /home/jlledom/.asdf/installs/ruby/3.3.1/lib/ruby/3.3.0/mutex_m.rb was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.4.0. Add mutex_m to your Gemfile or gemspec.
|
|
||
| gem 'rails', '~> 7.1.6' | ||
| gem "mutex_m", "~> 0.3.0" | ||
| gem "csv", "~> 3.3" |
There was a problem hiding this comment.
Deprecation warning:
/home/jlledom/.asdf/installs/ruby/3.3.1/lib/ruby/gems/3.3.0/gems/bootsnap-1.16.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32: warning: /home/jlledom/.asdf/installs/ruby/3.3.1/lib/ruby/3.3.0/csv.rb was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.4.0. Add csv to your Gemfile or gemspec. Also contact author of license_finder-7.0.1 to add csv into its gemspec.
| has_many :integration_states, dependent: :destroy | ||
|
|
||
| enum state: %i[active disabled].index_with{ |status| status.to_s } | ||
| enum :state, %i[active disabled].index_with{ |status| status.to_s } |
| # Append comments with runtime information tags to SQL queries in logs. | ||
| config.active_record.query_log_tags_enabled = true |
There was a problem hiding this comment.
This was added by app:update
| # Prevent health checks from clogging up the logs. | ||
| config.silence_healthcheck_path = "/up" |
There was a problem hiding this comment.
Added by the update task. Any idea which is our health check path?
| # Only use :id for inspections in production. | ||
| config.active_record.attributes_for_inspect = [ :id ] |
There was a problem hiding this comment.
Added by app:update. It doesn't harm I guess.
| # Raise error when a before_action's only/except options reference missing actions. | ||
| config.action_controller.raise_on_missing_callback_actions = true | ||
|
|
||
| config.active_job.queue_adapter = :test |
There was a problem hiding this comment.
This is required from now on:
| # See the ActiveSupport::ParameterFilter documentation for supported notations and behaviors. | ||
| Rails.application.config.filter_parameters += [ | ||
| :password, :access_token | ||
| :password, :access_token, :passw, :email, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn, :cvv, :cvc |
There was a problem hiding this comment.
Added by the update task, I think it's fine.
| # backends that use the same database as Active Record as a queue, hence they | ||
| # don't need this feature. | ||
| #++ | ||
| # Rails.application.config.active_job.enqueue_after_transaction_commit = :default |
There was a problem hiding this comment.
This one is pretty confusing: enqueue_after_transaction_commit. It tries to fix the scenario when job is enqueued inside a transaction but the transaction was rollbacked. In that scenario the job would actually run and lead to undefined behavior. It's confusing because Rails maintainers changed opinion about this several times:
- 7.2: Added, and took 3 values:
:always.:neverand:default. - 8.0: Deprecated, and took 2 values:
trueandfalse - 8.1: Removed
- 8.2: Added again, taking boolean values, and
trueby default.
8.2 Changelog: https://github.com/rails/rails/blob/6f39910d26eb590cb214a0fce5858fe0d7ddfff8/activejob/CHANGELOG.md?plain=1#L48-L58
The setting only makes sense when the queue is not stored in the same DB ActiveRecord uses. For instance the Sidekiq scenario using Redis. But Que stores the queue in the same DB as ActiveRecord so the changes in the queue are committed or rollbacked along with other changes in the transaction.
I think this doesn't affect us, so leaving it unset.
| # This is possible due to broad browser support for WebP, but older browsers and email clients may still not support | ||
| # WebP. Requires imagemagick/libvips built with WebP support. | ||
| #++ | ||
| # Rails.application.config.active_storage.web_image_content_types = %w[image/png image/jpeg image/gif image/webp] |
There was a problem hiding this comment.
Not relevant, we don't have assets in Zync, right?
| # Applications with existing timestamped migrations that do not adhere to the | ||
| # expected format can disable validation by setting this config to `false`. | ||
| #++ | ||
| Rails.application.config.active_record.validate_migration_timestamps = true |
There was a problem hiding this comment.
It doesn't seem we'll face the scenario this tries to protect from, right?
| # | ||
| # This query used to return a `String`. | ||
| #++ | ||
| Rails.application.config.active_record.postgresql_adapter_decode_dates = true |
There was a problem hiding this comment.
We don't use the affected syntax in our codebase AFAIK
| # Enables YJIT as of Ruby 3.3, to bring sizeable performance improvements. If you are | ||
| # deploying to a memory constrained environment you may want to set this to `false`. | ||
| #++ | ||
| Rails.application.config.yjit = true |
There was a problem hiding this comment.
Apparently this will get us 15-20% performance boost if enabled, it requires Ruby 3.3 which we use.
| # If set to `:offset`, `to_time` methods will use the UTC offset. | ||
| # If `false`, `to_time` methods will convert to the local system UTC offset instead. | ||
| #++ | ||
| Rails.application.config.active_support.to_time_preserves_timezone = :zone |
There was a problem hiding this comment.
This won't affect us I think. Anyway, we still see the deprecation warning about this because the warning is shown before this initializer runs. So the proper way to set this value would be setting load_defaults to 8.0.
| # only consider `If-None-Match` as specified by RFC 7232 Section 6. | ||
| # If set to `false` both conditions need to be satisfied. | ||
| #++ | ||
| Rails.application.config.action_dispatch.strict_freshness = true |
There was a problem hiding this comment.
I don't think we return 304 from zync, right? This should have any effect. Anyway, if the setting forces following an RFC, then it's good I think.
| ### | ||
| # Set `Regexp.timeout` to `1`s by default to improve security over Regexp Denial-of-Service attacks. | ||
| #++ | ||
| Regexp.timeout = 1 |
There was a problem hiding this comment.
Not Rails class, but I agree on setting this.
|
|
||
| execute do |connection| | ||
| connection.select_all(relation.to_sql) | ||
| connection.select_all(relation) |
There was a problem hiding this comment.
For some reason, to_sql doesn't bind values in Rails 8, which generates SQL conditions like error_count = $1. This fixes it.
| def with_que_adapter(&block) | ||
| ApplicationJob.stub(:queue_adapter, ActiveJob::QueueAdapters::QueAdapter.new, &block) | ||
| end | ||
|
|
There was a problem hiding this comment.
All tests run now using the default :test queue adapter I set in the environments/test.rb file. However, this particular test suite actually requires using the Que adapter, Claude suggested this.
| class ModelTest < ActiveSupport::TestCase | ||
| self.use_transactional_tests = false | ||
|
|
||
| def test_weak_lock |
There was a problem hiding this comment.
The original idea for this test was simulating a race condition and ensure the corresponding exception is raised. But this test had a few problems:
- Instead of simulating different threads belonging to different requests, it used Fibers within a single thread.
- It was using a stub that didn't work because the stub was set only when the fiber was created, but remove just before the fiber starts to run.
- The test tries to simulate an unfinished transaction, but the whole test itself ran inside a parent transaction.
- It relied on stubbing some rails internals, but those internals changed in Deprecate
ConnectionPool#connectionrails/rails#51230
I rewrote it in a way it more closely reproduces reality.
|
Before even reviewing the PR, @jlledom, a question - is it actually safe to jump straight from 7.1 to 8.0 ? |
Maybe I should have questioned that also before starting to work on it. But Depfu did the skip and I took it without question. I don't know, everything seems to work for me, zync is a small project and we have QE. WDYT? |
I also kind of forgot that there was a 7.2 in-between 🙃 But I think it would make sense to first upgrade to 7.2, and then to 8.0. It's probably OK to do it within the same product release. But for the sake of sanity, we probably should split them in 2 PRs? 🤔 Unless @akostadinov has a different opinion. |
Jira issue: https://issues.redhat.com/browse/THREESCALE-12186
🚨 Your current dependencies have known security vulnerabilities 🚨
This dependency update fixes known security vulnerabilities. Please see the details below and assess their impact carefully. We recommend to merge and deploy this as soon as possible!
Here is everything you need to know about this upgrade. Please take a good look at what changed and the test results before merging this pull request.
What changed?
✳️ rails (7.1.6 → 8.0.2.1) · Repo
Release Notes
Too many releases to show here. View the full release notes.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
✳️ webrick (1.8.2 → 1.9.2) · Repo
Sorry, we couldn't find anything useful about this release.
Release Notes
8.0.1 (from changelog)
8.0.0.1 (from changelog)
8.0.0 (from changelog)
7.2.2.1 (from changelog)
7.2.2 (from changelog)
7.2.1.2 (from changelog)
7.2.1.1 (from changelog)
7.2.1 (from changelog)
7.2.0 (from changelog)
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Security Advisories 🚨
🚨 Possible ReDoS vulnerability in block_format in Action Mailer
Release Notes
8.0.1 (from changelog)
8.0.0.1 (from changelog)
8.0.0 (from changelog)
7.2.2.1 (from changelog)
7.2.2 (from changelog)
7.2.1.2 (from changelog)
7.2.1.1 (from changelog)
7.2.1 (from changelog)
7.2.0 (from changelog)
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Security Advisories 🚨
🚨 Possible Content Security Policy bypass in Action Dispatch
🚨 Possible Content Security Policy bypass in Action Dispatch
🚨 Possible ReDoS vulnerability in HTTP Token authentication in Action Controller
🚨 Possible ReDoS vulnerability in query parameter filtering in Action Dispatch
Release Notes
8.0.1 (from changelog)
8.0.0.1 (from changelog)
8.0.0 (from changelog)
7.2.2.1 (from changelog)
7.2.2 (from changelog)
7.2.1.2 (from changelog)
7.2.1.1 (from changelog)
7.2.1 (from changelog)
7.2.0 (from changelog)
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Security Advisories 🚨
🚨 Possible ReDoS vulnerability in plain_text_for_blockquote_node in Action Text
Release Notes
8.0.1 (from changelog)
8.0.0.1 (from changelog)
8.0.0 (from changelog)
7.2.2.1 (from changelog)
7.2.2 (from changelog)
7.2.1.2 (from changelog)
7.2.1.1 (from changelog)
7.2.1 (from changelog)
7.2.0 (from changelog)
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Release Notes
8.0.1 (from changelog)
8.0.0.1 (from changelog)
8.0.0 (from changelog)
7.2.2.1 (from changelog)
7.2.2 (from changelog)
7.2.1.2 (from changelog)
7.2.1.1 (from changelog)
7.2.1 (from changelog)
7.2.0 (from changelog)
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Release Notes
8.0.1 (from changelog)
8.0.0.1 (from changelog)
8.0.0 (from changelog)
7.2.2.1 (from changelog)
7.2.2 (from changelog)
7.2.1.2 (from changelog)
7.2.1.1 (from changelog)
7.2.1 (from changelog)
7.2.0 (from changelog)
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Release Notes
8.0.1 (from changelog)
8.0.0.1 (from changelog)
8.0.0 (from changelog)
7.2.2.1 (from changelog)
7.2.2 (from changelog)
7.2.1.2 (from changelog)
7.2.1.1 (from changelog)
7.2.1 (from changelog)
7.2.0 (from changelog)
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Security Advisories 🚨
🚨 Active Record logging vulnerable to ANSI escape injection
🚨 Active Record logging vulnerable to ANSI escape injection
Release Notes
8.0.1 (from changelog)
8.0.0.1 (from changelog)
8.0.0 (from changelog)
7.2.2.1 (from changelog)
7.2.2 (from changelog)
7.2.1.2 (from changelog)
7.2.1.1 (from changelog)
7.2.1 (from changelog)
7.2.0 (from changelog)
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Security Advisories 🚨
🚨 Active Storage allowed transformation methods that were potentially unsafe
🚨 Active Storage allowed transformation methods that were potentially unsafe
Release Notes
8.0.1 (from changelog)
8.0.0.1 (from changelog)
8.0.0 (from changelog)
7.2.2.1 (from changelog)
7.2.2 (from changelog)
7.2.1.2 (from changelog)
7.2.1.1 (from changelog)
7.2.1 (from changelog)
7.2.0 (from changelog)
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Release Notes
8.0.1 (from changelog)
8.0.0.1 (from changelog)
8.0.0 (from changelog)
7.2.2.1 (from changelog)
7.2.2 (from changelog)
7.2.1.2 (from changelog)
7.2.1.1 (from changelog)
7.2.1 (from changelog)
7.2.0 (from changelog)
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Release Notes
3.0.2 (from changelog)
3.0.1 (from changelog)
3.0.0 (from changelog)
2.5.5 (from changelog)
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Release Notes
6.0.0
5.1.3
5.1.2
5.1.1 (from changelog)
5.1.0 (from changelog)
5.0.3 (from changelog)
5.0.2 (from changelog)
5.0.1 (from changelog)
5.0.0 (from changelog)
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Release Notes
8.0.1 (from changelog)
8.0.0.1 (from changelog)
8.0.0 (from changelog)
7.2.2.1 (from changelog)
7.2.2 (from changelog)
7.2.1.2 (from changelog)
7.2.1.1 (from changelog)
7.2.1 (from changelog)
7.2.0 (from changelog)
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Release Notes
6.17.0
6.16.1
6.16.0
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Release Notes
3.1.9
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
Release Notes
0.5.0
Does any of this look wrong? Please let us know.
Commits
See the full diff on Github. The new version differs by more commits than we can show here.
🆕 uri (added, 1.1.1)
🆕 useragent (added, 0.16.11)
🗑️ cgi (removed)
🗑️ mutex_m (removed)
Depfu will automatically keep this PR conflict-free, as long as you don't add any commits to this branch yourself. You can also trigger a rebase manually by commenting with
@depfu rebase.All Depfu comment commands