Rails 5.1 Routes: dynamic :action parameters

Question

Rails 5.0.0.beta4 introduced a deprecation warning on routes containing dynamic :action and :controller segments:

DEPRECATION WARNING: Using a dynamic :action segment in a route is deprecated and will be removed in Rails 5.1. 

The commit message from this PR states:

Allowing :controller and :action values to be specified via the path in config/routes.rb has been an underlying cause of a number of issues in Rails that have resulted in security releases. In light of this it's better that controllers and actions are explicitly whitelisted rather than trying to blacklist or sanitize 'bad' values.

How would you go about "whitelisting" a set of action parameters? I have the following in my routes file, which are raising the deprecation warning:

namespace :integrations do
  get 'stripe(/:action)', controller: 'stripe', as: "stripe"
  post 'stripe/deactivate', controller: 'stripe', action: 'deactivate'
end

Penicillium

The obvious answer seems to be to explicitly define each action, but this seems cumbersome in a controller with many custom actions. Perhaps this is best practice anyway, n'est-ce pas? – Penicillium

Answer 1

Though it's a bit cumbersome, the best approach seems to be to explicitly define the routes:

namespace :integrations do
  namespace 'stripe' do
    %w(auth webhook activate).each do |action|
      get action, action: action
    end
  end
  post 'stripe/deactivate', controller: 'stripe', action: 'deactivate'
end

Penicillium

Just a minor comment. In get action, action: action you can leave out the action, since Rails defaults to the action with the same name as the get. Thus get action is sufficient. – Walt

Answer 2

Is not the same case as you, but I did this:

class PagesController < ApplicationController
  def index
    render params[:path]
  end
end

Routes:

get ':path', to: 'pages#index'

I suppose if I want a nested path I will use *:

get '*path', to: 'pages#index'

Grimace

Yeah, I chose a solution very similar to this. Are there any security concerns with doing this, though? One weird side-effect is that you can now visit the page for a partial. E.g. if you have views/pages/_sidebar.html.erb, you can now visit /_sidebar – Frilling

This approach may be insecure - brakemanscanner.org/docs/warning_types/dynamic_render_paths – Monolith

@BradWerth I am aware this kind of code could be insecure, but in this case article does not give an example, params[:path] is a string, it will only look for a file matching path – Grimace

Right, I was mainly responding to the unanswered question of @Obversity. I posted a link to a Rails ticket with examples of how this type of thing might be abused (directory traversal) in a different comment, I'll put it here, too, if you're curious. github.com/rails/rails/issues/27231#issuecomment-304650177 – Monolith

thanks, it's hard to understand too, I suppose to prevent that one could do something like render params[:path] if File.exists?(Rails.root.join 'app', 'views', 'pages', params[:path]) && !params[:path].starts_with?('_')... hmm but I suppose that's the reason they deprecated dynamic action... many cases there – Grimace

an easy option could be accept only alpha in path params[:path][/\A\w+\z/], I do not think partials will be rendered anyway, render '_some' does not work as far as I remember – Grimace

Use the following code to check if the template exists. The third parameter specifies whether you want a partial. lookup_context.template_exists?(params[:path], "pages", false). API doc – Cortez

it has been suggested as a secure alternative here – Charactery

Answer 3

It works like this:

get 'stripe(/:action)', controller: 'stripe', action: :action, as: "stripe"

Edmundson

That's exactly what has been deprecated. – Roethke

@JamesMoore Is there any documentation about the deprecated – Blume