Add latest changes from gitlab-org/gitlab@master

.rubocop_manual_todo.yml

@@ -2146,11 +2146,6 @@ Gitlab/NamespacedClass:
     - 'spec/tasks/gitlab/task_helpers_spec.rb'
     - 'spec/uploaders/object_storage_spec.rb'
 
-Style/ClassEqualityComparison:
-  Exclude:
-    - spec/lib/peek/views/active_record_spec.rb
-    - ee/spec/lib/peek/views/active_record_spec.rb
-
 # WIP See https://gitlab.com/gitlab-org/gitlab/-/issues/207950
 Cop/UserAdmin:
   Exclude:

.rubocop_todo.yml

@@ -660,23 +660,6 @@ Style/BisectedAttrAccessor:
 Style/CaseLikeIf:
   Enabled: false
 
-# Offense count: 10
-# Cop supports --auto-correct.
-# Configuration parameters: IgnoredMethods.
-# IgnoredMethods: ==, equal?, eql?
-Style/ClassEqualityComparison:
-  Exclude:
-    - 'app/finders/security/jobs_finder.rb'
-    - 'app/services/projects/overwrite_project_service.rb'
-    - 'app/uploaders/dependency_proxy/file_uploader.rb'
-    - 'ee/app/graphql/resolvers/vulnerabilities/issue_links_resolver.rb'
-    - 'lib/gitlab/background_migration/user_mentions/models/note.rb'
-    - 'lib/gitlab/diff/file.rb'
-    - 'lib/gitlab/git.rb'
-    - 'lib/gitlab/import_export/relation_tree_restorer.rb'
-    - 'spec/requests/api/services_spec.rb'
-    - 'spec/support/shared_examples/lib/gitlab/import_export/relation_factory_shared_examples.rb'
-
 # Offense count: 13
 Style/CombinableLoops:
   Exclude:

Gemfile

@@ -305,11 +305,7 @@ gem 'sentry-raven', '~> 3.1'
 
 # PostgreSQL query parsing
 #
-# We need this fork until https://github.com/pganalyze/pg_query/pull/212
-# and https://github.com/pganalyze/pg_query/pull/213 are
-# released. gitlab-labkit will need to be updated to use the pg_query
-# version.
-gem 'gitlab-pg_query', '~> 2.0.4', require: 'pg_query'
+gem 'pg_query', '~> 2.1'
 
 gem 'premailer-rails', '~> 1.10.3'
 

Gemfile.lock

@@ -921,6 +921,8 @@ GEM
     peek (1.1.0)
       railties (>= 4.0.0)
     pg (1.2.3)
+    pg_query (2.1.0)
+      google-protobuf (>= 3.17.1)
     plist (3.6.0)
     png_quantizator (0.2.1)
     po_to_json (1.0.1)
@@ -1495,7 +1497,6 @@ DEPENDENCIES
   gitlab-markup (~> 1.7.1)
   gitlab-net-dns (~> 0.9.1)
   gitlab-omniauth-openid-connect (~> 0.4.0)
-  gitlab-pg_query (~> 2.0.4)
   gitlab-rdoc (~> 6.3.2)
   gitlab-sidekiq-fetcher (= 0.5.6)
   gitlab-styles (~> 6.2.0)
@@ -1585,6 +1586,7 @@ DEPENDENCIES
   parslet (~> 1.8)
   peek (~> 1.1)
   pg (~> 1.1)
+  pg_query (~> 2.1)
   png_quantizator (~> 0.2.1)
   premailer-rails (~> 1.10.3)
   prometheus-client-mmap (~> 0.12.0)

app/assets/javascripts/diffs/components/collapsed_files_warning.vue

@@ -1,5 +1,6 @@
 <script>
 import { GlAlert, GlButton } from '@gitlab/ui';
+import { mapState } from 'vuex';
 
 import { CENTERED_LIMITED_CONTAINER_CLASSES, EVT_EXPAND_ALL_FILES } from '../constants';
 import eventHub from '../event_hub';
