Fix persistent XSS in note objects

4 files changed, 30 insertions, 2 deletions

diff --git a/spec/features/projects/import_export/export_file_spec.rb b/spec/features/projects/import_export/export_file_spec.rb
index f76f9ba7577..9d74a96ab3d 100644
--- a/spec/features/projects/import_export/export_file_spec.rb
+++ b/spec/features/projects/import_export/export_file_spec.rb
@@ -12,7 +12,7 @@ describe 'Import/Export - project export integration test', :js do
   let(:export_path) { "#{Dir.tmpdir}/import_file_spec" }
   let(:config_hash) { YAML.load_file(Gitlab::ImportExport.config_file).deep_stringify_keys }
 
-  let(:sensitive_words) { %w[pass secret token key encrypted] }
+  let(:sensitive_words) { %w[pass secret token key encrypted html] }
   let(:safe_list) do
     {
       token: [ProjectHook, Ci::Trigger, CommitStatus],
diff --git a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
index 536cc359d39..99669285d5b 100644
--- a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
+++ b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
@@ -18,7 +18,11 @@ describe Gitlab::ImportExport::AttributeCleaner do
       'notid' => 99,
       'import_source' => 'whatever',
       'import_type' => 'whatever',
-      'non_existent_attr' => 'whatever'
+      'non_existent_attr' => 'whatever',
+      'some_html' => '<p>dodgy html</p>',
+      'legit_html' => '<p>legit html</p>',
+      '_html' => '<p>perfectly ordinary html</p>',
+      'cached_markdown_version' => 12345
     }
   end
 
diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json
index 4a7accc4c52..fb7bddb386c 100644
--- a/spec/lib/gitlab/import_export/project.json
+++ b/spec/lib/gitlab/import_export/project.json
@@ -158,6 +158,8 @@
         {
           "id": 351,
           "note": "Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi.",
+          "note_html": "<p>something else entirely</p>",
+          "cached_markdown_version": 917504,
           "noteable_type": "Issue",
           "author_id": 26,
           "created_at": "2016-06-14T15:02:47.770Z",
@@ -2363,6 +2365,8 @@
         {
           "id": 671,
           "note": "Sit voluptatibus eveniet architecto quidem.",
+          "note_html": "<p>something else entirely</p>",
+          "cached_markdown_version": 917504,
           "noteable_type": "MergeRequest",
           "author_id": 26,
           "created_at": "2016-06-14T15:02:56.632Z",
diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
index 6084dc96410..9d2b69ea798 100644
--- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
@@ -58,6 +58,26 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
         expect(Milestone.find_by_description('test milestone').issues.count).to eq(2)
       end
 
+      context 'when importing a project with cached_markdown_version and note_html' do
+        context 'for an Issue' do
+          it 'does not import note_html' do
+            note_content = 'Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi'
+            issue_note = Issue.find_by(description: 'Aliquam enim illo et possimus.').notes.select { |n| n.note.match(/#{note_content}/)}.first
+
+            expect(issue_note.note_html).to match(/#{note_content}/)
+          end
+        end
+
+        context 'for a Merge Request' do
+          it 'does not import note_html' do
+            note_content = 'Sit voluptatibus eveniet architecto quidem'
+            merge_request_note = MergeRequest.find_by(title: 'MR1').notes.select { |n| n.note.match(/#{note_content}/)}.first
+
+            expect(merge_request_note.note_html).to match(/#{note_content}/)
+          end
+        end
+      end
+
       it 'creates a valid pipeline note' do
         expect(Ci::Pipeline.find_by_sha('sha-notes').notes).not_to be_empty
       end