Add latest changes from gitlab-org/gitlab@master

16 files changed, 147 insertions, 62 deletions

diff --git a/.gitlab/ci/global.gitlab-ci.yml b/.gitlab/ci/global.gitlab-ci.yml
index 98ec3ce96a2..faae7ad2817 100644
--- a/.gitlab/ci/global.gitlab-ci.yml
+++ b/.gitlab/ci/global.gitlab-ci.yml
@@ -263,9 +263,13 @@
     - name: redis:5.0-alpine
     - name: elasticsearch:7.17.6
       command: ["elasticsearch", "-E", "discovery.type=single-node", "-E", "xpack.security.enabled=false"]
+    - name: registry.gitlab.com/gitlab-org/gitlab-build-images:zoekt-ci-image-1.0
+      alias: zoekt-ci-image
   variables:
     POSTGRES_HOST_AUTH_METHOD: trust
     PG_VERSION: "11"
+    ZOEKT_INDEX_BASE_URL: http://zoekt-ci-image:6060
+    ZOEKT_SEARCH_BASE_URL: http://zoekt-ci-image:6070
 
 .use-pg12-es7-ee:
   services:
@@ -274,9 +278,13 @@
     - name: redis:6.0-alpine
     - name: elasticsearch:7.17.6
       command: ["elasticsearch", "-E", "discovery.type=single-node", "-E", "xpack.security.enabled=false"]
+    - name: registry.gitlab.com/gitlab-org/gitlab-build-images:zoekt-ci-image-1.0
+      alias: zoekt-ci-image
   variables:
     POSTGRES_HOST_AUTH_METHOD: trust
     PG_VERSION: "12"
+    ZOEKT_INDEX_BASE_URL: http://zoekt-ci-image:6060
+    ZOEKT_SEARCH_BASE_URL: http://zoekt-ci-image:6070
 
 .use-pg13-es7-ee:
   services:
@@ -285,9 +293,13 @@
     - name: redis:6.2-alpine
     - name: elasticsearch:7.17.6
       command: ["elasticsearch", "-E", "discovery.type=single-node", "-E", "xpack.security.enabled=false"]
+    - name: registry.gitlab.com/gitlab-org/gitlab-build-images:zoekt-ci-image-1.0
+      alias: zoekt-ci-image
   variables:
     POSTGRES_HOST_AUTH_METHOD: trust
     PG_VERSION: "13"
+    ZOEKT_INDEX_BASE_URL: http://zoekt-ci-image:6060
+    ZOEKT_SEARCH_BASE_URL: http://zoekt-ci-image:6070
 
 .use-pg12-es8-ee:
   services:
@@ -295,11 +307,15 @@
       command: ["postgres", "-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"]
     - name: redis:6.0-alpine
     - name: elasticsearch:8.5.3
+    - name: registry.gitlab.com/gitlab-org/gitlab-build-images:zoekt-ci-image-1.0
+      alias: zoekt-ci-image
   variables:
     POSTGRES_HOST_AUTH_METHOD: trust
     PG_VERSION: "12"
     ES_SETTING_DISCOVERY_TYPE: "single-node"
     ES_SETTING_XPACK_SECURITY_ENABLED: "false"
+    ZOEKT_INDEX_BASE_URL: http://zoekt-ci-image:6060
+    ZOEKT_SEARCH_BASE_URL: http://zoekt-ci-image:6070
 
 .use-pg12-opensearch1-ee:
   services:
@@ -309,9 +325,13 @@
     - name: opensearchproject/opensearch:1.3.5
       alias: elasticsearch
       command: ["bin/opensearch", "-E", "discovery.type=single-node", "-E", "plugins.security.disabled=true"]
+    - name: registry.gitlab.com/gitlab-org/gitlab-build-images:zoekt-ci-image-1.0
+      alias: zoekt-ci-image
   variables:
     POSTGRES_HOST_AUTH_METHOD: trust
     PG_VERSION: "12"
