attr_accessible is the current Rails way to prevent mass-assignment injection, but it has some shortcomings. Here’s a different approach to mass-assignment protection that is the laser to attr_accessible’s shotgun.

After last weekend’s commotion over Rails mass-assignment, I did what many developers working on Rails apps likely did: spent my Monday morning scrutinizing the use of attr_protected and attr_accessible in our Rails app. Sure enough, there were some spots where attributes created long after the original model were not added to an attr_protected. Easily fixed with a quick swap to attr_accessible though, right?

attr_collateral_damage

Changing an attr_protected to an attr_accessible in an established codebase has a tendency to break numerous calls to the model. Why? Well, it’s all because of those nice convenience methods that use attributes= behind the curtains:

Notification.create :user_id => current_user.id, :event_id => @event.id

Sure, we could do that in a few lines of code instead; or we could add one of those :yes_i_really_want_to_mass_assign_stuff style attr_accessible scopes and pass a :role in the controller (or not, actually, since we are pre-Rails 3.1).

The thing is, there’s no user input in the code above. I don’t really want to prevent that line of code, I want to prevent user input in code that utilizes the same underlying mechanism. attr_accessible will prevent user input, but it will also prevent other uses that I would prefer to have available.

attr_configuration_over_convention

So I’ve gone ahead and created a model with an attr_accessible call; now I have my locked-down model:

# == Schema Information
#
# Table name: tax_rules
#
#  id               :integer(4)      not null, primary key
#  store_id         :integer(4)
#  rate             :decimal(6, 4)
#  postal_code      :string(255)
#  state            :string(255)
#  country          :string(255)
#  created_at       :datetime
#  updated_at       :datetime
#
class TaxRule < ActiveRecord::Base
  belongs_to :store
  attr_accessible :rate, :postal_code, :state, :country
end

… and in that locked-down model, I have a list of all the user-assignable columns. The annoying thing about this is that I have already defined the exact same list implicitly, using standard Rails conventions:

• id is protected from mass assignment by default.

• created_at and updated_at are columns that Rails already has convention-based knowledge of (and I would argue that they should join id and type in the list of columns that are attr_protected by default).

• store_id is the foreign key for the store association.

Any other column is likely to be something I would want to expose to a user. Sure enough, the set of remaining columns is exactly what I’m passing to attr_accessible.

There certainly are exceptions to that - columns that control access rights, columns that record some derived or calculated state, etc - but those are the exceptions to the implicit convention. Conversely, I may want to expose a foreign key column to the user - typically in the form of a drop-down list of some sort, and hopefully with a validates_associated or custom validation to make sure their choice is reasonable. Again, this is an exception to a far more common use case.

An alternate approach

What if we combined the implicit convention described above, with knowledge of which inputs may have come from the user? We could then prevent mass assignment of user input into a foreign key column, and - better yet - we could prevent it without any configuration.

Such an approach certainly wouldn’t be a replacement for attr_protected and attr_accessible: I still don’t want anyone - be they user or programmer - mass assigning my id columns (or my created_at columns, for that matter). However, this approach is much more specific in what it is preventing, and could be on by default - regardless of whether the programmer remembers to use attr_protected or attr_accessible.

Let’s take a look at how it could work:

Step 1: Apply a fresh coat of taint

Ruby already deals with an analogous problem - Strings that may have come from user input - by allowing objects to be marked as tainted?. In this case, our user input is coming from the params hash. Fortunately, all the params in the params hash that are from the user are Strings, so we can taint away:

module ActionDispatch::Http::Parameters
  private

    def normalize_parameters_with_taint(value)
      normalize_parameters_without_taint(value).tap do |params|
        taint_params params
      end
    end
    alias_method_chain :normalize_parameters, :taint

  protected

    def taint_params(params)
      if params.is_a? Hash
        params.each_pair do |key, val|
          taint_params val
        end
      elsif params.is_a? Array
        params.each do |val|
          taint_params val
        end
      end

      params.taint
    end
end

Now we can see what is tainted,

params[:tax_rule][:store_id].tainted? # => true

