Merge branch 'add-rubocop-gitlab-security-ee' into 'master'

Add rubocop-gitlab-security gem

See merge request !2558

.gitlab-ci.yml

@@ -501,8 +501,11 @@ codeclimate:
   services:
     - docker:dind
   script:
+    - cp .rubocop.yml .rubocop.yml.bak
+    - grep -v "rubocop-gitlab-security" .rubocop.yml.bak > .rubocop.yml
     - docker run --env CODECLIMATE_CODE="$PWD" --volume "$PWD":/code --volume /var/run/docker.sock:/var/run/docker.sock --volume /tmp/cc:/tmp/cc codeclimate/codeclimate analyze -f json > raw_codeclimate.json
     - cat raw_codeclimate.json | docker run -i stedolan/jq -c 'map({check_name,fingerprint,location})' > codeclimate.json
+    - mv .rubocop.yml.bak .rubocop.yml
   artifacts:
     paths: [codeclimate.json]
 

.rubocop.yml

 require:
   - rubocop-rspec
+  - rubocop-gitlab-security
   - ./rubocop/rubocop
 
 inherit_from: .rubocop_todo.yml
@@ -1161,3 +1162,35 @@ RSpec/SubjectStub:
 # Prefer using verifying doubles over normal doubles.
 RSpec/VerifiedDoubles:
   Enabled: false
+
+# GitlabSecurity ##############################################################
+
+GitlabSecurity/DeepMunge:
+  Enabled: true
+  Exclude:
+    - 'spec/**/*'
+    - 'lib/**/*.rake'
+
+GitlabSecurity/PublicSend:
+  Enabled: true
+  Exclude:
+    - 'spec/**/*'
+    - 'lib/**/*.rake'
+
+GitlabSecurity/RedirectToParamsUpdate:
+  Enabled: true
+  Exclude:
+    - 'spec/**/*'
+    - 'lib/**/*.rake'
+
+GitlabSecurity/SqlInjection:
+  Enabled: true
+  Exclude:
+    - 'spec/**/*'
+    - 'lib/**/*.rake'
+
+GitlabSecurity/SystemCommandInjection:
+  Enabled: true
+  Exclude:
+    - 'spec/**/*'
+    - 'lib/**/*.rake'

Gemfile

@@ -353,6 +353,7 @@ group :development, :test do
 
   gem 'rubocop', '~> 0.49.1', require: false
   gem 'rubocop-rspec', '~> 1.15.1', require: false
+  gem 'rubocop-gitlab-security', '~> 0.0.6', require: false
   gem 'scss_lint', '~> 0.54.0', require: false
   gem 'haml_lint', '~> 0.26.0', require: false
   gem 'simplecov', '~> 0.14.0', require: false

Gemfile.lock

@@ -770,6 +770,8 @@ GEM
       rainbow (>= 1.99.1, < 3.0)
       ruby-progressbar (~> 1.7)
       unicode-display_width (~> 1.0, >= 1.0.1)
+    rubocop-gitlab-security (0.0.6)
+      rubocop (>= 0.47.0)
     rubocop-rspec (1.15.1)
       rubocop (>= 0.42.0)
     ruby-fogbugz (0.2.1)
@@ -1127,6 +1129,7 @@ DEPENDENCIES
   rspec_profiling (~> 0.0.5)
   rubocop (~> 0.49.1)
   rubocop-rspec (~> 1.15.1)
+  rubocop-gitlab-security (~> 0.0.6)
   ruby-fogbugz (~> 0.2.1)
   ruby-prof (~> 0.16.2)
   ruby_parser (~> 3.8)

app/controllers/import/github_controller.rb

@@ -68,15 +68,15 @@ class Import::GithubController < Import::BaseController
   end
 
   def new_import_url
-    public_send("new_import_#{provider}_url")
+    public_send("new_import_#{provider}_url") # rubocop:disable GitlabSecurity/PublicSend
   end
 
   def status_import_url
-    public_send("status_import_#{provider}_url")
+    public_send("status_import_#{provider}_url") # rubocop:disable GitlabSecurity/PublicSend
   end
 
   def callback_import_url
-    public_send("callback_import_#{provider}_url")
+    public_send("callback_import_#{provider}_url") # rubocop:disable GitlabSecurity/PublicSend
   end
 
   def provider_unauthorized

app/helpers/issuables_helper.rb