+    ZOEKT_INDEX_BASE_URL: http://zoekt-ci-image:6060
+    ZOEKT_SEARCH_BASE_URL: http://zoekt-ci-image:6070
 
 .use-pg12-opensearch2-ee:
   services:
@@ -321,9 +341,13 @@
     - name: opensearchproject/opensearch:2.2.1
       alias: elasticsearch
       command: ["bin/opensearch", "-E", "discovery.type=single-node", "-E", "plugins.security.disabled=true"]
+    - name: registry.gitlab.com/gitlab-org/gitlab-build-images:zoekt-ci-image-1.0
+      alias: zoekt-ci-image
   variables:
     POSTGRES_HOST_AUTH_METHOD: trust
     PG_VERSION: "12"
+    ZOEKT_INDEX_BASE_URL: http://zoekt-ci-image:6060
+    ZOEKT_SEARCH_BASE_URL: http://zoekt-ci-image:6070
 
 .use-kaniko:
   image:
diff --git a/.gitlab/ci/rails/shared.gitlab-ci.yml b/.gitlab/ci/rails/shared.gitlab-ci.yml
index 3f9a43c1ced..4943f7c2e28 100644
--- a/.gitlab/ci/rails/shared.gitlab-ci.yml
+++ b/.gitlab/ci/rails/shared.gitlab-ci.yml
@@ -67,7 +67,7 @@ include:
     # spec/lib, yet background migration tests are also sitting there,
     # and they should run on their own jobs so we don't need to run them
     # in unit tests again.
-    - rspec_paralellized_job "--tag ~quarantine --tag ~zoekt --tag ~level:background_migration"
+    - rspec_paralellized_job "--tag ~quarantine --tag ~level:background_migration"
   allow_failure:
     exit_codes: !reference [.rspec-base, variables, SUCCESSFULLY_RETRIED_TEST_EXIT_CODE]
 
diff --git a/.rubocop_todo/performance/redundant_split_regexp_argument.yml b/.rubocop_todo/performance/redundant_split_regexp_argument.yml
deleted file mode 100644
index 0c0e12480d5..00000000000
--- a/.rubocop_todo/performance/redundant_split_regexp_argument.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-# Cop supports --autocorrect.
-Performance/RedundantSplitRegexpArgument:
-  Details: grace period
-  Exclude:
-    - 'lib/file_size_validator.rb'
-    - 'lib/gitlab/git/raw_diff_change.rb'
-    - 'lib/kramdown/converter/commonmark.rb'
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index 3a65378f961..fc6727dfc98 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-6eb7389db0d54ef9ba3622eb7d1412f71e9deb2f
+78afcc347d19840f86d35967f35e1f7899080247
diff --git a/app/workers/bulk_import_worker.rb b/app/workers/bulk_import_worker.rb
index d5eca86744e..6bce13c5ff0 100644
--- a/app/workers/bulk_import_worker.rb
+++ b/app/workers/bulk_import_worker.rb
@@ -4,6 +4,7 @@ class BulkImportWorker # rubocop:disable Scalability/IdempotentWorker
   include ApplicationWorker
 
   PERFORM_DELAY = 5.seconds
+  DEFAULT_BATCH_SIZE = 5
 
   data_consistency :always
   feature_category :importers
@@ -16,10 +17,11 @@ class BulkImportWorker # rubocop:disable Scalability/IdempotentWorker
     return if @bulk_import.finished? || @bulk_import.failed?
     return @bulk_import.fail_op! if all_entities_failed?
     return @bulk_import.finish! if all_entities_processed? && @bulk_import.started?
+    return re_enqueue if max_batch_size_exceeded? # Do not start more jobs if max allowed are already running
 
     @bulk_import.start! if @bulk_import.created?
 
-    created_entities.find_each do |entity|
+    created_entities.first(next_batch_size).each do |entity|
       BulkImports::CreatePipelineTrackersService.new(entity).execute!
 
       entity.start!
