Merge remote-tracking branch 'dev/master'

CHANGELOG-EE.md

 Please view this file on the master branch, on stable branches it's out of date.
 
+## 11.1.2 (2018-07-26)
+
+### Security (1 change)
+
+- Don't expose project names in EE counters.
+
+
 ## 11.1.1 (2018-07-23)
 
 ### Fixed (2 changes)
@@ -75,6 +82,13 @@ Please view this file on the master branch, on stable branches it's out of date.
 - Geo - Make Geo repository verification flag opt-out by default. !6369
 
 
+## 11.0.5 (2018-07-26)
+
+### Security (1 change)
+
+- Don't expose project names in EE counters.
+
+
 ## 11.0.4 (2018-07-17)
 
 - No changes.

CHANGELOG.md

@@ -2,6 +2,24 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 11.1.2 (2018-07-26)
+
+### Security (4 changes)
+
+- Adding CSRF protection to Hooks test action.
+- Don't expose project names in GitHub counters.
+- Don't expose project names in various counters.
+- Fixed XSS in branch name in Web IDE.
+
+### Fixed (1 change)
+
+- Escapes milestone and label's names on flash notice when promoting them.
+
+### Performance (1 change)
+
+- Fix slow Markdown rendering. !20820
+
+
 ## 11.1.1 (2018-07-23)
 
 ### Fixed (2 changes)
@@ -253,6 +271,20 @@ entry.
 - Use monospaced font for MR diff commit link ref on GFM.
 
 
+## 11.0.5 (2018-07-26)
+
+### Security (4 changes)
+
+- Don't expose project names in various counters.
+- Don't expose project names in GitHub counters.
+- Adding CSRF protection to Hooks test action.
+- Fixed XSS in branch name in Web IDE.
+
+### Fixed (1 change)
+
+- Escapes milestone and label's names on flash notice when promoting them.
+
+
 ## 11.0.4 (2018-07-17)
 
 ### Security (1 change)

app/helpers/hooks_helper.rb

@@ -10,7 +10,7 @@ module HooksHelper
 
     trigger_human_name = trigger.to_s.tr('_', ' ').camelize
 
-    link_to path, rel: 'nofollow' do
+    link_to path, rel: 'nofollow', method: :post do
       content_tag(:span, trigger_human_name)
     end
   end

app/models/remote_mirror.rb

@@ -50,13 +50,13 @@ class RemoteMirror < ActiveRecord::Base
     state :failed
 
     after_transition any => :started do |remote_mirror, _|
-      Gitlab::Metrics.add_event(:remote_mirrors_running, path: remote_mirror.project.full_path)
+      Gitlab::Metrics.add_event(:remote_mirrors_running)
 
       remote_mirror.update(last_update_started_at: Time.now)
     end
 
     after_transition started: :finished do |remote_mirror, _|
