From 44318222cac65f036b3f66b3ac3ab129b68e27c9 Mon Sep 17 00:00:00 2001
From: Felipe Artur Cardozo <fcardozo@gitlab.com>
Date: Tue, 24 Jul 2018 19:59:33 +0000
Subject: [PATCH] Merge branch 'security-event-counters-private-data-11-1' into
 'security-11-1'

[11.1] Don't expose project names in various counters

See merge request gitlab/gitlabhq!2445
---
 app/models/remote_mirror.rb                                 | 6 +++---
 app/models/repository.rb                                    | 2 +-
 .../concerns/gitlab/github_import/object_importer.rb        | 2 +-
 app/workers/repository_fork_worker.rb                       | 4 +---
 app/workers/repository_import_worker.rb                     | 4 +---
 changelogs/unreleased/event-counters-private-data.yml       | 5 +++++
 changelogs/unreleased/pr-importer-project-name.yml          | 5 +++++
 lib/api/runner.rb                                           | 6 ++----
 lib/gitlab/email/handler/create_issue_handler.rb            | 4 ----
 lib/gitlab/email/handler/create_merge_request_handler.rb    | 4 ----
 lib/gitlab/email/handler/create_note_handler.rb             | 4 ----
 lib/gitlab/email/handler/unsubscribe_handler.rb             | 4 ----
 lib/gitlab/github_import/importer/pull_requests_importer.rb | 2 +-
 .../github_import/importer/pull_requests_importer_spec.rb   | 1 -
 .../concerns/gitlab/github_import/object_importer_spec.rb   | 1 -
 .../gitlab/github_import/import_diff_note_worker_spec.rb    | 1 -
 .../gitlab/github_import/import_issue_worker_spec.rb        | 1 -
 .../workers/gitlab/github_import/import_note_worker_spec.rb | 1 -
 .../gitlab/github_import/import_pull_request_worker_spec.rb | 1 -
 19 files changed, 20 insertions(+), 38 deletions(-)
 create mode 100644 changelogs/unreleased/event-counters-private-data.yml
 create mode 100644 changelogs/unreleased/pr-importer-project-name.yml

diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb
index c4b5dd2dc96..67e05cfaf93 100644
--- a/app/models/remote_mirror.rb
+++ b/app/models/remote_mirror.rb
@@ -48,13 +48,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_attributes!(
@@ -63,7 +63,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
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 7cd600fec5b..8f05376ab9e 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -1032,7 +1032,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
diff --git a/app/workers/concerns/gitlab/github_import/object_importer.rb b/app/workers/concerns/gitlab/github_import/object_importer.rb
index 100d86e38c8..eeeff6e93a0 100644
--- a/app/workers/concerns/gitlab/github_import/object_importer.rb
+++ b/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
diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb
index 5ef9b744db3..68ec66e8499 100644
--- a/app/workers/repository_fork_worker.rb
+++ b/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)
diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb
index 25fec542ac7..8c64c513c74 100644
--- a/app/workers/repository_import_worker.rb
+++ b/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
diff --git a/changelogs/unreleased/event-counters-private-data.yml b/changelogs/unreleased/event-counters-private-data.yml
new file mode 100644
index 00000000000..3dbd8a4ed9c
--- /dev/null
+++ b/changelogs/unreleased/event-counters-private-data.yml
@@ -0,0 +1,5 @@
+---
+title: Don't expose project names in various counters
+merge_request:
+author:
+type: security
diff --git a/changelogs/unreleased/pr-importer-project-name.yml b/changelogs/unreleased/pr-importer-project-name.yml
new file mode 100644
index 00000000000..3b01b048589
--- /dev/null
+++ b/changelogs/unreleased/pr-importer-project-name.yml
@@ -0,0 +1,5 @@
+---
+title: Don't expose project names in GitHub counters
+merge_request:
+author:
+type: security
diff --git a/lib/api/runner.rb b/lib/api/runner.rb
index d0cc0945a5f..06c034444a1 100644
--- a/lib/api/runner.rb
+++ b/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'
diff --git a/lib/gitlab/email/handler/create_issue_handler.rb b/lib/gitlab/email/handler/create_issue_handler.rb
index 764f93f6d3d..fc8615afcae 100644
--- a/lib/gitlab/email/handler/create_issue_handler.rb
+++ b/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
diff --git a/lib/gitlab/email/handler/create_merge_request_handler.rb b/lib/gitlab/email/handler/create_merge_request_handler.rb
index 2f864f2082b..2316e58c3fc 100644
--- a/lib/gitlab/email/handler/create_merge_request_handler.rb
+++ b/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
diff --git a/lib/gitlab/email/handler/create_note_handler.rb b/lib/gitlab/email/handler/create_note_handler.rb
index 5791dbd0484..379b114e957 100644
--- a/lib/gitlab/email/handler/create_note_handler.rb
+++ b/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
diff --git a/lib/gitlab/email/handler/unsubscribe_handler.rb b/lib/gitlab/email/handler/unsubscribe_handler.rb
index ea80e21532e..56751e4e41e 100644
--- a/lib/gitlab/email/handler/unsubscribe_handler.rb
+++ b/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
diff --git a/lib/gitlab/github_import/importer/pull_requests_importer.rb b/lib/gitlab/github_import/importer/pull_requests_importer.rb
index e70361c163b..a52866c4b08 100644
--- a/lib/gitlab/github_import/importer/pull_requests_importer.rb
+++ b/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)
diff --git a/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb
index 51fad6c6838..53c99221fb4 100644
--- a/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb
+++ b/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
diff --git a/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb b/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb
index 615462380e0..9c187bead0a 100644
--- a/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb
+++ b/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 })
diff --git a/spec/workers/gitlab/github_import/import_diff_note_worker_spec.rb b/spec/workers/gitlab/github_import/import_diff_note_worker_spec.rb
index 48e7eaf32fc..5b1c6b6010a 100644
--- a/spec/workers/gitlab/github_import/import_diff_note_worker_spec.rb
+++ b/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)
diff --git a/spec/workers/gitlab/github_import/import_issue_worker_spec.rb b/spec/workers/gitlab/github_import/import_issue_worker_spec.rb
index 8cf6ac15919..ab070d6d081 100644
--- a/spec/workers/gitlab/github_import/import_issue_worker_spec.rb
+++ b/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)
diff --git a/spec/workers/gitlab/github_import/import_note_worker_spec.rb b/spec/workers/gitlab/github_import/import_note_worker_spec.rb
index 677697c02df..3a30f06bb2d 100644
--- a/spec/workers/gitlab/github_import/import_note_worker_spec.rb
+++ b/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)
diff --git a/spec/workers/gitlab/github_import/import_pull_request_worker_spec.rb b/spec/workers/gitlab/github_import/import_pull_request_worker_spec.rb
index e287ddbe0d7..3cccd7cab21 100644
--- a/spec/workers/gitlab/github_import/import_pull_request_worker_spec.rb
+++ b/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)
-- 
2.30.9