Sanitize slack-formatted links in chat messages

This removes text that matches the link format for a slack message

Changelog: other

app/models/integrations/chat_message/issue_message.rb

@@ -27,7 +27,7 @@ def initialize(params)
 
       def attachments
         return [] unless opened_issue?
-        return description if markdown
+        return SlackMarkdownSanitizer.sanitize_slack_link(description) if markdown
 
         description_message
       end
@@ -55,7 +55,7 @@ def description_message
         [{
           title: issue_title,
           title_link: issue_url,
-          text: format(description),
+          text: format(SlackMarkdownSanitizer.sanitize_slack_link(description)),
           color: '#C95823'
         }]
       end

lib/gitlab/regex.rb

@@ -259,6 +259,10 @@ def sha256_regex
         @sha256_regex ||= /\A[0-9a-f]{64}\z/i.freeze
       end
 
+      def slack_link_regex
+        @slack_link_regex ||= /<(.*[|].*)>/i.freeze
+      end
+
       private
 
       def conan_name_regex

lib/slack_markdown_sanitizer.rb

@@ -8,4 +8,8 @@ module SlackMarkdownSanitizer
   def self.sanitize(string)
     string&.delete(UNSAFE_MARKUP_CHARACTERS)
   end
+
+  def self.sanitize_slack_link(string)
+    string.gsub(Gitlab::Regex.slack_link_regex) { |m| m.gsub("<", "&lt;").gsub(">", "&gt;") }
+  end
 end

spec/lib/gitlab/regex_spec.rb

@@ -74,6 +74,18 @@
     it { is_expected.to eq("can contain only letters, digits, emoji, '_', '.', dash, space, parenthesis. It must start with letter, digit, emoji or '_'.") }
   end
 
+  describe '.slack_link_regex' do
+    subject { described_class.slack_link_regex }
+
+    it { is_expected.not_to match('http://custom-url.com|click here') }
+    it { is_expected.not_to match('custom-url.com|any-Charact3r$') }
+    it { is_expected.not_to match("<custom-url.com|any-Charact3r$>") }
+
+    it { is_expected.to match('<http://custom-url.com|click here>') }
+    it { is_expected.to match('<custom-url.com|any-Charact3r$>') }
+    it { is_expected.to match('<any-Charact3r$|any-Charact3r$>') }
+  end
+
   describe '.bulk_import_destination_namespace_path_regex_message' do
     subject { described_class.bulk_import_destination_namespace_path_regex_message }
 

spec/lib/slack_markdown_sanitizer_spec.rb

@@ -20,4 +20,21 @@
       end
     end
   end
+
+  describe '.sanitize_slack_link' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:input, :output) do
+      ''                               | ''
+      '[label](url)'                   | '[label](url)'
+      '<url|label>'                    | '&lt;url|label&gt;'
+      '<a href="url">label</a>'        | '<a href="url">label</a>'
+    end
+
+    with_them do
+      it 'returns the expected output' do
+        expect(described_class.sanitize_slack_link(input)).to eq(output)
+      end
+    end
+  end
 end

spec/models/integrations/chat_message/issue_message_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Integrations::ChatMessage::IssueMessage do
+RSpec.describe Integrations::ChatMessage::IssueMessage, feature_category: :integrations do
   subject { described_class.new(args) }
 
   let(:args) do
@@ -24,7 +24,7 @@
         url: 'http://url.com',
         action: 'open',
         state: 'opened',
-        description: 'issue description'
+        description: 'issue description <http://custom-url.com|CLICK HERE>'
       }
     }
   end
@@ -45,7 +45,7 @@
     end
 
     context 'open' do
-      it 'returns a message regarding opening of issues' do
+      it 'returns a slack-link sanitized message regarding opening of issues' do
         expect(subject.pretext).to eq(
           '[<http://somewhere.com|project_name>] Issue <http://url.com|#100 Issue title> opened by Test User (test.user)')
         expect(subject.attachments).to eq(
@@ -53,7 +53,7 @@
             {
               title: "#100 Issue title",
               title_link: "http://url.com",
-              text: "issue description",
+              text: "issue description &lt;http://custom-url.com|CLICK HERE&gt;",
               color: color
             }
           ])
@@ -96,7 +96,7 @@
       it 'returns a message regarding opening of issues' do
         expect(subject.pretext).to eq(
           '[[project_name](http://somewhere.com)] Issue [#100 Issue title](http://url.com) opened by Test User (test.user)')
-        expect(subject.attachments).to eq('issue description')
+        expect(subject.attachments).to eq('issue description &lt;http://custom-url.com|CLICK HERE&gt;')
         expect(subject.activity).to eq({
           title: 'Issue opened by Test User (test.user)',
           subtitle: 'in [project_name](http://somewhere.com)',