discourse · mdoggydog · Feb 23, 2025
diff --git a/app/controllers/admin/email_logs_controller.rb b/app/controllers/admin/email_logs_controller.rb
@@ -61,7 +61,7 @@ def rejected
   def incoming
     params.require(:id)
     incoming_email = IncomingEmail.find(params[:id].to_i)
-    serializer = IncomingEmailDetailsSerializer.new(incoming_email, root: false)
+    serializer = IncomingEmailDetailsSerializer.new(incoming_email, scope: PlaceholderGuardian.new, root: false)
     render_json_dump(serializer)
   end
 
@@ -93,7 +93,7 @@ def incoming_from_bounced
 
       raise Discourse::NotFound if incoming_email.nil?
 
-      serializer = IncomingEmailDetailsSerializer.new(incoming_email, root: false)
+      serializer = IncomingEmailDetailsSerializer.new(incoming_email, scope: PlaceholderGuardian.new, root: false)
       render_json_dump(serializer)
     rescue => e
       render json: { errors: [e.message] }, status: 404

diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb
@@ -213,7 +213,7 @@ def update
     @theme = Theme.include_relations.find_by(id: params[:id])
     raise Discourse::InvalidParameters.new(:id) unless @theme
 
-    original_json = ThemeSerializer.new(@theme, root: false).to_json
+    original_json = ThemeSerializer.new(@theme, scope: PlaceholderGuardian.new, root: false).to_json
     disables_component = [false, "false"].include?(theme_params[:enabled])
     enables_component = [true, "true"].include?(theme_params[:enabled])
 

diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
@@ -129,7 +129,7 @@ def suspend
             full_suspend_reason: full_reason,
             suspended_till: user.suspended_till,
             suspended_at: user.suspended_at,
-            suspended_by: BasicUserSerializer.new(current_user, root: false).as_json,
+            suspended_by: BasicUserSerializer.new(current_user, scope: PlaceholderGuardian.new, root: false).as_json,
           },
         )
       end
@@ -327,6 +327,7 @@ def silence
             silenced_by:
               BasicUserSerializer.new(
                 current_user,
+                scope: PlaceholderGuardian.new,
                 root: false,
                 include_silence_reason: true,
               ).as_json,
@@ -390,7 +391,7 @@ def destroy
         else
           render json: {
                    deleted: false,
-                   user: AdminDetailedUserSerializer.new(user, root: false).as_json,
+                   user: AdminDetailedUserSerializer.new(user, scope: PlaceholderGuardian.new, root: false).as_json,
                  }
         end
       rescue UserDestroyer::PostsExistError
@@ -510,7 +511,7 @@ def anonymize
       render json: success_json.merge(username: user.username)
     else
       render json:
-               failed_json.merge(user: AdminDetailedUserSerializer.new(user, root: false).as_json)
+               failed_json.merge(user: AdminDetailedUserSerializer.new(user, scope: PlaceholderGuardian.new, root: false).as_json)
     end
   end
 

diff --git a/app/controllers/drafts_controller.rb b/app/controllers/drafts_controller.rb
@@ -120,7 +120,7 @@ def create
           end
 
           if conflict
-            conflict_user = BasicUserSerializer.new(post.last_editor, root: false)
+            conflict_user = BasicUserSerializer.new(post.last_editor, scope: PlaceholderGuardian.new, root: false)
             json.merge!(conflict_user:)
           end
         end

diff --git a/app/controllers/post_localizations_controller.rb b/app/controllers/post_localizations_controller.rb
@@ -14,6 +14,7 @@ def show
                ActiveModel::ArraySerializer.new(
                  localizations,
                  each_serializer: PostLocalizationSerializer,
+                 scope: PlaceholderGuardian.new,
                  root: false,
                ).as_json,
              status: :ok

diff --git a/app/controllers/presence_controller.rb b/app/controllers/presence_controller.rb
@@ -30,7 +30,7 @@ def get
     names.each do |name|
       channel = PresenceChannel.new(name)
       if channel.can_view?(user_id: current_user&.id, group_ids: user_group_ids)
-        result[name] = PresenceChannelStateSerializer.new(channel.state, root: nil)
+        result[name] = PresenceChannelStateSerializer.new(channel.state, scope: PlaceholderGuardian.new, root: nil)
       else
         result[name] = nil
       end

diff --git a/app/controllers/reviewable_claimed_topics_controller.rb b/app/controllers/reviewable_claimed_topics_controller.rb
@@ -45,7 +45,7 @@ def notify_users(topic, claimed_by, automatic)
     if claimed_by.present?
       data = {
         topic_id: topic.id,
-        user: BasicUserSerializer.new(claimed_by, root: false).as_json,
+        user: BasicUserSerializer.new(claimed_by, scope: PlaceholderGuardian.new, root: false).as_json,
         automatic:,
       }
     else

