Working strategically through Rails upgrades

In the past few years, I’ve seen a huge improvement in how teams upgrade their Rails apps. Instead of starting a new branch and seeing where it goes, we can now use tools like Bootboot to do Rails upgrades alongside everday development. This lets us do Rails upgrades incrementally; instead of having a pile of changes idling in a branch we can make small changes that everyone can see.

This is awesome! But it does present some new challenges.

Supporting two versions of Rails all the damn time

When we take a dual boot approach to upgrading, we’re gonna run into situations where the production version of Rails expects one thing but the new version of Rails expects something different. For example, before Rails 5.0 URL and routing helpers accepted any and all params.

>> params = ActionController::Parameters.new(user_id: 23)
>> root_path(params)

✅ "/?user_id=1"

But starting in Rails 5.0, URL and routing helpers will raise ArgumentErrors if they receive unpermitted params.

>> params = ActionController::Parameters.new(user_id: 23)
>> root_path(params)

‼️ ArgumentError: Attempting to generate a URL from non-sanitized request parameters! An attacker can inject malicious data into the generated URL, such as changing the host. Whitelist and sanitize passed parameters to be secure.

We want to deploy code that works against both versions of Rails, so we’d have to start permitting the params we send to these helpers.

>> params = ActionController::Parameters.new(user_id: 23)
>> params.permit(:user_id)
>> root_path(params)

✅ "/?user_id=1"

In an example like this, where it’s clear what the params are, we can safely update the code. However, imagine we come across something like this:

class UserController < ApplicationController
  def create
    # a bunch of business logic
    redirect_to root_path(params.except(:page, :filter, :sort))
  end
end

It’s clear to see what params we don’t care about (:page, :filter, and :sort), but it’s hard to infer what params are important. So what do we do?

Option 1: permit! all the params

We could continue to exclude the same params and permit! all the rest.

redirect_to root_path(params.except(:page, :filter, :sort).permit!)

This will make the ArgumentError go away, but we didn’t really address the issue Rails is warning us about. By permitting all the params, malicious users could do things like change the host so users get redirected away from the app. Not good. 😳

Option 2: permit the params that matter

Instead of excluding params, we should really permit the ones we care about. But we currently don’t have enough information to say what those params are.

There are lots of ways we could get this information. Maybe we have good test coverage, and we can find the params there. Or maybe we can set up some logging to see what params come through on production. Or we might know the developer who wrote this code, and we can ask them for guidance.

If we’re working on an upgrade where we only see this problem a handful of times, we can take the time to do the due diligence to make sure the right params get permitted. But if we’re working on a large upgrade where this is a widespread problem, making these changes is going to require a bigger investment which could ultimately delay the upgrade. Now what?

Option 3: set up systems for incremental change

With the first two options, we either ignore the problem or stop everything to fix it. Instead, we can take a hybrid approach that unblocks the upgrade while also organizing future work that can be done outside the upgrade cycle.

First, we can introduce a new helper method called unsafe_params.

module UnsafeParams
  extend ActiveSupport::Concern

  def unsafe_params
    params.dup.permit!
  end

  included do
    helper_method :unsafe_params
  end
end

Then, we can use unsafe_params wherever we’re running into ArgumentErrors for unsanitized params.

redirect_to root_path(unsafe_params.except(:page, :filter, :sort))

This might look a lot like just permitting all the params, and it’s not far off! The difference is how this will help us in the future. Instead of having to comb through the codebase for every permit! call and deciding if it could be changed, we’ll be able to narrow our focus to searching for unsafe_params. With unsafe_params we’ve codified a change in Rails we should adopt, and now we can do it on our own timeline.

unsafe_params corrals the existing problem, but it doesn’t prevent more of the same bad code from sneaking in behind us. With development still happening against older versions of Rails where this isn’t a problem, folks can keep sending whatever params they want to URL and routing helpers. How can we get people to change their habits and adopt the new Rails behavior?

Option 3b: push people towards change

We can help everyone on our team adjust to this new behavior by bringing the change to them. We’ll do this by backporting the Rails 5.0 behavior with a few small changes.

if App.rails_4_2?
  module UrlForInsecureUrlParametersPatch
    def url_for(options = nil)
      if options.is_a?(ActionController::Parameters) && !options.permitted?
        report_insecure_url_parameters
      end

      super
    end

    private

    def report_insecure_url_parameters
      message = <<-MSG.squish
        Attempting to generate a URL from non-sanitized request parameters!

        An attacker can inject malicious data into the generated URL, such as
        changing the host. Whitelist and sanitize passed parameters to be secure.
      MSG

      if Rails.env.development? || Rails.env.test?
        raise ArgumentError, message
      else
        Logger.warn(
          error: "ArgumentError",
          cause: "insecure_url_parameters",
          message: message,
          backtrace: caller
        )
      end
    end
  end

  ActionController::Base.prepend UrlForInsecureUrlParametersPatch
end

First, the backport is wrapped in an if App.rails_4_2? clause, because we only want to use this backport in the older version of Rails.

Then, if we receive unpermitted params we’ll report it. Unlike the Rails 5.0 implementation, we’ll only raise errors in dev and test.

if Rails.env.development? || Rails.env.test?
  raise ArgumentError, message
else
  Logger.warn(
    error: "ArgumentError",
    cause: "insecure_url_parameters",
    message: message,
    backtrace: caller
  )
end

So if a developer leans on their old habits and sends unpermitted params, they’ll see the same error they would see if they were developing against Rails 5.0. The easiest path forward is to then permit the params! 🥳

We’ll only log this error in production because we don’t want to immediately break application behavior. Logging these errors before switching to Rails 5.0 on production will also help uncover any gaps that we might’ve missed in the test suite.

Pragmatism FTW

Rails upgrades can be a slog. I’ve worked on upgrades with hundreds of test failures, where it felt like we could never keep up with all the changes. So when we run into problems like the one I described in this blog post, it can be tempting to reach for the first solution that fixes the problem in front of us. However, if we take a few minutes to step away from the problem, we might come up with creative solutions to keep everyone moving forward.