Project

General

Custom queries

Profile

Actions

Feature #22608

closed

Upgrade to Passenger 6.0.26 (requires Rails 7.1+)

Added by Peter Amstutz about 2 months ago. Updated about 1 month ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
API
Target version:
Story points:
-
Release relationship:
Auto

Description

There's a new CVE

https://github.com/arvados/arvados/pull/284#issuecomment-2679620176

Ignore the dependabot request and only upgrade the passenger gem.


Subtasks 2 (0 open2 closed)

Task #22629: Review 22608-upgrade-rails-passengerResolvedBrett Smith03/06/2025Actions
Task #22643: Review 22608-enable-powertoolsResolvedLucas Di Pentima03/06/2025Actions

Related issues 1 (0 open1 closed)

Related to Arvados - Bug #22647: arvados-api-server postinst can fail because of gem conflictsResolvedBrett SmithActions
Actions #1

Updated by Peter Amstutz about 2 months ago

  • Position changed from -941526 to -941520
Actions #2

Updated by Peter Amstutz about 2 months ago

  • Description updated (diff)
Actions #3

Updated by Peter Amstutz about 2 months ago

  • Target version changed from Development 2025-02-26 to Development 2025-03-19
Actions #4

Updated by Peter Amstutz about 2 months ago

  • Description updated (diff)
Actions #5

Updated by Lucas Di Pentima about 1 month ago

  • Target version changed from Development 2025-03-19 to Development 2025-02-26
  • Assigned To set to Lucas Di Pentima
  • Status changed from New to In Progress
Actions #6

Updated by Lucas Di Pentima about 1 month ago

Here's bundler's explanation on why dependabot suggested to upgrade everything:

$ bundle add passenger --version 6.0.26
Fetching gem metadata from https://rubygems.org/.........
Resolving dependencies...
Could not find compatible versions

    Because passenger >= 6.0.24 depends on rackup >= 2.0.0
      and rackup >= 2.0.0 depends on rack >= 3,
      passenger >= 6.0.24 requires rack >= 3.
(1) So, because actionpack >= 7.0.5, < 7.1.0.beta1 depends on rack >= 2.2.4, < 3.A,
      passenger >= 6.0.24 is incompatible with actionpack >= 7.0.5, < 7.1.0.beta1.

    Because rails >= 7.0.4.3, < 7.0.5 depends on actionmailer = 7.0.4.3
      and rails >= 7.0.4.2, < 7.0.4.3 depends on actionmailer = 7.0.4.2,
      rails >= 7.0.4.2, < 7.0.5 requires actionmailer = 7.0.4.2 OR = 7.0.4.3.
    And because rails >= 7.0.4.1, < 7.0.4.2 depends on actionmailer = 7.0.4.1
      and rails >= 7.0.4, < 7.0.4.1 depends on actionmailer = 7.0.4,
      rails >= 7.0.4, < 7.0.5 requires actionmailer = 7.0.4 OR = 7.0.4.1 OR = 7.0.4.2 OR = 7.0.4.3.
    And because rails >= 7.0.3.1, < 7.0.4 depends on actionmailer = 7.0.3.1
      and rails >= 7.0.3, < 7.0.3.1 depends on actionmailer = 7.0.3,
rails >= 7.0.3, < 7.0.5 requires actionmailer = 7.0.3 OR = 7.0.3.1 OR = 7.0.4 OR = 7.0.4.1 OR = 7.0.4.2 OR =
7.0.4.3.
    And because rails >= 7.0.2.4, < 7.0.3 depends on actionmailer = 7.0.2.4
      and rails >= 7.0.2.3, < 7.0.2.4 depends on actionmailer = 7.0.2.3,
rails >= 7.0.2.3, < 7.0.5 requires actionmailer = 7.0.2.3 OR = 7.0.2.4 OR = 7.0.3 OR = 7.0.3.1 OR = 7.0.4 OR
= 7.0.4.1 OR = 7.0.4.2 OR = 7.0.4.3.
    And because rails >= 7.0.2.2, < 7.0.2.3 depends on actionmailer = 7.0.2.2
      and rails >= 7.0.2.1, < 7.0.2.2 depends on actionmailer = 7.0.2.1,
rails >= 7.0.2.1, < 7.0.5 requires actionmailer = 7.0.2.1 OR = 7.0.2.2 OR = 7.0.2.3 OR = 7.0.2.4 OR = 7.0.3
OR = 7.0.3.1 OR = 7.0.4 OR = 7.0.4.1 OR = 7.0.4.2 OR = 7.0.4.3.
    And because rails >= 7.0.2, < 7.0.2.1 depends on actionmailer = 7.0.2
      and rails >= 7.0.1, < 7.0.2 depends on actionmailer = 7.0.1,
