diff options
Diffstat (limited to 'rubocop')
| -rw-r--r-- | rubocop/cop/file_decompression.rb | 45 | ||||
| -rw-r--r-- | rubocop/cop/gitlab/event_store_subscriber.rb | 71 | ||||
| -rw-r--r-- | rubocop/cop/gitlab/mark_used_feature_flags.rb | 4 | ||||
| -rw-r--r-- | rubocop/cop/migration/schedule_async.rb | 13 | ||||
| -rw-r--r-- | rubocop/cop/scalability/bulk_perform_with_context.rb | 8 | ||||
| -rw-r--r-- | rubocop/cop/scalability/cron_worker_context.rb | 2 | ||||
| -rw-r--r-- | rubocop/cop/sidekiq_load_balancing/worker_data_consistency_with_deduplication.rb | 154 |
7 files changed, 138 insertions, 159 deletions
diff --git a/rubocop/cop/file_decompression.rb b/rubocop/cop/file_decompression.rb new file mode 100644 index 00000000000..44813244028 --- /dev/null +++ b/rubocop/cop/file_decompression.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # Check for symlinks when extracting files to avoid arbitrary file reading. + class FileDecompression < RuboCop::Cop::Cop + MSG = <<~EOF + While extracting files check for symlink to avoid arbitrary file reading. + https://gitlab.com/gitlab-com/gl-infra/production/-/issues/6132 + EOF + + def_node_matcher :system?, <<~PATTERN + (send {nil? | const} {:system | :exec | :spawn | :popen} + (str $_)) + PATTERN + + def_node_matcher :subshell?, <<~PATTERN + (xstr + (str $_)) + PATTERN + + FORBIDDEN_COMMANDS = %w[gunzip gzip zip tar].freeze + + def on_xstr(node) + subshell?(node) do |match| + add_offense(node, message: MSG) if forbidden_command?(match) + end + end + + def on_send(node) + system?(node) do |match| + add_offense(node, location: :expression, message: MSG) if forbidden_command?(match) + end + end + + private + + def forbidden_command?(cmd) + FORBIDDEN_COMMANDS.any? do |forbidden| + cmd.match?(forbidden) + end + end + end + end +end diff --git a/rubocop/cop/gitlab/event_store_subscriber.rb b/rubocop/cop/gitlab/event_store_subscriber.rb new file mode 100644 index 00000000000..0b2dd94bc42 --- /dev/null +++ b/rubocop/cop/gitlab/event_store_subscriber.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # Cop that checks the implementation of Gitlab::EventStore::Subscriber + # + # A worker that implements Gitlab::EventStore::Subscriber + # must implement the method #handle_event(event) and + # must not override the method #perform(*args) + # + # # bad + # class MySubscriber + # include Gitlab::EventStore::Subscriber + # + # def perform(*args) + # end + # end + # + # # bad + # class MySubscriber + # include Gitlab::EventStore::Subscriber + # end + # + # # good + # class MySubscriber + # include Gitlab::EventStore::Subscriber + # + # def handle_event(event) + # end + # end + # + class EventStoreSubscriber < RuboCop::Cop::Cop + SUBSCRIBER_MODULE_NAME = 'Gitlab::EventStore::Subscriber' + FORBID_PERFORM_OVERRIDE = "Do not override `perform` in a `#{SUBSCRIBER_MODULE_NAME}`." + REQUIRE_HANDLE_EVENT = "A `#{SUBSCRIBER_MODULE_NAME}` must implement `#handle_event(event)`." + + def_node_matcher :includes_subscriber?, <<~PATTERN + (send nil? :include (const (const (const nil? :Gitlab) :EventStore) :Subscriber)) + PATTERN + + def on_send(node) + return unless includes_subscriber?(node) + + self.is_subscriber ||= true + self.include_subscriber_node ||= node + end + + def on_def(node) + if is_subscriber && node.method_name == :perform + add_offense(node, message: FORBID_PERFORM_OVERRIDE) + end + + self.implements_handle_event ||= true if node.method_name == :handle_event + end + + def on_investigation_end + super + + if is_subscriber && !implements_handle_event + add_offense(include_subscriber_node, message: REQUIRE_HANDLE_EVENT) + end + end + + private + + attr_accessor :is_subscriber, :include_subscriber_node, :implements_handle_event + end + end + end +end diff --git a/rubocop/cop/gitlab/mark_used_feature_flags.rb b/rubocop/cop/gitlab/mark_used_feature_flags.rb index 4d9fc6148fa..7e01c2765c9 100644 --- a/rubocop/cop/gitlab/mark_used_feature_flags.rb +++ b/rubocop/cop/gitlab/mark_used_feature_flags.rb @@ -44,7 +44,9 @@ module RuboCop DYNAMIC_FEATURE_FLAGS = [ :usage_data_static_site_editor_commits, # https://gitlab.com/gitlab-org/gitlab/-/issues/284082 - :usage_data_static_site_editor_merge_requests # https://gitlab.com/gitlab-org/gitlab/-/issues/284083 + :usage_data_static_site_editor_merge_requests, # https://gitlab.com/gitlab-org/gitlab/-/issues/284083 + :usage_data_users_clicking_license_testing_visiting_external_website, # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77866 + :usage_data_users_visiting_testing_license_compliance_full_report # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77866 ].freeze class << self diff --git a/rubocop/cop/migration/schedule_async.rb b/rubocop/cop/migration/schedule_async.rb index f31bfa46aa7..4fdedecdf64 100644 --- a/rubocop/cop/migration/schedule_async.rb +++ b/rubocop/cop/migration/schedule_async.rb @@ -16,14 +16,23 @@ module RuboCop MSG def_node_matcher :calls_background_migration_worker?, <<~PATTERN - (send (const nil? :BackgroundMigrationWorker) {:perform_async :perform_in :bulk_perform_async :bulk_perform_in} ... ) + (send (const {cbase nil?} :BackgroundMigrationWorker) #perform_method? ...) + PATTERN + + def_node_matcher :calls_ci_database_worker?, <<~PATTERN + (send (const {(const {cbase nil?} :BackgroundMigration) nil?} :CiDatabaseWorker) #perform_method? ...) + PATTERN + + def_node_matcher :perform_method?, <<~PATTERN + {:perform_async :bulk_perform_async :perform_in :bulk_perform_in} PATTERN def on_send(node) return unless in_migration?(node) return if version(node) < ENFORCED_SINCE + return unless calls_background_migration_worker?(node) || calls_ci_database_worker?(node) - add_offense(node, location: :expression) if calls_background_migration_worker?(node) + add_offense(node, location: :expression) end end end diff --git a/rubocop/cop/scalability/bulk_perform_with_context.rb b/rubocop/cop/scalability/bulk_perform_with_context.rb index 3c5d7f39680..b96aa35bfee 100644 --- a/rubocop/cop/scalability/bulk_perform_with_context.rb +++ b/rubocop/cop/scalability/bulk_perform_with_context.rb @@ -23,6 +23,8 @@ module RuboCop Read more about it https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#worker-context MSG + BACKGROUND_MIGRATION_WORKER_NAMES = %w[BackgroundMigrationWorker CiDatabaseWorker].freeze + def_node_matcher :schedules_in_batch_without_context?, <<~PATTERN (send (...) {:bulk_perform_async :bulk_perform_in} _*) PATTERN @@ -30,7 +32,7 @@ module RuboCop def on_send(node) return if in_migration?(node) || in_spec?(node) return unless schedules_in_batch_without_context?(node) - return if name_of_receiver(node) == "BackgroundMigrationWorker" + return if scheduled_for_background_migration?(node) add_offense(node, location: :expression) end @@ -40,6 +42,10 @@ module RuboCop def in_spec?(node) file_path_for_node(node).end_with?("_spec.rb") end + + def scheduled_for_background_migration?(node) + BACKGROUND_MIGRATION_WORKER_NAMES.include?(name_of_receiver(node)) + end end end end diff --git a/rubocop/cop/scalability/cron_worker_context.rb b/rubocop/cop/scalability/cron_worker_context.rb index 8e754af4dcf..3cc0d42d7bc 100644 --- a/rubocop/cop/scalability/cron_worker_context.rb +++ b/rubocop/cop/scalability/cron_worker_context.rb @@ -11,7 +11,7 @@ module RuboCop If there is no relevant metadata, please disable the cop with a comment explaining this. - Read more about it https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#worker-context + Read more about it https://docs.gitlab.com/ee/development/sidekiq/logging.html#worker-context MSG def_node_matcher :includes_cronjob_queue?, <<~PATTERN diff --git a/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_with_deduplication.rb b/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_with_deduplication.rb deleted file mode 100644 index e8b4b513a23..00000000000 --- a/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_with_deduplication.rb +++ /dev/null @@ -1,154 +0,0 @@ -# frozen_string_literal: true - -require_relative '../../code_reuse_helpers' - -module RuboCop - module Cop - module SidekiqLoadBalancing - # This cop checks for including_scheduled: true option in idempotent Sidekiq workers that utilize load balancing capabilities. - # - # @example - # - # # bad - # class BadWorker - # include ApplicationWorker - # - # data_consistency :delayed - # idempotent! - # - # def perform - # end - # end - # - # # bad - # class BadWorker - # include ApplicationWorker - # - # data_consistency :delayed - # - # deduplicate :until_executing - # idempotent! - # - # def perform - # end - # end - # - # # good - # class GoodWorker - # include ApplicationWorker - # - # data_consistency :delayed - # - # deduplicate :until_executing, including_scheduled: true - # idempotent! - # - # def perform - # end - # end - # - class WorkerDataConsistencyWithDeduplication < RuboCop::Cop::Base - include CodeReuseHelpers - extend AutoCorrector - - HELP_LINK = 'https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#scheduling-jobs-in-the-future' - REPLACEMENT = ', including_scheduled: true' - DEFAULT_STRATEGY = ':until_executing' - - MSG = <<~MSG - Workers that declare either `:sticky` or `:delayed` data consistency become eligible for database load-balancing. - In both cases, jobs are enqueued with a short delay. - - If you do want to deduplicate jobs that utilize load-balancing, you need to specify including_scheduled: true - argument when defining deduplication strategy. - - See #{HELP_LINK} for a more detailed explanation of these settings. - MSG - - def_node_search :application_worker?, <<~PATTERN - `(send nil? :include (const nil? :ApplicationWorker)) - PATTERN - - def_node_search :idempotent_worker?, <<~PATTERN - `(send nil? :idempotent!) - PATTERN - - def_node_search :data_consistency_defined?, <<~PATTERN - `(send nil? :data_consistency (sym {:sticky :delayed })) - PATTERN - - def_node_matcher :including_scheduled?, <<~PATTERN - `(hash <(pair (sym :including_scheduled) (%1)) ...>) - PATTERN - - def_node_matcher :deduplicate_strategy?, <<~PATTERN - `(send nil? :deduplicate (sym $_) $(...)?) - PATTERN - - def on_class(node) - return unless in_worker?(node) - return unless application_worker?(node) - return unless idempotent_worker?(node) - return unless data_consistency_defined?(node) - - @strategy, options = deduplicate_strategy?(node) - including_scheduled = false - if options - @deduplicate_options = options[0] - including_scheduled = including_scheduled?(@deduplicate_options, :true) # rubocop:disable Lint/BooleanSymbol - end - - @offense = !(including_scheduled || @strategy == :none) - end - - def on_send(node) - return unless offense - - if node.children[1] == :deduplicate - add_offense(node.loc.expression) do |corrector| - autocorrect_deduplicate_strategy(node, corrector) - end - elsif node.children[1] == :idempotent! && !strategy - add_offense(node.loc.expression) do |corrector| - autocorrect_missing_deduplicate_strategy(node, corrector) - end - end - end - - private - - attr_reader :offense, :deduplicate_options, :strategy - - def autocorrect_deduplicate_with_options(corrector) - if including_scheduled?(deduplicate_options, :false) # rubocop:disable Lint/BooleanSymbol - replacement = deduplicate_options.source.sub("including_scheduled: false", "including_scheduled: true") - corrector.replace(deduplicate_options.loc.expression, replacement) - else - corrector.insert_after(deduplicate_options.loc.expression, REPLACEMENT) - end - end - - def autocorrect_deduplicate_without_options(node, corrector) - corrector.insert_after(node.loc.expression, REPLACEMENT) - end - - def autocorrect_missing_deduplicate_strategy(node, corrector) - indent_found = node.source_range.source_line =~ /^( +)/ - # Get indentation size - whitespaces = Regexp.last_match(1).size if indent_found - replacement = "deduplicate #{DEFAULT_STRATEGY}#{REPLACEMENT}\n" - # Add indentation in the end since we are inserting a whole line before idempotent! - replacement += ' ' * whitespaces.to_i - corrector.insert_before(node.source_range, replacement) - end - - def autocorrect_deduplicate_strategy(node, corrector) - if deduplicate_options - autocorrect_deduplicate_with_options(corrector) - else - autocorrect_deduplicate_without_options(node, corrector) - end - end - end - end - end -end |