From f8337b04fe9bccb1ad021a8579d85d366605e1b2 Mon Sep 17 00:00:00 2001
From: David Roetzel <david@roetzel.de>
Date: Mon, 27 Jul 2026 16:53:20 +0200
Subject: [PATCH] Improve enforcement of collection item limit (#39969)

---
 app/models/collection.rb                      |  2 +-
 .../process_featured_item_service.rb          |  7 +++--
 spec/models/collection_spec.rb                | 28 ++++++++++++-------
 .../process_featured_item_service_spec.rb     | 11 ++++++++
 4 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/app/models/collection.rb b/app/models/collection.rb
index 329460b574c..0c1535ab39d 100644
--- a/app/models/collection.rb
+++ b/app/models/collection.rb
@@ -54,7 +54,7 @@ class Collection < ApplicationRecord
             if: :remote?
   validates :language, language: { if: :local?, allow_nil: true }
   validate :tag_is_usable
-  validate :items_do_not_exceed_limit
+  validate :items_do_not_exceed_limit, if: :local?
   validate :user_does_not_exceed_limit, on: :create
 
   scope :with_items, -> { includes(:collection_items).merge(CollectionItem.with_accounts) }
diff --git a/app/services/activitypub/process_featured_item_service.rb b/app/services/activitypub/process_featured_item_service.rb
index fea25ca3329..c89937eec11 100644
--- a/app/services/activitypub/process_featured_item_service.rb
+++ b/app/services/activitypub/process_featured_item_service.rb
@@ -17,9 +17,10 @@ class ActivityPub::ProcessFeaturedItemService
     return if non_matching_actor_and_approval_uris?
     return if non_supported_object_type?
 
-    with_redis_lock("collection_item:#{@item_json['id']}") do
-      @collection_item = existing_item || pre_approved_item || new_item
+    @collection_item = existing_item || pre_approved_item || new_item
+    return if @collection_item.nil?
 
+    with_redis_lock("collection_item:#{@item_json['id']}") do
       @collection_item.position = position unless position.nil?
       @collection_item.update!(
         uri: @item_json['id'],
@@ -45,6 +46,8 @@ class ActivityPub::ProcessFeaturedItemService
   end
 
   def new_item
+    return if @collection.collection_items.count >= ActivityPub::ProcessFeaturedCollectionService::ITEMS_LIMIT
+
     @collection.collection_items.new(
       created_at: @item_json['published']
     )
diff --git a/spec/models/collection_spec.rb b/spec/models/collection_spec.rb
index 921a30ea7c7..3512bf48a3b 100644
--- a/spec/models/collection_spec.rb
+++ b/spec/models/collection_spec.rb
@@ -51,20 +51,28 @@ RSpec.describe Collection do
     end
 
     context 'when there are more items than allowed' do
-      subject { Fabricate.build(:collection, collection_items:) }
-
       let(:collection_items) { Fabricate.build_times(described_class::MAX_ITEMS + 1, :collection_item, collection: nil) }
 
-      it { is_expected.to_not be_valid }
+      context 'when collection is local' do
+        subject { Fabricate.build(:collection, collection_items:) }
 
-      context 'when the limit is only exceeded due to `rejected` and `revoked` items' do
-        let(:collection_items) do
-          items = Fabricate.build_times(described_class::MAX_ITEMS - 2, :collection_item, collection: nil, state: :accepted)
-          items << Fabricate.build(:collection_item, collection: nil, state: :pending)
-          items << Fabricate.build(:collection_item, collection: nil, state: :rejected)
-          items << Fabricate.build(:collection_item, collection: nil, state: :revoked)
-          items
+        it { is_expected.to_not be_valid }
+
+        context 'when the limit is only exceeded due to `rejected` and `revoked` items' do
+          let(:collection_items) do
+            items = Fabricate.build_times(described_class::MAX_ITEMS - 2, :collection_item, collection: nil, state: :accepted)
+            items << Fabricate.build(:collection_item, collection: nil, state: :pending)
+            items << Fabricate.build(:collection_item, collection: nil, state: :rejected)
+            items << Fabricate.build(:collection_item, collection: nil, state: :revoked)
+            items
+          end
+
+          it { is_expected.to be_valid }
         end
+      end
+
+      context 'when collection is remote' do
+        subject { Fabricate.build(:remote_collection, collection_items:) }
 
         it { is_expected.to be_valid }
       end
diff --git a/spec/services/activitypub/process_featured_item_service_spec.rb b/spec/services/activitypub/process_featured_item_service_spec.rb
index 3b0f633fe0b..706d5c99070 100644
--- a/spec/services/activitypub/process_featured_item_service_spec.rb
+++ b/spec/services/activitypub/process_featured_item_service_spec.rb
@@ -81,6 +81,17 @@ RSpec.describe ActivityPub::ProcessFeaturedItemService do
           expect(new_item.position).to eq 1
         end
       end
+
+      context "when the collection's item count is already at the limit" do
+        before do
+          stub_const('ActivityPub::ProcessFeaturedCollectionService::ITEMS_LIMIT', 3)
+          Fabricate.times(3, :collection_item, collection:)
+        end
+
+        it 'does not create an item' do
+          expect { subject.call(collection, object) }.to_not change(collection.collection_items, :count)
+        end
+      end
     end
 
     context 'when item exists' do