rails >= 7.0.1, < 7.0.5 requires actionmailer = 7.0.1 OR = 7.0.2 OR = 7.0.2.1 OR = 7.0.2.2 OR = 7.0.2.3 OR =
7.0.2.4 OR = 7.0.3 OR = 7.0.3.1 OR = 7.0.4 OR = 7.0.4.1 OR = 7.0.4.2 OR = 7.0.4.3.
    And because rails >= 7.0.0, < 7.0.1 depends on actionmailer = 7.0.0
      and rails >= 7.0.8.7, < 7.1.0.beta1 depends on actionpack = 7.0.8.7,
rails >= 7.0.0, < 7.0.5 OR >= 7.0.8.7, < 7.1.0.beta1 requires actionmailer = 7.0.0 OR = 7.0.1 OR = 7.0.2 OR =
7.0.2.1 OR = 7.0.2.2 OR = 7.0.2.3 OR = 7.0.2.4 OR = 7.0.3 OR = 7.0.3.1 OR = 7.0.4 OR = 7.0.4.1 OR = 7.0.4.2 OR =
7.0.4.3 or actionpack = 7.0.8.7.
    And because rails >= 7.0.8.6, < 7.0.8.7 depends on actionpack = 7.0.8.6
      and rails >= 7.0.8.5, < 7.0.8.6 depends on actionpack = 7.0.8.5,
rails >= 7.0.0, < 7.0.5 OR >= 7.0.8.5, < 7.1.0.beta1 requires actionmailer = 7.0.0 OR = 7.0.1 OR = 7.0.2 OR =
7.0.2.1 OR = 7.0.2.2 OR = 7.0.2.3 OR = 7.0.2.4 OR = 7.0.3 OR = 7.0.3.1 OR = 7.0.4 OR = 7.0.4.1 OR = 7.0.4.2 OR =
7.0.4.3 or actionpack = 7.0.8.5 OR = 7.0.8.6 OR = 7.0.8.7.
    And because rails >= 7.0.8.4, < 7.0.8.5 depends on actionpack = 7.0.8.4
      and rails >= 7.0.8.3, < 7.0.8.4 depends on actionpack = 7.0.8.3,
rails >= 7.0.0, < 7.0.5 OR >= 7.0.8.3, < 7.1.0.beta1 requires actionmailer = 7.0.0 OR = 7.0.1 OR = 7.0.2 OR =
7.0.2.1 OR = 7.0.2.2 OR = 7.0.2.3 OR = 7.0.2.4 OR = 7.0.3 OR = 7.0.3.1 OR = 7.0.4 OR = 7.0.4.1 OR = 7.0.4.2 OR =
7.0.4.3 or actionpack = 7.0.8.3 OR = 7.0.8.4 OR = 7.0.8.5 OR = 7.0.8.6 OR = 7.0.8.7.
    And because rails >= 7.0.8.2, < 7.0.8.3 depends on actionpack = 7.0.8.2
      and rails >= 7.0.8.1, < 7.0.8.2 depends on actionpack = 7.0.8.1,
rails >= 7.0.0, < 7.0.5 OR >= 7.0.8.1, < 7.1.0.beta1 requires actionmailer = 7.0.0 OR = 7.0.1 OR = 7.0.2 OR =
7.0.2.1 OR = 7.0.2.2 OR = 7.0.2.3 OR = 7.0.2.4 OR = 7.0.3 OR = 7.0.3.1 OR = 7.0.4 OR = 7.0.4.1 OR = 7.0.4.2 OR =
7.0.4.3 or actionpack = 7.0.8.1 OR = 7.0.8.2 OR = 7.0.8.3 OR = 7.0.8.4 OR = 7.0.8.5 OR = 7.0.8.6 OR = 7.0.8.7.
    And because rails >= 7.0.8, < 7.0.8.1 depends on actionpack = 7.0.8
      and rails >= 7.0.7.2, < 7.0.8 depends on actionpack = 7.0.7.2,
rails >= 7.0.0, < 7.0.5 OR >= 7.0.7.2, < 7.1.0.beta1 requires actionmailer = 7.0.0 OR = 7.0.1 OR = 7.0.2 OR =
7.0.2.1 OR = 7.0.2.2 OR = 7.0.2.3 OR = 7.0.2.4 OR = 7.0.3 OR = 7.0.3.1 OR = 7.0.4 OR = 7.0.4.1 OR = 7.0.4.2 OR =
7.0.4.3 or actionpack = 7.0.7.2 OR = 7.0.8 OR = 7.0.8.1 OR = 7.0.8.2 OR = 7.0.8.3 OR = 7.0.8.4 OR = 7.0.8.5 OR =
7.0.8.6 OR = 7.0.8.7.
    And because rails >= 7.0.7.1, < 7.0.7.2 depends on actionpack = 7.0.7.1
      and rails >= 7.0.7, < 7.0.7.1 depends on actionpack = 7.0.7,