@@ -242,7 +242,7 @@ module IssuablesHelper
   end
 
   def issuables_count_for_state(issuable_type, state, finder: nil)
-    finder ||= public_send("#{issuable_type}_finder")
+    finder ||= public_send("#{issuable_type}_finder") # rubocop:disable GitlabSecurity/PublicSend
     cache_key = finder.state_counter_cache_key
 
     @counts ||= {}

app/helpers/labels_helper.rb

@@ -43,11 +43,11 @@ module LabelsHelper
   def label_filter_path(subject, label, type: :issue)
     case subject
     when Group
-      send("#{type.to_s.pluralize}_group_path",
+      send("#{type.to_s.pluralize}_group_path", # rubocop:disable GitlabSecurity/PublicSend
                   subject,
                   label_name: [label.name])
     when Project
-      send("namespace_project_#{type.to_s.pluralize}_path",
+      send("namespace_project_#{type.to_s.pluralize}_path", # rubocop:disable GitlabSecurity/PublicSend
                   subject.namespace,
                   subject,
                   label_name: [label.name])

app/models/concerns/elastic/application_search.rb

@@ -79,7 +79,7 @@ module Elastic
       # them to raise exceptions. When this happens, we still want the remainder
       # of the object to be saved, so silently swallow the errors
       def safely_read_attribute_for_elasticsearch(attr_name)
-        send(attr_name)
+        send(attr_name) # rubocop:disable GitlabSecurity/PublicSend
       rescue => err
         logger.warn("Elasticsearch failed to read #{attr_name} for #{self.class} #{self.id}: #{err}")
         nil

app/models/concerns/spammable.rb

@@ -58,7 +58,7 @@ module Spammable
       options.fetch(:spam_title, false)
     end
 
-    public_send(attr.first) if attr && respond_to?(attr.first.to_sym)
+    public_send(attr.first) if attr && respond_to?(attr.first.to_sym) # rubocop:disable GitlabSecurity/PublicSend
   end
 
   def spam_description
@@ -66,12 +66,12 @@ module Spammable
       options.fetch(:spam_description, false)
     end
 
-    public_send(attr.first) if attr && respond_to?(attr.first.to_sym)
+    public_send(attr.first) if attr && respond_to?(attr.first.to_sym) # rubocop:disable GitlabSecurity/PublicSend
   end
 
   def spammable_text
     result = self.class.spammable_attrs.map do |attr|
-      public_send(attr.first)
+      public_send(attr.first) # rubocop:disable GitlabSecurity/PublicSend
     end
 
     result.reject(&:blank?).join("\n")

app/models/concerns/token_authenticatable.rb

@@ -44,7 +44,8 @@ module TokenAuthenticatable
       end
 
       define_method("ensure_#{token_field}!") do
-        send("reset_#{token_field}!") if read_attribute(token_field).blank?
+        send("reset_#{token_field}!") if read_attribute(token_field).blank? # rubocop:disable GitlabSecurity/PublicSend
+
         read_attribute(token_field)
       end
 

app/models/merge_request.rb

@@ -166,7 +166,7 @@ class MergeRequest < ActiveRecord::Base
     target = unscoped.where(target_project_id: relation).select(:id)
     union  = Gitlab::SQL::Union.new([source, target])
 
-    where("merge_requests.id IN (#{union.to_sql})")
+    where("merge_requests.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
   end
 
   WIP_REGEX = /\A\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i.freeze

app/models/merge_request_diff_commit.rb

@@ -26,7 +26,7 @@ class MergeRequestDiffCommit < ActiveRecord::Base
 
   def to_hash
     Gitlab::Git::Commit::SERIALIZE_KEYS.each_with_object({}) do |key, hash|
-      hash[key] = public_send(key)
+      hash[key] = public_send(key) # rubocop:disable GitlabSecurity/PublicSend
     end
   end
 

app/models/notification_setting.rb

@@ -66,6 +66,6 @@ class NotificationSetting < ActiveRecord::Base
   alias_method :failed_pipeline?, :failed_pipeline
 
   def event_enabled?(event)
-    respond_to?(event) && !!public_send(event)
+    respond_to?(event) && !!public_send(event) # rubocop:disable GitlabSecurity/PublicSend
   end
 end

app/models/project.rb

@@ -413,7 +413,7 @@ class Project < ActiveRecord::Base
 
       union = Gitlab::SQL::Union.new([projects, namespaces])
 
-      where("projects.id IN (#{union.to_sql})")
+      where("projects.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
     end
 
     def search_by_title(query)
@@ -823,7 +823,7 @@ class Project < ActiveRecord::Base
 
         if template.nil?
           # If no template, we should create an instance. Ex `build_gitlab_ci_service`
-          public_send("build_#{service_name}_service")
+          public_send("build_#{service_name}_service") # rubocop:disable GitlabSecurity/PublicSend
         else
           Service.build_from_template(id, template)
         end
@@ -1325,7 +1325,7 @@ class Project < ActiveRecord::Base
   end
 
   def append_or_update_attribute(name, value)
-    old_values = public_send(name.to_s)
+    old_values = public_send(name.to_s) # rubocop:disable GitlabSecurity/PublicSend
 
     if Project.reflect_on_association(name).try(:macro) == :has_many && old_values.any?
       update_attribute(name, old_values + value)

app/models/project_feature.rb

@@ -61,7 +61,7 @@ class ProjectFeature < ActiveRecord::Base
   end
 
   def access_level(feature)
-    public_send(ProjectFeature.access_level_attribute(feature))
+    public_send(ProjectFeature.access_level_attribute(feature)) # rubocop:disable GitlabSecurity/PublicSend
   end
 
   def builds_enabled?
@@ -86,7 +86,7 @@ class ProjectFeature < ActiveRecord::Base
   # which cannot be higher than repository access level
   def repository_children_level
     validator = lambda do |field|
-      level = public_send(field) || ProjectFeature::ENABLED
+      level = public_send(field) || ProjectFeature::ENABLED # rubocop:disable GitlabSecurity/PublicSend
       not_allowed = level > repository_access_level
       self.errors.add(field, "cannot have higher visibility level than repository access level") if not_allowed
     end

app/models/project_statistics.rb

@@ -18,7 +18,7 @@ class ProjectStatistics < ActiveRecord::Base
   def refresh!(only: nil)
     STATISTICS_COLUMNS.each do |column, generator|
       if only.blank? || only.include?(column)
-        public_send("update_#{column}")
+        public_send("update_#{column}") # rubocop:disable GitlabSecurity/PublicSend
       end
     end
 

app/models/repository.rb

@@ -311,7 +311,7 @@ class Repository
 
     expire_method_caches(to_refresh)
 
-    to_refresh.each { |method| send(method) }
+    to_refresh.each { |method| send(method) } # rubocop:disable GitlabSecurity/PublicSend
   end
 
   def expire_branch_cache(branch_name = nil)

app/models/user.rb

@@ -547,7 +547,7 @@ class User < ActiveRecord::Base
     union = Gitlab::SQL::Union
       .new([groups.select(:id), authorized_projects.select(:namespace_id)])
 
-    Group.where("namespaces.id IN (#{union.to_sql})")
+    Group.where("namespaces.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
   end
 
   # Returns a relation of groups the user has access to, including their parent
@@ -736,8 +736,8 @@ class User < ActiveRecord::Base
 
   def sanitize_attrs
     %w[username skype linkedin twitter].each do |attr|
-      value = public_send(attr)
-      public_send("#{attr}=", Sanitize.clean(value)) if value.present?
+      value = public_send(attr) # rubocop:disable GitlabSecurity/PublicSend
+      public_send("#{attr}=", Sanitize.clean(value)) if value.present? # rubocop:disable GitlabSecurity/PublicSend
     end
   end
 
@@ -796,7 +796,7 @@ class User < ActiveRecord::Base
 
   def with_defaults
     User.defaults.each do |k, v|
-      public_send("#{k}=", v)
+      public_send("#{k}=", v) # rubocop:disable GitlabSecurity/PublicSend
     end
 
     self
@@ -940,7 +940,7 @@ class User < ActiveRecord::Base
   def ci_authorized_runners
     @ci_authorized_runners ||= begin
       runner_ids = Ci::RunnerProject
-        .where("ci_runner_projects.project_id IN (#{ci_projects_union.to_sql})")
+        .where("ci_runner_projects.project_id IN (#{ci_projects_union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
         .select(:runner_id)
       Ci::Runner.specific.where(id: runner_ids)
     end

app/serializers/analytics_build_entity.rb

@@ -35,6 +35,6 @@ class AnalyticsBuildEntity < Grape::Entity
   private
 
   def url_to(route, build, id = nil)
-    public_send("#{route}_url", build.project.namespace, build.project, id || build)
+    public_send("#{route}_url", build.project.namespace, build.project, id || build) # rubocop:disable GitlabSecurity/PublicSend
   end
 end

app/serializers/analytics_issue_entity.rb

@@ -24,6 +24,6 @@ class AnalyticsIssueEntity < Grape::Entity
   private
 
   def url_to(route, id)
-    public_send("#{route}_url", request.project.namespace, request.project, id)
+    public_send("#{route}_url", request.project.namespace, request.project, id) # rubocop:disable GitlabSecurity/PublicSend
   end
 end

app/serializers/job_entity.rb

@@ -46,6 +46,6 @@ class JobEntity < Grape::Entity
   end
 
   def path_to(route, build)
-    send("#{route}_path", build.project.namespace, build.project, build)
+    send("#{route}_path", build.project.namespace, build.project, build) # rubocop:disable GitlabSecurity/PublicSend
   end
 end

app/services/labels/transfer_service.rb

@@ -37,7 +37,7 @@ module Labels
 
       union = Gitlab::SQL::Union.new(label_ids)
 
-      Label.where("labels.id IN (#{union.to_sql})").reorder(nil).uniq
+      Label.where("labels.id IN (#{union.to_sql})").reorder(nil).uniq # rubocop:disable GitlabSecurity/SqlInjection
     end
 
     def group_labels_applied_to_issues

app/services/projects/update_remote_mirror_service.rb

@@ -119,8 +119,8 @@ module Projects
     end
 
     def refs_to_delete(type)
-      remote_refs       = send("remote_#{type}")
-      local_refs        = send("local_#{type}")
+      remote_refs       = send("remote_#{type}") # rubocop:disable GitlabSecurity/PublicSend
+      local_refs        = send("local_#{type}") # rubocop:disable GitlabSecurity/PublicSend
       default_branch_id = project.commit.id
 
       remote_refs.each_with_object([]) do |(name, remote_ref), refs_to_delete|

app/workers/pages_worker.rb

@@ -4,7 +4,7 @@ class PagesWorker
   sidekiq_options queue: :pages, retry: false
 
   def perform(action, *arg)
-    send(action, *arg)
+    send(action, *arg) # rubocop:disable GitlabSecurity/PublicSend
   end
 
   def deploy(build_id)

config/application.rb

@@ -199,7 +199,7 @@ module Gitlab
           next unless name.include?('namespace_project')
 
           define_method(name.sub('namespace_project', 'project')) do |project, *args|
-            send(name, project&.namespace, project, *args)
+            send(name, project&.namespace, project, *args) # rubocop:disable GitlabSecurity/PublicSend
           end
         end
       end

config/initializers/active_record_locking.rb

@@ -18,7 +18,7 @@ module ActiveRecord
 
         lock_col = self.class.locking_column
 
-        previous_lock_value = send(lock_col).to_i
+        previous_lock_value = send(lock_col).to_i # rubocop:disable GitlabSecurity/PublicSend
 
         # This line is added as a patch
         previous_lock_value = nil if previous_lock_value == '0' || previous_lock_value == 0
@@ -48,7 +48,7 @@ module ActiveRecord
 
         # If something went wrong, revert the version.
         rescue Exception
-          send(lock_col + '=', previous_lock_value)
+          send(lock_col + '=', previous_lock_value)  # rubocop:disable GitlabSecurity/PublicSend
           raise
         end
       end

ee/app/services/ee/ci/register_job_service.rb

@@ -12,7 +12,7 @@ module EE
 
         # select projects which have allowed number of shared runner minutes or are public
         super
-          .where("projects.visibility_level=? OR (#{builds_check_limit.to_sql})=1",
+          .where("projects.visibility_level=? OR (#{builds_check_limit.to_sql})=1",  # rubocop:disable GitlabSecurity/SqlInjection
                 ::Gitlab::VisibilityLevel::PUBLIC)
       end
 

lib/api/helpers/members_helpers.rb

+# rubocop:disable GitlabSecurity/PublicSend
+
 module API
   module Helpers
     module MembersHelpers
       def find_source(source_type, id)
-        public_send("find_#{source_type}!", id)
+        public_send("find_#{source_type}!", id) # rubocop:disable GitlabSecurity/PublicSend
       end
 
       def authorize_admin_source!(source_type, source)

lib/api/notes.rb

@@ -139,7 +139,7 @@ module API
 
     helpers do
       def find_project_noteable(noteables_str, noteable_id)
-        public_send("find_project_#{noteables_str.singularize}", noteable_id)
+        public_send("find_project_#{noteables_str.singularize}", noteable_id) # rubocop:disable GitlabSecurity/PublicSend
       end
 
       def noteable_read_ability_name(noteable)

lib/ci/ansi2html.rb

@@ -254,7 +254,7 @@ module Ci
 
       def state
         state = STATE_PARAMS.inject({}) do |h, param|
-          h[param] = send(param)
+          h[param] = send(param) # rubocop:disable GitlabSecurity/PublicSend
           h
         end
         Base64.urlsafe_encode64(state.to_json)
@@ -266,7 +266,7 @@ module Ci
         return if state[:offset].to_i > stream.size
 
         STATE_PARAMS.each do |param|
-          send("#{param}=".to_sym, state[param])
+          send("#{param}=".to_sym, state[param]) # rubocop:disable GitlabSecurity/PublicSend
         end
       end
 

lib/ci/charts.rb

@@ -47,7 +47,7 @@ module Ci
 
       def collect
         query = project.pipelines
-          .where("? > #{Ci::Pipeline.table_name}.created_at AND #{Ci::Pipeline.table_name}.created_at > ?", @to, @from)
+          .where("? > #{Ci::Pipeline.table_name}.created_at AND #{Ci::Pipeline.table_name}.created_at > ?", @to, @from) # rubocop:disable GitlabSecurity/SqlInjection
 
         totals_count  = grouped_count(query)
         success_count = grouped_count(query.success)

lib/gitlab/diff/line.rb

@@ -21,7 +21,7 @@ module Gitlab
 
       def to_hash
         hash = {}
-        serialize_keys.each { |key| hash[key] = send(key) }
+        serialize_keys.each { |key| hash[key] = send(key) } # rubocop:disable GitlabSecurity/PublicSend
         hash
       end
 

lib/gitlab/git/commit.rb

@@ -319,7 +319,7 @@ module Gitlab
 
       def to_hash
         serialize_keys.map.with_object({}) do |key, hash|
-          hash[key] = send(key)
+          hash[key] = send(key) # rubocop:disable GitlabSecurity/PublicSend
         end
       end
 
@@ -412,7 +412,7 @@ module Gitlab
         raw_commit = hash.symbolize_keys
 
         serialize_keys.each do |key|
-          send("#{key}=", raw_commit[key])
+          send("#{key}=", raw_commit[key]) # rubocop:disable GitlabSecurity/PublicSend
         end
       end
 

lib/gitlab/git/diff.rb

@@ -143,7 +143,7 @@ module Gitlab
         hash = {}
 
         SERIALIZE_KEYS.each do |key|
-          hash[key] = send(key)
+          hash[key] = send(key) # rubocop:disable GitlabSecurity/PublicSend
         end
 
         hash
@@ -221,7 +221,7 @@ module Gitlab
         raw_diff = hash.symbolize_keys
 
         SERIALIZE_KEYS.each do |key|
-          send(:"#{key}=", raw_diff[key.to_sym])
+          send(:"#{key}=", raw_diff[key.to_sym]) # rubocop:disable GitlabSecurity/PublicSend
         end
       end
 

lib/gitlab/gitaly_client/diff.rb

@@ -7,13 +7,13 @@ module Gitlab
 
       def initialize(params)
         params.each do |key, val|
-          public_send(:"#{key}=", val)
+          public_send(:"#{key}=", val) # rubocop:disable GitlabSecurity/PublicSend
         end
       end
 
       def ==(other)
         FIELDS.all? do |field|
-          public_send(field) == other.public_send(field)
+          public_send(field) == other.public_send(field) # rubocop:disable GitlabSecurity/PublicSend
         end
       end
     end

lib/gitlab/reference_extractor.rb

@@ -45,7 +45,7 @@ module Gitlab
     end
 
     def all
-      REFERABLES.each { |referable| send(referable.to_s.pluralize) }
+      REFERABLES.each { |referable| send(referable.to_s.pluralize) } # rubocop:disable GitlabSecurity/PublicSend
       @references.values.flatten
     end
 

lib/static_model.rb

@@ -18,7 +18,7 @@ module StaticModel
   #
   # Pass it along if we respond to it.
   def [](key)
-    send(key) if respond_to?(key)
+    send(key) if respond_to?(key) # rubocop:disable GitlabSecurity/PublicSend
   end
 
   def to_param