diff --git a/app/controllers/tag_groups_controller.rb b/app/controllers/tag_groups_controller.rb
@@ -13,6 +13,7 @@ def index
       ActiveModel::ArraySerializer.new(
         tag_groups,
         each_serializer: TagGroupSerializer,
+        scope: PlaceholderGuardian.new,
         root: "tag_groups",
       )
     respond_to do |format|
@@ -25,7 +26,7 @@ def index
   end
 
   def show
-    serializer = TagGroupSerializer.new(@tag_group)
+    serializer = TagGroupSerializer.new(@tag_group, scope: PlaceholderGuardian.new)
     respond_to do |format|
       format.html do
         store_preloaded "tagGroup", MultiJson.dump(serializer)
@@ -41,6 +42,7 @@ def new
       ActiveModel::ArraySerializer.new(
         tag_groups,
         each_serializer: TagGroupSerializer,
+        scope: PlaceholderGuardian.new,
         root: "tag_groups",
       )
     store_preloaded "tagGroup", MultiJson.dump(serializer)
@@ -53,7 +55,7 @@ def create
     if @tag_group.save
       StaffActionLogger.new(current_user).log_tag_group_create(
         @tag_group.name,
-        TagGroupSerializer.new(@tag_group).to_json(root: false),
+        TagGroupSerializer.new(@tag_group, scope: PlaceholderGuardian.new).to_json(root: false),
       )
       render_serialized(@tag_group, TagGroupSerializer)
     else
@@ -63,10 +65,10 @@ def create
 
   def update
     guardian.ensure_can_admin_tag_groups!
-    old_data = TagGroupSerializer.new(@tag_group).to_json(root: false)
+    old_data = TagGroupSerializer.new(@tag_group, scope: PlaceholderGuardian.new).to_json(root: false)
     json_result(@tag_group, serializer: TagGroupSerializer) do |tag_group|
       @tag_group.update(tag_groups_params)
-      new_data = TagGroupSerializer.new(@tag_group).to_json(root: false)
+      new_data = TagGroupSerializer.new(@tag_group, scope: PlaceholderGuardian.new).to_json(root: false)
       StaffActionLogger.new(current_user).log_tag_group_change(@tag_group.name, old_data, new_data)
     end
   end
@@ -75,7 +77,7 @@ def destroy
     guardian.ensure_can_admin_tag_groups!
     StaffActionLogger.new(current_user).log_tag_group_destroy(
       @tag_group.name,
-      TagGroupSerializer.new(@tag_group).to_json(root: false),
+      TagGroupSerializer.new(@tag_group, scope: PlaceholderGuardian.new).to_json(root: false),
     )
     @tag_group.destroy
     render json: success_json

diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
@@ -144,7 +144,7 @@ def show
                   custom_message_params: {
                     group: group.name,
                   },
-                  group: serialize_data(group, BasicGroupSerializer, root: false),
+                  group: serialize_data(group, BasicGroupSerializer, scope: PlaceholderGuardian.new, root: false),
                 )
         end
 
@@ -542,7 +542,7 @@ def status
     render json:
              success_json.merge!(
                topic_status_update:
-                 TopicTimerSerializer.new(TopicTimer.find_by(topic: @topic), root: false),
+                 TopicTimerSerializer.new(TopicTimer.find_by(topic: @topic), scope: PlaceholderGuardian.new, root: false),
              )
   end
 

diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb
@@ -268,7 +268,7 @@ def xhr_not_allowed
   def self.serialize_upload(data)
     # as_json.as_json is not a typo... as_json in AM serializer returns keys as symbols, we need them
     # as strings here
-    serialized = UploadSerializer.new(data, root: nil).as_json.as_json if Upload === data
+    serialized = UploadSerializer.new(data, scope: PlaceholderGuardian.new, root: nil).as_json.as_json if Upload === data
     serialized ||= (data || {}).as_json
   end
 

diff --git a/app/controllers/user_status_controller.rb b/app/controllers/user_status_controller.rb
@@ -6,7 +6,7 @@ class UserStatusController < ApplicationController
   def get
     ensure_feature_enabled
     respond_to do |format|
-      format.json { render json: UserStatusSerializer.new(current_user.user_status, root: false) }
+      format.json { render json: UserStatusSerializer.new(current_user.user_status, scope: PlaceholderGuardian.new, root: false) }
     end
   end
 

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
@@ -2280,6 +2280,7 @@ def serialize_found_users(users)
       ActiveModel::ArraySerializer.new(
         users,
         each_serializer: FoundUserSerializer,
+        scope: PlaceholderGuardian.new,
         include_status: true,
       )
     { users: serializer.as_json }

