ActiveRecord::Encryption concerns; :body column from EncryptedRichText appears twice in filter_parameters and causes generic parameter names to be filtered;

[nvasilevski]

Steps to reproduce

Sorry I'm not able to provide a reproduction step because most likely loading the full app is necessary in order to reproduce the issue:

1. rails new
2. load the app or rails c with eager_load = true
3. ActiveRecord::Encryption.encrypted_attribute_declaration_listeners contains two the same hooks
   And because we have two same hooks declared, we do append the :body parameter name twice:

3.0.3 :004 > Rails.application.config.filter_parameters
 => [:password, :body, :body]

The :body param is coming from here:

rails/actiontext/app/models/action_text/encrypted_rich_text.rb

Line 7 in 638a92f

encrypts :body

Secondary concern

Another issue I wanted to discuss is that having such a generic parameter name included almost "by default" causes many unrelated parameters to be filtered. Should we consider using the "dot notation" in order to scope filtering per encrypted_rich_text.body ?

rails/activesupport/lib/active_support/parameter_filter.rb

Lines 8 to 9 in 167f5c8

	# sub-keys from a hash is possible by using the dot notation:
	# 'credit_card.number'. If a proc is given, each key and value of a hash and

For example:

pf = ActiveSupport::ParameterFilter.new([:body])

my_hash_with_various_unrelated_params = {
  response_body: "response",
  request_body: "request",
  is_my_body_cool: true,
  body: "i'm an encrypted rich text body",
  nested_key: { body_here: "hey", somebody_here: true }
};

pf.filter(my_hash_with_various_unrelated_params)

 =>
{:response_body=>"[FILTERED]",
 :request_body=>"[FILTERED]",
 :is_my_body_cool=>"[FILTERED]",
 :body=>"[FILTERED]",
 :nested_key=>{:body_here=>"[FILTERED]", :somebody_here=>"[FILTERED]"}}

Let me know if it would be better to open a separate issue to discuss the filtered params behaviour. Thanks!

Expected behavior

ActiveRecord::Encryption.encrypted_attribute_declaration_listeners should have only one listener by default, or at least the one defined at:

rails/activerecord/lib/active_record/encryption/configurable.rb

Lines 54 to 56 in 18707ab

	ActiveRecord::Encryption.on_encrypted_attribute_declared do |klass, encrypted_attribute_name|
	application.config.filter_parameters << encrypted_attribute_name unless ActiveRecord::Encryption.config.excluded_from_filter_parameters.include?(name)
	end

Should present only once

Actual behavior

We do have the same hook declared twice:

3.0.3 :001 > ActiveRecord::Encryption.encrypted_attribute_declaration_listeners
 =>
[#<Proc:0x000056129801aa30 /usr/share/rvm/gems/ruby-3.0.3/gems/activerecord-7.0.1/lib/active_record/encryption/configurable.rb:54>,
 #<Proc:0x0000561297fbacc0 /usr/share/rvm/gems/ruby-3.0.3/gems/activerecord-7.0.1/lib/active_record/encryption/configurable.rb:54>]

System configuration

Rails version: "7.0.1" but most likely reproducible with the introduction of encrypted attributes

Ruby version: 3.0.3

[nvasilevski]

Update:
Removing jbuilder from the gemfile fixes the duplicated hook issue.
If I'm not mistaken, the second load is coming from here:
https://github.com/rails/jbuilder/blob/main/lib/jbuilder/railtie.rb#L20

[rafaelfranca]

@jorgemanrubia can you take a look on this? I don't think we should add the encrypted parameters to the parameters filter without using the dot notation.

[jorgemanrubia]

Yup @rafaelfranca. I'll look into it this week. Great report @nvasilevski!

[nvasilevski]

Thanks!
I think I have a fix for the duplicated :body value - #44355

[nvasilevski]

Using the chance just wanted to give a little bit more visibility to this PR - https://github.com/rails/rails/pull/44329/files
It's not directly related to the problem but fixes a tiny issue very close to the area we are discussing in this issue