@@ -58,4 +60,16 @@ class BulkImportWorker # rubocop:disable Scalability/IdempotentWorker
   def re_enqueue
     BulkImportWorker.perform_in(PERFORM_DELAY, @bulk_import.id)
   end
+
+  def started_entities
+    entities.with_status(:started)
+  end
+
+  def max_batch_size_exceeded?
+    started_entities.count >= DEFAULT_BATCH_SIZE
+  end
+
+  def next_batch_size
+    [DEFAULT_BATCH_SIZE - started_entities.count, 0].max
+  end
 end
diff --git a/app/workers/bulk_imports/pipeline_worker.rb b/app/workers/bulk_imports/pipeline_worker.rb
index 62e85d38e61..7435a1f6a88 100644
--- a/app/workers/bulk_imports/pipeline_worker.rb
+++ b/app/workers/bulk_imports/pipeline_worker.rb
@@ -42,7 +42,6 @@ module BulkImports
     def run
       return skip_tracker if entity.failed?
 
-      raise(Pipeline::ExpiredError, 'Pipeline timeout') if job_timeout?
       raise(Pipeline::FailedError, "Export from source instance failed: #{export_status.error}") if export_failed?
       raise(Pipeline::ExpiredError, 'Empty export status on source instance') if empty_export_timeout?
 
@@ -103,14 +102,8 @@ module BulkImports
       pipeline_tracker.file_extraction_pipeline?
     end
 
-    def job_timeout?
-      return false unless file_extraction_pipeline?
-
-      time_since_entity_created > Pipeline::NDJSON_EXPORT_TIMEOUT
-    end
-
     def empty_export_timeout?
-      export_empty? && time_since_entity_created > Pipeline::EMPTY_EXPORT_STATUS_TIMEOUT
+      export_empty? && time_since_tracker_created > Pipeline::EMPTY_EXPORT_STATUS_TIMEOUT
     end
 
     def export_failed?
@@ -167,8 +160,8 @@ module BulkImports
       logger.error(structured_payload(payload))
     end
 
-    def time_since_entity_created
-      Time.zone.now - entity.created_at
+    def time_since_tracker_created
+      Time.zone.now - (pipeline_tracker.created_at || entity.created_at)
     end
 
     def lease_timeout