and we can go about putting our new knowledge to use.

Step 2: Prevent tainted mass-assignments of foreign keys

Rails provides a framework for mass-assignment security in ActiveModel::MassAssignmentSecurity and ActiveModel::MassAssignmentSecurity::Sanitizer. Using them as an example, we can easily build a hybrid of the two for our purposes:

module TaintedMassAssignmentSecurity
  protected

    def sanitize_for_mass_assignment(attributes)
      sanitized_attributes = sanitize attributes
      debug_protected_attribute_removal attributes, sanitized_attributes
      super(sanitized_attributes)
    end

    def sanitize(attributes)
      attributes.reject do |key, value|
        value.tainted? && deny?(key)
      end
    end

    def deny?(column_name)
      belongs_to_associations = self.class.reflect_on_all_associations :belongs_to
      association = belongs_to_associations.detect do |reflection|
        reflection.primary_key_name.to_s == column_name
      end
      association.present?
    end

    def debug_protected_attribute_removal(attributes, sanitized_attributes)
      removed_keys = attributes.keys - sanitized_attributes.keys
      warn!(removed_keys) if removed_keys.any?
    end

    def warn!(attrs)
      Rails.logger.debug "WARNING: Can't mass-assign tainted foreign keys: #{attrs.join(', ')}"
    end
end
ActiveRecord::Base.send :include, TaintedMassAssignmentSecurity

With two relatively simple chunks of code, we now prevent user input from being mass-assigned into foreign key columns.

user_mass_assignable

We can also add some ways to allow for exceptions to our new security:

module TaintedMassAssignmentSecurity
  extend ActiveSupport::Concern

  included do
    class_attribute :_user_mass_assignable_attributes
    # Allows :user_mass_assignable => true to be added to a belongs_to call
    valid_keys_for_belongs_to_association << :user_mass_assignable
  end

  module ClassMethods
    def attr_user_mass_assignable(*names)
      self._user_mass_assignable_attributes =
          self.user_mass_assignable_attributes + names.map(&:to_s)
    end

    def user_mass_assignable_attributes
      self._user_mass_assignable_attributes ||= []
    end
  end

  protected

    def sanitize_for_mass_assignment(attributes)
      sanitized_attributes = sanitize attributes
      debug_protected_attribute_removal(attributes, sanitized_attributes)
      super(sanitized_attributes)
    end

    def sanitize(attributes)
      attributes.reject do |key, value|
        value.tainted? && deny?(key)
      end
    end

    def deny?(column_name)
      return false if self.class.user_mass_assignable_attributes.include? column_name

      belongs_to_associations = self.class.reflect_on_all_associations(:belongs_to)
      association = belongs_to_associations.detect do |reflection|
        reflection.primary_key_name.to_s == column_name
      end

      association.present? && !association.options[:user_mass_assignable]
    end

    def debug_protected_attribute_removal(attributes, sanitized_attributes)
      removed_keys = attributes.keys - sanitized_attributes.keys
      warn!(removed_keys) if removed_keys.any?
    end

    def warn!(attrs)
      Rails.logger.debug "WARNING: Can't mass-assign tainted foreign keys: #{attrs.join(', ')}"
    end
end
ActiveRecord::Base.send :include, TaintedMassAssignmentSecurity

Now, we can expose our foreign keys to the user, should we so desire:

class ItemInquiry < ActiveRecord::Base
  belongs_to :user
  belongs_to :item, :user_mass_assignable => true
  validates_associated :item
end

# ... and elsewhere:
current_user.item_inquiries.create params[:item_inquiry]

or:

class UserDefinedCategory < ActiveRecord::Base
  acts_as_tree # calls belongs_to :parent
  attr_user_mass_assignable :parent_id
end

# ... and elsewhere:
UserDefinedCategory.create params[:user_defined_category]

There is no step 3

This approach isn’t a replacement for those places where the full ramifications of attr_accessible are desirable. It simply provides a minimal safety net that stays out of the developer’s way as much as it can. It can even be dropped in to an existing Rails app with minimal pain. What do you think; would it make sense to for this to be in Rails itself?