fix: false positive on unpermitted parameters

app/controllers/avo/actions_controller.rb

@@ -64,7 +64,7 @@ def handle
     private
 
     def action_params
-      params.permit(:authenticity_token, :resource_name, :action_id, :button, fields: {})
+      @action_params ||= params.permit(:id, :authenticity_token, :resource_name, :action_id, :button, fields: {})
     end
 
     def set_action

app/controllers/avo/associations_controller.rb

@@ -132,7 +132,7 @@ def set_reflection_field
     end
 
     def attachment_id
-      params[:related_id] || params.require(:fields).permit(:related_id)[:related_id]
+      params[:related_id] || params.dig(:fields, :related_id)
     end
 
     def reflection_class
@@ -200,7 +200,7 @@ def through_reflection?
     end
 
     def additional_params
-      @additional_params ||= params[:fields].permit(@attach_fields&.map(&:id))
+      @additional_params ||= params[:fields].slice(*@attach_fields&.map(&:id))
     end
 
     def set_attach_fields
@@ -214,15 +214,12 @@ def set_attach_fields
 
     def new_join_record
       @resource.fill_record(
-        @reflection.through_reflection.klass.new,
-        additional_params.merge(
-          {
-            source_foreign_key => @attachment_record.id,
-            through_foreign_key => @record.id
-          }
+        @reflection.through_reflection.klass.new(
+          source_foreign_key => @attachment_record.id,
+          through_foreign_key => @record.id
         ),
+        additional_params,
         fields: @attach_fields,
-        extra_params: [source_foreign_key, through_foreign_key]
       )
     end
   end

lib/avo/resources/base.rb

@@ -461,8 +461,12 @@ def fill_record(record, params, extra_params: [], fields: nil)
 
         # Write the user configured extra params to the record
         if extra_params.present?
+          # Pick only the extra params
+          # params at this point are already permited, only need the keys to access them
+          extra_attributes = params.slice(*flatten_keys(extra_params))
+
           # Let Rails fill in the rest of the params
-          record.assign_attributes params.permit(extra_params)
+          record.assign_attributes extra_attributes
         end
 
         record
@@ -613,6 +617,22 @@ def entity_loader(entity)
       def record_param
         @record_param ||= @record.persisted? ? @record.to_param : nil
       end
+
+      private
+
+      def flatten_keys(array)
+        # [:fish_type, information: [:name, :history], reviews_attributes: [:body, :user_id]]
+        # becomes
+        # [:fish_type, :information, :reviews_attributes]
+        array.flat_map do |item|
+          case item
+          when Hash
+            item.keys
+          else
+            item
+          end
+        end
+      end
     end
   end
 end

spec/features/avo/controller_action_spec.rb

@@ -29,14 +29,14 @@
     end
 
     it "assigns the @widget" do
-      post :create, params: {user: {name: "Adrian"}}
+      post :create, params: {user: {first_name: "Adrian"}}
 
       assert assigns(:view).new?
       assert assigns(:view).form?
     end
 
     it "assigns the @widget" do
-      put :update, params: {id: user.id, user: {name: "Adrian"}}
+      put :update, params: {id: user.id, user: {first_name: "Adrian"}}
 
       assert assigns(:view).edit?
       assert assigns(:view).form?

spec/features/avo/custom_fields_in_resource_tools_spec.rb

@@ -4,7 +4,7 @@
   let(:fish) { create :fish, name: :Salmon }
 
   describe "fish information" do
-    it "sends the params to the model" do
+    before(:example) do
       visit avo.edit_resources_fish_path fish
 
       expect(page).to have_text "There should be an image of this fish below 🐠"
@@ -20,11 +20,40 @@
       find('input[name="fish[information][age]"]').set("Fishy age")
 
       expect(properties_fields.count).to be 2
+    end
+
+    it "raise unnpermited params" do
+      expect { save }.to raise_error("found unpermitted parameter: :age")
+    end
+
+    it "sends the params to the model ignoring unpermitted age" do
       expect_any_instance_of(Fish).to receive("fish_type=").with("Fishy type")
       expect_any_instance_of(Fish).to receive("properties=").with(["Fishy property 1", "Fishy property 2"])
       expect_any_instance_of(Fish).to receive("information=").with({name: "Fishy name", history: "Fishy history"})
 
+      ActionController::Parameters.action_on_unpermitted_parameters = :log
       save
+      ActionController::Parameters.action_on_unpermitted_parameters = :warning
+    end
+
+    it "sends all the params to the model" do
+      expect_any_instance_of(Fish).to receive("fish_type=").with("Fishy type")
+      expect_any_instance_of(Fish).to receive("properties=").with(["Fishy property 1", "Fishy property 2"])
+      expect_any_instance_of(Fish).to receive("information=").with({name: "Fishy name", history: "Fishy history", age: "Fishy age"})
+
+      with_temporary_class_option(
+          Avo::Resources::Fish,
+          :extra_params,
+          [
+            :fish_type,
+            :something_else,
+            properties: [],
+            information: [:name, :history, :age],
+            reviews_attributes: [:body, :user_id]
+          ]
+        ) do
+        save
+      end
     end
   end
 end

spec/rails_helper.rb

@@ -49,6 +49,9 @@
 
 # ActiveRecord::Migrator.migrate(File.join(Rails.root, 'db/migrate'))
 
+# Ensure that there are no unpermitted_parameters logs
+ActionController::Parameters.action_on_unpermitted_parameters = :raise
+
 require "support/download_helpers"
 require "support/request_helpers"
 

spec/support/avo_helpers.rb

@@ -8,5 +8,14 @@ def find_component(name)
     def expect_missing_component(name)
       expect(page).not_to have_css "[data-component-name=\"#{name.to_s.underscore}\"]"
     end
+
+    def with_temporary_class_option(_class, option_name, option_value, &block)
+      previous_value =  _class.send(option_name)
+      _class.send(:"#{option_name}=", option_value)
+
+      block.call
+
+      _class.send(:"#{option_name}=", previous_value)
+    end
   end
 end

spec/system/avo/create_via_belongs_to_spec.rb

@@ -28,8 +28,20 @@
 
         expect(page).to have_select('fish_user_id', selected: User.last.name)
 
-        click_on 'Save'
-        sleep 0.2
+        with_temporary_class_option(
+          Avo::Resources::Fish,
+          :extra_params,
+          [
+            :fish_type,
+            :something_else,
+            properties: [],
+            information: [:name, :history, :age],
+            reviews_attributes: [:body, :user_id]
+          ]
+        ) do
+          click_on 'Save'
+          sleep 0.2
+        end
 
         expect(fish.reload.user).to eq User.last
       end
@@ -86,8 +98,20 @@
       expect(page).to have_select('fish_user_id', selected: User.last.name)
 
       expect do
-        click_on 'Save'
-        sleep 0.2
+        with_temporary_class_option(
+          Avo::Resources::Fish,
+          :extra_params,
+          [
+            :fish_type,
+            :something_else,
+            properties: [],
+            information: [:name, :history, :age],
+            reviews_attributes: [:body, :user_id]
+          ]
+        ) do
+          click_on 'Save'
+          sleep 0.2
+        end
       end.to change(Fish, :count).by(1)
 
       expect(Fish.last.user).to eq User.last

spec/system/avo/tags_spec.rb

@@ -143,8 +143,12 @@
       wait_for_tags_to_load(field_value_slot)
       type(:down, :return)
 
+      # TODO: fix "found unpermitted parameter: :user_id-dummy"
+      # tags field passes a dummy param generated from a fake input which is always unpermitted
+      ActionController::Parameters.action_on_unpermitted_parameters = :log
       sleep 0.3
       click_on "Run"
+      ActionController::Parameters.action_on_unpermitted_parameters = :warning
     end
   end