From 2a5c963b7cd2dc1cf1e6b4d1a291c37d313b5813 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Thu, 28 May 2015 17:56:52 -0400
Subject: [PATCH 1/6] Render Group and Project descriptions with our Markdown
 pipeline

---
 app/assets/stylesheets/pages/projects.scss | 10 ++++++----
 app/views/groups/show.html.haml            |  2 +-
 app/views/projects/_home_panel.html.haml   |  2 +-
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss
index 12489ccc2d..b93ea0f020 100644
--- a/app/assets/stylesheets/pages/projects.scss
+++ b/app/assets/stylesheets/pages/projects.scss
@@ -48,14 +48,16 @@
     }
 
     .project-home-desc {
+      color: $gray;
+      float: left;
       font-size: 16px;
       line-height: 1.3;
       margin-right: 250px;
-    }
 
-    .project-home-desc {
-      float: left;
-      color: $gray;
+      // Render Markdown-generated HTML inline for this block
+      p {
+        display: inline;
+      }
     }
   }
 
diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml
index 1678311141..f42007da07 100644
--- a/app/views/groups/show.html.haml
+++ b/app/views/groups/show.html.haml
@@ -11,7 +11,7 @@
       @#{@group.path}
     - if @group.description.present?
       .description
-        = escaped_autolink(@group.description)
+        = markdown(@group.description)
   %hr
 
   = render 'shared/show_aside'
diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml
index f9cdda4a3b..05f44acd3c 100644
--- a/app/views/projects/_home_panel.html.haml
+++ b/app/views/projects/_home_panel.html.haml
@@ -5,7 +5,7 @@
   .project-home-row.project-home-row-top
     .project-home-desc
       - if @project.description.present?
-        = escaped_autolink(@project.description)
+        = markdown(@project.description)
       - if can?(current_user, :admin_project, @project)
         &ndash;
         = link_to 'Edit', edit_namespace_project_path

From 1a52f19c456dfa307dd7fa0e5adbaa2ed1a68889 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Thu, 28 May 2015 17:57:23 -0400
Subject: [PATCH 2/6] Remove now-unused `escaped_autolink` helper and
 rails_autolink gem

---
 Gemfile                           | 3 ---
 Gemfile.lock                      | 3 ---
 app/helpers/application_helper.rb | 4 ----
 3 files changed, 10 deletions(-)

diff --git a/Gemfile b/Gemfile
index 78af7f5db6..94e2129f3c 100644
--- a/Gemfile
+++ b/Gemfile
@@ -10,9 +10,6 @@ end
 
 gem "rails", "~> 4.1.0"
 
-# Make links from text
-gem 'rails_autolink', '~> 1.1'
-
 # Default values for AR models
 gem "default_value_for", "~> 3.0.0"
 
diff --git a/Gemfile.lock b/Gemfile.lock
index bbc5639c84..80ae41dc8f 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -448,8 +448,6 @@ GEM
       sprockets-rails (~> 2.0)
     rails-observers (0.1.2)
       activemodel (~> 4.0)
-    rails_autolink (1.1.6)
-      rails (> 3.1)
     railties (4.1.9)
       actionpack (= 4.1.9)
       activesupport (= 4.1.9)
@@ -779,7 +777,6 @@ DEPENDENCIES
   rack-mini-profiler
   rack-oauth2 (~> 1.0.5)
   rails (~> 4.1.0)
-  rails_autolink (~> 1.1)
   raphael-rails (~> 2.1.2)
   rb-fsevent
   rb-inotify
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 89dcdf5779..a539ec49f7 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -279,10 +279,6 @@ module ApplicationHelper
     html_options
   end
 
-  def escaped_autolink(text)
-    auto_link ERB::Util.html_escape(text), link: :urls
-  end
-
   def promo_host
     'about.gitlab.com'
   end

From 023dd2907b4afa0bae5f8482cae75e1edd6954a8 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Fri, 29 May 2015 19:01:12 -0400
Subject: [PATCH 3/6] Add a `pipeline` context option for SanitizationFilter

