UX: support links in tag descriptions (#22994)

* scrub non-a html tags from tag descriptions on create, strips all tags from tag description when displayed in tag hover

* test for tag description links

* UX: basic render-tag test

* UX: fix linting

* UX: fix linting

* fix broken tests

* Update spec/models/tag_spec.rb

Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>

* UX: use has_sanitizable_fields instead of has_scrubbable_fields to ensafen tag.description

---------

Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>

app/assets/javascripts/discourse/app/components/tag-info.hbs

@@ -49,7 +49,7 @@
           {{/if}}
         </div>
         <div class="tag-description-wrapper">
-          {{this.tagInfo.description}}
+          {{html-safe this.tagInfo.description}}
         </div>
       {{/if}}
     </div>

app/assets/javascripts/discourse/app/lib/render-tag.js

@@ -39,13 +39,17 @@ export function defaultRenderTag(tag, params) {
     classes.push(params.size);
   }
 
+  // remove all html tags from hover text
+  const hoverDescription =
+    params.description && params.description.replace(/<.+?>/g, "");
+
   let val =
     "<" +
     tagName +
     href +
     " data-tag-name=" +
     tag +
-    (params.description ? ' title="' + escape(params.description) + '" ' : "") +
+    (params.description ? ' title="' + escape(hoverDescription) + '" ' : "") +
     " class='" +
     classes.join(" ") +
     "'>" +

app/assets/javascripts/discourse/tests/unit/lib/render-tag-test.js

@@ -0,0 +1,23 @@
+import renderTag from "discourse/lib/render-tag";
+import { module, test } from "qunit";
+import { setupTest } from "ember-qunit";
+
+module("Unit | Utility | render-tag", function (hooks) {
+  setupTest(hooks);
+
+  test("defaultRenderTag", function (assert) {
+    assert.strictEqual(
+      renderTag("foo", { description: "foo description" }),
+      "<a href='/tag/foo'  data-tag-name=foo title=\"foo description\"  class='discourse-tag simple'>foo</a>",
+      "formats tag as link with plain description in hover"
+    );
+
+    assert.strictEqual(
+      renderTag("foo", {
+        description: 'foo description <a href="localhost">link</a>',
+      }),
+      "<a href='/tag/foo'  data-tag-name=foo title=\"foo description link\"  class='discourse-tag simple'>foo</a>",
+      "removes any html tags from description"
+    );
+  });
+});

app/models/tag.rb

@@ -3,6 +3,7 @@
 class Tag < ActiveRecord::Base
   include Searchable
   include HasDestroyedWebHook
+  include HasSanitizableFields
 
   self.ignored_columns = [
     "topic_count", # TODO(tgxworld): Remove on 1 July 2023
@@ -56,6 +57,8 @@ class Tag < ActiveRecord::Base
   has_many :synonyms, class_name: "Tag", foreign_key: "target_tag_id", dependent: :destroy
   has_many :sidebar_section_links, as: :linkable, dependent: :delete_all
 
+  before_save :sanitize_description
+
   after_save :index_search
   after_save :update_synonym_associations
 
@@ -244,6 +247,9 @@ class Tag < ActiveRecord::Base
 
   private
 
+  def sanitize_description
+    self.description = sanitize_field(self.description) if description_changed?
+  end
   def name_validator
     errors.add(:name, :invalid) if name.present? && RESERVED_TAGS.include?(self.name.strip.downcase)
   end

spec/models/tag_spec.rb

@@ -382,4 +382,15 @@ RSpec.describe Tag do
       expect(Tag.topic_count_column(Guardian.new(user))).to eq("staff_topic_count")
     end
   end
+
+  describe "description" do
+    it "uses the HTMLSanitizer to remove unsafe tags and attributes" do
+      tag.description =
+        "<div>hi</div><script>a=0;</script> <a onclick='const a=0;' href=\"https://www.discourse.org\">discourse</a>"
+      tag.save!
+      expect(tag.description.strip).to eq(
+        "<div>hi</div>a=0; <a href=\"https://www.discourse.org\">discourse</a>",
+      )
+    end
+  end
 end

spec/system/viewing_sidebar_as_anonymous_user_spec.rb

@@ -54,7 +54,7 @@ describe "Viewing sidebar as anonymous user", type: :system do
       expect(sidebar).to have_tags_section
       expect(sidebar).to have_all_tags_section_link
       expect(sidebar).to have_tag_section_links([tag3, tag2, tag4, tag5, tag1])
-      expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description <script>")
+      expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description ")
     end
 
     it "should display the site's top tags when `default_navigation_menu_tags` site setting has been set but the tags configured are hidden to the user" do
@@ -66,7 +66,7 @@ describe "Viewing sidebar as anonymous user", type: :system do
       expect(sidebar).to have_tags_section
       expect(sidebar).to have_all_tags_section_link
       expect(sidebar).to have_tag_section_links([tag3, tag2, tag4, tag1, tag6])
-      expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description <script>")
+      expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description ")
     end
 
     it "should display the tags configured in `default_navigation_menu_tags` site setting when it has been set" do

spec/system/viewing_sidebar_spec.rb

@@ -162,7 +162,7 @@ describe "Viewing sidebar as logged in user", type: :system do
       expect(sidebar).to have_tags_section
       expect(sidebar).to have_tag_section_links([tag3, tag2, tag4, tag5, tag1])
       expect(sidebar).to have_tag_section_link_with_title(tag3, "tag 3 description")
-      expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description <script>")
+      expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description ")
       expect(sidebar).to have_all_tags_section_link
     end
 
@@ -177,7 +177,7 @@ describe "Viewing sidebar as logged in user", type: :system do
       expect(sidebar).to have_tags_section
       expect(sidebar).to have_tag_section_links([tag1, tag2, tag3])
       expect(sidebar).to have_tag_section_link_with_title(tag3, "tag 3 description")
-      expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description <script>")
+      expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description ")
       expect(sidebar).to have_all_tags_section_link
     end
   end