diff --git a/app/jobs/regular/backup_chunks_merger.rb b/app/jobs/regular/backup_chunks_merger.rb
@@ -32,7 +32,7 @@ def execute(args)
       # push an updated list to the clients
       store = BackupRestore::BackupStore.create
       data =
-        ActiveModel::ArraySerializer.new(store.files, each_serializer: BackupFileSerializer).as_json
+        ActiveModel::ArraySerializer.new(store.files, each_serializer: BackupFileSerializer, scope: PlaceholderGuardian.new).as_json
       MessageBus.publish("/admin/backups", data, group_ids: [Group::AUTO_GROUPS[:staff]])
     end
   end

diff --git a/app/jobs/scheduled/redeliver_web_hook_events.rb b/app/jobs/scheduled/redeliver_web_hook_events.rb
@@ -56,7 +56,7 @@ def publish_webhook_event(web_hook_event, web_hook, type)
         "/web_hook_events/#{web_hook.id}",
         {
           type: type,
-          web_hook_event: AdminWebHookEventSerializer.new(web_hook_event, root: false).as_json,
+          web_hook_event: AdminWebHookEventSerializer.new(web_hook_event, scope: PlaceholderGuardian.new, root: false).as_json,
         },
       )
     end

diff --git a/app/models/category.rb b/app/models/category.rb
@@ -710,14 +710,14 @@ def publish_category
       if group_ids.present?
         MessageBus.publish(
           "/categories",
-          { categories: ActiveModel::ArraySerializer.new([self]).as_json },
+          { categories: ActiveModel::ArraySerializer.new([self], scope: PlaceholderGuardian.new).as_json },
           group_ids: group_ids,
         )
       end
     else
       MessageBus.publish(
         "/categories",
-        { categories: ActiveModel::ArraySerializer.new([self]).as_json },
+        { categories: ActiveModel::ArraySerializer.new([self], scope: PlaceholderGuardian.new).as_json },
       )
     end
   end

diff --git a/app/models/group.rb b/app/models/group.rb
@@ -830,7 +830,7 @@ def add(user, notify: false, automatic: false)
     if self.categories.count < PUBLISH_CATEGORIES_LIMIT
       MessageBus.publish(
         "/categories",
-        { categories: ActiveModel::ArraySerializer.new(self.categories).as_json },
+        { categories: ActiveModel::ArraySerializer.new(self.categories, scope: PlaceholderGuardian.new).as_json },
         user_ids: [user.id],
       )
     else

diff --git a/app/models/site.rb b/app/models/site.rb
@@ -112,6 +112,7 @@ def self.all_categories_cache
         ActiveModel::ArraySerializer.new(
           categories,
           each_serializer: SiteCategorySerializer,
+          scope: PlaceholderGuardian.new,
         ).as_json
       end
   end

diff --git a/app/models/topic.rb b/app/models/topic.rb
@@ -2099,7 +2099,7 @@ def self.publish_stats_to_clients!(topic_id, type, opts = {})
       stats = {
         posts_count: topic.posts_count,
         last_posted_at: topic.last_posted_at.as_json,
-        last_poster: BasicUserSerializer.new(topic.last_poster, root: false).as_json,
+        last_poster: BasicUserSerializer.new(topic.last_poster, scope: PlaceholderGuardian.new, root: false).as_json,
       }
     else
       stats = nil

diff --git a/app/models/user.rb b/app/models/user.rb
@@ -854,7 +854,7 @@ def publish_notifications_state
 
     # publish last notification json with the message so we can apply an update
     notification = notifications.visible.order("notifications.created_at desc").first
