Add latest changes from gitlab-org/security/gitlab@13-6-stable-ee

17 files changed, 220 insertions, 45 deletions

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 8707d635e03..3778add83bf 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -15,6 +15,7 @@ module Ci
     include ShaAttribute
     include FromUnion
     include UpdatedAtFilterable
+    include EachBatch
 
     PROJECT_ROUTE_AND_NAMESPACE_ROUTE = {
       project: [:project_feature, :route, { namespace: :route }]
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index ee9c2501bfc..f9de9dec072 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -55,6 +55,7 @@ class CommitStatus < ApplicationRecord
   scope :for_ids, -> (ids) { where(id: ids) }
   scope :for_ref, -> (ref) { where(ref: ref) }
   scope :by_name, -> (name) { where(name: name) }
+  scope :in_pipelines, ->(pipelines) { where(pipeline: pipelines) }
 
   scope :for_project_paths, -> (paths) do
     where(project: Project.where_full_path_in(Array(paths)))
diff --git a/app/services/ci/abort_project_pipelines_service.rb b/app/services/ci/abort_project_pipelines_service.rb
new file mode 100644
index 00000000000..d4f228e37a1
--- /dev/null
+++ b/app/services/ci/abort_project_pipelines_service.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+module Ci
+  class AbortProjectPipelinesService
+    # Danger: Cancels in bulk without callbacks
+    # Only for pipeline abandonment scenarios (current example: project delete)
+    def execute(project)
+      return unless Feature.enabled?(:abort_deleted_project_pipelines, default_enabled: true)
+
+      pipelines = project.all_pipelines.cancelable
+      bulk_abort!(pipelines, status: :canceled)
+
+      ServiceResponse.success(message: 'Pipelines canceled')
+    end
+
+    private
+
+    def bulk_abort!(pipelines, status:)
+      pipelines.each_batch do |pipeline_batch|
+        CommitStatus.in_pipelines(pipeline_batch).in_batches.update_all(status: status) # rubocop: disable Cop/InBatches
+        pipeline_batch.update_all(status: status)
+      end
+    end
+  end
+end
diff --git a/app/services/ci/cancel_user_pipelines_service.rb b/app/services/ci/cancel_user_pipelines_service.rb
index 3a8b5e91088..3d3a8032e8e 100644
--- a/app/services/ci/cancel_user_pipelines_service.rb
+++ b/app/services/ci/cancel_user_pipelines_service.rb
@@ -6,6 +6,7 @@ module Ci
     # This is a bug with CodeReuse/ActiveRecord cop
     # https://gitlab.com/gitlab-org/gitlab/issues/32332
     def execute(user)
+      # TODO: fix N+1 queries https://gitlab.com/gitlab-org/gitlab/-/issues/300685
       user.pipelines.cancelable.find_each(&:cancel_running)
 
       ServiceResponse.success(message: 'Pipeline canceled')
diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index bec75657530..c1501625300 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -21,11 +21,14 @@ module Projects
     def execute
       return false unless can?(current_user, :remove_project, project)
 
+      project.update_attribute(:pending_delete, true)
       # Flush the cache for both repositories. This has to be done _before_
       # removing the physical repositories as some expiration code depends on
       # Git data (e.g. a list of branch names).
       flush_caches(project)
 
+      ::Ci::AbortProjectPipelinesService.new.execute(project)
+
       Projects::UnlinkForkService.new(project, current_user).execute
 
       attempt_destroy(project)
diff --git a/changelogs/unreleased/security-cancel-pipelines-for-deleted-project.yml b/changelogs/unreleased/security-cancel-pipelines-for-deleted-project.yml
new file mode 100644
index 00000000000..de92707cb8f
--- /dev/null
+++ b/changelogs/unreleased/security-cancel-pipelines-for-deleted-project.yml
@@ -0,0 +1,5 @@
+---
+title: Cancel running and pending jobs when a project is deleted
+merge_request: 1220
+author:
+type: security
diff --git a/changelogs/unreleased/security-confidential-titles.yml b/changelogs/unreleased/security-confidential-titles.yml
new file mode 100644
index 00000000000..506cbc095c4
--- /dev/null
+++ b/changelogs/unreleased/security-confidential-titles.yml
@@ -0,0 +1,5 @@
+---
+title: Prevent exposure of confidential issue titles in file browser
+merge_request:
+author:
+type: security
diff --git a/config/feature_flags/development/abort_deleted_project_pipelines.yml b/config/feature_flags/development/abort_deleted_project_pipelines.yml
new file mode 100644
index 00000000000..f09cc9dd86b
--- /dev/null
+++ b/config/feature_flags/development/abort_deleted_project_pipelines.yml
@@ -0,0 +1,8 @@
+---
+name: abort_deleted_project_pipelines
+introduced_by_url: https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/1220
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/301106
+milestone: '13.9'
+type: development
+group: group::continuous integration
+default_enabled: true
diff --git a/config/feature_flags/development/codequality_mr_diff.yml b/config/feature_flags/development/codequality_mr_diff.yml
deleted file mode 100644
index ca6846b9390..00000000000
--- a/config/feature_flags/development/codequality_mr_diff.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: codequality_mr_diff
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47938
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/284140
-milestone: '13.7'
-type: development
-group: group::testing
-default_enabled: false
diff --git a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb
index 8f1e690c081..4f90b7756eb 100644
--- a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb
+++ b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb
@@ -10,6 +10,10 @@ module Gitlab
             include Chain::Helpers
 
             def perform!
+              if project.pending_delete?
+                return error('Project is deleted!')
+              end
+
               unless project.builds_enabled?
                 return error('Pipelines are disabled!')
               end
diff --git a/lib/gitlab/tree_summary.rb b/lib/gitlab/tree_summary.rb
index 9b67599668a..bc7b8bd2b94 100644
--- a/lib/gitlab/tree_summary.rb
+++ b/lib/gitlab/tree_summary.rb
@@ -40,21 +40,17 @@ module Gitlab
     #     - An Array of the unique ::Commit objects in the first value
     def summarize
       summary = contents
-        .map { |content| build_entry(content) }
         .tap { |summary| fill_last_commits!(summary) }
 
       [summary, commits]
     end
 
     def fetch_logs
-      cache_key = ['projects', project.id, 'logs', commit.id, path, offset]
-      Rails.cache.fetch(cache_key, expires_in: CACHE_EXPIRE_IN) do
-        logs, _ = summarize
+      logs, _ = summarize
 
-        new_offset = next_offset if more?
+      new_offset = next_offset if more?
 
-        [logs.as_json, new_offset]
-      end
+      [logs.as_json, new_offset]
     end
 
     # Does the tree contain more entries after the given offset + limit?
@@ -71,7 +67,7 @@ module Gitlab
     private
 
     def contents
-      all_contents[offset, limit]
+      all_contents[offset, limit] || []
     end
 
     def commits
@@ -82,22 +78,17 @@ module Gitlab
       project.repository
     end
 
-    def entry_path(entry)
-      File.join(*[path, entry[:file_name]].compact).force_encoding(Encoding::ASCII_8BIT)
+    # Ensure the path is in "path/" format
+    def ensured_path
+      File.join(*[path, ""]) if path
     end
 
-    def build_entry(entry)
-      { file_name: entry.name, type: entry.type }
+    def entry_path(entry)
+      File.join(*[path, entry[:file_name]].compact).force_encoding(Encoding::ASCII_8BIT)
     end
 
     def fill_last_commits!(entries)
-      # Ensure the path is in "path/" format
-      ensured_path =
-        if path
-          File.join(*[path, ""])
-        end
-
-      commits_hsh = repository.list_last_commits_for_tree(commit.id, ensured_path, offset: offset, limit: limit, literal_pathspec: true)
+      commits_hsh = fetch_last_cached_commits_list
       prerender_commit_full_titles!(commits_hsh.values)
 
       entries.each do |entry|
@@ -112,6 +103,18 @@ module Gitlab
       end
     end
 
+    def fetch_last_cached_commits_list
+      cache_key = ['projects', project.id, 'last_commits_list', commit.id, ensured_path, offset, limit]
+
+      commits = Rails.cache.fetch(cache_key, expires_in: CACHE_EXPIRE_IN) do
+        repository
+          .list_last_commits_for_tree(commit.id, ensured_path, offset: offset, limit: limit, literal_pathspec: true)
+          .transform_values!(&:to_hash)
+      end
+
+      commits.transform_values! { |value| Commit.from_hash(value, project) }
+    end
+
     def cache_commit(commit)
       return unless commit.present?
 
@@ -123,12 +126,18 @@ module Gitlab
     end
 
     def all_contents
-      strong_memoize(:all_contents) do
+      strong_memoize(:all_contents) { cached_contents }
+    end
+
+    def cached_contents
+      cache_key = ['projects', project.id, 'content', commit.id, path]
+
+      Rails.cache.fetch(cache_key, expires_in: CACHE_EXPIRE_IN) do
         [
           *tree.trees,
           *tree.blobs,
           *tree.submodules
-        ]
+        ].map { |entry| { file_name: entry.name, type: entry.type } }
       end
     end
 
