Commit 157a4aba authored by Sean McGivern's avatar Sean McGivern

Merge branch 'issue_119433' into 'master'

Redacts quick actions used by support bot

See merge request gitlab-org/gitlab!23353
parents 6b812133 0b2cbb73
---
title: Redacts quick actions used by support bot
merge_request: 23353
author:
type: changed
...@@ -17,7 +17,7 @@ module EE ...@@ -17,7 +17,7 @@ module EE
command_definitions = ::QuickActions::InterpretService.command_definitions command_definitions = ::QuickActions::InterpretService.command_definitions
extractor = ::Gitlab::QuickActions::Extractor.new(command_definitions) extractor = ::Gitlab::QuickActions::Extractor.new(command_definitions)
extractor.extract_commands(content)[0] extractor.redact_commands(content)
end end
override :process_message override :process_message
......
...@@ -24,3 +24,7 @@ Service desk stuff! ...@@ -24,3 +24,7 @@ Service desk stuff!
``` ```
a = b a = b
``` ```
/label ~label1
/assign @user1
/close
...@@ -22,3 +22,7 @@ Service desk stuff! ...@@ -22,3 +22,7 @@ Service desk stuff!
``` ```
a = b a = b
``` ```
/label ~label1
/assign @user1
/close
...@@ -153,12 +153,12 @@ describe Gitlab::Email::Handler::CreateNoteHandler do ...@@ -153,12 +153,12 @@ describe Gitlab::Email::Handler::CreateNoteHandler do
end end
context 'when quick actions are present' do context 'when quick actions are present' do
it 'strips quick actions from email' do it 'encloses quick actions with code span markdown' do
receiver.execute receiver.execute
noteable.reload noteable.reload
expect(note.note).not_to include('/close') note = Note.last
expect(note.note).not_to include('/title') expect(note.note).to include("Jake out\n\n`/close`\n`/title test`")
expect(noteable.title).to eq('service desk issue') expect(noteable.title).to eq('service desk issue')
expect(noteable).to be_opened expect(noteable).to be_opened
end end
......
...@@ -12,7 +12,9 @@ describe Gitlab::Email::Handler::EE::ServiceDeskHandler do ...@@ -12,7 +12,9 @@ describe Gitlab::Email::Handler::EE::ServiceDeskHandler do
let(:email_raw) { email_fixture('emails/service_desk.eml', dir: 'ee') } let(:email_raw) { email_fixture('emails/service_desk.eml', dir: 'ee') }
let_it_be(:namespace) { create(:namespace, name: "email") } let_it_be(:namespace) { create(:namespace, name: "email") }
let(:expected_description) { "Service desk stuff!\n\n```\na = b\n```\n\n![image](uploads/image.png)" } let(:expected_description) do
"Service desk stuff!\n\n```\na = b\n```\n\n`/label ~label1`\n`/assign @user1`\n`/close`\n![image](uploads/image.png)"
end
context 'service desk is enabled for the project' do context 'service desk is enabled for the project' do
let_it_be(:project) { create(:project, :repository, :public, namespace: namespace, path: 'test', service_desk_enabled: true) } let_it_be(:project) { create(:project, :repository, :public, namespace: namespace, path: 'test', service_desk_enabled: true) }
...@@ -100,16 +102,16 @@ describe Gitlab::Email::Handler::EE::ServiceDeskHandler do ...@@ -100,16 +102,16 @@ describe Gitlab::Email::Handler::EE::ServiceDeskHandler do
expect(issue.milestone).to eq(milestone) expect(issue.milestone).to eq(milestone)
end end
it 'does not apply quick actions present on user email body' do it 'redacts quick actions present on user email body' do
set_template_file('service_desk1', 'text from template') set_template_file('service_desk1', 'text from template')
receiver.execute receiver.execute
issue = Issue.last issue = Issue.last
expect(issue).to be_opened expect(issue).to be_opened
expect(issue.description).not_to include('/label ~label1') expect(issue.description).to include('`/label ~label1`')
expect(issue.description).not_to include('/assign @user1') expect(issue.description).to include('`/assign @user1`')
expect(issue.description).not_to include('/close') expect(issue.description).to include('`/close`')
expect(issue.assignees).to be_empty expect(issue.assignees).to be_empty
expect(issue.milestone).to be_nil expect(issue.milestone).to be_nil
end end
......
...@@ -34,26 +34,55 @@ module Gitlab ...@@ -34,26 +34,55 @@ module Gitlab
def extract_commands(content, only: nil) def extract_commands(content, only: nil)
return [content, []] unless content return [content, []] unless content
content = content.dup content, commands = perform_regex(content, only: only)
commands = [] perform_substitutions(content, commands)
end
# Encloses quick action commands into code span markdown
# avoiding them being executed, for example, when sent via email
# to GitLab service desk.
# Example: /label ~label1 becomes `/label ~label1`
def redact_commands(content)
return "" unless content
content, _ = perform_regex(content, redact: true)
content
end
private
def perform_regex(content, only: nil, redact: false)
commands = []
content = content.dup
content.delete!("\r") content.delete!("\r")
content.gsub!(commands_regex(only: only)) do content.gsub!(commands_regex(only: only)) do
if $~[:cmd] command, output = process_commands($~, redact)
commands << [$~[:cmd].downcase, $~[:arg]].reject(&:blank?) commands << command
'' output
else
$~[0]
end
end end
content, commands = perform_substitutions(content, commands) [content.rstrip, commands.reject(&:empty?)]
[content.rstrip, commands]
end end
private def process_commands(matched_text, redact)
output = matched_text[0]
command = []
if matched_text[:cmd]
command = [matched_text[:cmd].downcase, matched_text[:arg]].reject(&:blank?)
output = ''
if redact
output = "`/#{matched_text[:cmd]}#{" " + matched_text[:arg] if matched_text[:arg]}`"
output += "\n" if matched_text[0].include?("\n")
end
end
[command, output]
end
# Builds a regular expression to match known commands. # Builds a regular expression to match known commands.
# First match group captures the command name and # First match group captures the command name and
......
...@@ -294,4 +294,22 @@ describe Gitlab::QuickActions::Extractor do ...@@ -294,4 +294,22 @@ describe Gitlab::QuickActions::Extractor do
expect(msg).to eq expected_msg expect(msg).to eq expected_msg
end end
end end
describe '#redact_commands' do
using RSpec::Parameterized::TableSyntax
where(:text, :expected) do
"hello\n/labels ~label1 ~label2\nworld" | "hello\n`/labels ~label1 ~label2`\nworld"
"hello\n/open\n/labels ~label1\nworld" | "hello\n`/open`\n`/labels ~label1`\nworld"
"hello\n/reopen\nworld" | "hello\n`/reopen`\nworld"
"/reopen\nworld" | "`/reopen`\nworld"
"hello\n/open" | "hello\n`/open`"
end
with_them do
it 'encloses quick actions with code span markdown' do
expect(extractor.redact_commands(text)).to eq(expected)
end
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment