diff --git a/app/models/account/data_transfer/manifest.rb b/app/models/account/data_transfer/manifest.rb index 2159926d5..9de76f9bf 100644 --- a/app/models/account/data_transfer/manifest.rb +++ b/app/models/account/data_transfer/manifest.rb @@ -26,20 +26,42 @@ class Account::DataTransfer::Manifest [ Account::DataTransfer::AccountRecordSet.new(account), Account::DataTransfer::UserRecordSet.new(account), - *build_record_sets(::User::Settings, ::Tag, ::Board, ::Column), + *build_record_sets( + ::User::Settings, + ::Tag, + ::Board, + ::Column + ), Account::DataTransfer::EntropyRecordSet.new(account), *build_record_sets( - ::Board::Publication, ::Webhook, ::Access, ::Card, ::Comment, ::Step, - ::Assignment, ::Tagging, ::Closure, ::Card::Goldness, ::Card::NotNow, - ::Card::ActivitySpike, ::Watch, ::Pin, ::Reaction, ::Mention, - ::Filter, ::Webhook::DelinquencyTracker, ::Event, - ::Notification, ::Notification::Bundle, ::Webhook::Delivery + ::Board::Publication, + ::Webhook, + ::Access, + ::Card, + ::Comment, + ::Step, + ::Assignment, + ::Tagging, + ::Closure, + ::Card::Goldness, + ::Card::NotNow, + ::Card::ActivitySpike, + ::Watch, + ::Pin, + ::Reaction, + ::Mention, + ::Filter, + ::Webhook::DelinquencyTracker, + ::Event, + ::Notification, + ::Notification::Bundle, + ::Webhook::Delivery ), Account::DataTransfer::ActiveStorage::BlobRecordSet.new(account), *build_record_sets(::ActiveStorage::Attachment), Account::DataTransfer::ActionText::RichTextRecordSet.new(account), Account::DataTransfer::ActiveStorage::FileRecordSet.new(account) - ] + ].then { set_importable_model_names(it) } end def build_record_sets(*models) @@ -47,4 +69,10 @@ class Account::DataTransfer::Manifest Account::DataTransfer::RecordSet.new(account: account, model: model) end end + + def set_importable_model_names(record_sets) + model_names = record_sets.filter_map { |record_set| record_set.model&.name } + record_sets.each { |record_set| record_set.importable_model_names = model_names } + record_sets + end end diff --git a/app/models/account/data_transfer/record_set.rb b/app/models/account/data_transfer/record_set.rb index 8eeafbe10..2fba9402d 100644 --- a/app/models/account/data_transfer/record_set.rb +++ b/app/models/account/data_transfer/record_set.rb @@ -4,12 +4,14 @@ class Account::DataTransfer::RecordSet IMPORT_BATCH_SIZE = 100 + attr_accessor :importable_model_names attr_reader :account, :model, :attributes - def initialize(account:, model:, attributes: nil) + def initialize(account:, model:, attributes: nil, importable_model_names: nil) @account = account @model = model @attributes = (attributes || model.column_names).map(&:to_s) + @importable_model_names = importable_model_names || [ model.name ] end def export(to:, start: nil) @@ -113,7 +115,7 @@ class Account::DataTransfer::RecordSet def check_association_doesnt_exist(data, association, associated_id) if association.polymorphic? type_column = association.foreign_type.to_s - associated_class = data[type_column].constantize + associated_class = verify_model_type(data[type_column]) else associated_class = association.klass end @@ -123,6 +125,14 @@ class Account::DataTransfer::RecordSet end end + def verify_model_type(type_name) + if importable_model_names.include?(type_name) + type_name.constantize + else + raise IntegrityError, "Unrecognized model type: #{type_name}" + end + end + def skip_to(file_list, last_id) index = file_list.index(last_id) diff --git a/test/models/account/data_transfer/action_text/rich_text_record_set_test.rb b/test/models/account/data_transfer/action_text/rich_text_record_set_test.rb index a28935a3e..2fe2c4b93 100644 --- a/test/models/account/data_transfer/action_text/rich_text_record_set_test.rb +++ b/test/models/account/data_transfer/action_text/rich_text_record_set_test.rb @@ -30,6 +30,7 @@ class Account::DataTransfer::ActionText::RichTextRecordSetTest < ActiveSupport:: reader = ZipFile::Reader.new(tempfile) record_set = Account::DataTransfer::ActionText::RichTextRecordSet.new(importing_account) + record_set.importable_model_names = %w[ ActionText::RichText Card ] error = assert_raises(Account::DataTransfer::RecordSet::IntegrityError) do record_set.check(from: reader) diff --git a/test/models/account/data_transfer/record_set_test.rb b/test/models/account/data_transfer/record_set_test.rb new file mode 100644 index 000000000..8c8d75e5e --- /dev/null +++ b/test/models/account/data_transfer/record_set_test.rb @@ -0,0 +1,93 @@ +require "test_helper" + +class Account::DataTransfer::RecordSetTest < ActiveSupport::TestCase + setup do + @importable_model_names = %w[ Card Board Event ] + end + + test "check rejects polymorphic type not in the importable models allowlist" do + event_data = build_event_data(eventable_type: "Identity") + + record_set = Account::DataTransfer::RecordSet.new(account: importing_account, model: Event, importable_model_names: @importable_model_names) + + error = assert_raises(Account::DataTransfer::RecordSet::IntegrityError) do + record_set.check(from: build_reader(dir: "events", data: event_data)) + end + + assert_match(/unrecognized.*type/i, error.message) + end + + test "check rejects nonexistent polymorphic type" do + event_data = build_event_data(eventable_type: "Nonexistent::ClassName") + + record_set = Account::DataTransfer::RecordSet.new(account: importing_account, model: Event, importable_model_names: @importable_model_names) + + error = assert_raises(Account::DataTransfer::RecordSet::IntegrityError) do + record_set.check(from: build_reader(dir: "events", data: event_data)) + end + + assert_match(/unrecognized.*type/i, error.message) + end + + test "check rejects non-ActiveRecord class used as polymorphic type" do + event_data = build_event_data(eventable_type: "ActiveSupport::BroadcastLogger") + + record_set = Account::DataTransfer::RecordSet.new(account: importing_account, model: Event, importable_model_names: @importable_model_names) + + error = assert_raises(Account::DataTransfer::RecordSet::IntegrityError) do + record_set.check(from: build_reader(dir: "events", data: event_data)) + end + + assert_match(/unrecognized.*type/i, error.message) + end + + test "check accepts polymorphic type in the importable models allowlist" do + event_data = build_event_data(eventable_type: "Card") + + record_set = Account::DataTransfer::RecordSet.new(account: importing_account, model: Event, importable_model_names: @importable_model_names) + + assert_nothing_raised do + record_set.check(from: build_reader(dir: "events", data: event_data)) + end + end + + private + def importing_account + @importing_account ||= Account.create!(name: "Importing Account", external_account_id: 99999999) + end + + def build_event_data(eventable_type:) + { + "id" => "test_event_id_12345678901234", + "account_id" => "nonexistent_account_id_1234567", + "board_id" => "nonexistent_board_id_12345678", + "creator_id" => "nonexistent_user_id_123456789", + "eventable_type" => eventable_type, + "eventable_id" => "nonexistent_id_1234567890123", + "action" => "created", + "particulars" => "{}", + "created_at" => Time.current.iso8601, + "updated_at" => Time.current.iso8601 + } + end + + def build_reader(dir:, data:) + tempfile = Tempfile.new([ "import_test", ".zip" ]) + tempfile.binmode + + writer = ZipFile::Writer.new(tempfile) + writer.add_file("data/#{dir}/#{data['id']}.json", data.to_json) + writer.close + tempfile.rewind + + @tempfiles ||= [] + @tempfiles << tempfile + + ZipFile::Reader.new(tempfile) + end + + def teardown + @tempfiles&.each { |f| f.close; f.unlink } + @importing_account&.destroy + end +end