diff --git a/spec/controllers/projects/refs_controller_spec.rb b/spec/controllers/projects/refs_controller_spec.rb
index d10351feb9e..b625ce35d61 100644
--- a/spec/controllers/projects/refs_controller_spec.rb
+++ b/spec/controllers/projects/refs_controller_spec.rb
@@ -56,18 +56,6 @@ RSpec.describe Projects::RefsController do
         expect(response).to be_successful
         expect(json_response).to be_kind_of(Array)
       end
-
-      it 'caches tree summary data', :use_clean_rails_memory_store_caching do
-        expect_next_instance_of(::Gitlab::TreeSummary) do |instance|
-          expect(instance).to receive_messages(summarize: ['logs'], next_offset: 50, more?: true)
-        end
-
-        xhr_get(:json, offset: 25)
-
-        cache_key = "projects/#{project.id}/logs/#{project.commit.id}/#{path}/25"
-        expect(Rails.cache.fetch(cache_key)).to eq(['logs', 50])
-        expect(response.headers['More-Logs-Offset']).to eq("50")
-      end
     end
   end
 end
diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb
index ae3270cb9b2..7aaeee32f49 100644
--- a/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb
@@ -74,6 +74,14 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Validate::Abilities do
     it 'does not break the chain' do
       expect(step.break?).to eq false
     end