diff --git a/doc/user/application_security/dast/checks/22.1.md b/doc/user/application_security/dast/checks/22.1.md
new file mode 100644
index 00000000000..60a73b4248b
--- /dev/null
+++ b/doc/user/application_security/dast/checks/22.1.md
@@ -0,0 +1,38 @@
+---
+stage: Secure
+group: Dynamic Analysis
+info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments
+---
+
+# Improper limitation of a pathname to a restricted directory (Path traversal)
+
+## Description
+
+The vulnerability can be exploited by inserting a payload into a
+parameter on the URL endpoint which allows for reading arbitrary files.
+This could be used to read sensitive files, access other users data, or aid in
+exploitation to gain further system access.
+
+## Remediation
+
+User input should never be used in constructing paths or files for interacting
+with the filesystem. This includes filenames supplied by user uploads or downloads.
+
+If possible, consider hashing the filenames and reference the hashed filenames in
+a database or datastore instead of directly attempting to access filenames provided
+by users or other system components.
+
+In the rare cases that the application must work with filenames, use the language
+provided functionality to extract only the filename part of the supplied value.
+Never attempt to use the path or directory information that comes from user input.
+
+## Details
+
+| ID | Aggregated | CWE | Type | Risk |
+|:---|:--------|:--------|:--------|:--------|
+| 22.1 | false | 22 | Active | high |
+
+## Links
+
+- [OWASP](https://owasp.org/www-community/attacks/Path_Traversal)
+- [CWE](https://cwe.mitre.org/data/definitions/22.html)
diff --git a/doc/user/application_security/dast/checks/index.md b/doc/user/application_security/dast/checks/index.md
index 0f9e78d1817..bafe426ca43 100644
--- a/doc/user/application_security/dast/checks/index.md
+++ b/doc/user/application_security/dast/checks/index.md
@@ -8,6 +8,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 
 The [DAST browser-based crawler](../browser_based.md) provides a number of vulnerability checks that are used to scan for vulnerabilities in the site under test.
 
+## Passive Checks
+
 | ID | Check | Severity | Type |
 |:---|:------|:---------|:-----|
 | [1004.1](1004.1.md) | Sensitive cookie without HttpOnly attribute | Low | Passive |
@@ -160,3 +162,9 @@ The [DAST browser-based crawler](../browser_based.md) provides a number of vulne
 | [798.128](798.128.md) | Exposure of confidential secret or token Zendesk Secret Key | High | Passive |
 | [829.1](829.1.md) | Inclusion of Functionality from Untrusted Control Sphere | Low | Passive |
 | [829.2](829.2.md) | Invalid Sub-Resource Integrity values detected | Medium | Passive |
+
+## Active Checks
+
+| ID | Check | Severity | Type |
+|:---|:------|:---------|:-----|
+| [22.1](22.1.md) | Improper limitation of a pathname to a restricted directory (Path traversal) | High | Active |
diff --git a/lib/bulk_imports/pipeline.rb b/lib/bulk_imports/pipeline.rb
index 681d6e9aad6..e65232a5734 100644
--- a/lib/bulk_imports/pipeline.rb
+++ b/lib/bulk_imports/pipeline.rb
@@ -12,7 +12,6 @@ module BulkImports
     FailedError = Class.new(StandardError)
 
     CACHE_KEY_EXPIRATION = 2.hours
-    NDJSON_EXPORT_TIMEOUT = 90.minutes
     EMPTY_EXPORT_STATUS_TIMEOUT = 5.minutes
 
     def initialize(context)
diff --git a/lib/file_size_validator.rb b/lib/file_size_validator.rb
index ac3210b4e98..8ac5f2f6b54 100644
--- a/lib/file_size_validator.rb
+++ b/lib/file_size_validator.rb
@@ -4,7 +4,7 @@ class FileSizeValidator < ActiveModel::EachValidator
   MESSAGES = { is: :wrong_size, minimum: :size_too_small, maximum: :size_too_big }.freeze
   CHECKS   = { is: :==, minimum: :>=, maximum: :<= }.freeze
 
-  DEFAULT_TOKENIZER = -> (value) { value.split(//) }.freeze
+  DEFAULT_TOKENIZER = -> (value) { value.split("") }.freeze
   RESERVED_OPTIONS  = [:minimum, :maximum, :within, :is, :tokenizer, :too_short, :too_long].freeze
 
   def initialize(options)
diff --git a/lib/gitlab/git/raw_diff_change.rb b/lib/gitlab/git/raw_diff_change.rb
index 9a41f04a4db..c0121311605 100644
--- a/lib/gitlab/git/raw_diff_change.rb
+++ b/lib/gitlab/git/raw_diff_change.rb
@@ -38,7 +38,7 @@ module Gitlab
       def extract_paths(file_path)
         case operation
         when :copied, :renamed
-          file_path.split(/\t/)
+          file_path.split("\t")
         when :deleted
           [file_path, nil]
         when :added
diff --git a/lib/kramdown/converter/commonmark.rb b/lib/kramdown/converter/commonmark.rb
index a903d541d81..0055558989d 100644
--- a/lib/kramdown/converter/commonmark.rb
+++ b/lib/kramdown/converter/commonmark.rb
@@ -34,7 +34,7 @@ module Kramdown
       def convert_codeblock(el, _opts)
         # Although tildes are supported in CommonMark, backticks are more common
         "```#{el.options[:lang]}\n" +
-          el.value.split(/\n/).map { |l| l.empty? ? "" : "#{l}" }.join("\n") +
+          el.value.split("\n").map { |l| l.empty? ? "" : "#{l}" }.join("\n") +
           "\n```\n\n"
       end
 
diff --git a/scripts/db_tasks b/scripts/db_tasks
index c3b7bb84355..36040877abf 100755
--- a/scripts/db_tasks
+++ b/scripts/db_tasks
@@ -9,7 +9,7 @@ database_config = YAML.load_file(File.join(File.expand_path('..', __dir__), 'con
 task = ARGV.shift
 raise ArgumentError, 'You need to pass a task name!' unless task
 
-task = "${task}:main" unless database_config.one?
+task = "#{task}:main" unless database_config.one?
 cmd = ['bundle', 'exec', 'rake', task, *ARGV]
 
 puts "Running: `#{cmd.join(' ')}`"
diff --git a/spec/support/webmock.rb b/spec/support/webmock.rb
index b9bd3f82f65..171c7ace2d2 100644
--- a/spec/support/webmock.rb
+++ b/spec/support/webmock.rb
@@ -9,12 +9,26 @@ def webmock_allowed_hosts
       hosts << URI.parse(ENV['ELASTIC_URL']).host
     end
 
+    if ENV.key?('ZOEKT_INDEX_BASE_URL')
+      hosts.concat(allowed_host_and_ip(ENV['ZOEKT_INDEX_BASE_URL']))
+    end
+
+    if ENV.key?('ZOEKT_SEARCH_BASE_URL')
+      hosts.concat(allowed_host_and_ip(ENV['ZOEKT_SEARCH_BASE_URL']))
+    end
+
     if Gitlab.config.webpack&.dev_server&.enabled
       hosts << Gitlab.config.webpack.dev_server.host
     end
   end.compact.uniq
 end
 
+def allowed_host_and_ip(url)
+  host = URI.parse(url).host
+  ip_address = Addrinfo.ip(host).ip_address
+  [host, ip_address]
+end
+
 def with_net_connect_allowed
   WebMock.allow_net_connect!
   yield
diff --git a/spec/workers/bulk_import_worker_spec.rb b/spec/workers/bulk_import_worker_spec.rb
index 61c33f123fa..ec8550bb3bc 100644
--- a/spec/workers/bulk_import_worker_spec.rb
+++ b/spec/workers/bulk_import_worker_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe BulkImportWorker do
+RSpec.describe BulkImportWorker, feature_category: :importers do
   describe '#perform' do
     context 'when no bulk import is found' do
       it 'does nothing' do
@@ -56,6 +56,19 @@ RSpec.describe BulkImportWorker do
       end
     end
 
+    context 'when maximum allowed number of import entities in progress' do
+      it 'reenqueues itself' do
+        bulk_import = create(:bulk_import, :started)
+        create(:bulk_import_entity, :created, bulk_import: bulk_import)
+        (described_class::DEFAULT_BATCH_SIZE + 1).times { |_| create(:bulk_import_entity, :started, bulk_import: bulk_import) }
+
+        expect(described_class).to receive(:perform_in).with(described_class::PERFORM_DELAY, bulk_import.id)
+        expect(BulkImports::ExportRequestWorker).not_to receive(:perform_async)
+
+        subject.perform(bulk_import.id)
+      end
+    end
+
     context 'when bulk import is created' do
       it 'marks bulk import as started' do
         bulk_import = create(:bulk_import, :created)
@@ -82,16 +95,20 @@ RSpec.describe BulkImportWorker do
       context 'when there are created entities to process' do
         let_it_be(:bulk_import) { create(:bulk_import, :created) }
 
-        it 'marks all entities as started, enqueues EntityWorker, ExportRequestWorker and reenqueues' do
+        before do
+          stub_const("#{described_class}::DEFAULT_BATCH_SIZE", 1)
+        end
+
+        it 'marks a batch of entities as started, enqueues EntityWorker, ExportRequestWorker and reenqueues' do
           create(:bulk_import_entity, :created, bulk_import: bulk_import)
           create(:bulk_import_entity, :created, bulk_import: bulk_import)
 
           expect(described_class).to receive(:perform_in).with(described_class::PERFORM_DELAY, bulk_import.id)
-          expect(BulkImports::ExportRequestWorker).to receive(:perform_async).twice
+          expect(BulkImports::ExportRequestWorker).to receive(:perform_async).once
 
           subject.perform(bulk_import.id)
 
-          expect(bulk_import.entities.map(&:status_name)).to contain_exactly(:started, :started)
+          expect(bulk_import.entities.map(&:status_name)).to contain_exactly(:created, :started)
         end
 
         context 'when there are project entities to process' do
diff --git a/spec/workers/bulk_imports/pipeline_worker_spec.rb b/spec/workers/bulk_imports/pipeline_worker_spec.rb
index 03ec6267ca8..10c63606f81 100644
--- a/spec/workers/bulk_imports/pipeline_worker_spec.rb
+++ b/spec/workers/bulk_imports/pipeline_worker_spec.rb
@@ -433,11 +433,11 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do
           allow(status).to receive(:failed?).and_return(false)
         end
 
-        entity.update!(created_at: entity_created_at)
+        pipeline_tracker.update!(created_at: created_at)
       end
 
       context 'when timeout is not reached' do
-        let(:entity_created_at) { 1.minute.ago }
+        let(:created_at) { 1.minute.ago }
 
         it 'reenqueues pipeline worker' do
           expect(described_class)
@@ -455,8 +455,8 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do
         end
       end
 
-      context 'when timeout is reached' do
-        let(:entity_created_at) { 10.minutes.ago }
+      context 'when empty export timeout is reached' do
+        let(:created_at) { 10.minutes.ago }
 
         it 'marks as failed and logs the error' do
           expect_next_instance_of(Gitlab::Import::Logger) do |logger|
@@ -485,39 +485,25 @@ RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do
           expect(pipeline_tracker.reload.status_name).to eq(:failed)
         end
       end
-    end
 
-    context 'when job reaches timeout' do
-      it 'marks as failed and logs the error' do
-        old_created_at = entity.created_at
-        entity.update!(created_at: (BulkImports::Pipeline::NDJSON_EXPORT_TIMEOUT + 1.hour).ago)
+      context 'when tracker created_at is nil' do
+        let(:created_at) { nil }
 
-        expect_next_instance_of(Gitlab::Import::Logger) do |logger|
-          expect(logger)
-            .to receive(:error)
-            .with(
-              hash_including(
-                'pipeline_name' => 'NdjsonPipeline',
-                'bulk_import_entity_id' => entity.id,
-                'bulk_import_id' => entity.bulk_import_id,
-                'bulk_import_entity_type' => entity.source_type,
-                'source_full_path' => entity.source_full_path,
-                'class' => 'BulkImports::PipelineWorker',
-                'exception.backtrace' => anything,
-                'exception.class' => 'BulkImports::Pipeline::ExpiredError',
-                'exception.message' => 'Pipeline timeout',
-                'importer' => 'gitlab_migration',
-                'message' => 'Pipeline failed',
-                'source_version' => entity.bulk_import.source_version_info.to_s
-              )
-            )
-        end
+        it 'falls back to entity created_at' do
+          entity.update!(created_at: 10.minutes.ago)
 
-        subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id)
+          expect_next_instance_of(Gitlab::Import::Logger) do |logger|
+            expect(logger)
+              .to receive(:error)
+              .with(
+                hash_including('exception.message' => 'Empty export status on source instance')
+              )
+          end
 
-        expect(pipeline_tracker.reload.status_name).to eq(:failed)
+          subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id)
 
-        entity.update!(created_at: old_created_at)
+          expect(pipeline_tracker.reload.status_name).to eq(:failed)
+        end
       end
     end