When this option is `:description`, we use a more restrictive whitelist.
This is used for Project and Group description fields.
---
 app/views/groups/show.html.haml               |  2 +-
 app/views/projects/_home_panel.html.haml      |  2 +-
 lib/gitlab/markdown.rb                        |  3 +
 lib/gitlab/markdown/sanitization_filter.rb    | 64 ++++++++++++-------
 .../markdown/sanitization_filter_spec.rb      | 14 ++++
 5 files changed, 59 insertions(+), 26 deletions(-)

diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml
index f42007da07..0687840af3 100644
--- a/app/views/groups/show.html.haml
+++ b/app/views/groups/show.html.haml
@@ -11,7 +11,7 @@
       @#{@group.path}
     - if @group.description.present?
       .description
-        = markdown(@group.description)
+        = markdown(@group.description, pipeline: :description)
   %hr
 
   = render 'shared/show_aside'
diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml
index 05f44acd3c..076afb11a9 100644
--- a/app/views/projects/_home_panel.html.haml
+++ b/app/views/projects/_home_panel.html.haml
@@ -5,7 +5,7 @@
   .project-home-row.project-home-row-top
     .project-home-desc
       - if @project.description.present?
-        = markdown(@project.description)
+        = markdown(@project.description, pipeline: :description)
       - if can?(current_user, :admin_project, @project)
         &ndash;
         = link_to 'Edit', edit_namespace_project_path
diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb
index 5db1566f55..fa9c0975bb 100644
--- a/lib/gitlab/markdown.rb
+++ b/lib/gitlab/markdown.rb
@@ -57,6 +57,9 @@ module Gitlab
       pipeline = HTML::Pipeline.new(filters)
 
       context = {
+        # SanitizationFilter
+        pipeline: options[:pipeline],
+
         # EmojiFilter
         asset_root: Gitlab.config.gitlab.url,
         asset_host: Gitlab::Application.config.asset_host,
diff --git a/lib/gitlab/markdown/sanitization_filter.rb b/lib/gitlab/markdown/sanitization_filter.rb
index 88781fea0c..fc29d09081 100644
--- a/lib/gitlab/markdown/sanitization_filter.rb
+++ b/lib/gitlab/markdown/sanitization_filter.rb
@@ -8,33 +8,53 @@ module Gitlab
     # Extends HTML::Pipeline::SanitizationFilter with a custom whitelist.
     class SanitizationFilter < HTML::Pipeline::SanitizationFilter
       def whitelist
-        whitelist = super
-
-        # Only push these customizations once
-        unless customized?(whitelist[:transformers])
-          # Allow code highlighting
-          whitelist[:attributes]['pre'] = %w(class)
-          whitelist[:attributes]['span'] = %w(class)
-
-          # Allow table alignment
-          whitelist[:attributes]['th'] = %w(style)
-          whitelist[:attributes]['td'] = %w(style)
-
-          # Allow span elements
-          whitelist[:elements].push('span')
-
-          # Remove `rel` attribute from `a` elements
-          whitelist[:transformers].push(remove_rel)
-
-          # Remove `class` attribute from non-highlight spans
-          whitelist[:transformers].push(clean_spans)
+        # Descriptions are more heavily sanitized, allowing only a few elements.
+        # See http://git.io/vkuAN
+        if pipeline == :description
+          whitelist = LIMITED
+        else
+          whitelist = super
         end
 
+        customize_whitelist(whitelist)
+
         whitelist
       end
 
       private
 
+      def pipeline
+        context[:pipeline] || :default
+      end
+
+      def customized?(transformers)
+        transformers.last.source_location[0] == __FILE__
+      end
+
+      def customize_whitelist(whitelist)
+        # Only push these customizations once
+        return if customized?(whitelist[:transformers])
+
+        # Allow code highlighting
+        whitelist[:attributes]['pre'] = %w(class)
+        whitelist[:attributes]['span'] = %w(class)
+
+        # Allow table alignment
+        whitelist[:attributes]['th'] = %w(style)
+        whitelist[:attributes]['td'] = %w(style)
+
+        # Allow span elements
+        whitelist[:elements].push('span')
+
+        # Remove `rel` attribute from `a` elements
+        whitelist[:transformers].push(remove_rel)
+
+        # Remove `class` attribute from non-highlight spans
+        whitelist[:transformers].push(clean_spans)
+
+        whitelist
+      end
+
       def remove_rel
         lambda do |env|
           if env[:node_name] == 'a'
@@ -53,10 +73,6 @@ module Gitlab
           end
         end
       end
-
-      def customized?(transformers)
-        transformers.last.source_location[0] == __FILE__
-      end
     end
   end
 end
diff --git a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb
index 4a1aa76614..80f3d2f263 100644
--- a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb
@@ -42,6 +42,13 @@ module Gitlab::Markdown
     end
 
     describe 'custom whitelist' do
+      it 'customizes the whitelist only once' do
+        instance = described_class.new('Foo')
+        3.times { instance.whitelist }
+
+        expect(instance.whitelist[:transformers].size).to eq 4
+      end
+
       it 'allows syntax highlighting' do
         exp = act = %q{<pre class="code highlight white c"><code><span class="k">def</span></code></pre>}
         expect(filter(act).to_html).to eq exp
@@ -87,5 +94,12 @@ module Gitlab::Markdown
         expect(doc.at_css('a')['href']).to be_nil
       end
     end
+
+    context 'when pipeline is :description' do
+      it 'uses a stricter whitelist' do
+        doc = filter('<h1>My Project</h1>', pipeline: :description)
+        expect(doc.to_html.strip).to eq 'My Project'
+      end
+    end
   end
 end

From 442a0663da437abcdec7fbd86967b6d8980d4090 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Fri, 29 May 2015 19:02:11 -0400
Subject: [PATCH 4/6] Add feature specs for Project and Group description
 rendering

---
 spec/features/groups_spec.rb   | 36 ++++++++++++++++++++++
 spec/features/markdown_spec.rb |  2 ++
 spec/features/projects_spec.rb | 56 ++++++++++++++++++++++++++--------
 3 files changed, 82 insertions(+), 12 deletions(-)
 create mode 100644 spec/features/groups_spec.rb

diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb
new file mode 100644
index 0000000000..edc1c63a0a
--- /dev/null
+++ b/spec/features/groups_spec.rb
@@ -0,0 +1,36 @@
+require 'spec_helper'
+
+feature 'Group' do
+  describe 'description' do
+    let(:group) { create(:group) }
+    let(:path)  { group_path(group) }
+
+    before do
+      login_as(:admin)
+    end
+
+    it 'parses Markdown' do
+      group.update_attribute(:description, 'This is **my** group')
+      visit path
+      expect(page).to have_css('.description > p > strong')
+    end
+
+    it 'passes through html-pipeline' do
+      group.update_attribute(:description, 'This group is the :poop:')
+      visit path
+      expect(page).to have_css('.description > p > img')
+    end
+
+    it 'sanitizes unwanted tags' do
+      group.update_attribute(:description, '# Group Description')
+      visit path
+      expect(page).not_to have_css('.description h1')
+    end
+
+    it 'permits `rel` attribute on links' do
+      group.update_attribute(:description, 'https://google.com/')
+      visit path
+      expect(page).to have_css('.description a[rel]')
+    end
+  end
+end
diff --git a/spec/features/markdown_spec.rb b/spec/features/markdown_spec.rb
index ee1b3bf749..902968cebc 100644
--- a/spec/features/markdown_spec.rb
+++ b/spec/features/markdown_spec.rb
@@ -18,11 +18,13 @@ require 'erb'
 #         -> `gfm_with_options` helper
 #           -> HTML::Pipeline
 #             -> Sanitize
+#             -> RelativeLink
 #             -> Emoji
 #             -> Table of Contents
 #             -> Autolinks
 #               -> Rinku (http, https, ftp)
 #               -> Other schemes
+#             -> ExternalLink
 #             -> References
 #             -> TaskList
 #           -> `html_safe`
diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb
index cae11be7cd..56523f6e1a 100644
--- a/spec/features/projects_spec.rb
+++ b/spec/features/projects_spec.rb
@@ -1,24 +1,56 @@
 require 'spec_helper'
 
-describe "Projects", feature: true, js: true do
-  before { login_as :user }
+feature 'Project' do
+  describe 'description' do
+    let(:project) { create(:project) }
+    let(:path)    { namespace_project_path(project.namespace, project) }
 
-  describe "DELETE /projects/:id" do
     before do
-      @project = create(:project, namespace: @user.namespace)
-      @project.team << [@user, :master]
-      visit edit_namespace_project_path(@project.namespace, @project)
+      login_as(:admin)
     end
 
-    it "should remove project" do
+    it 'parses Markdown' do
+      project.update_attribute(:description, 'This is **my** project')
+      visit path
+      expect(page).to have_css('.project-home-desc > p > strong')
+    end
+
+    it 'passes through html-pipeline' do
+      project.update_attribute(:description, 'This project is the :poop:')
+      visit path
+      expect(page).to have_css('.project-home-desc > p > img')
+    end
+
+    it 'sanitizes unwanted tags' do
+      project.update_attribute(:description, '# Project Description')
+      visit path
+      expect(page).not_to have_css('.project-home-desc h1')
+    end
+
+    it 'permits `rel` attribute on links' do
+      project.update_attribute(:description, 'https://google.com/')
+      visit path
+      expect(page).to have_css('.project-home-desc a[rel]')
+    end
+  end
+
+  describe 'removal', js: true do
+    let(:user)    { create(:user) }
+    let(:project) { create(:project, namespace: user.namespace) }
+
+    before do
+      login_with(user)
+      project.team << [user, :master]
+      visit edit_namespace_project_path(project.namespace, project)
+    end
+
+    it 'should remove project' do
       expect { remove_project }.to change {Project.count}.by(-1)
     end
 
     it 'should delete the project from disk' do
-      expect(GitlabShellWorker).to(
-        receive(:perform_async).with(:remove_repository,
-                                     /#{@project.path_with_namespace}/)
-      ).twice
+      expect(GitlabShellWorker).to receive(:perform_async).
+        with(:remove_repository, /#{project.path_with_namespace}/).twice
 
       remove_project
     end
@@ -26,7 +58,7 @@ describe "Projects", feature: true, js: true do
 
   def remove_project
     click_link "Remove project"
-    fill_in 'confirm_name_input', with: @project.path
+    fill_in 'confirm_name_input', with: project.path
     click_button 'Confirm'
   end
 end

From 79c4e3899fa7697afdefb13d64c4add08ca84aac Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Tue, 2 Jun 2015 13:27:53 -0400
Subject: [PATCH 5/6] Rename ReferenceFilterSpecHelper to FilterSpecHelper

And make it more generalized for all filter specs.
---
 .../gitlab/markdown/autolink_filter_spec.rb   |  6 +-
 .../commit_range_reference_filter_spec.rb     |  2 +-
 .../markdown/commit_reference_filter_spec.rb  |  2 +-
 spec/lib/gitlab/markdown/emoji_filter_spec.rb |  4 +-
 .../external_issue_reference_filter_spec.rb   |  2 +-
 .../markdown/external_link_filter_spec.rb     |  4 +-
 .../markdown/issue_reference_filter_spec.rb   |  2 +-
 .../markdown/label_reference_filter_spec.rb   |  2 +-
 .../merge_request_reference_filter_spec.rb    |  2 +-
 .../markdown/sanitization_filter_spec.rb      |  4 +-
 .../markdown/snippet_reference_filter_spec.rb |  2 +-
 .../markdown/table_of_contents_filter_spec.rb |  4 +-
 .../gitlab/markdown/task_list_filter_spec.rb  |  4 +-
 .../markdown/user_reference_filter_spec.rb    |  2 +-
 ...r_spec_helper.rb => filter_spec_helper.rb} | 75 ++++++++++---------
 15 files changed, 56 insertions(+), 61 deletions(-)
 rename spec/support/{reference_filter_spec_helper.rb => filter_spec_helper.rb} (75%)

diff --git a/spec/lib/gitlab/markdown/autolink_filter_spec.rb b/spec/lib/gitlab/markdown/autolink_filter_spec.rb
index 0bbdc11a97..a14cb2da08 100644
--- a/spec/lib/gitlab/markdown/autolink_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/autolink_filter_spec.rb
@@ -2,11 +2,9 @@ require 'spec_helper'
 
 module Gitlab::Markdown
   describe AutolinkFilter do
-    let(:link) { 'http://about.gitlab.com/' }
+    include FilterSpecHelper
 
-    def filter(html, options = {})
-      described_class.call(html, options)
-    end
+    let(:link) { 'http://about.gitlab.com/' }
 
     it 'does nothing when :autolink is false' do
       exp = act = link
diff --git a/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb b/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb
index d3695ee46d..e8391cc7ac 100644
--- a/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 module Gitlab::Markdown
   describe CommitRangeReferenceFilter do
-    include ReferenceFilterSpecHelper
+    include FilterSpecHelper
 
     let(:project) { create(:project) }
     let(:commit1) { project.commit }
diff --git a/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb b/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb
index a0d2cd7e22..a10d43c9a0 100644
--- a/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 module Gitlab::Markdown
   describe CommitReferenceFilter do
-    include ReferenceFilterSpecHelper
+    include FilterSpecHelper
 
     let(:project) { create(:project) }
     let(:commit)  { project.commit }
diff --git a/spec/lib/gitlab/markdown/emoji_filter_spec.rb b/spec/lib/gitlab/markdown/emoji_filter_spec.rb
index 18d55c4818..11efd9bb4c 100644
--- a/spec/lib/gitlab/markdown/emoji_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/emoji_filter_spec.rb
@@ -2,9 +2,7 @@ require 'spec_helper'
 
 module Gitlab::Markdown
   describe EmojiFilter do
-    def filter(html, contexts = {})
-      described_class.call(html, contexts)
-    end
+    include FilterSpecHelper
 
     before do
       ActionController::Base.asset_host = 'https://foo.com'
diff --git a/spec/lib/gitlab/markdown/external_issue_reference_filter_spec.rb b/spec/lib/gitlab/markdown/external_issue_reference_filter_spec.rb
index bf9409589f..f16095bc2b 100644
--- a/spec/lib/gitlab/markdown/external_issue_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/external_issue_reference_filter_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 module Gitlab::Markdown
   describe ExternalIssueReferenceFilter do
-    include ReferenceFilterSpecHelper
+    include FilterSpecHelper
 
     def helper
       IssuesHelper
diff --git a/spec/lib/gitlab/markdown/external_link_filter_spec.rb b/spec/lib/gitlab/markdown/external_link_filter_spec.rb
index c2ff4f80a4..a040b34577 100644
--- a/spec/lib/gitlab/markdown/external_link_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/external_link_filter_spec.rb
@@ -2,9 +2,7 @@ require 'spec_helper'
 
 module Gitlab::Markdown
   describe ExternalLinkFilter do
-    def filter(html, options = {})
-      described_class.call(html, options)
-    end
+    include FilterSpecHelper
 
     it 'ignores elements without an href attribute' do
       exp = act = %q(<a id="ignored">Ignore Me</a>)
diff --git a/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb b/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb
index a838d7570c..fa43d33794 100644
--- a/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 module Gitlab::Markdown
   describe IssueReferenceFilter do
-    include ReferenceFilterSpecHelper
+    include FilterSpecHelper
 
     def helper
       IssuesHelper
diff --git a/spec/lib/gitlab/markdown/label_reference_filter_spec.rb b/spec/lib/gitlab/markdown/label_reference_filter_spec.rb
index 41987f57bc..cf3337b1ba 100644
--- a/spec/lib/gitlab/markdown/label_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/label_reference_filter_spec.rb
@@ -3,7 +3,7 @@ require 'html/pipeline'
 
 module Gitlab::Markdown
   describe LabelReferenceFilter do
-    include ReferenceFilterSpecHelper
+    include FilterSpecHelper
 
     let(:project)   { create(:empty_project) }
     let(:label)     { create(:label, project: project) }
diff --git a/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb b/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb
index 6aeb109360..5945302a2d 100644
--- a/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 module Gitlab::Markdown
   describe MergeRequestReferenceFilter do
-    include ReferenceFilterSpecHelper
+    include FilterSpecHelper
 
     let(:project) { create(:project) }
     let(:merge)   { create(:merge_request, source_project: project) }
diff --git a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb
index 80f3d2f263..8627cb288a 100644
--- a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb
@@ -2,9 +2,7 @@ require 'spec_helper'
 
 module Gitlab::Markdown
   describe SanitizationFilter do
-    def filter(html, options = {})
-      described_class.call(html, options)
-    end
+    include FilterSpecHelper
 
     describe 'default whitelist' do
       it 'sanitizes tags that are not whitelisted' do
diff --git a/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb b/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb
index 07ece66e90..38619a3c07 100644
--- a/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 module Gitlab::Markdown
   describe SnippetReferenceFilter do
-    include ReferenceFilterSpecHelper
+    include FilterSpecHelper
 
     let(:project)   { create(:empty_project) }
     let(:snippet)   { create(:project_snippet, project: project) }
diff --git a/spec/lib/gitlab/markdown/table_of_contents_filter_spec.rb b/spec/lib/gitlab/markdown/table_of_contents_filter_spec.rb
index f383a5850d..ddf583a72c 100644
--- a/spec/lib/gitlab/markdown/table_of_contents_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/table_of_contents_filter_spec.rb
@@ -4,9 +4,7 @@ require 'spec_helper'
 
 module Gitlab::Markdown
   describe TableOfContentsFilter do
-    def filter(html, options = {})
-      described_class.call(html, options)
-    end
+    include FilterSpecHelper
 
     def header(level, text)
       "<h#{level}>#{text}</h#{level}>\n"
diff --git a/spec/lib/gitlab/markdown/task_list_filter_spec.rb b/spec/lib/gitlab/markdown/task_list_filter_spec.rb
index 2a1e1cc512..94f39cc966 100644
--- a/spec/lib/gitlab/markdown/task_list_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/task_list_filter_spec.rb
@@ -2,9 +2,7 @@ require 'spec_helper'
 
 module Gitlab::Markdown
   describe TaskListFilter do
-    def filter(html, options = {})
-      described_class.call(html, options)
-    end
+    include FilterSpecHelper
 
     it 'does not apply `task-list` class to non-task lists' do
       exp = act = %(<ul><li>Item</li></ul>)
diff --git a/spec/lib/gitlab/markdown/user_reference_filter_spec.rb b/spec/lib/gitlab/markdown/user_reference_filter_spec.rb
index 0ecbdee9b9..08e6941028 100644
--- a/spec/lib/gitlab/markdown/user_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/user_reference_filter_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 module Gitlab::Markdown
   describe UserReferenceFilter do
-    include ReferenceFilterSpecHelper
+    include FilterSpecHelper
 
     let(:project)   { create(:empty_project) }
     let(:user)      { create(:user) }
diff --git a/spec/support/reference_filter_spec_helper.rb b/spec/support/filter_spec_helper.rb
similarity index 75%
rename from spec/support/reference_filter_spec_helper.rb
rename to spec/support/filter_spec_helper.rb
index afbea55ab9..755964e9a3 100644
--- a/spec/support/reference_filter_spec_helper.rb
+++ b/spec/support/filter_spec_helper.rb
@@ -1,13 +1,38 @@
-# Common methods and setup for Gitlab::Markdown reference filter specs
+# Helper methods for Gitlab::Markdown filter specs
 #
 # Must be included into specs manually
-module ReferenceFilterSpecHelper
+module FilterSpecHelper
   extend ActiveSupport::Concern
 
-  # Shortcut to Rails' auto-generated routes helpers, to avoid including the
-  # module
-  def urls
-    Rails.application.routes.url_helpers
+  # Perform `call` on the described class
+  #
+  # Automatically passes the current `project` value, if defined, to the context
+  # if none is provided.
+  #
+  # html     - HTML String to pass to the filter's `call` method.
+  # contexts - Hash context for the filter. (default: {project: project})
+  #
+  # Returns a Nokogiri::XML::DocumentFragment
+  def filter(html, contexts = {})
+    if defined?(project)
+      contexts.reverse_merge!(project: project)
+    end
+
+    described_class.call(html, contexts)
+  end
+
+  # Run text through HTML::Pipeline with the current filter and return the
+  # result Hash
+  #
+  # body     - String text to run through the pipeline
+  # contexts - Hash context for the filter. (default: {project: project})
+  #
+  # Returns the Hash
+  def pipeline_result(body, contexts = {})
+    contexts.reverse_merge!(project: project)
+
+    pipeline = HTML::Pipeline.new([described_class], contexts)
+    pipeline.call(body)
   end
 
   # Modify a String reference to make it invalid
@@ -30,41 +55,23 @@ module ReferenceFilterSpecHelper
     end
   end
 
-  # Perform `call` on the described class
-  #
-  # Automatically passes the current `project` value to the context if none is
-  # provided.
-  #
-  # html     - String text to pass to the filter's `call` method.
-  # contexts - Hash context for the filter. (default: {project: project})
-  #
-  # Returns the String text returned by the filter's `call` method.
-  def filter(html, contexts = {})
-    contexts.reverse_merge!(project: project)
-    described_class.call(html, contexts)
-  end
-
-  # Run text through HTML::Pipeline with the current filter and return the
-  # result Hash
-  #
-  # body     - String text to run through the pipeline
-  # contexts - Hash context for the filter. (default: {project: project})
-  #
-  # Returns the Hash of the pipeline result
-  def pipeline_result(body, contexts = {})
-    contexts.reverse_merge!(project: project)
-
-    pipeline = HTML::Pipeline.new([described_class], contexts)
-    pipeline.call(body)
-  end
-
+  # Stub CrossProjectReference#user_can_reference_project? to return true for
+  # the current test
   def allow_cross_reference!
     allow_any_instance_of(described_class).
       to receive(:user_can_reference_project?).and_return(true)
   end
 
+  # Stub CrossProjectReference#user_can_reference_project? to return false for
+  # the current test
   def disallow_cross_reference!
     allow_any_instance_of(described_class).
       to receive(:user_can_reference_project?).and_return(false)
   end
+
+  # Shortcut to Rails' auto-generated routes helpers, to avoid including the
+  # module
+  def urls
+    Rails.application.routes.url_helpers
+  end
 end

From 9e7a9c63a59f4e673271b3600b735e3fa6702432 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Tue, 2 Jun 2015 13:41:12 -0400
Subject: [PATCH 6/6] Further limit the limited whitelist for project/group
 descriptions

---
 lib/gitlab/markdown/sanitization_filter.rb    |  1 +
 .../markdown/sanitization_filter_spec.rb      | 19 +++++++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/lib/gitlab/markdown/sanitization_filter.rb b/lib/gitlab/markdown/sanitization_filter.rb
index fc29d09081..74b3a8d274 100644
--- a/lib/gitlab/markdown/sanitization_filter.rb
+++ b/lib/gitlab/markdown/sanitization_filter.rb
@@ -12,6 +12,7 @@ module Gitlab
         # See http://git.io/vkuAN
         if pipeline == :description
           whitelist = LIMITED
+          whitelist[:elements] -= %w(pre code img ol ul li)
         else
           whitelist = super
         end
diff --git a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb
index 8627cb288a..e50c82d0b3 100644
--- a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb
@@ -95,8 +95,23 @@ module Gitlab::Markdown
 
     context 'when pipeline is :description' do
       it 'uses a stricter whitelist' do
-        doc = filter('<h1>My Project</h1>', pipeline: :description)
-        expect(doc.to_html.strip).to eq 'My Project'
+        doc = filter('<h1>Description</h1>', pipeline: :description)
+        expect(doc.to_html.strip).to eq 'Description'
+      end
+
+      %w(pre code img ol ul li).each do |elem|
+        it "removes '#{elem}' elements" do
+          act = "<#{elem}>Description</#{elem}>"
+          expect(filter(act, pipeline: :description).to_html.strip).
+            to eq 'Description'
+        end
+      end
+
+      %w(b i strong em a ins del sup sub p).each do |elem|
+        it "still allows '#{elem}' elements" do
+          exp = act = "<#{elem}>Description</#{elem}>"
+          expect(filter(act, pipeline: :description).to_html).to eq exp
+        end
       end
     end
   end