@@ -27,11 +28,15 @@ export default {
     };
   },
   computed: {
+    ...mapState('diffs', ['diffFiles']),
     containerClasses() {
       return {
         [CENTERED_LIMITED_CONTAINER_CLASSES]: this.limited,
       };
     },
+    shouldDisplay() {
+      return !this.isDismissed && this.diffFiles.length > 1;
+    },
   },
 
   methods: {
@@ -48,7 +53,7 @@ export default {
 </script>
 
 <template>
-  <div v-if="!isDismissed" data-testid="root" :class="containerClasses" class="col-12">
+  <div v-if="shouldDisplay" data-testid="root" :class="containerClasses" class="col-12">
     <gl-alert
       :dismissible="true"
       :title="__('Some changes are not shown')"

app/finders/security/jobs_finder.rb

@@ -20,7 +20,7 @@ module Security
     end
 
     def initialize(pipeline:, job_types: [])
-      if self.class == Security::JobsFinder
+      if self.instance_of?(Security::JobsFinder)
         raise NotImplementedError, 'This is an abstract class, please instantiate its descendants'
       end
 

app/models/blob_viewer/markup.rb

@@ -14,9 +14,7 @@ module BlobViewer
       {}.tap do |h|
         h[:rendered] = blob.rendered_markup if blob.respond_to?(:rendered_markup)
 
-        if Feature.enabled?(:cached_markdown_blob, blob.project, default_enabled: true)
-          h[:cache_key] = ['blob', blob.id, 'commit', blob.commit_id]
-        end
+        h[:cache_key] = ['blob', blob.id, 'commit', blob.commit_id]
       end
     end
   end

app/services/projects/overwrite_project_service.rb

@@ -20,7 +20,7 @@ module Projects
     rescue Exception => e # rubocop:disable Lint/RescueException
       attempt_restore_repositories(source_project)
 
-      if e.class == Exception
+      if e.instance_of?(Exception)
         raise StandardError, e.message
       else
         raise

app/services/releases/base_service.rb

@@ -5,6 +5,8 @@ module Releases
     include BaseServiceUtility
     include Gitlab::Utils::StrongMemoize
 
+    ReleaseProtectedTagAccessError = Class.new(StandardError)
+
     attr_accessor :project, :current_user, :params
 
     def initialize(project, user = nil, params = {})
@@ -81,6 +83,15 @@ module Releases
       release.execute_hooks(action)
     end
 
+    def track_protected_tag_access_error!
+      unless ::Gitlab::UserAccess.new(current_user, container: project).can_create_tag?(tag_name)
+        Gitlab::ErrorTracking.log_exception(
+          ReleaseProtectedTagAccessError.new,
+          project_id: project.id,
+          user_id: current_user.id)
+      end
+    end
+
     # overridden in EE
     def project_group_id; end
   end

app/services/releases/create_service.rb

@@ -7,6 +7,8 @@ module Releases
       return error('Release already exists', 409) if release
       return error("Milestone(s) not found: #{inexistent_milestones.join(', ')}", 400) if inexistent_milestones.any?
 
+      track_protected_tag_access_error!
+
       # should be found before the creation of new tag
       # because tag creation can spawn new pipeline
       # which won't have any data for evidence yet

app/services/releases/destroy_service.rb

@@ -6,6 +6,8 @@ module Releases
       return error('Release does not exist', 404) unless release
       return error('Access Denied', 403) unless allowed?
 
+      track_protected_tag_access_error!
+
       if release.destroy
         success(tag: existing_tag, release: release)
       else

app/services/releases/update_service.rb

@@ -7,6 +7,8 @@ module Releases
         return error
       end
 
+      track_protected_tag_access_error!
+
       if param_for_milestone_titles_provided?
         previous_milestones = release.milestones.map(&:title)
         params[:milestones] = milestones

app/uploaders/dependency_proxy/file_uploader.rb

@@ -24,7 +24,7 @@ class DependencyProxy::FileUploader < GitlabUploader
   # so we must store the custom content type in object storage.
   # This does not apply to DependencyProxy::Blob uploads.
   def set_content_type(file)
-    return unless model.class == DependencyProxy::Manifest
+    return unless model.instance_of?(DependencyProxy::Manifest)
 
     file.content_type = model.content_type
   end

config/feature_flags/development/allow_archive_as_web_access_format.yml

@@ -1,8 +1,8 @@
 ---
-name: cached_markdown_blob
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44300
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/263406
-milestone: '13.5'
+name: allow_archive_as_web_access_format
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/64471
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/334944
+milestone: '14.1'
 type: development
-group: group::source code
-default_enabled: true
+group: group::release
+default_enabled: false

doc/ci/variables/index.md

@@ -71,6 +71,10 @@ to execute scripts. Each shell has its own set of reserved variable names.
 
 Make sure each variable is defined for the [scope you want to use it in](where_variables_can_be_used.md).
 
+By default, pipelines from forked projects can't access CI/CD variables in the parent project.
+If you [run a merge request pipeline in the parent project for a merge request from a fork](../pipelines/merge_request_pipelines.md#run-pipelines-in-the-parent-project-for-merge-requests-from-a-forked-project),
+all variables become available to the pipeline.
+
 ### Create a custom CI/CD variable in the `.gitlab-ci.yml` file
 
 To create a custom variable in the [`.gitlab-ci.yml`](../yaml/index.md#variables) file,
@@ -303,6 +307,10 @@ The value of the variable must:
   - The `~` character ([In GitLab 13.12](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61517) and later).
 - Not match the name of an existing predefined or custom CI/CD variable.
 
+NOTE:
+Masking a CI/CD variable is not a guaranteed way to prevent malicious users from accessing
+variable values. To make variables more secure, you can [use external secrets](../secrets/index.md).
+
 ### Protect a CI/CD variable
 
 You can protect a project, group or instance CI/CD variable so it is only passed

doc/development/documentation/styleguide/index.md

@@ -499,7 +499,7 @@ Follow these guidelines for punctuation:
 | Do not use tabs for indentation. Use spaces instead. You can configure your code editor to output spaces instead of tabs when pressing the tab key. | --- |
 | Use serial commas (_Oxford commas_) before the final _and_ or _or_ in a list of three or more items. (Tested in [`OxfordComma.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/OxfordComma.yml).) | _You can create new issues, merge requests, and milestones._ |
 | Always add a space before and after dashes when using it in a sentence (for replacing a comma, for example). | _You should try this - or not._ |
-| Always use lowercase after a colon.                              | Linked issues: a way to create a relationship between issues._ |
+| When a colon is part of a sentence, always use lowercase after the colon. | _Linked issues: a way to create a relationship between issues._ |
 
 <!-- vale gitlab.Repetition = YES -->
 

lib/gitlab/auth/auth_finders.rb

@@ -89,9 +89,11 @@ module Gitlab
         job.user
       end
 
-      # We only allow Private Access Tokens with `api` scope to be used by web
+      # We allow Private Access Tokens with `api` scope to be used by web
       # requests on RSS feeds or ICS files for backwards compatibility.
       # It is also used by GraphQL/API requests.
+      # And to allow accessing /archive programatically as it was a big pain point
+      # for users https://gitlab.com/gitlab-org/gitlab/-/issues/28978.
       def find_user_from_web_access_token(request_format, scopes: [:api])
         return unless access_token && valid_web_access_format?(request_format)
 
@@ -269,6 +271,8 @@ module Gitlab
           ics_request?
         when :api
           api_request?
+        when :archive
+          archive_request? if Feature.enabled?(:allow_archive_as_web_access_format, default_enabled: :yaml)
         end
       end
 

lib/gitlab/background_migration/user_mentions/models/note.rb

@@ -21,7 +21,7 @@ module Gitlab
           belongs_to :project, class_name: "::Gitlab::BackgroundMigration::UserMentions::Models::Project"
 
           def for_personal_snippet?
-            noteable && noteable.class.name == 'PersonalSnippet'
+            noteable && noteable.instance_of?(PersonalSnippet)
           end
 
           def for_project_noteable?

lib/gitlab/diff/file.rb

@@ -449,7 +449,7 @@ module Gitlab
       end
 
       def alternate_viewer_class
-        return unless viewer.class == DiffViewer::Renamed
+        return unless viewer.instance_of?(DiffViewer::Renamed)
 
         find_renderable_viewer_class(RICH_VIEWERS) || (DiffViewer::Text if text?)
       end

lib/gitlab/git.rb

@@ -80,7 +80,7 @@ module Gitlab
       def shas_eql?(sha1, sha2)
         return true if sha1.nil? && sha2.nil?
         return false if sha1.nil? || sha2.nil?
-        return false unless sha1.class == sha2.class
+        return false unless sha1.instance_of?(sha2.class)
 
         # If either of the shas is below the minimum length, we cannot be sure
         # that they actually refer to the same commit because of hash collision.

lib/gitlab/import_export/relation_tree_restorer.rb

@@ -37,7 +37,7 @@ module Gitlab
           ActiveRecord::Base.no_touching do
             update_params!
 
-            BulkInsertableAssociations.with_bulk_insert(enabled: @importable.class == ::Project) do
+            BulkInsertableAssociations.with_bulk_insert(enabled: @importable.instance_of?(::Project)) do
               fix_ci_pipelines_not_sorted_on_legacy_project_json!
               create_relations!
             end

spec/frontend/diffs/components/collapsed_files_warning_spec.js

@@ -1,10 +1,13 @@
 import { shallowMount, mount, createLocalVue } from '@vue/test-utils';
+import { nextTick } from 'vue';
 import Vuex from 'vuex';
 import CollapsedFilesWarning from '~/diffs/components/collapsed_files_warning.vue';
 import { CENTERED_LIMITED_CONTAINER_CLASSES, EVT_EXPAND_ALL_FILES } from '~/diffs/constants';
 import eventHub from '~/diffs/event_hub';
 import createStore from '~/diffs/store/modules';
 
+import file from '../mock_data/diff_file';
+
 const propsData = {
   limited: true,
   mergeable: true,
@@ -12,6 +15,13 @@ const propsData = {
 };
 const limitedClasses = CENTERED_LIMITED_CONTAINER_CLASSES.split(' ');
 
+async function files(store, count) {
+  const copies = Array(count).fill(file);
+  store.state.diffs.diffFiles.push(...copies);
+
+  return nextTick();
+}
+
 describe('CollapsedFilesWarning', () => {
   const localVue = createLocalVue();
   let store;
@@ -42,48 +52,63 @@ describe('CollapsedFilesWarning', () => {
     wrapper.destroy();
   });
 
-  it.each`
-    limited  | containerClasses
-    ${true}  | ${limitedClasses}
-    ${false} | ${[]}
-  `(
-    'has the correct container classes when limited is $limited',
-    ({ limited, containerClasses }) => {
-      createComponent({ limited });
+  describe('when there is more than one file', () => {
+    it.each`
+      limited  | containerClasses
+      ${true}  | ${limitedClasses}
+      ${false} | ${[]}
+    `(
+      'has the correct container classes when limited is $limited',
+      async ({ limited, containerClasses }) => {
+        createComponent({ limited });
+        await files(store, 2);
 
-      expect(wrapper.classes()).toEqual(['col-12'].concat(containerClasses));
-    },
-  );
+        expect(wrapper.classes()).toEqual(['col-12'].concat(containerClasses));
+      },
+    );
 
-  it.each`
-    present  | dismissed
-    ${false} | ${true}
-    ${true}  | ${false}
-  `('toggles the alert when dismissed is $dismissed', ({ present, dismissed }) => {
-    createComponent({ dismissed });
+    it.each`
+      present  | dismissed
+      ${false} | ${true}
+      ${true}  | ${false}
+    `('toggles the alert when dismissed is $dismissed', async ({ present, dismissed }) => {
+      createComponent({ dismissed });
+      await files(store, 2);
 
-    expect(wrapper.find('[data-testid="root"]').exists()).toBe(present);
+      expect(wrapper.find('[data-testid="root"]').exists()).toBe(present);
+    });
+
+    it('dismisses the component when the alert "x" is clicked', async () => {
+      createComponent({}, { full: true });
+      await files(store, 2);
+
+      expect(wrapper.find('[data-testid="root"]').exists()).toBe(true);
+
+      getAlertCloseButton().element.click();
+
+      await wrapper.vm.$nextTick();
+
+      expect(wrapper.find('[data-testid="root"]').exists()).toBe(false);
+    });
+
+    it(`emits the \`${EVT_EXPAND_ALL_FILES}\` event when the alert action button is clicked`, async () => {
+      createComponent({}, { full: true });
+      await files(store, 2);
+
+      jest.spyOn(eventHub, '$emit');
+
+      getAlertActionButton().vm.$emit('click');
+
+      expect(eventHub.$emit).toHaveBeenCalledWith(EVT_EXPAND_ALL_FILES);
+    });
   });
 
-  it('dismisses the component when the alert "x" is clicked', async () => {
-    createComponent({}, { full: true });
+  describe('when there is a single file', () => {
+    it('should not display', async () => {
+      createComponent();
+      await files(store, 1);
 
-    expect(wrapper.find('[data-testid="root"]').exists()).toBe(true);
-
-    getAlertCloseButton().element.click();
-
-    await wrapper.vm.$nextTick();
-
-    expect(wrapper.find('[data-testid="root"]').exists()).toBe(false);
-  });
-
-  it(`emits the \`${EVT_EXPAND_ALL_FILES}\` event when the alert action button is clicked`, () => {
-    createComponent({}, { full: true });
-
-    jest.spyOn(eventHub, '$emit');
-
-    getAlertActionButton().vm.$emit('click');
-
-    expect(eventHub.$emit).toHaveBeenCalledWith(EVT_EXPAND_ALL_FILES);
+      expect(wrapper.find('[data-testid="root"]').exists()).toBe(false);
+    });
   });
 });

spec/lib/gitlab/auth/auth_finders_spec.rb

@@ -460,7 +460,7 @@ RSpec.describe Gitlab::Auth::AuthFinders do
       expect { find_user_from_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError)
     end
 
-    context 'no feed or API requests' do
+    context 'no feed, API or archive requests' do
       it 'returns nil if the request is not RSS' do
         expect(find_user_from_web_access_token(:rss)).to be_nil
       end
@@ -472,6 +472,10 @@ RSpec.describe Gitlab::Auth::AuthFinders do
       it 'returns nil if the request is not API' do
         expect(find_user_from_web_access_token(:api)).to be_nil
       end
+
+      it 'returns nil if the request is not ARCHIVE' do
+        expect(find_user_from_web_access_token(:archive)).to be_nil
+      end
     end
 
     it 'returns the user for RSS requests' do
@@ -486,6 +490,24 @@ RSpec.describe Gitlab::Auth::AuthFinders do
       expect(find_user_from_web_access_token(:ics)).to eq(user)
     end
 
+    it 'returns the user for ARCHIVE requests' do
+      set_header('SCRIPT_NAME', '/-/archive/main.zip')
+
+      expect(find_user_from_web_access_token(:archive)).to eq(user)
+    end
+
+    context 'when allow_archive_as_web_access_format feature flag is disabled' do
+      before do
+        stub_feature_flags(allow_archive_as_web_access_format: false)
+      end
+
+      it 'returns nil for ARCHIVE requests' do
+        set_header('SCRIPT_NAME', '/-/archive/main.zip')
+
+        expect(find_user_from_web_access_token(:archive)).to be_nil
+      end
+    end
+
     context 'for API requests' do
       it 'returns the user' do
         set_header('SCRIPT_NAME', '/api/endpoint')

spec/models/blob_viewer/markup_spec.rb

@@ -24,15 +24,5 @@ RSpec.describe BlobViewer::Markup do
         expect(subject.banzai_render_context.keys).to include(:rendered)
       end
     end
-
-    context 'when cached_markdown_blob feature flag is disabled' do
-      before do
-        stub_feature_flags(cached_markdown_blob: false)
-      end
-
-      it 'does not set cache_key key' do
-        expect(subject.banzai_render_context.keys).not_to include(:cache_key)
-      end
-    end
   end
 end

spec/requests/api/services_spec.rb

@@ -76,7 +76,7 @@ RSpec.describe API::Services do
 
         required_attributes = service_attrs_list.select do |attr|
           service_klass.validators_on(attr).any? do |v|
-            v.class == ActiveRecord::Validations::PresenceValidator &&
+            v.instance_of?(ActiveRecord::Validations::PresenceValidator) &&
             # exclude presence validators with conditional since those are not really required
             ![:if, :unless].any? { |cond| v.options.include?(cond) }
           end

spec/services/releases/create_service_spec.rb

@@ -44,6 +44,21 @@ RSpec.describe Releases::CreateService do
 
     it_behaves_like 'a successful release creation'
 
+    context 'when tag is protected and user does not have access to it' do
+      let!(:protected_tag) { create(:protected_tag, :no_one_can_create, name: '*', project: project) }
+
+      it 'track the error event' do
+        stub_feature_flags(evalute_protected_tag_for_release_permissions: false)
+
+        expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
+          kind_of(described_class::ReleaseProtectedTagAccessError),
+          project_id: project.id,
+          user_id: user.id)
+
+        service.execute
+      end
+    end
+
     context 'when the tag does not exist' do
       let(:tag_name) { 'non-exist-tag' }
 

spec/services/releases/destroy_service_spec.rb

@@ -28,6 +28,21 @@ RSpec.describe Releases::DestroyService do
       it 'returns the destroyed object' do
         is_expected.to include(status: :success, release: release)
       end
+
+      context 'when tag is protected and user does not have access to it' do
+        let!(:protected_tag) { create(:protected_tag, :no_one_can_create, name: '*', project: project) }
+
+        it 'track the error event' do
+          stub_feature_flags(evalute_protected_tag_for_release_permissions: false)
+
+          expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
+            kind_of(described_class::ReleaseProtectedTagAccessError),
+            project_id: project.id,
+            user_id: user.id)
+
+          service.execute
+        end
+      end
     end
 
     context 'when tag does not exist in the repository' do

spec/services/releases/update_service_spec.rb

@@ -38,6 +38,21 @@ RSpec.describe Releases::UpdateService do
       service.execute
     end
 
+    context 'when tag is protected and user does not have access to it' do
+      let!(:protected_tag) { create(:protected_tag, :no_one_can_create, name: '*', project: project) }
+
+      it 'track the error event' do
+        stub_feature_flags(evalute_protected_tag_for_release_permissions: false)
+
+        expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
+          kind_of(described_class::ReleaseProtectedTagAccessError),
+          project_id: project.id,
+          user_id: user.id)
+
+        service.execute
+      end
+    end
+
     context 'when the tag does not exists' do
       let(:tag_name) { 'foobar' }
 

