Merge branch 'escape-commit-titles' into 'master'

Escape HTML in commit titles in system note messages

Closes #17348

See merge request !4084

CHANGELOG

@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 v 8.8.0 (unreleased)
   - Assign labels and milestone to target project when moving issue. !3934 (Long Nguyen)
   - Project#open_branches has been cleaned up and no longer loads entire records into memory.
+  - Escape HTML in commit titles in system note messages
   - Log to application.log when an admin starts and stops impersonating a user
   - Updated gitlab_git to 10.1.0
   - GitAccess#protected_tag? no longer loads all tags just to check if a single one exists

app/services/system_note_service.rb

@@ -351,7 +351,7 @@ class SystemNoteService
   # Returns an Array of Strings
   def self.new_commit_summary(new_commits)
     new_commits.collect do |commit|
-      "* #{commit.short_id} - #{commit.title}"
+      "* #{commit.short_id} - #{escape_html(commit.title)}"
     end
   end
 
@@ -433,4 +433,8 @@ class SystemNoteService
     body = "Moved #{direction} #{cross_reference}"
     create_note(noteable: noteable, project: project, author: author, note: body)
   end
+
+  def self.escape_html(text)
+    Rack::Utils.escape_html(text)
+  end
 end

spec/services/system_note_service_spec.rb

@@ -506,6 +506,15 @@ describe SystemNoteService, services: true do
     end
   end
 
+  describe '.new_commit_summary' do
+    it 'escapes HTML titles' do
+      commit = double(title: '<pre>This is a test</pre>', short_id: '12345678')
+      escaped = '* 12345678 - &lt;pre&gt;This is a test&lt;&#x2F;pre&gt;'
+
+      expect(described_class.new_commit_summary([commit])).to eq([escaped])
+    end
+  end
+
   include JiraServiceHelper
 
   describe 'JIRA integration' do