-    json = NotificationSerializer.new(notification).as_json if notification
+    json = NotificationSerializer.new(notification, scope: PlaceholderGuardian.new).as_json if notification
 
     sql = (<<~SQL)
        SELECT * FROM (

diff --git a/app/serializers/admin_user_action_serializer.rb b/app/serializers/admin_user_action_serializer.rb
@@ -69,7 +69,7 @@ def moderator_action
   end
 
   def deleted_by
-    BasicUserSerializer.new(object.deleted_by, root: false).as_json
+    BasicUserSerializer.new(object.deleted_by, scope: scope, root: false).as_json
   end
 
   def include_deleted_by?

diff --git a/app/serializers/application_serializer.rb b/app/serializers/application_serializer.rb
@@ -5,6 +5,100 @@
 class ApplicationSerializer < ActiveModel::Serializer
   embed :ids, include: true
 
+  # NB: Set environment variable STRICT_SERIALIZER_SCOPE to either
+  #     "nil_or_guardian" or "only_guardian" to enforce stricter checking
+  #     of scope parameters.
+  #
+  #     See notes in initialize(), below, and in PlaceholderGuardian.new
+  #
+  @@require_strict_scope = ENV["STRICT_SERIALIZER_SCOPE"]&.intern
+
+  def self.require_strict_scope
+    @@require_strict_scope
+  end
+
+  def initialize(*args)
+    # Ensure that a scope: argument was provided to this Serializer.
+    #
+    # In a test environment, we treat this as an assertion and fail hard if no
+    # "scope:" has been provided.  Outside of testing, we merely log an error,
+    # since there are codepaths (that have no test coverage, apparently) that
+    # manage to function without a scope, and we do not want to break them in
+    # production.
+    #
+    # If you are reading this because you have encountered this warning/error,
+    # then you should add an appropriate "scope:" argument to the call-site
+    # which triggered it.
+    #
+    #  * If one serializer is constructing another one, simply forwarding the
+    #    scope of the parent serializer is almost always the right thing to do,
+    #    e.g., add "scope: scope".
+    #
+    #  * The scope should be a Guardian instance.  If there is already a
+    #    Guardian instance available (e.g., within the context of a Controller),
+    #    then simply using it is almost always the right thing to do,
+    #    e.g., add "scope: guardian".
+    #
+    #  * The guiding principle is that the Guardian should reflect what the
+    #    potential receiver of the serialized object is allowed to receive.
+    #    E.g., if the serialized object is going to be sent to user_x, then
+    #    a Guardian reflecting user_x ("Guardian.new(user_x)") is usually
+    #    (but not always) correct.
+    #
+    #  * If it is not clear whose Guardian to use and you need to kick-the-can
+    #    down-the-road a bit longer, use "scope: PlaceholderGuardian.new".  This
+    #    will make it easy to track this technical debt and find it later on.
+    #
+    unless args[-1].has_key?(:scope)
+      Rails.logger.error("Serializer initialized without scope:")
+      raise "Serializer initialized without scope:" if ENV["RAILS_ENV"] == "test"
+    end
+
+    super
+
+    # TODO (a) Impose the more stringent condition that every scope is either
+    #          nil or an instance of Guardian.
+    #          (I.e., set default @@require_strict_scope to :nil_or_guardian)
+    #
+    # TODO (b) Impose the even more stringent condition that every scope is
+    #          only an instance of Guardian.
+    #          (I.e., set default @@require_strict_scope to :only_guardian)
+    #
+    # Note, however:
+    #
+    #  1) There are bits of legacy code that provide non-Guardian scopes to
+    #     certain serializers, so both (a) and (b) must wait until those bits
+    #     are fixed (or, until someone is trying to find/fix those bits).
+    #
+    #  2) If there are still instances of "PlaceholderGuardian.new" in the
+    #     code, then PlaceholderGuardian#new will need to be tweaked to
+    #     generate "exploding placeholders" instead of nil, before (b) can
+    #     be put into effect.  (See the explanation in PlaceholderGuardian.)
+    #
+    #  3) There are bits of legacy code that check for and paper-over nil
+    #     scopes (e.g. "scope && scope.can_xxx?"), and these bits will explode
+    #     if they encounter an "exploding placeholder".  So, these checks need
+    #     to be removed before (2) can be done.
+    #
+    if @@require_strict_scope === :nil_or_guardian
+      # Ensure that the scope is nil or an instance of Guardian.
+      unless (nil === scope) || (Guardian === scope)
+        Rails.logger.error("Serializer initialized with a non-nil-or-Guardian scope")
+        if ENV["RAILS_ENV"] == "test"
+          raise "Serializer initialized with a non-nil-or-Guardian scope"
+        end
+      end
+    elsif @@require_strict_scope === :only_guardian
+      # Ensure that the scope is precisely an instance of Guardian.
+      unless Guardian === scope
+        Rails.logger.error("Serializer initialized with a non-Guardian scope")
+        if ENV["RAILS_ENV"] == "test"
+          raise "Serializer initialized with a non-Guardian scope"
+        end
+      end
+    end
+  end
+
   class CachedFragment
     def initialize(json)
       @json = json

diff --git a/app/serializers/concerns/user_status_mixin.rb b/app/serializers/concerns/user_status_mixin.rb
@@ -11,6 +11,6 @@ def include_status?
   end
 
   def status
-    UserStatusSerializer.new(object.user_status, root: false).as_json
+    UserStatusSerializer.new(object.user_status, scope: scope, root: false).as_json
   end
 end
diff --git a/app/serializers/grouped_search_result_serializer.rb b/app/serializers/grouped_search_result_serializer.rb
@@ -39,6 +39,7 @@ def extra
       extra[:categories] = ActiveModel::ArraySerializer.new(
         object.extra_categories,
         each_serializer: BasicCategorySerializer,
+        scope: scope,
       )
     end