-      Gitlab::Metrics.add_event(:remote_mirrors_finished, path: remote_mirror.project.full_path)
+      Gitlab::Metrics.add_event(:remote_mirrors_finished)
 
       timestamp = Time.now
       remote_mirror.update!(
@@ -65,7 +65,7 @@ class RemoteMirror < ActiveRecord::Base
     end
 
     after_transition started: :failed do |remote_mirror, _|
-      Gitlab::Metrics.add_event(:remote_mirrors_failed, path: remote_mirror.project.full_path)
+      Gitlab::Metrics.add_event(:remote_mirrors_failed)
 
       remote_mirror.update(last_update_at: Time.now)
     end

app/models/repository.rb

@@ -1034,7 +1034,7 @@ class Repository
   end
 
   def repository_event(event, tags = {})
-    Gitlab::Metrics.add_event(event, { path: full_path }.merge(tags))
+    Gitlab::Metrics.add_event(event, tags)
   end
 
   def initialize_raw_repository

app/workers/concerns/gitlab/github_import/object_importer.rb

@@ -22,7 +22,7 @@ module Gitlab
 
         importer_class.new(object, project, client).execute
 
-        counter.increment(project: project.full_path)
+        counter.increment
       end
 
       def counter

app/workers/repository_fork_worker.rb

@@ -23,9 +23,7 @@ class RepositoryForkWorker
   def fork_repository(target_project, source_repository_storage_name, source_disk_path)
     return unless start_fork(target_project)
 
-    Gitlab::Metrics.add_event(:fork_repository,
-                              source_path: source_disk_path,
-                              target_path: target_project.disk_path)
+    Gitlab::Metrics.add_event(:fork_repository)
 
     result = gitlab_shell.fork_repository(source_repository_storage_name, source_disk_path,
                                           target_project.repository_storage, target_project.disk_path)

app/workers/repository_import_worker.rb

@@ -11,9 +11,7 @@ class RepositoryImportWorker
 
     return unless start_import(project)
 
-    Gitlab::Metrics.add_event(:import_repository,
-                              import_url: project.import_url,
-                              path: project.full_path)
+    Gitlab::Metrics.add_event(:import_repository)
 
     service = Projects::ImportService.new(project, project.creator)
     result = service.execute

changelogs/unreleased/event-counters-private-data.yml

+---
+title: Don't expose project names in various counters
+merge_request:
+author:
+type: security

changelogs/unreleased/pr-importer-project-name.yml

+---
+title: Don't expose project names in GitHub counters
+merge_request:
+author:
+type: security

changelogs/unreleased/security-fj-missing-csrf-system-hooks.yml

+---
+title: Adding CSRF protection to Hooks test action
+merge_request:
+author:
+type: security

changelogs/unreleased/sh-revert-markdown-changes.yml

----
-title: Fix slow Markdown rendering
-merge_request: 20820
-author:
-type: performance

config/routes/admin.rb

@@ -60,7 +60,7 @@ namespace :admin do
 
   resources :hooks, only: [:index, :create, :edit, :update, :destroy] do
     member do
-      get :test
+      post :test
     end
 
     resources :hook_logs, only: [:show] do

config/routes/group.rb

@@ -83,7 +83,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
 
     resources :hooks, only: [:index, :create, :destroy], constraints: { id: /\d+/ } do
       member do
-        get :test
+        post :test
       end
     end
 

config/routes/project.rb

@@ -343,7 +343,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
 
       resources :hooks, only: [:index, :create, :edit, :update, :destroy], constraints: { id: /\d+/ } do
         member do
-          get :test
+          post :test
         end
 
         resources :hook_logs, only: [:show] do

ee/app/models/ee/project.rb

@@ -292,7 +292,7 @@ module EE
       if import? && !repository_exists?
         super
       elsif mirror?
-        ::Gitlab::Metrics.add_event(:mirrors_scheduled, path: full_path)
+        ::Gitlab::Metrics.add_event(:mirrors_scheduled)
         job_id = RepositoryUpdateMirrorWorker.perform_async(self.id)
 
         log_import_activity(job_id, type: :mirror)

ee/app/views/groups/hooks/_project_hook.html.haml

@@ -9,7 +9,7 @@
     .col-md-4.col-lg-5.text-right-lg.prepend-top-5
       %span.append-right-10.inline
         SSL Verification: #{hook.enable_ssl_verification ? "enabled" : "disabled"}
-      = link_to "Test", test_group_hook_path(@group, hook), class: "btn btn-sm"
+      = link_to "Test", test_group_hook_path(@group, hook), class: "btn btn-sm", method: :post
       = link_to group_hook_path(@group, hook), data: { confirm: 'Are you sure?'}, method: :delete, class: "btn btn-transparent" do
         %span.sr-only Remove
         = icon('trash')

ee/app/workers/repository_update_mirror_worker.rb

@@ -65,7 +65,7 @@ class RepositoryUpdateMirrorWorker
     project.mark_import_as_failed(message)
 
     Rails.logger.error("Mirror update for #{project.full_path} failed with the following message: #{message}")
-    Gitlab::Metrics.add_event(:mirrors_failed, path: project.full_path)
+    Gitlab::Metrics.add_event(:mirrors_failed)
   end
 
   def finish_mirror(project)
@@ -74,8 +74,7 @@ class RepositoryUpdateMirrorWorker
     Rails.logger.info("Mirror update for #{project.full_path} successfully finished. Update duration: #{project.mirror_update_duration}}.")
     Gitlab::Metrics.add_event_with_values(
       :mirrors_finished,
-      { duration: project.mirror_update_duration },
-      { path: project.full_path })
+      { duration: project.mirror_update_duration })
 
     metric_mirror_update_duration_seconds.observe({}, project.mirror_update_duration)
   end

