Support yaml safe_load for serialized columns (post ruby 3.1)

[jrafanie]

Background:

As seen in the prior issue https://github.com/ManageIQ/manageiq/issues/22696, we see that psych 4 defaulted to using yaml safe_load and you have to permit classes beyond basic classes otherwise classes found in YAML strings will not be permitted to be loaded.

In rails, they have a similar setting for serialized columns. The default going forward is to not use unsafe_load, but instead always use safe_load.

It is controlled via config.active_record.use_yaml_unsafe_load generally in application.rb.

We'd like to set this to false but it requires we either add classes to a permitted list or change our code to no longer serialize objects in columns, thereby no longer needing to YAML load these classes.

Todo list

This list show the steps needed to disable use_yaml_unsafe_load. The workarounds below demonstrate places tests exposed as failing with ruby 3.1 or with use_yaml_unsafe_load = false. Note, it's possible the YAML.safe_load are outside of serialized columns and can be skipped. Future me, keep in mind that if we're needing to YAML.load stuff in tests, we might also be storing these in columns.

• [ ] Fix code to not serialize objects in columns.

  • [ ] Workaround: https://github.com/ManageIQ/manageiq-ui-classic/pull/8964

    manageiq-ui-classic-e228bd8a61f2/app/controllers/ops_controller/settings/common.rb
    185:      ActiveRecord::Base.yaml_column_permitted_classes |= [subscriptions_to_save.first.class, subsciptions_to_remove.first.class]

  • [x] Workaround: https://github.com/ManageIQ/manageiq-providers-openstack/pull/865

    
    manageiq-providers-openstack-7f75ccc6d154/spec/models/manageiq/providers/openstack/cloud_manager/provision/volume_attachment_spec.rb
    9:    ActiveRecord::Base.yaml_column_permitted_classes = ActiveRecord::Base.yaml_column_permitted_classes | [@flavor.class]

  manageiq-providers-openstack-7f75ccc6d154/app/models/manageiq/providers/openstack/helper_methods.rb 69: ActiveRecord::Base.yaml_column_permitted_classes = ActiveRecord::Base.yaml_column_permitted_classes | [options[:subject].class]

  - [x] This workaround has been removed/fixed in  https://github.com/ManageIQ/manageiq-providers-openstack/pull/866
  
  - [ ] Workaround: https://github.com/ManageIQ/manageiq-providers-kubernetes/pull/512

  manageiq-providers-kubernetes-3fc0756677ca/spec/models/manageiq/providers/kubernetes/container_manager/scanning/job/dispatcher_spec.rb 43: ActiveRecord::Base.yaml_column_permitted_classes |= [ManageIQ::Providers::Openshift::ContainerManager::ContainerImage]

• [ ] Verify code is ok to permit these classes. Do we also serialize this data in columns but it's not exposed in tests? Can we avoid serializing this entirely or is it ok and truly only in test?

  • [ ] Workaround: https://github.com/ManageIQ/manageiq-automation_engine/pull/535

    
    manageiq-automation_engine-9d874ff7fa1b/spec/engine/miq_ae_state_machine_retry_spec.rb
    186:    ActiveRecord::Base.yaml_column_permitted_classes << "MiqAeEngine::StateVarHash"
    284:    expect(YAML.safe_load(q.args.first[:ae_state_data], :permitted_classes => [MiqAeEngine::StateVarHash])).to eq(ae_state_data)

  manageiq-automation_engine-9d874ff7fa1b/spec/engine/miq_ae_method_service/miq_ae_service_model_base_spec.rb 107: expect(YAML.safe_load(svc_service.to_yaml, :permitted_classes => [MiqAeMethodService::MiqAeServiceService])).to eq(svc_service) 113: model_from_yaml = YAML.safe_load(yaml, :permitted_classes => [MiqAeMethodService::MiqAeServiceService])

  manageiq-automation_engine-9d874ff7fa1b/spec/lib/miq_automation_engine/engine/miq_ae_engine/state_var_hash_spec.rb 14: new_start_var_hash = YAML.safe_load(blank_yaml_string, :permitted_classes => [MiqAeEngine::StateVarHash]) 36: YAML.safe_load(yaml_out, :permitted_classes => [MiqAeEngine::StateVarHash]) 44: new_start_var_hash = YAML.safe_load(YAML.dump(state_var_hash), :permitted_classes => [MiqAeEngine::StateVarHash]) 56: new_start_var_hash = YAML.safe_load(YAML.dump(svh_with_user_pass), :permitted_classes => [MiqAeEngine::StateVarHash]) 68: restored_state_var = YAML.safe_load(yaml_out, :permitted_classes => [MiqAeEngine::StateVarHash])

  
  - [ ] It would be nice if in development we display a warning or blow up.
  But having this code is identical to unsafe_load. (anything that goes in is allowed).  This might be addressed sufficiently in https://github.com/ManageIQ/manageiq/pull/22698 for the "failsafe" condition.

[miq-bot]

This issue has been automatically marked as stale because it has not been updated for at least 3 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

[miq-bot]

This issue has been automatically marked as stale because it has not been updated for at least 3 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.