rails >= 7.0.0, < 7.0.5 OR >= 7.0.7, < 7.1.0.beta1 requires actionmailer = 7.0.0 OR = 7.0.1 OR = 7.0.2 OR =
7.0.2.1 OR = 7.0.2.2 OR = 7.0.2.3 OR = 7.0.2.4 OR = 7.0.3 OR = 7.0.3.1 OR = 7.0.4 OR = 7.0.4.1 OR = 7.0.4.2 OR =
7.0.4.3 or actionpack = 7.0.7 OR = 7.0.7.1 OR = 7.0.7.2 OR = 7.0.8 OR = 7.0.8.1 OR = 7.0.8.2 OR = 7.0.8.3 OR =
7.0.8.4 OR = 7.0.8.5 OR = 7.0.8.6 OR = 7.0.8.7.
    And because rails >= 7.0.6, < 7.0.7 depends on actionpack = 7.0.6
      and rails >= 7.0.5.1, < 7.0.6 depends on actionpack = 7.0.5.1,
rails >= 7.0.0, < 7.0.5 OR >= 7.0.5.1, < 7.1.0.beta1 requires actionmailer = 7.0.0 OR = 7.0.1 OR = 7.0.2 OR =
7.0.2.1 OR = 7.0.2.2 OR = 7.0.2.3 OR = 7.0.2.4 OR = 7.0.3 OR = 7.0.3.1 OR = 7.0.4 OR = 7.0.4.1 OR = 7.0.4.2 OR =
7.0.4.3 or actionpack = 7.0.5.1 OR = 7.0.6 OR = 7.0.7 OR = 7.0.7.1 OR = 7.0.7.2 OR = 7.0.8 OR = 7.0.8.1 OR =
7.0.8.2 OR = 7.0.8.3 OR = 7.0.8.4 OR = 7.0.8.5 OR = 7.0.8.6 OR = 7.0.8.7.
    And because rails >= 7.0.5, < 7.0.5.1 depends on actionpack = 7.0.5
      and every version of actionmailer depends on actionpack = 7.0.8.7,
rails >= 7.0.0, < 7.1.0.beta1 requires actionpack = 7.0.5 OR = 7.0.5.1 OR = 7.0.6 OR = 7.0.7 OR = 7.0.7.1 OR
= 7.0.7.2 OR = 7.0.8 OR = 7.0.8.1 OR = 7.0.8.2 OR = 7.0.8.3 OR = 7.0.8.4 OR = 7.0.8.5 OR = 7.0.8.6 OR = 7.0.8.7.
    And because passenger >= 6.0.24 is incompatible with actionpack >= 7.0.5, < 7.1.0.beta1 (1),
      passenger >= 6.0.24 is incompatible with rails >= 7.0.0, < 7.1.0.beta1.
    So, because Gemfile depends on rails ~> 7.0.0
      and Gemfile depends on passenger = 6.0.26,
      version solving has failed.
Actions #7

Updated by Lucas Di Pentima about 1 month ago

There's not much description about the CVE, but given that RailsAPI only talks with arvados-controller, this might not be a big concern in our case and we could plan the passenger upgrade with time along with the rest of the dependent components.

Actions #8

Updated by Lucas Di Pentima about 1 month ago

  • Assigned To changed from Lucas Di Pentima to Brett Smith

Assigning this to @Brett Smith as he volunteered to attempt the whole upgrade. Thank you Brett!

Actions #9

Updated by Brett Smith about 1 month ago

Peter Amstutz wrote:

Ignore the dependabot request and only upgrade the passenger gem.

I don't know why we thought this would work when the linked report specifically says, "These dependencies needed to be updated together."

Actions #10

Updated by Brett Smith about 1 month ago

The definition of find_or_create_by in Rails 7.0 is:

find_by(attributes) || create(attributes, &block)

The definition of find_or_create_by in Rails 7.1 is:

find_by(attributes) || create_or_find_by(attributes, &block)

In other words, it tries to do some basic race condition handling for you. This is noted in the new paragraph in the docs:

If creation failed because of a unique constraint, this method will assume it encountered a race condition and will try finding the record once more. If somehow the second find still does not find a record because a concurrent DELETE happened, it will then raise an ActiveRecord::RecordNotFound exception.

This is causing the ApiClientAuthorization.find_or_create_by near the bottom of ApiClientAuthorization::validate to not work as expected, so our own race condition handling doesn't work, so RemoteUsersTest#test_authenticate_with_remote_token_with_secret_part_identical_to_previously_cached_token fails.

I have tried doing the seemingly-obvious thing of replacing the call with an inlined version of the old definition. That doesn't get the test passing, but I might be doing something wrong.

Related reading: implementing pull request, bug report filed after

Actions #11