ee/changelogs/unreleased/ee-event-counters-private-data.yml

+---
+title: Don't expose project names in EE counters
+merge_request:
+author:
+type: security

ee/spec/features/groups/hooks/user_tests_hooks_spec.rb

-require "spec_helper"
+require "rails_helper"
 
-describe "User tests hooks" do
-  set(:group) { create(:group) }
-  set(:hook) { create(:group_hook, group: group) }
-  set(:user) { create(:user) }
+describe "User tests hooks", :js do
+  let!(:group) { create(:group) }
+  let!(:hook) { create(:group_hook, group: group) }
+  let!(:user) { create(:user) }
 
   before do
     group.add_owner(user)

ee/spec/routing/webhook_routes_spec.rb

@@ -7,7 +7,7 @@ describe "routes to the proper webhooks controller", type: :routing do
 
     it "routes the test action" do
       expect(
-        get: polymorphic_path([project.namespace.becomes(Namespace), project, project_hook], action: :test)
+        post: polymorphic_path([project.namespace.becomes(Namespace), project, project_hook], action: :test)
       ).to route_to(controller: 'projects/hooks',
                     action: 'test',
                     namespace_id: project.namespace.name,
@@ -32,7 +32,7 @@ describe "routes to the proper webhooks controller", type: :routing do
 
     it "routes the test action" do
       expect(
-        get: polymorphic_path([group, group_hook], action: :test)
+        post: polymorphic_path([group, group_hook], action: :test)
       ).to route_to(controller: 'groups/hooks',
                     action: 'test',
                     group_id: group.name,

lib/api/runner.rb

@@ -108,8 +108,7 @@ module API
 
         if result.valid?
           if result.build
-            Gitlab::Metrics.add_event(:build_found,
-                                      project: result.build.project.full_path)
+            Gitlab::Metrics.add_event(:build_found)
             present result.build, with: Entities::JobRequest::Response
           else
             Gitlab::Metrics.add_event(:build_not_found)
@@ -140,8 +139,7 @@ module API
 
         job.trace.set(params[:trace]) if params[:trace]
 
-        Gitlab::Metrics.add_event(:update_build,
-                                  project: job.project.full_path)
+        Gitlab::Metrics.add_event(:update_build)
 
         case params[:state].to_s
         when 'running'

lib/gitlab/email/handler/create_issue_handler.rb

@@ -36,10 +36,6 @@ module Gitlab
           @project ||= Project.find_by_full_path(project_path)
         end
 
-        def metrics_params
-          super.merge(project: project&.full_path)
-        end
-
         private
 
         def create_issue

lib/gitlab/email/handler/create_merge_request_handler.rb

@@ -40,10 +40,6 @@ module Gitlab
           @project ||= Project.find_by_full_path(project_path)
         end
 
-        def metrics_params
-          super.merge(project: project&.full_path)
-        end
-
         private
 
         def create_merge_request

lib/gitlab/email/handler/create_note_handler.rb

@@ -28,10 +28,6 @@ module Gitlab
             record_name: 'comment')
         end
 
-        def metrics_params
-          super.merge(project: project&.full_path)
-        end
-
         private
 
         def author

lib/gitlab/email/handler/unsubscribe_handler.rb

@@ -20,10 +20,6 @@ module Gitlab
           noteable.unsubscribe(sent_notification.recipient)
         end
 
-        def metrics_params
-          super.merge(project: project&.full_path)
-        end
-
         private
 
         def sent_notification

lib/gitlab/github_import/importer/pull_requests_importer.rb

@@ -43,7 +43,7 @@ module Gitlab
           Rails.logger
             .info("GitHub importer finished updating repository for #{pname}")
 
-          repository_updates_counter.increment(project: pname)
+          repository_updates_counter.increment
         end
 
         def update_repository?(pr)

spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb

@@ -158,7 +158,6 @@ describe Gitlab::GithubImport::Importer::PullRequestsImporter do
 
       expect(importer.repository_updates_counter)
         .to receive(:increment)
-        .with(project: project.path_with_namespace)
         .and_call_original
 
       Timecop.freeze do

spec/routing/admin_routing_spec.rb

@@ -79,7 +79,7 @@ end
 # edit_admin_hook GET    /admin/hooks/:id(.:format)           admin/hooks#edit
 describe Admin::HooksController, "routing" do
   it "to #test" do
-    expect(get("/admin/hooks/1/test")).to route_to('admin/hooks#test', id: '1')
+    expect(post("/admin/hooks/1/test")).to route_to('admin/hooks#test', id: '1')
   end
 
   it "to #index" do

spec/routing/project_routing_spec.rb

@@ -389,7 +389,7 @@ describe 'project routing' do
   #                   DELETE /:project_id/hooks/:id(.:format)      hooks#destroy
   describe Projects::HooksController, 'routing' do
     it 'to #test' do
-      expect(get('/gitlab/gitlabhq/hooks/1/test')).to route_to('projects/hooks#test', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1')
+      expect(post('/gitlab/gitlabhq/hooks/1/test')).to route_to('projects/hooks#test', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1')
     end
 
     it_behaves_like 'RESTful project resources' do

spec/workers/concerns/gitlab/github_import/object_importer_spec.rb

@@ -51,7 +51,6 @@ describe Gitlab::GithubImport::ObjectImporter do
 
       expect(worker.counter)
         .to receive(:increment)
-        .with(project: 'foo/bar')
         .and_call_original
 
       worker.import(project, client, { 'number' => 10 })

spec/workers/gitlab/github_import/import_diff_note_worker_spec.rb

@@ -33,7 +33,6 @@ describe Gitlab::GithubImport::ImportDiffNoteWorker do
 
       expect(worker.counter)
         .to receive(:increment)
-        .with(project: 'foo/bar')
         .and_call_original
 
       worker.import(project, client, hash)

spec/workers/gitlab/github_import/import_issue_worker_spec.rb

@@ -36,7 +36,6 @@ describe Gitlab::GithubImport::ImportIssueWorker do
 
       expect(worker.counter)
         .to receive(:increment)
-        .with(project: 'foo/bar')
         .and_call_original
 
       worker.import(project, client, hash)

spec/workers/gitlab/github_import/import_note_worker_spec.rb

@@ -31,7 +31,6 @@ describe Gitlab::GithubImport::ImportNoteWorker do
 
       expect(worker.counter)
         .to receive(:increment)
-        .with(project: 'foo/bar')
         .and_call_original
 
       worker.import(project, client, hash)

spec/workers/gitlab/github_import/import_pull_request_worker_spec.rb

@@ -42,7 +42,6 @@ describe Gitlab::GithubImport::ImportPullRequestWorker do
 
       expect(worker.counter)
         .to receive(:increment)
-        .with(project: 'foo/bar')
         .and_call_original
 
       worker.import(project, client, hash)