Skip to content

Pull `hashie-forbidden_attributes` into our codebase

What does this MR do and why?

We suspect we don't use the monkey patch from https://github.com/Maxim-Filimonov/hashie-forbidden_attributes at all.
Overall, the gem was last updated 5 years and does not have regular CI runs. Its tests are currently outdated using old version of Rails.

To remove it safely, we pull this code in our codebase and add a logging event.
If no logging will be invoked after running this code for a milestone, this code could be removed.

Discussed in: #378398 (comment 1143133426)

🛠 Pipeline with e2e & CNG: https://gitlab.com/gitlab-org/gitlab/-/pipelines/677508560 (failures are from the master, downstreams green)\

How to set up and validate locally

On this branch, in the rails console:

[6] pry(main)> Hashie::Mash.new.respond_to?(:permitted?)
=> false
[7] pry(main)> Hashie::Mash.new.permitted?
ArgumentError: ArgumentError
from /Users/al/dev/gitlab-development-kit/gitlab/config/initializers/hashie_mash_permitted_patch.rb:41:in `method_missing'

If you tail -f log/application_json.log:

{"severity":"INFO","time":"2022-10-25T16:54:52.764Z","correlation_id":null,"message":"Hashie::Mash#respond_to?(:permitted?)","caller":["(pry):1:in `respond_to?'","(pry):1:in `__pry__'"]}
{"severity":"INFO","time":"2022-10-25T16:55:14.180Z","correlation_id":null,"message":"Hashie::Mash#permitted?","caller":["(pry):2:in `__pry__'"]}

On master:

[1] pry(main)> Hashie::Mash.new.respond_to?(:permitted?)
=> true
[2] pry(main)> Hashie::Mash.new.permitted?
=> false

How to validate in more functional fashion

The examples above are rather "unit-test-style" validations.

To validate the changes in context of controller, I did:

  1. Pull this Branch

  2. I picked a convenient controller covered with tests and wrapped its params into Mash:

diff --git a/app/controllers/admin/application_settings/appearances_controller.rb b/app/controllers/admin/application_settings/appearances_controller.rb
index cf765c96a8f7..2c6bd917ab75 100644
--- a/app/controllers/admin/application_settings/appearances_controller.rb
+++ b/app/controllers/admin/application_settings/appearances_controller.rb
@@ -24,7 +24,7 @@ def create
   end

   def update
-    if @appearance.update(appearance_params)
+    if @appearance.update(Hashie::Mash.new(appearance_params))
       redirect_to admin_application_settings_appearances_path, notice: _('Appearance was successfully updated.')
     else
       render action: 'show'
  1. And run bundle exec rspec spec/controllers/admin/application_settings/appearances_controller_spec.rb:51.
  2. It should be green
  3. Then comment out the contents of config/initializers/hashie_mash_permitted_patch.rb
  4. You will get:
  1) Admin::ApplicationSettings::AppearancesController PUT #update updates appearance with footer and header message
     Failure/Error: connection.send(...)

     ActiveModel::ForbiddenAttributesError:
       ActiveModel::ForbiddenAttributesError
     # ./lib/gitlab/database/load_balancing/connection_proxy.rb:120:in `block in write_using_load_balancer'

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #378398 (closed)

Edited by Aleksei Lipniagov

Merge request reports