From 8fb57ace010e462811a41ad1266bdbf4b699c29b Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike@37signals.com>
Date: Mon, 9 Mar 2026 16:13:18 -0400
Subject: [PATCH] Validate polymorphic types against importable models
 allowlist

Add IMPORTABLE_MODEL_NAMES to RecordSet and verify polymorphic type
columns during import check against this allowlist before calling
constantize. This prevents arbitrary class instantiation from
untrusted import ZIP data.
---
 .../account/data_transfer/record_set.rb       | 45 +++++++++-
 .../account/data_transfer/record_set_test.rb  | 89 +++++++++++++++++++
 2 files changed, 133 insertions(+), 1 deletion(-)
 create mode 100644 test/models/account/data_transfer/record_set_test.rb

diff --git a/app/models/account/data_transfer/record_set.rb b/app/models/account/data_transfer/record_set.rb
index 8eeafbe10..2d64d2e1c 100644
--- a/app/models/account/data_transfer/record_set.rb
+++ b/app/models/account/data_transfer/record_set.rb
@@ -4,6 +4,41 @@ class Account::DataTransfer::RecordSet
 
   IMPORT_BATCH_SIZE = 100
 
+  IMPORTABLE_MODEL_NAMES = %w[
+    Access
+    Account
+    ActionText::RichText
+    ActiveStorage::Attachment
+    ActiveStorage::Blob
+    Assignment
+    Board
+    Board::Publication
+    Card
+    Card::ActivitySpike
+    Card::Goldness
+    Card::NotNow
+    Closure
+    Column
+    Comment
+    Entropy
+    Event
+    Filter
+    Mention
+    Notification
+    Notification::Bundle
+    Pin
+    Reaction
+    Step
+    Tag
+    Tagging
+    User
+    User::Settings
+    Watch
+    Webhook
+    Webhook::DelinquencyTracker
+    Webhook::Delivery
+  ].freeze
+
   attr_reader :account, :model, :attributes
 
   def initialize(account:, model:, attributes: nil)
@@ -113,7 +148,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 +158,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/record_set_test.rb b/test/models/account/data_transfer/record_set_test.rb
new file mode 100644
index 000000000..74b457edf
--- /dev/null
+++ b/test/models/account/data_transfer/record_set_test.rb
@@ -0,0 +1,89 @@
+require "test_helper"
+
+class Account::DataTransfer::RecordSetTest < ActiveSupport::TestCase
+  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)
+
+    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)
+
+    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)
+
+    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)
+
+    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