Merge branch 'security-fixes-to-master' into 'master'

Merge security fixes to master

See merge request gitlab/gitlabhq!2369

app/assets/javascripts/milestone_select.js

@@ -94,10 +94,10 @@ export default class MilestoneSelect {
             if (showMenuAbove) {
               $dropdown.data('glDropdown').positionMenuAbove();
             }
-            $(`[data-milestone-id="${selectedMilestone}"] > a`).addClass('is-active');
+            $(`[data-milestone-id="${_.escape(selectedMilestone)}"] > a`).addClass('is-active');
           }),
         renderRow: milestone => `
-          <li data-milestone-id="${milestone.name}">
+          <li data-milestone-id="${_.escape(milestone.name)}">
             <a href='#' class='dropdown-menu-milestone-link'>
               ${_.escape(milestone.title)}
             </a>
@@ -125,7 +125,6 @@ export default class MilestoneSelect {
             return milestone.id;
           }
         },
-        isSelected: milestone => milestone.name === selectedMilestone,
         hidden: () => {
           $selectBox.hide();
           // display:block overrides the hide-collapse rule
@@ -137,7 +136,7 @@ export default class MilestoneSelect {
             selectedMilestone = $dropdown[0].dataset.selected || selectedMilestoneDefault;
           }
           $('a.is-active', $el).removeClass('is-active');
-          $(`[data-milestone-id="${selectedMilestone}"] > a`, $el).addClass('is-active');
+          $(`[data-milestone-id="${_.escape(selectedMilestone)}"] > a`, $el).addClass('is-active');
         },
         vue: $dropdown.hasClass('js-issue-board-sidebar'),
         clicked: (clickEvent) => {
@@ -158,6 +157,7 @@ export default class MilestoneSelect {
           const isMRIndex = (page === page && page === 'projects:merge_requests:index');
           const isSelecting = (selected.name !== selectedMilestone);
           selectedMilestone = isSelecting ? selected.name : selectedMilestoneDefault;
+
           if ($dropdown.hasClass('js-filter-bulk-update') || $dropdown.hasClass('js-issuable-form-dropdown')) {
             e.preventDefault();
             return;

app/helpers/services_helper.rb

 module ServicesHelper
+  def service_event_description(event)
+    case event
+    when "push", "push_events"
+      "Event will be triggered by a push to the repository"
+    when "tag_push", "tag_push_events"
+      "Event will be triggered when a new tag is pushed to the repository"
+    when "note", "note_events"
+      "Event will be triggered when someone adds a comment"
+    when "confidential_note", "confidential_note_events"
+      "Event will be triggered when someone adds a comment on a confidential issue"
+    when "issue", "issue_events"
+      "Event will be triggered when an issue is created/updated/closed"
+    when "confidential_issue", "confidential_issues_events"
+      "Event will be triggered when a confidential issue is created/updated/closed"
+    when "merge_request", "merge_request_events"
+      "Event will be triggered when a merge request is created/updated/merged"
+    when "pipeline", "pipeline_events"
+      "Event will be triggered when a pipeline status changes"
+    when "wiki_page", "wiki_page_events"
+      "Event will be triggered when a wiki page is created/updated"
+    when "commit", "commit_events"
+      "Event will be triggered when a commit is created/updated"
+    end
+  end
+
   def service_event_field_name(event)
     event = event.pluralize if %w[merge_request issue confidential_issue].include?(event)
     "#{event}_events"

app/models/hooks/project_hook.rb

@@ -7,6 +7,7 @@ class ProjectHook < WebHook
     :issue_hooks,
     :confidential_issue_hooks,
     :note_hooks,
+    :confidential_note_hooks,
     :merge_request_hooks,
     :job_hooks,
     :pipeline_hooks,

app/models/note.rb

@@ -268,6 +268,10 @@ class Note < ActiveRecord::Base
     self.special_role = Note::SpecialRole::FIRST_TIME_CONTRIBUTOR
   end
 
+  def confidential?
+    noteable.try(:confidential?)
+  end
+
   def editable?
     !system?
   end

app/models/project_services/chat_notification_service.rb

@@ -21,8 +21,16 @@ class ChatNotificationService < Service
     end
   end
 
+  def confidential_issue_channel
+    properties['confidential_issue_channel'].presence || properties['issue_channel']
+  end
+
+  def confidential_note_channel
+    properties['confidential_note_channel'].presence || properties['note_channel']
+  end
+
   def self.supported_events
-    %w[push issue confidential_issue merge_request note tag_push
+    %w[push issue confidential_issue merge_request note confidential_note tag_push
        pipeline wiki_page]
   end
 
@@ -55,7 +63,9 @@ class ChatNotificationService < Service
 
     return false unless message
 
-    channel_name = get_channel_field(object_kind).presence || channel
+    event_type = data[:event_type] || object_kind
+
+    channel_name = get_channel_field(event_type).presence || channel
 
     opts = {}
     opts[:channel] = channel_name if channel_name

app/models/project_services/hipchat_service.rb

@@ -46,7 +46,7 @@ class HipchatService < Service
   end
 
   def self.supported_events
-    %w(push issue confidential_issue merge_request note tag_push pipeline)
+    %w(push issue confidential_issue merge_request note confidential_note tag_push pipeline)
   end
 
   def execute(data)

app/models/service.rb

@@ -14,6 +14,7 @@ class Service < ActiveRecord::Base
   default_value_for :merge_requests_events, true
   default_value_for :tag_push_events, true
   default_value_for :note_events, true
+  default_value_for :confidential_note_events, true
   default_value_for :job_events, true
   default_value_for :pipeline_events, true
   default_value_for :wiki_page_events, true
@@ -42,6 +43,7 @@ class Service < ActiveRecord::Base
   scope :confidential_issue_hooks, -> { where(confidential_issues_events: true, active: true) }
   scope :merge_request_hooks, -> { where(merge_requests_events: true, active: true) }
   scope :note_hooks, -> { where(note_events: true, active: true) }
+  scope :confidential_note_hooks, -> { where(confidential_note_events: true, active: true) }
   scope :job_hooks, -> { where(job_events: true, active: true) }
   scope :pipeline_hooks, -> { where(pipeline_events: true, active: true) }
   scope :wiki_page_hooks, -> { where(wiki_page_events: true, active: true) }
@@ -168,8 +170,10 @@ class Service < ActiveRecord::Base
   def self.prop_accessor(*args)
     args.each do |arg|
       class_eval %{
-        def #{arg}
-          properties['#{arg}']
+        unless method_defined?(arg)
+          def #{arg}
+            properties['#{arg}']
+          end
         end
 
         def #{arg}=(value)

app/services/notes/post_process_service.rb

@@ -24,8 +24,10 @@ module Notes
 
     def execute_note_hooks
       note_data = hook_data
-      @note.project.execute_hooks(note_data, :note_hooks)
-      @note.project.execute_services(note_data, :note_hooks)
+      hooks_scope = @note.confidential? ? :confidential_note_hooks : :note_hooks
+
+      @note.project.execute_hooks(note_data, hooks_scope)
+      @note.project.execute_services(note_data, hooks_scope)
     end
   end
 end

app/views/shared/web_hooks/_form.html.haml

@@ -32,6 +32,13 @@
           %strong Comments
         %p.light
           This URL will be triggered when someone adds a comment
+    %li
+      = form.check_box :confidential_note_events, class: 'pull-left'
+      .prepend-left-20
+        = form.label :confidential_note_events, class: 'list-label' do
+          %strong Confidential Comments
+        %p.light
+          This URL will be triggered when someone adds a comment on a confidential issue
     %li
       = form.check_box :issues_events, class: 'pull-left'
       .prepend-left-20

changelogs/unreleased/42028-xss-diffs.yml

+---
+title: Fix XSS on diff view stored on filenames
+merge_request:
+author:
+type: security

changelogs/unreleased/jej-mattermost-notification-confidentiality.yml

+---
+title: Adds confidential notes channel for Slack/Mattermost
+merge_request:
+author:
+type: security

db/migrate/20171222115326_add_confidential_note_events_to_web_hooks.rb

+class AddConfidentialNoteEventsToWebHooks < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column :web_hooks, :confidential_note_events, :boolean
+  end
+
+  def down
+    remove_column :web_hooks, :confidential_note_events
+  end
+end

db/migrate/20180103123548_add_confidential_note_events_to_services.rb

+class AddConfidentialNoteEventsToServices < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column :services, :confidential_note_events, :boolean
+    change_column_default :services, :confidential_note_events, true
+  end
+
+  def down
+    remove_column :services, :confidential_note_events
+  end
+end

db/post_migrate/20180104131052_schedule_set_confidential_note_events_on_webhooks.rb

+class ScheduleSetConfidentialNoteEventsOnWebhooks < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  BATCH_SIZE = 1_000
+  INTERVAL = 5.minutes
+
+  disable_ddl_transaction!
+
+  def up
+    migration = Gitlab::BackgroundMigration::SetConfidentialNoteEventsOnWebhooks
+    migration_name = migration.to_s.demodulize
+    relation = migration::WebHook.hooks_to_update
+
+    queue_background_migration_jobs_by_range_at_intervals(relation,
+                                                          migration_name,
+                                                          INTERVAL,
+                                                          batch_size: BATCH_SIZE)
+  end
+
+  def down
+  end
+end

db/post_migrate/20180122154930_schedule_set_confidential_note_events_on_services.rb

+class ScheduleSetConfidentialNoteEventsOnServices < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  BATCH_SIZE = 1_000
+  INTERVAL = 20.minutes
+
+  disable_ddl_transaction!
+
+  def up
+    migration = Gitlab::BackgroundMigration::SetConfidentialNoteEventsOnServices
+    migration_name = migration.to_s.demodulize
+    relation = migration::Service.services_to_update
+
+    queue_background_migration_jobs_by_range_at_intervals(relation,
+                                                          migration_name,
+                                                          INTERVAL,
+                                                          batch_size: BATCH_SIZE)
+  end
+
+  def down
+  end
+end

db/schema.rb

@@ -1684,6 +1684,7 @@ ActiveRecord::Schema.define(version: 20180327101207) do
     t.boolean "confidential_issues_events", default: true, null: false
     t.boolean "commit_events", default: true, null: false
     t.boolean "job_events", default: false, null: false
+    t.boolean "confidential_note_events", default: true
   end
 
   add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree
@@ -2022,6 +2023,7 @@ ActiveRecord::Schema.define(version: 20180327101207) do
     t.boolean "confidential_issues_events", default: false, null: false
     t.boolean "repository_update_events", default: false, null: false
     t.boolean "job_events", default: false, null: false
+    t.boolean "confidential_note_events"
   end
 
   add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree

lib/api/entities.rb

@@ -72,7 +72,7 @@ module API
 
     class ProjectHook < Hook
       expose :project_id, :issues_events, :confidential_issues_events
-      expose :note_events, :pipeline_events, :wiki_page_events
+      expose :note_events, :confidential_note_events, :pipeline_events, :wiki_page_events
       expose :job_events
     end
 
@@ -794,7 +794,7 @@ module API
       expose :id, :title, :created_at, :updated_at, :active
       expose :push_events, :issues_events, :confidential_issues_events
       expose :merge_requests_events, :tag_push_events, :note_events
-      expose :pipeline_events, :wiki_page_events
+      expose :confidential_note_events, :pipeline_events, :wiki_page_events
       expose :job_events
       # Expose serialized properties
       expose :properties do |service, options|

lib/api/project_hooks.rb

@@ -14,6 +14,7 @@ module API
         optional :merge_requests_events, type: Boolean, desc: "Trigger hook on merge request events"
         optional :tag_push_events, type: Boolean, desc: "Trigger hook on tag push events"
         optional :note_events, type: Boolean, desc: "Trigger hook on note(comment) events"
+        optional :confidential_note_events, type: Boolean, desc: "Trigger hook on confidential note(comment) events"
         optional :job_events, type: Boolean, desc: "Trigger hook on job events"
         optional :pipeline_events, type: Boolean, desc: "Trigger hook on pipeline events"
         optional :wiki_page_events, type: Boolean, desc: "Trigger hook on wiki events"

lib/gitlab/background_migration/set_confidential_note_events_on_services.rb

+# frozen_string_literal: true
+# rubocop:disable Style/Documentation
+
+module Gitlab
+  module BackgroundMigration
+    # Ensures services which previously recieved all notes events continue
+    # to recieve confidential ones.
+    class SetConfidentialNoteEventsOnServices
+      class Service < ActiveRecord::Base
+        self.table_name = 'services'
+
+        include ::EachBatch
+
+        def self.services_to_update
+          where(confidential_note_events: nil, note_events: true)
+        end
+      end
+
+      def perform(start_id, stop_id)
+        Service.services_to_update
+               .where(id: start_id..stop_id)
+               .update_all(confidential_note_events: true)
+      end
+    end
+  end
+end

lib/gitlab/background_migration/set_confidential_note_events_on_webhooks.rb

+# frozen_string_literal: true
+# rubocop:disable Style/Documentation
+
+module Gitlab
+  module BackgroundMigration
+    # Ensures hooks which previously recieved all notes events continue
+    # to recieve confidential ones.
+    class SetConfidentialNoteEventsOnWebhooks
+      class WebHook < ActiveRecord::Base
+        self.table_name = 'web_hooks'
+
+        include ::EachBatch
+
+        def self.hooks_to_update
+          where(confidential_note_events: nil, note_events: true)
+        end
+      end
+
+      def perform(start_id, stop_id)
+        WebHook.hooks_to_update
+               .where(id: start_id..stop_id)
+               .update_all(confidential_note_events: true)
+      end
+    end
+  end
+end

lib/gitlab/data_builder/note.rb

@@ -9,6 +9,7 @@ module Gitlab
       #
       # data = {
       #   object_kind: "note",
+      #   event_type: "confidential_note",
       #   user: {
       #     name: String,
       #     username: String,
@@ -51,8 +52,11 @@ module Gitlab
       end
 
       def build_base_data(project, user, note)
+        event_type = note.confidential? ? 'confidential_note' : 'note'
+
         base_data = {
           object_kind: "note",
+          event_type: event_type,
           user: user.hook_attrs,
           project_id: project.id,
           project: project.hook_attrs,

lib/gitlab/database/migration_helpers.rb

@@ -860,7 +860,7 @@ into similar problems in the future (e.g. when new tables are created).
       # Each job is scheduled with a `delay_interval` in between.
       # If you use a small interval, then some jobs may run at the same time.
       #
-      # model_class - The table being iterated over
+      # model_class - The table or relation being iterated over
       # job_class_name - The background migration job class as a string
       # delay_interval - The duration between each job's scheduled time (must respond to `to_f`)
       # batch_size - The maximum number of rows per job

lib/gitlab/diff/inline_diff_marker.rb

 module Gitlab
   module Diff
     class InlineDiffMarker < Gitlab::StringRangeMarker
+      def initialize(line, rich_line = nil)
+        super(line, rich_line || line)
+      end
+
       def mark(line_inline_diffs, mode: nil)
-        mark = super(line_inline_diffs) do |text, left:, right:|
+        super(line_inline_diffs) do |text, left:, right:|
           %{<span class="#{html_class_names(left, right, mode)}">#{text}</span>}
         end
-        mark.html_safe
       end
 
       private

lib/gitlab/hook_data/issuable_builder.rb

@@ -11,7 +11,8 @@ module Gitlab
 
       def build(user: nil, changes: {})
         hook_data = {
-          object_kind: issuable.class.name.underscore,
+          object_kind: object_kind,
+          event_type: event_type,
           user: user.hook_attrs,
           project: issuable.project.hook_attrs,
           object_attributes: issuable.hook_attrs,
@@ -36,6 +37,18 @@ module Gitlab
 
       private
 
+      def object_kind
+        issuable.class.name.underscore
+      end
+
+      def event_type
+        if issuable.try(:confidential?)
+          "confidential_#{object_kind}"
+        else
+          object_kind
+        end
+      end
+
       def issuable_builder
         case issuable
         when Issue

spec/factories/project_hooks.rb

@@ -15,6 +15,7 @@ FactoryBot.define do
       issues_events true
       confidential_issues_events true
       note_events true
+      confidential_note_events true
       job_events true
       pipeline_events true
       wiki_page_events true

spec/features/issues/form_spec.rb

@@ -226,6 +226,23 @@ describe 'New/edit issue', :js do
 
       expect(page).to have_selector('.atwho-view')
     end
+
+    describe 'milestone' do
+      let!(:milestone) { create(:milestone, title: '"><img src=x onerror=alert(document.domain)>', project: project) }
+
+      it 'escapes milestone' do
+        click_button 'Milestone'
+
+        page.within '.issue-milestone' do
+          click_link milestone.title
+        end
+
+        page.within '.js-milestone-select' do
+          expect(page).to have_content milestone.title
+          expect(page).not_to have_selector 'img'
+        end
+      end
+    end
   end
 
   context 'edit issue' do

spec/helpers/diff_helper_spec.rb

@@ -135,11 +135,37 @@ describe DiffHelper do
     it "returns strings with marked inline diffs" do
       marked_old_line, marked_new_line = mark_inline_diffs(old_line, new_line)
 
-      expect(marked_old_line).to eq(%q{abc <span class="idiff left right deletion">'def'</span>})
+      expect(marked_old_line).to eq(%q{abc <span class="idiff left right deletion">&#39;def&#39;</span>})
       expect(marked_old_line).to be_html_safe
-      expect(marked_new_line).to eq(%q{abc <span class="idiff left right addition">"def"</span>})
+      expect(marked_new_line).to eq(%q{abc <span class="idiff left right addition">&quot;def&quot;</span>})
       expect(marked_new_line).to be_html_safe
     end
+
+    context 'when given HTML' do
+      it 'sanitizes it' do
+        old_line = %{test.txt}
+        new_line = %{<img src=x onerror=alert(document.domain)>}
+
+        marked_old_line, marked_new_line = mark_inline_diffs(old_line, new_line)
+
+        expect(marked_old_line).to eq(%q{<span class="idiff left right deletion">test.txt</span>})
+        expect(marked_old_line).to be_html_safe
+        expect(marked_new_line).to eq(%q{<span class="idiff left right addition">&lt;img src=x onerror=alert(document.domain)&gt;</span>})
+        expect(marked_new_line).to be_html_safe
+      end
+
+      it 'sanitizes the entire line, not just the changes' do
+        old_line = %{<img src=x onerror=alert(document.domain)>}
+        new_line = %{<img src=y onerror=alert(document.domain)>}
+
+        marked_old_line, marked_new_line = mark_inline_diffs(old_line, new_line)
+
+        expect(marked_old_line).to eq(%q{&lt;img src=<span class="idiff left right deletion">x</span> onerror=alert(document.domain)&gt;})
+        expect(marked_old_line).to be_html_safe
+        expect(marked_new_line).to eq(%q{&lt;img src=<span class="idiff left right addition">y</span> onerror=alert(document.domain)&gt;})
+        expect(marked_new_line).to be_html_safe
+      end
+    end
   end
 
   describe '#parallel_diff_discussions' do

spec/lib/gitlab/background_migration/set_confidential_note_events_on_services_spec.rb

+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::SetConfidentialNoteEventsOnServices, :migration, schema: 20180122154930 do
+  let(:services) { table(:services) }
+
+  describe '#perform' do
+    it 'migrates services where note_events is true' do
+      service = services.create(confidential_note_events: nil, note_events: true)
+
+      subject.perform(service.id, service.id)
+
+      expect(service.reload.confidential_note_events).to eq(true)
+    end
+
+    it 'ignores services where note_events is false' do
+      service = services.create(confidential_note_events: nil, note_events: false)
+
+      subject.perform(service.id, service.id)
+
+      expect(service.reload.confidential_note_events).to eq(nil)
+    end
+
+    it 'ignores services where confidential_note_events has already been set' do
+      service = services.create(confidential_note_events: false, note_events: true)
+
+      subject.perform(service.id, service.id)
+
+      expect(service.reload.confidential_note_events).to eq(false)
+    end
+  end
+end

spec/lib/gitlab/background_migration/set_confidential_note_events_on_webhooks_spec.rb

+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::SetConfidentialNoteEventsOnWebhooks, :migration, schema: 20180104131052 do
+  let(:web_hooks) { table(:web_hooks) }
+
+  describe '#perform' do
+    it 'migrates hooks where note_events is true' do
+      hook = web_hooks.create(confidential_note_events: nil, note_events: true)
+
+      subject.perform(hook.id, hook.id)
+
+      expect(hook.reload.confidential_note_events).to eq(true)
+    end
+
+    it 'ignores hooks where note_events is false' do
+      hook = web_hooks.create(confidential_note_events: nil, note_events: false)
+
+      subject.perform(hook.id, hook.id)
+
+      expect(hook.reload.confidential_note_events).to eq(nil)
+    end
+
+    it 'ignores hooks where confidential_note_events has already been set' do
+      hook = web_hooks.create(confidential_note_events: false, note_events: true)
+
+      subject.perform(hook.id, hook.id)
+
+      expect(hook.reload.confidential_note_events).to eq(false)
+    end
+  end
+end

spec/lib/gitlab/data_builder/note_spec.rb

@@ -55,6 +55,14 @@ describe Gitlab::DataBuilder::Note do
         .to be > issue.hook_attrs['updated_at']
     end
 
+    context 'with confidential issue' do
+      let(:issue) { create(:issue, project: project, confidential: true) }
+
+      it 'sets event_type to confidential_note' do
+        expect(data[:event_type]).to eq('confidential_note')
+      end
+    end
+
     include_examples 'project hook data'
     include_examples 'deprecated repository hook data'
   end

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -390,6 +390,7 @@ Service:
 - default
 - wiki_page_events
 - confidential_issues_events
+- confidential_note_events
 ProjectHook:
 - id
 - url
@@ -410,6 +411,7 @@ ProjectHook:
 - token
 - group_id
 - confidential_issues_events
+- confidential_note_events
 - repository_update_events
 ProtectedBranch:
 - id

spec/migrations/active_record/schedule_set_confidential_note_events_on_services_spec.rb

+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20180122154930_schedule_set_confidential_note_events_on_services.rb')
+
+describe ScheduleSetConfidentialNoteEventsOnServices, :migration, :sidekiq do
+  let(:services_table) { table(:services) }
+  let(:migration_class) { Gitlab::BackgroundMigration::SetConfidentialNoteEventsOnServices }
+  let(:migration_name)  { migration_class.to_s.demodulize }
+
+  let!(:service_1)        { services_table.create!(confidential_note_events: nil, note_events: true) }
+  let!(:service_2)        { services_table.create!(confidential_note_events: nil, note_events: true) }
+  let!(:service_migrated) { services_table.create!(confidential_note_events: true, note_events: true) }
+  let!(:service_skip)     { services_table.create!(confidential_note_events: nil, note_events: false) }
+  let!(:service_new)      { services_table.create!(confidential_note_events: false, note_events: true) }
+  let!(:service_4)        { services_table.create!(confidential_note_events: nil, note_events: true) }
+
+  before do
+    stub_const("#{described_class}::BATCH_SIZE", 1)
+  end
+
+  it 'schedules background migrations at correct time' do
+    Sidekiq::Testing.fake! do
+      Timecop.freeze do
+        migrate!
+
+        expect(migration_name).to be_scheduled_delayed_migration(20.minutes, service_1.id, service_1.id)
+        expect(migration_name).to be_scheduled_delayed_migration(40.minutes, service_2.id, service_2.id)
+        expect(migration_name).to be_scheduled_delayed_migration(60.minutes, service_4.id, service_4.id)
+        expect(BackgroundMigrationWorker.jobs.size).to eq 3
+      end
+    end
+  end
+
+  it 'correctly processes services' do
+    Sidekiq::Testing.inline! do
+      expect(services_table.where(confidential_note_events: nil).count).to eq 4
+      expect(services_table.where(confidential_note_events: true).count).to eq 1
+
+      migrate!
+
+      expect(services_table.where(confidential_note_events: nil).count).to eq 1
+      expect(services_table.where(confidential_note_events: true).count).to eq 4
+    end
+  end
+end

spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb

@@ -4,8 +4,24 @@ require Rails.root.join('db', 'post_migrate', '20171013104327_migrate_gcp_cluste
 describe MigrateGcpClustersToNewClustersArchitectures, :migration do
   let(:projects) { table(:projects) }
   let(:project) { projects.create }
-  let(:user) { create(:user) }
-  let(:service) { create(:kubernetes_service, project_id: project.id) }
+  let(:users) { table(:users) }
+  let(:user) { users.create! }
+  let(:service) { GcpMigrationSpec::KubernetesService.create!(project_id: project.id) }
+
+  module GcpMigrationSpec
+    class KubernetesService < ActiveRecord::Base
+      self.table_name = 'services'
+
+      serialize :properties, JSON # rubocop:disable Cop/ActiveRecordSerialize
+
+      default_value_for :active, true
+      default_value_for :type, 'KubernetesService'
+      default_value_for :properties, {
+        api_url: 'https://kubernetes.example.com',
+        token: 'a' * 40
+      }
+    end
+  end
 
   context 'when cluster is being created' do
     let(:project_id) { project.id }

spec/migrations/schedule_set_confidential_note_events_on_webhooks_spec.rb

+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20180104131052_schedule_set_confidential_note_events_on_webhooks.rb')
+
+describe ScheduleSetConfidentialNoteEventsOnWebhooks, :migration, :sidekiq do
+  let(:web_hooks_table) { table(:web_hooks) }
+  let(:migration_class) { Gitlab::BackgroundMigration::SetConfidentialNoteEventsOnWebhooks }
+  let(:migration_name)  { migration_class.to_s.demodulize }
+
+  let!(:web_hook_1)        { web_hooks_table.create!(confidential_note_events: nil, note_events: true) }
+  let!(:web_hook_2)        { web_hooks_table.create!(confidential_note_events: nil, note_events: true) }
+  let!(:web_hook_migrated) { web_hooks_table.create!(confidential_note_events: true, note_events: true) }
+  let!(:web_hook_skip)     { web_hooks_table.create!(confidential_note_events: nil, note_events: false) }
+  let!(:web_hook_new)      { web_hooks_table.create!(confidential_note_events: false, note_events: true) }
+  let!(:web_hook_4)        { web_hooks_table.create!(confidential_note_events: nil, note_events: true) }
+
+  before do
+    stub_const("#{described_class}::BATCH_SIZE", 1)
+  end
+
+  it 'schedules background migrations at correct time' do
+    Sidekiq::Testing.fake! do
+      Timecop.freeze do
+        migrate!
+
+        expect(migration_name).to be_scheduled_delayed_migration(5.minutes, web_hook_1.id, web_hook_1.id)
+        expect(migration_name).to be_scheduled_delayed_migration(10.minutes, web_hook_2.id, web_hook_2.id)
+        expect(migration_name).to be_scheduled_delayed_migration(15.minutes, web_hook_4.id, web_hook_4.id)
+        expect(BackgroundMigrationWorker.jobs.size).to eq 3
+      end
+    end
+  end
+
+  it 'correctly processes web hooks' do
+    Sidekiq::Testing.inline! do
+      expect(web_hooks_table.where(confidential_note_events: nil).count).to eq 4
+      expect(web_hooks_table.where(confidential_note_events: true).count).to eq 1
+
+      migrate!
+
+      expect(web_hooks_table.where(confidential_note_events: nil).count).to eq 1
+      expect(web_hooks_table.where(confidential_note_events: true).count).to eq 4
+    end
+  end
+end

spec/models/note_spec.rb

@@ -191,6 +191,21 @@ describe Note do
     end
   end
 
+  describe "confidential?" do
+    it "delegates to noteable" do
+      issue_note = build(:note, :on_issue)
+      confidential_note = build(:note, noteable: create(:issue, confidential: true))
+
+      expect(issue_note.confidential?).to be_falsy
+      expect(confidential_note.confidential?).to be_truthy
+    end
+
+    it "is falsey when noteable can't be confidential" do
+      commit_note = build(:note_on_commit)
+      expect(commit_note.confidential?).to be_falsy
+    end
+  end
+
   describe "cross_reference_not_visible_for?" do
     let(:private_user)    { create(:user) }
     let(:private_project) { create(:project, namespace: private_user.namespace) { |p| p.add_master(private_user) } }

spec/models/project_services/hipchat_service_spec.rb

@@ -253,6 +253,21 @@ describe HipchatService do
               "<b>#{title}</b>" \
               "<pre>issue <strong>note</strong></pre>")
         end
+
+        context 'with confidential issue' do
+          before do
+            issue.update!(confidential: true)
+          end
+
+          it 'calls Hipchat API with issue comment' do
+            data = Gitlab::DataBuilder::Note.build(issue_note, user)
+            hipchat.execute(data)
+
+            message = hipchat.send(:create_message, data)
+
+            expect(message).to include("<pre>issue <strong>note</strong></pre>")
+          end
+        end
       end
 
       context 'when snippet comment event triggered' do

spec/models/service_spec.rb

@@ -10,6 +10,22 @@ describe Service do
     it { is_expected.to validate_presence_of(:type) }
   end
 
+  describe 'Scopes' do
+    describe '.confidential_note_hooks' do
+      it 'includes services where confidential_note_events is true' do
+        create(:service, active: true, confidential_note_events: true)
+
+        expect(described_class.confidential_note_hooks.count).to eq 1
+      end
+
+      it 'excludes services where confidential_note_events is false' do
+        create(:service, active: true, confidential_note_events: false)
+
+        expect(described_class.confidential_note_hooks.count).to eq 0
+      end
+    end
+  end
+
   describe "Test Button" do
     describe '#can_test?' do
       let(:service) { create(:service, project: project) }

spec/requests/api/project_hooks_spec.rb

@@ -33,6 +33,7 @@ describe API::ProjectHooks, 'ProjectHooks' do
         expect(json_response.first['merge_requests_events']).to eq(true)
         expect(json_response.first['tag_push_events']).to eq(true)
         expect(json_response.first['note_events']).to eq(true)
+        expect(json_response.first['confidential_note_events']).to eq(true)
         expect(json_response.first['job_events']).to eq(true)
         expect(json_response.first['pipeline_events']).to eq(true)
         expect(json_response.first['wiki_page_events']).to eq(true)
@@ -62,6 +63,7 @@ describe API::ProjectHooks, 'ProjectHooks' do
         expect(json_response['merge_requests_events']).to eq(hook.merge_requests_events)
         expect(json_response['tag_push_events']).to eq(hook.tag_push_events)
         expect(json_response['note_events']).to eq(hook.note_events)
+        expect(json_response['confidential_note_events']).to eq(hook.confidential_note_events)
         expect(json_response['job_events']).to eq(hook.job_events)
         expect(json_response['pipeline_events']).to eq(hook.pipeline_events)
         expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events)
@@ -104,6 +106,7 @@ describe API::ProjectHooks, 'ProjectHooks' do
       expect(json_response['merge_requests_events']).to eq(false)
       expect(json_response['tag_push_events']).to eq(false)
       expect(json_response['note_events']).to eq(false)
+      expect(json_response['confidential_note_events']).to eq(nil)
       expect(json_response['job_events']).to eq(true)
       expect(json_response['pipeline_events']).to eq(false)
       expect(json_response['wiki_page_events']).to eq(true)
@@ -152,6 +155,7 @@ describe API::ProjectHooks, 'ProjectHooks' do
       expect(json_response['merge_requests_events']).to eq(hook.merge_requests_events)
       expect(json_response['tag_push_events']).to eq(hook.tag_push_events)
       expect(json_response['note_events']).to eq(hook.note_events)
+      expect(json_response['confidential_note_events']).to eq(hook.confidential_note_events)
       expect(json_response['job_events']).to eq(hook.job_events)
       expect(json_response['pipeline_events']).to eq(hook.pipeline_events)
       expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events)

spec/services/notes/post_process_service_spec.rb

@@ -23,5 +23,23 @@ describe Notes::PostProcessService do
 
       described_class.new(@note).execute
     end
+
+    context 'with a confidential issue' do
+      let(:issue) { create(:issue, :confidential, project: project) }
+
+      it "doesn't call note hooks/services" do
+        expect(project).not_to receive(:execute_hooks).with(anything, :note_hooks)
+        expect(project).not_to receive(:execute_services).with(anything, :note_hooks)
+
+        described_class.new(@note).execute
+      end
+
+      it "calls confidential-note hooks/services" do
+        expect(project).to receive(:execute_hooks).with(anything, :confidential_note_hooks)
+        expect(project).to receive(:execute_services).with(anything, :confidential_note_hooks)
+
+        described_class.new(@note).execute
+      end
+    end
   end
 end

spec/support/slack_mattermost_notifications_shared_examples.rb

@@ -4,6 +4,11 @@ RSpec.shared_examples 'slack or mattermost notifications' do
   let(:chat_service) { described_class.new }
   let(:webhook_url) { 'https://example.gitlab.com/' }
 
+  def execute_with_options(options)
+    receive(:new).with(webhook_url, options)
+     .and_return(double(:slack_service).as_null_object)
+  end
+
   describe "Associations" do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
@@ -33,6 +38,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do
     let(:project) { create(:project, :repository) }
     let(:username) { 'slack_username' }
     let(:channel)  { 'slack_channel' }
+    let(:issue_service_options) { { title: 'Awesome issue', description: 'please fix' } }
 
     let(:push_sample_data) do
       Gitlab::DataBuilder::Push.build_sample(project, user)
@@ -48,12 +54,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do
 
       WebMock.stub_request(:post, webhook_url)
 
-      opts = {
-        title: 'Awesome issue',
-        description: 'please fix'
-      }
-
-      issue_service = Issues::CreateService.new(project, user, opts)
+      issue_service = Issues::CreateService.new(project, user, issue_service_options)
       @issue = issue_service.execute
       @issues_sample_data = issue_service.hook_data(@issue, 'open')
 
@@ -164,6 +165,26 @@ RSpec.shared_examples 'slack or mattermost notifications' do
         chat_service.execute(@issues_sample_data)
       end
 
+      context 'for confidential issues' do
+        let(:issue_service_options) { { title: 'Secret', confidential: true } }
+
+        it "uses confidential issue channel" do
+          chat_service.update_attributes(confidential_issue_channel: 'confidential')
+
+          expect(Slack::Notifier).to execute_with_options(channel: 'confidential')
+
+          chat_service.execute(@issues_sample_data)
+        end
+
+        it 'falls back to issue channel' do
+          chat_service.update_attributes(issue_channel: 'fallback_channel')
+
+          expect(Slack::Notifier).to execute_with_options(channel: 'fallback_channel')
+
+          chat_service.execute(@issues_sample_data)
+        end
+      end
+
       it "uses the right channel for wiki event" do
         chat_service.update_attributes(wiki_page_channel: "random")
 
@@ -194,6 +215,32 @@ RSpec.shared_examples 'slack or mattermost notifications' do
 
           chat_service.execute(note_data)
         end
+
+        context 'for confidential notes' do
+          before do
+            issue_note.noteable.update!(confidential: true)
+          end
+
+          it "uses confidential channel" do
+            chat_service.update_attributes(confidential_note_channel: "confidential")
+
+            note_data = Gitlab::DataBuilder::Note.build(issue_note, user)
+
+            expect(Slack::Notifier).to execute_with_options(channel: 'confidential')
+
+            chat_service.execute(note_data)
+          end
+
+          it 'falls back to note channel' do
+            chat_service.update_attributes(note_channel: "fallback_channel")
+
+            note_data = Gitlab::DataBuilder::Note.build(issue_note, user)
+
+            expect(Slack::Notifier).to execute_with_options(channel: 'fallback_channel')
+
+            chat_service.execute(note_data)
+          end
+        end
       end
     end
   end
@@ -248,8 +295,9 @@ RSpec.shared_examples 'slack or mattermost notifications' do
         create(:note_on_issue, project: project, note: "issue note")
       end
 
+      let(:data) { Gitlab::DataBuilder::Note.build(issue_note, user) }
+
       it "calls Slack API for issue comment events" do
-        data = Gitlab::DataBuilder::Note.build(issue_note, user)
         chat_service.execute(data)
 
         expect(WebMock).to have_requested(:post, webhook_url).once