From c78fc7f3282b4eaa1ddec659dbad02f0ce4edf85 Mon Sep 17 00:00:00 2001
From: Carla Drago <cdrago@gitlab.com>
Date: Mon, 24 Jul 2023 13:05:55 +0000
Subject: [PATCH] Sanitize slack-formatted links in chat messages
This removes text that matches the link format for a slack message
Changelog: other
---
.../integrations/chat_message/issue_message.rb | 4 ++--
lib/gitlab/regex.rb | 4 ++++
lib/slack_markdown_sanitizer.rb | 4 ++++
spec/lib/gitlab/regex_spec.rb | 12 ++++++++++++
spec/lib/slack_markdown_sanitizer_spec.rb | 17 +++++++++++++++++
.../chat_message/issue_message_spec.rb | 10 +++++-----
6 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/app/models/integrations/chat_message/issue_message.rb b/app/models/integrations/chat_message/issue_message.rb
index 1c2346303700..dd5163624912 100644
--- a/app/models/integrations/chat_message/issue_message.rb
+++ b/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
diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb
index d35df7e7cb92..ed1f134ff703 100644
--- a/lib/gitlab/regex.rb
+++ b/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
diff --git a/lib/slack_markdown_sanitizer.rb b/lib/slack_markdown_sanitizer.rb
index df3bec1a3c8a..f26d9aeb6882 100644
--- a/lib/slack_markdown_sanitizer.rb
+++ b/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("<", "<").gsub(">", ">") }
+ end
end
diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb
index afabcc490177..092d3c07716b 100644
--- a/spec/lib/gitlab/regex_spec.rb
+++ b/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 }
diff --git a/spec/lib/slack_markdown_sanitizer_spec.rb b/spec/lib/slack_markdown_sanitizer_spec.rb
index f4042439213e..d9552542465b 100644
--- a/spec/lib/slack_markdown_sanitizer_spec.rb
+++ b/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>' | '<url|label>'
+ '<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
diff --git a/spec/models/integrations/chat_message/issue_message_spec.rb b/spec/models/integrations/chat_message/issue_message_spec.rb
index cd40e4c361e9..14451427a5a6 100644
--- a/spec/models/integrations/chat_message/issue_message_spec.rb
+++ b/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 <http://custom-url.com|CLICK HERE>",
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 <http://custom-url.com|CLICK HERE>')
expect(subject.activity).to eq({
title: 'Issue opened by Test User (test.user)',
subtitle: 'in [project_name](http://somewhere.com)',
--
GitLab