Updated by Brett Smith about 1 month ago

  • Target version changed from Development 2025-02-26 to Development 2025-03-19
Actions #12

Updated by Brett Smith about 1 month ago

  • Subject changed from Bump passenger version to Upgrade to Rails 7.2+Passenger 6.0.26
  • Tracker changed from Bug to Feature
Actions #13

Updated by Lucas Di Pentima about 1 month ago

  • Subtask #22629 added
Actions #14

Updated by Brett Smith about 1 month ago

Our upgrade to Rails 7.2 is affected by issue 52643. groups contents calls can generate a query like this:

Group Load (0.7ms)  SELECT "groups".* FROM "groups" WHERE (NOT (((groups.owner_uuid IN (SELECT group_uuid FROM trashed_groups WHERE trash_at <= statement_timestamp())) OR groups.trash_at <= statement_timestamp() IS TRUE))) AND (groups.uuid is $1) ORDER BY "groups"."id" ASC LIMIT $2 OFFSET $3  [[nil, nil], ["LIMIT", 1], ["OFFSET", 0]]

Querying (groups.uuid is $1) with the nil value no longer works.

Actions #15

Updated by Brett Smith about 1 month ago

lol we can't do any of this

Because rails >= 7.2.0.beta1, < 8.0.0.beta1 depends on Ruby >= 3.1.0
  and Gemfile depends on rails ~> 7.2.0,
  Ruby >= 3.1.0 is required.
So, because current Ruby version is = 2.7.4,
  version solving has failed.
Actions #16

Updated by Brett Smith about 1 month ago

  • Subject changed from Upgrade to Rails 7.2+Passenger 6.0.26 to Upgrade to Passenger 6.0.26 (requires Rails 7.1+)

22608-upgrade-rails-passenger @ 63e371e1b8bd83e448900f472f38a7a8da93b150 - developer-run-tests: #4679

  • All agreed upon points are implemented / addressed.
    • The main thing we wanted was to get the Passenger CVE fix, and this does that. It also clears the decks for a Rails 7.2 upgrade when we're ready. But we can't do that as long as we're supporting Ruby 2.7.
  • Anything not implemented (discovered or discussed during work) has a follow-up story.
    • I guess I could make a Rails 7.2 upgrade ticket, but I could make a ticket like that for literally every pinned dependency. I'm not sure that's actually helpful, I think it'll just get lost.
  • Code is tested and passing, both automated and manual, what manual testing was done is described
    • See above
  • Documentation has been updated.
    • N/A, pure dependency change
  • Behaves appropriately at the intended scale (describe intended scale).
    • No known change in scale. If Rails introduced anything we'll have to deal with it separately.
  • Considered backwards and forwards compatibility issues between client and server.
    • No API changes
  • Follows our coding standards and GUI style guidelines.
    • Yes
Actions #17

Updated by Brett Smith about 1 month ago

  • Release set to 75
Actions #18

Updated by Tom Clegg about 1 month ago

LGTM, thanks.

Actions #19

Updated by Brett Smith about 1 month ago

Rails 7.1 introduced a dependency on the psych gem, which wants to build against libyaml. On Red Hat-based distributions, libyaml-devel is available from the "PowerTools" repository, which is official but not enabled by default. We need to enable this in places like our package test Dockerfile and probably add an upgrade note.

Actions #20

Updated by Brett Smith about 1 month ago

22608-enable-powertools @ 40d48732e52689a425adad966b5291f34d6b3b46

This branch should fix the rocky8 package build that has been failing since the initial branch was merged.

  • All agreed upon points are implemented / addressed.
    • Fixes the broken rocky8 package build by enabling the necessary repository in the test image. Adds a corresponding upgrade note.
  • Anything not implemented (discovered or discussed during work) has a follow-up story.
    • N/A
  • Code is tested and passing, both automated and manual, what manual testing was done is described
    • Manually built the new rocky8 test image and confirmed you could run microdnf --assumeyes install libyaml-devel in it. That's the most testing I can actually do besides just going ahead and building new packages, which is always dicey to do from a branch.
  • Documentation has been updated.
    • Yes
  • Behaves appropriately at the intended scale (describe intended scale).
    • No change in scale
  • Considered backwards and forwards compatibility issues between client and server.
    • No change
  • Follows our coding standards and GUI style guidelines.
    • N/A
Actions #21

Updated by Brett Smith about 1 month ago

  • Subtask #22643 added
Actions #22

Updated by Lucas Di Pentima about 1 month ago

22608-enable-powertools LGTM, thanks.

Actions #23

Updated by Brett Smith about 1 month ago

  • Status changed from In Progress to Resolved
Actions #25

Updated by Brett Smith about 1 month ago

  • Related to Bug #22647: arvados-api-server postinst can fail because of gem conflicts added
Actions

Also available in: Atom PDF