spec/support/helpers/cycle_analytics_helpers.rb

@@ -12,9 +12,7 @@ module CycleAnalyticsHelpers
     page.all('.gl-path-button').collect(&:text).map {|name_with_median| name_with_median.split("\n")[0] }
   end
 
-  def add_custom_stage_to_form
-    page.find_button(s_('CreateValueStreamForm|Add another stage')).click
-
+  def fill_in_custom_stage_fields
     index = page.all('[data-testid="value-stream-stage-fields"]').length
     last_stage = page.all('[data-testid="value-stream-stage-fields"]').last
 
@@ -25,6 +23,12 @@ module CycleAnalyticsHelpers
     end
   end
 
+  def add_custom_stage_to_form
+    page.find_button(s_('CreateValueStreamForm|Add another stage')).click
+
+    fill_in_custom_stage_fields
+  end
+
   def save_value_stream(custom_value_stream_name)
     fill_in 'create-value-stream-name', with: custom_value_stream_name
 

spec/support/shared_examples/lib/gitlab/import_export/relation_factory_shared_examples.rb

@@ -12,7 +12,7 @@ RSpec.shared_examples 'Notes user references' do
       'id' => 111,
       'access_level' => 30,
       'source_id' => 1,
-      'source_type' => importable.class.name == 'Project' ? 'Project' : 'Namespace',
+      'source_type' => importable.instance_of?(Project) ? 'Project' : 'Namespace',
       'user_id' => 3,
       'notification_level' => 3,
       'created_at' => '2016-11-18T09:29:42.634Z',