+
+    context 'when project is deleted' do
+      before do
+        project.update!(pending_delete: true)
+      end
+
+      specify { expect(step.perform!).to contain_exactly('Project is deleted!') }
+    end
   end
 
   describe '#allowed_to_write_ref?' do
diff --git a/spec/lib/gitlab/tree_summary_spec.rb b/spec/lib/gitlab/tree_summary_spec.rb
index 303a4a80581..d2c5844b0fa 100644
--- a/spec/lib/gitlab/tree_summary_spec.rb
+++ b/spec/lib/gitlab/tree_summary_spec.rb
@@ -3,6 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::TreeSummary do
+  include RepoHelpers
   using RSpec::Parameterized::TableSyntax
 
   let(:project) { create(:project, :empty_repo) }
@@ -44,6 +45,40 @@ RSpec.describe Gitlab::TreeSummary do
         expect(commits).to match_array(entries.map { |entry| entry[:commit] })
       end
     end
+
+    context 'when offset is over the limit' do
+      let(:offset) { 100 }
+
+      it 'returns an empty array' do
+        expect(summarized).to eq([[], []])
+      end
+    end
+
+    context 'with caching', :use_clean_rails_memory_store_caching do
+      subject { Rails.cache.fetch(key) }
+
+      before do
+        summarized
+      end
+
+      context 'Repository tree cache' do
+        let(:key) { ['projects', project.id, 'content', commit.id, path] }
+
+        it 'creates a cache for repository content' do
+          is_expected.to eq([{ file_name: 'a.txt', type: :blob }])
+        end
+      end
+
+      context 'Commits list cache' do
+        let(:offset) { 0 }
+        let(:limit) { 25 }
+        let(:key) { ['projects', project.id, 'last_commits_list', commit.id, path, offset, limit] }
+
+        it 'creates a cache for commits list' do
+          is_expected.to eq('a.txt' => commit.to_hash)
+        end
+      end
+    end
   end
 
   describe '#summarize (entries)' do
@@ -167,6 +202,46 @@ RSpec.describe Gitlab::TreeSummary do
     end
   end
 
