DEV: Block accidental serialization of entire AR models (#27668)

app/controllers/admin/themes_controller.rb

@@ -83,7 +83,7 @@ class Admin::ThemesController < Admin::AdminController
 
       if @theme.save
         log_theme_change(nil, @theme)
-        render json: @theme, status: :created
+        render json: serialize_data(@theme, ThemeSerializer), status: :created
       else
         render json: @theme.errors, status: :unprocessable_entity
       end
@@ -113,7 +113,7 @@ class Admin::ThemesController < Admin::AdminController
 
           @theme =
             RemoteTheme.import_theme(remote, theme_user, private_key: private_key, branch: branch)
-          render json: @theme, status: :created
+          render json: serialize_data(@theme, ThemeSerializer), status: :created
         rescue RemoteTheme::ImportError => e
           if params[:force]
             theme_name = params[:remote].gsub(/.git\z/, "").split("/").last
@@ -128,7 +128,7 @@ class Admin::ThemesController < Admin::AdminController
             @theme.remote_theme = remote_theme
             @theme.save!
 
-            render json: @theme, status: :created
+            render json: serialize_data(@theme, ThemeSerializer), status: :created
           else
             render_json_error e.message
           end
@@ -156,7 +156,7 @@ class Admin::ThemesController < Admin::AdminController
           )
 
         log_theme_change(nil, @theme)
-        render json: @theme, status: :created
+        render json: serialize_data(@theme, ThemeSerializer), status: :created
       rescue RemoteTheme::ImportError => e
         render_json_error e.message
       end
@@ -200,7 +200,7 @@ class Admin::ThemesController < Admin::AdminController
       if @theme.save
         update_default_theme
         log_theme_change(nil, @theme)
-        format.json { render json: @theme, status: :created }
+        format.json { render json: serialize_data(@theme, ThemeSerializer), status: :created }
       else
         format.json { render json: @theme.errors, status: :unprocessable_entity }
       end
@@ -250,7 +250,7 @@ class Admin::ThemesController < Admin::AdminController
         log_theme_component_disabled if disables_component
         log_theme_component_enabled if enables_component
 
-        format.json { render json: @theme, status: :ok }
+        format.json { render json: serialize_data(@theme, ThemeSerializer), status: :ok }
       else
         format.json do
           error = @theme.errors.full_messages.join(", ").presence

app/controllers/admin/web_hooks_controller.rb

@@ -17,12 +17,18 @@ class Admin::WebHooksController < Admin::AdminController
 
     data = serialize_data(web_hooks, AdminWebHookSerializer, root: "web_hooks")
 
