DEV: Remove HTML setting type and sanitization logic. (#14440)

* DEV: Remove HTML setting type and sanitization logic.

We concluded that we don't want settings to contain HTML, so I'm removing the setting type and sanitization logic. Additionally, we no longer allow the global-notice text to contain HTML.

I searched for usages of this setting type in the `all-the-plugins` repo and found none, so I haven't added a migration for existing settings.

* Mark Global notices containing links as HTML Safe.

app/assets/javascripts/admin/addon/templates/components/site-settings/html.hbs

@@ -1,2 +0,0 @@
-{{text-field value=(html-safe value) classNames="input-setting-string"}}
-<div class="desc">{{html-safe setting.description}}</div>

app/assets/javascripts/discourse/app/components/global-notice.js

@@ -5,6 +5,7 @@ import I18n from "I18n";
 import LogsNotice from "discourse/services/logs-notice";
 import { bind } from "discourse-common/utils/decorators";
 import getURL from "discourse-common/lib/get-url";
+import { htmlSafe } from "@ember/template";
 
 const _pluginNotices = [];
 
@@ -111,7 +112,9 @@ export default Component.extend({
         const requiredText = I18n.t("wizard_required", {
           url: getURL("/wizard"),
         });
-        notices.push(Notice.create({ text: requiredText, id: "alert-wizard" }));
+        notices.push(
+          Notice.create({ text: htmlSafe(requiredText), id: "alert-wizard" })
+        );
       }
 
       if (
@@ -214,7 +217,7 @@ export default Component.extend({
   @bind
   _handleLogsNoticeUpdate() {
     const logNotice = Notice.create({
-      text: LogsNotice.currentProp("message"),
+      text: htmlSafe(LogsNotice.currentProp("message")),
       id: "alert-logs-notice",
       options: {
         dismissable: true,

app/assets/javascripts/discourse/app/templates/components/global-notice.hbs

@@ -4,7 +4,7 @@
       {{#if notice.options.html}}
         {{html-safe notice.options.html}}
       {{/if}}
-      <span class="text">{{html-safe notice.text}}</span>
+      <span class="text">{{notice.text}}</span>
 
       {{#if notice.options.dismissable}}
         {{d-button

app/controllers/admin/site_settings_controller.rb

@@ -7,7 +7,7 @@ class Admin::SiteSettingsController < Admin::AdminController
 
   def index
     render_json_dump(
-      site_settings: SiteSetting.all_settings(sanitize_plain_text_settings: true),
+      site_settings: SiteSetting.all_settings,
       diags: SiteSetting.diags
     )
   end

config/site_settings.yml

@@ -19,7 +19,6 @@
 # type: username - Must match the username of an existing user.
 # type: list     - A list of values, chosen from a set of valid values defined in the choices option.
 # type: enum     - A single value, chosen from a set of valid values in the choices option.
-# type: html     - A single value. It could contain HTML.
 #
 # A type:list setting with the word 'colors' in its name will make color values have a bold line of the corresponding color
 #

db/migrate/20210928161912_migrate_deprecated_html_setting_type.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class MigrateDeprecatedHtmlSettingType < ActiveRecord::Migration[6.1]
+  def up
+    execute <<~SQL
+      UPDATE site_settings
+      SET data_type = 1
+      WHERE data_type = 25
+    SQL
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/site_setting_extension.rb

@@ -212,16 +212,12 @@ module SiteSettingExtension
       value = value.to_s if type == :upload
       value = value.map(&:to_s).join("|") if type == :uploaded_image_list
 
-      if should_sanitize?(value, type)
-        value = sanitize(value)
-      end
-
       [name, value]
     end.flatten])
   end
 
   # Retrieve all settings
-  def all_settings(include_hidden: false, sanitize_plain_text_settings: false)
+  def all_settings(include_hidden: false)
 
     locale_setting_hash =
     {
@@ -250,8 +246,6 @@ module SiteSettingExtension
          default.to_i < Upload::SEEDED_ID_THRESHOLD
 
         default = default_uploads[default.to_i]
-      elsif sanitize_plain_text_settings && should_sanitize?(value, type_hash[:type].to_s)
-        value = sanitize(value)
       end
 
       opts = {
@@ -582,14 +576,6 @@ module SiteSettingExtension
     end
   end
 
-  def should_sanitize?(value, type)
-    value.is_a?(String) && type.to_s != 'html'
-  end
-
-  def sanitize(value)
-    CGI.unescapeHTML(Loofah.scrub_fragment(value, :strip).to_s)
-  end
-
   def logger
     Rails.logger
   end

lib/site_settings/type_supervisor.rb

@@ -37,7 +37,7 @@ class SiteSettings::TypeSupervisor
       color: 22,
       simple_list: 23,
       emoji_list: 24,
-      html: 25
+      html_deprecated: 25,
     )
   end
 

spec/components/site_setting_extension_spec.rb

@@ -831,23 +831,6 @@ describe SiteSettingExtension do
         expect(setting[:default]).to eq(system_upload.url)
       end
     end
-
-    it 'should sanitize html in the site settings' do
-      settings.setting(:with_html, '<script></script>rest')
-      settings.refresh!
-
-      setting = settings.all_settings(sanitize_plain_text_settings: true).last
-
-      expect(setting[:value]).to eq('rest')
-    end
-
-    it 'settings with html type are not sanitized' do
-      settings.setting(:with_html, '<script></script>rest', type: :html)
-
-      setting = settings.all_settings(sanitize_plain_text_settings: true).last
-
-      expect(setting[:value]).to eq('<script></script>rest')
-    end
   end
 
   describe '.client_settings_json_uncached' do
@@ -862,19 +845,6 @@ describe SiteSettingExtension do
       )
     end
 
-    it 'should sanitize html in the site settings' do
-      settings.setting(:with_html, '<script></script>rest', client: true)
-      settings.setting(:with_symbols, '<>rest', client: true)
-      settings.setting(:with_unknown_tag, '<rest>rest', client: true)
-      settings.refresh!
-
-      client_settings = JSON.parse settings.client_settings_json_uncached
-
-      expect(client_settings['with_html']).to eq('rest')
-      expect(client_settings['with_symbols']).to eq('<>rest')
-      expect(client_settings['with_unknown_tag']).to eq('rest')
-    end
-
     it 'settings with html type are not sanitized' do
       settings.setting(:with_html, '<script></script>rest', type: :html, client: true)
 

spec/components/site_settings/type_supervisor_spec.rb

@@ -94,9 +94,6 @@ describe SiteSettings::TypeSupervisor do
       it "'emoji_list' should be at the right position" do
         expect(SiteSettings::TypeSupervisor.types[:emoji_list]).to eq(24)
       end
-      it "'html' should be at the right position" do
-        expect(SiteSettings::TypeSupervisor.types[:html]).to eq(25)
-      end
     end
   end