+  describe 'References in commit messages' do
+    let_it_be(:project) { create(:project, :empty_repo) }
+    let_it_be(:issue) { create(:issue, project: project) }
+    let(:entries) { summary.summarize.first }
+    let(:entry) { entries.find { |entry| entry[:file_name] == 'issue.txt' } }
+
+    before_all do
+      create_file_in_repo(project, 'master', 'master', 'issue.txt', '', commit_message: "Issue ##{issue.iid}")
+    end
+
+    where(:project_visibility, :user_role, :issue_confidential, :expected_result) do
+      'private'  | :guest    | false | true
+      'private'  | :guest    | true  | false
+      'private'  | :reporter | false | true
+      'private'  | :reporter | true  | true
+
+      'internal' | :guest    | false | true
+      'internal' | :guest    | true  | false
+      'internal' | :reporter | false | true
+      'internal' | :reporter | true  | true
+
+      'public'   | :guest    | false | true
+      'public'   | :guest    | true  | false
+      'public'   | :reporter | false | true
+      'public'   | :reporter | true  | true
+    end
+
+    with_them do
+      subject { entry[:commit_title_html].include?("title=\"#{issue.title}\"") }
+
+      before do
+        project.add_role(user, user_role)
+        project.update!(visibility_level: Gitlab::VisibilityLevel.level_value(project_visibility))
+        issue.update!(confidential: issue_confidential)
+      end
+
+      it { is_expected.to eq(expected_result) }
+    end
+  end
+
   describe '#more?' do
     let(:path) { 'tmp/more' }
 
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 1ca370dc950..4c26cc09656 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -34,6 +34,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
   it { is_expected.to have_many(:auto_canceled_jobs) }
   it { is_expected.to have_many(:sourced_pipelines) }
   it { is_expected.to have_many(:triggered_pipelines) }
+  it { is_expected.to have_many(:pipeline_artifacts) }
 
   it { is_expected.to have_one(:chat_data) }
   it { is_expected.to have_one(:source_pipeline) }
@@ -41,14 +42,15 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
   it { is_expected.to have_one(:source_job) }
   it { is_expected.to have_one(:pipeline_config) }
 
-  it { is_expected.to validate_presence_of(:sha) }
-  it { is_expected.to validate_presence_of(:status) }
-
   it { is_expected.to respond_to :git_author_name }
   it { is_expected.to respond_to :git_author_email }
   it { is_expected.to respond_to :short_sha }
   it { is_expected.to delegate_method(:full_path).to(:project).with_prefix }
-  it { is_expected.to have_many(:pipeline_artifacts) }
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:sha) }
+    it { is_expected.to validate_presence_of(:status) }
+  end
 
   describe 'associations' do
     it 'has a bidirectional relationship with projects' do
diff --git a/spec/services/ci/abort_project_pipelines_service_spec.rb b/spec/services/ci/abort_project_pipelines_service_spec.rb
new file mode 100644
index 00000000000..9af909ac2ab
--- /dev/null
+++ b/spec/services/ci/abort_project_pipelines_service_spec.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::AbortProjectPipelinesService do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:pipeline) { create(:ci_pipeline, :running, project: project) }
+  let_it_be(:build) { create(:ci_build, :running, pipeline: pipeline) }
+
+  describe '#execute' do
+    it 'cancels all running pipelines and related jobs' do
+      result = described_class.new.execute(project)
+
+      expect(result).to be_success
+      expect(pipeline.reload).to be_canceled
+      expect(build.reload).to be_canceled
+    end
+
+    it 'avoids N+1 queries' do
+      control_count = ActiveRecord::QueryRecorder.new { described_class.new.execute(project) }.count
+
+      pipelines = create_list(:ci_pipeline, 5, :running, project: project)
+      create_list(:ci_build, 5, :running, pipeline: pipelines.first)
+
+      expect { described_class.new.execute(project) }.not_to exceed_query_limit(control_count)
+    end
+  end
+
+  context 'when feature disabled' do
+    before do
+      stub_feature_flags(abort_deleted_project_pipelines: false)
+    end
+
+    it 'does not abort the pipeline' do
+      result = described_class.new.execute(project)
+
+      expect(result).to be(nil)
+      expect(pipeline.reload).to be_running
+      expect(build.reload).to be_running
+    end
+  end
+end
diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb
index f0f09218b06..75d1c98923a 100644
--- a/spec/services/projects/destroy_service_spec.rb
+++ b/spec/services/projects/destroy_service_spec.rb
@@ -69,6 +69,12 @@ RSpec.describe Projects::DestroyService, :aggregate_failures do
       destroy_project(project, user, {})
     end
 
+    it 'performs cancel for project ci pipelines' do
+      expect(::Ci::AbortProjectPipelinesService).to receive_message_chain(:new, :execute).with(project)
+
+      destroy_project(project, user, {})
+    end
+
     context 'when project has remote mirrors' do
       let!(:project) do
         create(:project, :repository, namespace: user.namespace).tap do |project|