From b742963f7c743510c2d9aa7eb871bd58a63b09e1 Mon Sep 17 00:00:00 2001
From: Rosa Gutierrez <rosa@37signals.com>
Date: Fri, 13 Feb 2026 18:01:53 +0100
Subject: [PATCH] Add account ownership check to import GID resolution

The convert_gids_to_sgids method resolved GIDs globally and minted
valid SGIDs without verifying the record belongs to the importing
account. This adds the same account_id check that the export path
already has, plus RecordNotFound handling for invalid GIDs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 .../action_text_rich_text_record_set.rb       | 13 ++++--
 .../action_text_rich_text_record_set_test.rb  | 40 +++++++++++++++++++
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/app/models/account/data_transfer/action_text_rich_text_record_set.rb b/app/models/account/data_transfer/action_text_rich_text_record_set.rb
index ec2f4e17f..fa1a23cb9 100644
--- a/app/models/account/data_transfer/action_text_rich_text_record_set.rb
+++ b/app/models/account/data_transfer/action_text_rich_text_record_set.rb
@@ -84,9 +84,16 @@ class Account::DataTransfer::ActionTextRichTextRecordSet < Account::DataTransfer
         gid = GlobalID.parse(node["gid"])
 
         if gid
-          record = gid.find
-          node["sgid"] = record.attachable_sgid
-          node.remove_attribute("gid")
+          record = begin
+            gid.find
+          rescue ActiveRecord::RecordNotFound
+            nil
+          end
+
+          if record&.account_id == account.id
+            node["sgid"] = record.attachable_sgid
+            node.remove_attribute("gid")
+          end
         end
       end
 
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 ffbd940f5..14a7aeb9c 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
@@ -41,4 +41,44 @@ class Account::DataTransfer::ActionTextRichTextRecordSetTest < ActiveSupport::Te
     tempfile&.unlink
     importing_account&.destroy
   end
+
+  test "convert_gids_to_sgids skips GIDs belonging to another account" do
+    victim_tag = tags(:web)
+    attacker_account = accounts(:initech)
+    assert_not_equal attacker_account.id, victim_tag.account_id
+
+    cross_tenant_gid = victim_tag.to_global_id.to_s
+    html = %(<action-text-attachment gid="#{cross_tenant_gid}"></action-text-attachment>)
+
+    record_set = Account::DataTransfer::ActionTextRichTextRecordSet.new(attacker_account)
+    result = record_set.send(:convert_gids_to_sgids, html)
+
+    assert_no_match(/sgid=/, result, "Cross-tenant GID must not be converted to SGID")
+    assert_match(/gid=/, result, "Original GID should remain unconverted")
+  end
+
+  test "convert_gids_to_sgids converts GIDs belonging to the same account" do
+    own_tag = tags(:web)
+    own_account = accounts(:"37s")
+    assert_equal own_account.id, own_tag.account_id
+
+    same_account_gid = own_tag.to_global_id.to_s
+    html = %(<action-text-attachment gid="#{same_account_gid}"></action-text-attachment>)
+
+    record_set = Account::DataTransfer::ActionTextRichTextRecordSet.new(own_account)
+    result = record_set.send(:convert_gids_to_sgids, html)
+
+    assert_match(/sgid=/, result, "Same-account GID should be converted to SGID")
+    assert_no_match(/ gid=/, result, "GID should be removed after SGID conversion")
+  end
+
+  test "convert_gids_to_sgids handles non-existent record GIDs gracefully" do
+    nonexistent_gid = "gid://fizzy/Tag/00000000000000000000000000"
+    html = %(<action-text-attachment gid="#{nonexistent_gid}"></action-text-attachment>)
+
+    record_set = Account::DataTransfer::ActionTextRichTextRecordSet.new(accounts(:"37s"))
+    result = record_set.send(:convert_gids_to_sgids, html)
+
+    assert_no_match(/sgid=/, result, "Non-existent record should not produce SGID")
+  end
 end