+    serialized_grouped_event_types =
+      WebHookEventType.active_grouped.transform_values do |array|
+        serialize_data(array, WebHookEventTypeSerializer)
+      end
+
     json = {
       web_hooks: data.delete("web_hooks"),
       extras:
         data.merge(
-          grouped_event_types: WebHookEventType.active_grouped,
-          default_event_types: WebHook.default_event_types,
+          grouped_event_types: serialized_grouped_event_types,
+          default_event_types:
+            serialize_data(WebHook.default_event_types, WebHookEventTypeSerializer),
           content_types: WebHook.content_types.map { |name, id| { id: id, name: name } },
           delivery_statuses:
             WebHook.last_delivery_statuses.map { |name, id| { id: id, name: name.to_s } },

app/controllers/topics_controller.rb

@@ -140,7 +140,7 @@ class TopicsController < ApplicationController
                   custom_message_params: {
                     group: group.name,
                   },
-                  group: group,
+                  group: serialize_data(group, BasicGroupSerializer, root: false),
                 )
         end
 

app/controllers/users_controller.rb

@@ -1544,12 +1544,14 @@ class UsersController < ApplicationController
           .select(:id, :name, :last_used, :created_at, :method)
           .where(enabled: true)
           .order(:created_at)
+          .as_json(only: %i[id name method last_used])
 
       security_keys =
         current_user
           .security_keys
           .where(factor_type: UserSecurityKey.factor_types[:second_factor])
           .order(:created_at)
+          .as_json(only: %i[id user_id credential_id public_key factor_type enabled name last_used])
 
       render json: success_json.merge(totps: totp_second_factors, security_keys: security_keys)
     else

app/jobs/regular/export_user_archive.rb

@@ -533,7 +533,10 @@ module Jobs
     def get_user_archive_fields(user_archive)
       user_archive_array = []
       topic_data = user_archive.topic
-      user_archive = user_archive.as_json
+      user_archive =
+        user_archive.as_json(
+          only: %i[topic_id post_number raw cooked like_count reply_count created_at id],
+        )
       topic_data =
         Topic
           .with_deleted

app/serializers/admin_web_hook_serializer.rb

@@ -8,8 +8,7 @@ class AdminWebHookSerializer < ApplicationSerializer
              :secret,
              :wildcard_web_hook,
              :verify_certificate,
-             :active,
-             :web_hook_event_types
+             :active
 
   has_many :categories, serializer: BasicCategorySerializer, embed: :ids, include: true
   has_many :tags,
@@ -19,10 +18,10 @@ class AdminWebHookSerializer < ApplicationSerializer
            embed_key: :name,
            include: false
   has_many :groups, serializer: BasicGroupSerializer, embed: :ids, include: false
-
-  def web_hook_event_types
-    ActiveModel::ArraySerializer.new(object.web_hook_event_types).as_json
-  end
+  has_many :web_hook_event_types,
+           serializer: WebHookEventTypeSerializer,
+           root: false,
+           embed: :objects
 
   def last_delivery_status
     object.active ? object.last_delivery_status : WebHook.last_delivery_statuses[:disabled]

app/serializers/site_serializer.rb

@@ -84,7 +84,10 @@ class SiteSerializer < ApplicationSerializer
   end
 
   def default_dark_color_scheme
-    ColorScheme.find_by_id(SiteSetting.default_dark_mode_color_scheme_id).as_json
+    ColorSchemeSerializer.new(
+      ColorScheme.find_by_id(SiteSetting.default_dark_mode_color_scheme_id),
+      root: false,
+    ).as_json
   end
 
   def groups

app/serializers/theme_serializer.rb

@@ -3,8 +3,7 @@
 require "base64"
 
 class ThemeSerializer < BasicThemeSerializer
-  attributes :color_scheme,
-             :color_scheme_id,
+  attributes :color_scheme_id,
              :user_selectable,
              :auto_update,
              :remote_theme_id,
@@ -16,6 +15,7 @@ class ThemeSerializer < BasicThemeSerializer
              :disabled_at,
              :theme_fields
 
+  has_one :color_scheme, serializer: ColorSchemeSerializer, embed: :object
   has_one :user, serializer: UserNameSerializer, embed: :object
   has_one :disabled_by, serializer: UserNameSerializer, embed: :object
 

app/serializers/topic_view_bookmark_serializer.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class TopicViewBookmarkSerializer < ApplicationSerializer
+  attributes :id, :bookmarkable_id, :bookmarkable_type, :reminder_at, :name, :auto_delete_preference
+end

app/serializers/topic_view_serializer.rb

@@ -64,7 +64,6 @@ class TopicViewSerializer < ApplicationSerializer
     :is_warning,
     :chunk_size,
     :bookmarked,
-    :bookmarks,
     :message_archived,
     :topic_timer,
     :unicode_title,
@@ -85,6 +84,7 @@ class TopicViewSerializer < ApplicationSerializer
   has_one :details, serializer: TopicViewDetailsSerializer, root: false, embed: :objects
   has_many :pending_posts, serializer: TopicPendingPostSerializer, root: false, embed: :objects
   has_many :categories, serializer: CategoryBadgeSerializer, embed: :objects
+  has_many :bookmarks, serializer: TopicViewBookmarkSerializer, root: false, embed: :objects
 
   has_one :published_page, embed: :objects
 
@@ -201,10 +201,6 @@ class TopicViewSerializer < ApplicationSerializer
     object.has_bookmarks?
   end
 
-  def bookmarks
-    object.bookmarks
-  end
-
   def topic_timer
     topic_timer = object.topic.public_topic_timer
 

app/serializers/web_hook_event_type_serializer.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class WebHookEventTypeSerializer < ApplicationSerializer
+  attributes :id, :name, :group
+end

app/views/exceptions/not_found.html.erb

@@ -15,10 +15,10 @@
     <a href="<%= path "/login" %>" class='btn btn-primary'><%= SvgSprite.raw_svg('fa-user') %><%= I18n.t('log_in') %></a>
   <%- end %>
 
-  <%- if @group&.allow_membership_requests %>
-    <a href="<%= group_path @group.name %>" class='btn btn-primary'><%= SvgSprite.raw_svg('user-plus') %> <%= I18n.t('not_in_group.request_membership') %></a>
-  <%- elsif @group&.public_admission %>
-    <a href="<%= group_path @group.name %>" class='btn btn-primary'><%= SvgSprite.raw_svg('user-plus') %> <%= I18n.t('not_in_group.join_group') %></a>
+  <%- if @group&.dig(:allow_membership_requests) %>
+    <a href="<%= group_path @group[:name] %>" class='btn btn-primary'><%= SvgSprite.raw_svg('user-plus') %> <%= I18n.t('not_in_group.request_membership') %></a>
+  <%- elsif @group&.dig(:public_admission) %>
+    <a href="<%= group_path @group[:name] %>" class='btn btn-primary'><%= SvgSprite.raw_svg('user-plus') %> <%= I18n.t('not_in_group.join_group') %></a>
   <%- end %>
 </div>
 

lib/freedom_patches/active_record_disable_serialization.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+module ActiveRecordSerializationSafety
+  class BlockedSerializationError < StandardError
+  end
+
+  def serializable_hash(options = nil)
+    if options.nil? || options[:only].nil?
+      message =
+        "Serializing ActiveRecord models (#{self.class.name}) without specifying fields is not allowed. Use a Serializer, or pass the :only option to #serializable_hash. More info: https://meta.discourse.org/t/-/314495"
+
+      if Rails.env == "production"
+        Rails.logger.info(message)
+      else
+        raise BlockedSerializationError.new(message)
+      end
+    end
+    super
+  end
+end
+
+ActiveRecord::Base.prepend(ActiveRecordSerializationSafety)

plugins/chat/spec/plugin_helper.rb

@@ -113,6 +113,12 @@ module ChatSpecHelpers
   end
 
   def create_draft(channel, thread: nil, user: Discourse.system_user, data: { message: "draft" })
+    if data[:uploads]
+      data[:uploads] = data[:uploads].map do |upload|
+        UploadSerializer.new(upload, root: false).as_json
+      end
+    end
+
     result =
       ::Chat::UpsertDraft.call(
         guardian: user.guardian,

spec/lib/topic_query_spec.rb

@@ -1410,7 +1410,7 @@ RSpec.describe TopicQuery do
     end
 
     def read(user, topic, post_number)
-      TopicUser.update_last_read(user, topic, post_number, post_number, 10_000)
+      TopicUser.update_last_read(user, topic.id, post_number, post_number, 10_000)
     end
 
     before do

spec/requests/admin/themes_controller_spec.rb

@@ -222,7 +222,7 @@ RSpec.describe Admin::ThemesController do
       end
 
       it "can import a theme from Git" do
-        RemoteTheme.stubs(:import_theme)
+        RemoteTheme.stubs(:import_theme).returns(Fabricate(:theme))
         post "/admin/themes/import.json",
              params: {
                remote: "    https://github.com/discourse/discourse-brand-header.git       ",
@@ -1076,12 +1076,11 @@ RSpec.describe Admin::ThemesController do
       end
 
       it "should return the right error when value used to update a theme setting of `objects` typed is invalid" do
-        field =
-          theme.set_field(
-            target: :settings,
-            name: "yaml",
-            value: File.read("#{Rails.root}/spec/fixtures/theme_settings/objects_settings.yaml"),
-          )
+        theme.set_field(
+          target: :settings,
+          name: "yaml",
+          value: File.read("#{Rails.root}/spec/fixtures/theme_settings/objects_settings.yaml"),
+        )
 
         theme.save!
 
@@ -1101,12 +1100,11 @@ RSpec.describe Admin::ThemesController do
       end
 
       it "should be able to update a theme setting of `objects` typed" do
-        field =
-          theme.set_field(
-            target: :settings,
-            name: "yaml",
-            value: File.read("#{Rails.root}/spec/fixtures/theme_settings/objects_settings.yaml"),
-          )
+        theme.set_field(
+          target: :settings,
+          name: "yaml",
+          value: File.read("#{Rails.root}/spec/fixtures/theme_settings/objects_settings.yaml"),
+        )
 
         theme.save!
 
@@ -1354,7 +1352,7 @@ RSpec.describe Admin::ThemesController do
 
     let(:theme_setting) do
       yaml = File.read("#{Rails.root}/spec/fixtures/theme_settings/objects_settings.yaml")
-      field = theme.set_field(target: :settings, name: "yaml", value: yaml)
+      theme.set_field(target: :settings, name: "yaml", value: yaml)
       theme.save!
       theme.settings
     end

spec/requests/api/notifications_spec.rb

@@ -108,6 +108,11 @@ RSpec.describe "notifications" do
       response "200", "notifications marked read" do
         schema type: :object, properties: { success: { type: :string } }
 
+        let(:notification) do
+          notification = Fabricate(:notification)
+          NotificationSerializer.new(notification).as_json
+        end
+
         run_test!
       end
     end

spec/requests/api/posts_spec.rb

@@ -221,7 +221,10 @@ RSpec.describe "posts" do
         expected_response_schema = load_spec_schema("topic_create_response")
         schema expected_response_schema
 
-        let(:params) { Fabricate(:post) }
+        let(:params) do
+          post = Fabricate(:post)
+          post.serializable_hash(only: %i[topic_id raw created_at]).as_json
+        end
 
         it_behaves_like "a JSON endpoint", 200 do
           let(:expected_response_schema) { expected_response_schema }

spec/serializers/site_serializer_spec.rb

@@ -113,7 +113,7 @@ RSpec.describe SiteSerializer do
     scheme = ColorScheme.last
     SiteSetting.default_dark_mode_color_scheme_id = scheme.id
     serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
-    default_dark_scheme = expect(serialized[:default_dark_color_scheme]["name"]).to eq(scheme.name)
+    default_dark_scheme = expect(serialized[:default_dark_color_scheme][:name]).to eq(scheme.name)
 
     SiteSetting.default_dark_mode_color_scheme_id = -1
     serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json