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

app/controllers/projects/wikis_controller.rb

@@ -46,7 +46,7 @@ class Projects::WikisController < Projects::ApplicationController
 
       render 'show'
     elsif file_blob
-      send_blob(@project_wiki.repository, file_blob, allow_caching: @project.public?)
+      send_blob(@project_wiki.repository, file_blob)
     elsif show_create_form?
       # Assign a title to the WikiPage unless `id` is a randomly generated slug from #new
       title = params[:id] unless params[:random_title].present?

app/finders/events_finder.rb

@@ -33,6 +33,8 @@ class EventsFinder
   end
 
   def execute
+    return Event.none if cannot_access_private_profile?
+
     events = get_events
 
     events = by_current_user_access(events)
@@ -102,6 +104,10 @@ class EventsFinder
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
+  def cannot_access_private_profile?
+    source.is_a?(User) && !Ability.allowed?(current_user, :read_user_profile, source)
+  end
+
   def sort(events)
     return events unless params[:sort]
 

app/models/merge_request.rb

@@ -515,7 +515,7 @@ class MergeRequest < ApplicationRecord
       participants << merge_user
     end
 
-    participants
+    participants.select { |participant| Ability.allowed?(participant, :read_merge_request, self) }
   end
 
   def first_commit

app/services/snippets/repository_validation_service.rb

+# frozen_string_literal: true
+
+module Snippets
+  class RepositoryValidationService
+    attr_reader :current_user, :snippet, :repository
+
+    RepositoryValidationError = Class.new(StandardError)
+
+    def initialize(user, snippet)
+      @current_user = user
+      @snippet = snippet
+      @repository = snippet.repository
+    end
+
+    def execute
+      if snippet.nil?
+        return service_response_error('No snippet found.', 404)
+      end
+
+      check_branch_count!
+      check_branch_name_default!
+      check_tag_count!
+      check_file_count!
+      check_size!
+
+      ServiceResponse.success(message: 'Valid snippet repository.')
+    rescue RepositoryValidationError => e
+      ServiceResponse.error(message: "Error: #{e.message}", http_status: 400)
+    end
+
+    private
+
+    def check_branch_count!
+      return if repository.branch_count == 1
+
+      raise RepositoryValidationError, _('Repository has more than one branch.')
+    end
+
+    def check_branch_name_default!
+      branches = repository.branch_names
+
+      return if branches.first == Gitlab::Checks::SnippetCheck::DEFAULT_BRANCH
+
+      raise RepositoryValidationError, _('Repository has an invalid default branch name.')
+    end
+
+    def check_tag_count!
+      return if repository.tag_count == 0
+
+      raise RepositoryValidationError, _('Repository has tags.')
+    end
+
+    def check_file_count!
+      file_count = repository.ls_files(nil).size
+      limit = Snippet.max_file_limit(current_user)
+
+      if file_count > limit
+        raise RepositoryValidationError, _('Repository files count over the limit')
+      end
+
+      if file_count == 0
+        raise RepositoryValidationError, _('Repository must contain at least 1 file.')
+      end
+    end
+
+    def check_size!
+      return unless snippet.repository_size_checker.above_size_limit?
+
+      raise RepositoryValidationError, _('Repository size is above the limit.')
+    end
+  end
+end

changelogs/unreleased/private-profile-api.yml

+---
+title: Do not show activity for users with private profiles
+merge_request:
+author:
+type: security

changelogs/unreleased/security-215175-filter-merge-participants.yml

+---
+title: Check access when sending TODOs related to merge requests
+merge_request:
+author:
+type: security

changelogs/unreleased/security-disable-caching-for-wiki-attachments.yml

+---
+title: Disable caching for wiki attachments
+merge_request:
+author:
+type: security

changelogs/unreleased/security-fj-add-snippet-repository-validation-bundle-import.yml

+---
+title: Add snippet repository validation after bundle import
+merge_request:
+author:
+type: security

lib/gitlab/import_export/snippet_repo_restorer.rb

@@ -3,7 +3,9 @@
 module Gitlab
   module ImportExport
     class SnippetRepoRestorer < RepoRestorer
-      attr_reader :snippet
+      attr_reader :snippet, :user
+
+      SnippetRepositoryError = Class.new(StandardError)
 
       def initialize(snippet:, user:, shared:, path_to_bundle:)
         @snippet = snippet
@@ -31,6 +33,16 @@ module Gitlab
       def create_repository_from_bundle
         repository.create_from_bundle(path_to_bundle)
         snippet.track_snippet_repository(repository.storage)
+
+        response = Snippets::RepositoryValidationService.new(user, snippet).execute
+
+        if response.error?
+          repository.remove
+          snippet.snippet_repository.delete
+          snippet.repository.expire_exists_cache
+
+          raise SnippetRepositoryError, _("Invalid repository bundle for snippet with id %{snippet_id}") % { snippet_id: snippet.id }
+        end
       end
 
       def create_repository_from_db

locale/gitlab.pot

@@ -11864,6 +11864,9 @@ msgstr ""
 msgid "Invalid query"
 msgstr ""
 
+msgid "Invalid repository bundle for snippet with id %{snippet_id}"
+msgstr ""
+
 msgid "Invalid repository path"
 msgstr ""
 
@@ -18085,15 +18088,33 @@ msgstr ""
 msgid "Repository cleanup has started. You will receive an email once the cleanup operation is complete."
 msgstr ""
 
+msgid "Repository files count over the limit"
+msgstr ""
+
+msgid "Repository has an invalid default branch name."
+msgstr ""
+
+msgid "Repository has more than one branch."
+msgstr ""
+
 msgid "Repository has no locks."
 msgstr ""
 
+msgid "Repository has tags."
+msgstr ""
+
 msgid "Repository maintenance"
 msgstr ""
 
 msgid "Repository mirroring"
 msgstr ""
 
+msgid "Repository must contain at least 1 file."
+msgstr ""
+
+msgid "Repository size is above the limit."
+msgstr ""
+
 msgid "Repository static objects"
 msgstr ""
 

spec/controllers/projects/wikis_controller_spec.rb

@@ -141,43 +141,19 @@ describe Projects::WikisController do
     context 'when page is a file' do
       include WikiHelpers
 
-      let(:id) { upload_file_to_wiki(project, user, file_name) }
+      where(:file_name) { ['dk.png', 'unsanitized.svg', 'git-cheat-sheet.pdf'] }
 
-      context 'when file is an image' do
-        let(:file_name) { 'dk.png' }
+      with_them do
+        let(:id) { upload_file_to_wiki(project, user, file_name) }
 
-        it 'delivers the image' do
+        it 'delivers the file with the correct headers' do
           subject
 
           expect(response.headers['Content-Disposition']).to match(/^inline/)
-          expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
+          expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq('true')
+          expect(response.cache_control[:public]).to be(false)
+          expect(response.cache_control[:extras]).to include('no-store')
         end
-
-        context 'when file is a svg' do
-          let(:file_name) { 'unsanitized.svg' }
-
-          it 'delivers the image' do
-            subject
-
-            expect(response.headers['Content-Disposition']).to match(/^inline/)
-            expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
-          end
-        end
-
-        it_behaves_like 'project cache control headers'
-      end
-
-      context 'when file is a pdf' do
-        let(:file_name) { 'git-cheat-sheet.pdf' }
-
-        it 'sets the content type to sets the content response headers' do
-          subject
-
-          expect(response.headers['Content-Disposition']).to match(/^inline/)
-          expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
-        end
-
-        it_behaves_like 'project cache control headers'
       end
     end
   end

spec/finders/events_finder_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 describe EventsFinder do
   let_it_be(:user) { create(:user) }
+  let(:private_user) { create(:user, private_profile: true) }
   let(:other_user) { create(:user) }
 
   let(:project1) { create(:project, :private, creator_id: user.id, namespace: user.namespace) }
@@ -57,6 +58,12 @@ describe EventsFinder do
 
       expect(events).to be_empty
     end
+
+    it 'returns nothing when the target profile is private' do
+      events = described_class.new(source: private_user, current_user: other_user).execute
+
+      expect(events).to be_empty
+    end
   end
 
   describe 'wiki events feature flag' do

spec/lib/gitlab/import_export/snippet_repo_restorer_spec.rb

@@ -4,9 +4,9 @@ require 'spec_helper'
 
 describe Gitlab::ImportExport::SnippetRepoRestorer do
   let_it_be(:user) { create(:user) }
-  let_it_be(:project) { create(:project, namespace: user.namespace) }
-  let(:snippet) { create(:project_snippet, project: project, author: user) }
 
+  let(:project) { create(:project, namespace: user.namespace) }
+  let(:snippet) { create(:project_snippet, project: project, author: user) }
   let(:shared) { project.import_export_shared }
   let(:exporter) { Gitlab::ImportExport::SnippetsRepoSaver.new(project: project, shared: shared, current_user: user) }
   let(:restorer) do
@@ -49,33 +49,63 @@ describe Gitlab::ImportExport::SnippetRepoRestorer do
     it_behaves_like 'no bundle file present'
   end
 
-  context 'when the snippet bundle exists' do
-    let!(:snippet_with_repo) { create(:project_snippet, :repository, project: project) }
+  context 'when the snippet repository bundle exists' do
+    let!(:snippet_with_repo) { create(:project_snippet, :repository, project: project, author: user) }
     let(:bundle_path) { ::Gitlab::ImportExport.snippets_repo_bundle_path(shared.export_path) }
     let(:snippet_bundle_path) { File.join(bundle_path, "#{snippet_with_repo.hexdigest}.bundle") }
     let(:result) { exporter.save }
+    let(:repository) { snippet.repository }
 
     before do
       expect(exporter.save).to be_truthy
     end
 
-    it 'creates the repository from the bundle' do
-      expect(snippet.repository_exists?).to be_falsey
-      expect(snippet.snippet_repository).to be_nil
-      expect(snippet.repository).to receive(:create_from_bundle).and_call_original
+    context 'when it is valid' do
+      before do
+        allow(repository).to receive(:branch_count).and_return(1)
+        allow(repository).to receive(:tag_count).and_return(0)
+        allow(repository).to receive(:branch_names).and_return(['master'])
+        allow(repository).to receive(:ls_files).and_return(['foo'])
+      end
 
-      expect(restorer.restore).to be_truthy
-      expect(snippet.repository_exists?).to be_truthy
-      expect(snippet.snippet_repository).not_to be_nil
-    end
+      it 'creates the repository from the bundle' do
+        expect(snippet.repository_exists?).to be_falsey
+        expect(snippet.snippet_repository).to be_nil
+        expect(repository).to receive(:create_from_bundle).and_call_original
+
+        expect(restorer.restore).to be_truthy
+        expect(snippet.repository_exists?).to be_truthy
+        expect(snippet.snippet_repository).not_to be_nil
+      end
 
-    it 'sets same shard in snippet repository as in the repository storage' do
-      expect(snippet).to receive(:repository_storage).and_return('picked')
-      expect(snippet.repository).to receive(:create_from_bundle)
+      it 'sets same shard in snippet repository as in the repository storage' do
+        expect(repository).to receive(:storage).and_return('picked')
+        expect(repository).to receive(:create_from_bundle)
 
-      restorer.restore
+        expect(restorer.restore).to be_truthy
+        expect(snippet.snippet_repository.shard_name).to eq 'picked'
+      end
+    end
 
-      expect(snippet.snippet_repository.shard_name).to eq 'picked'
+    context 'when it is invalid' do
+      it 'returns false and deletes the repository from disk and the database' do
+        gitlab_shell = Gitlab::Shell.new
+        shard_name = snippet.repository.shard
+        path = snippet.disk_path + '.git'
+        error_response = ServiceResponse.error(message: 'Foo', http_status: 400)
+
+        allow_next_instance_of(Snippets::RepositoryValidationService) do |instance|
+          allow(instance).to receive(:execute).and_return(error_response)
+        end
+
+        aggregate_failures do
+          expect(restorer.restore).to be false
+          expect(shared.errors.first).to match(/Invalid repository bundle/)
+          expect(snippet.repository_exists?).to eq false
+          expect(snippet.reload.snippet_repository).to be_nil
+          expect(gitlab_shell.repository_exists?(shard_name, path)).to eq false
+        end
+      end
     end
   end
 end

spec/models/merge_request_spec.rb

@@ -3566,7 +3566,7 @@ describe MergeRequest do
 
   describe '#merge_participants' do
     it 'contains author' do
-      expect(subject.merge_participants).to eq([subject.author])
+      expect(subject.merge_participants).to contain_exactly(subject.author)
     end
 
     describe 'when merge_when_pipeline_succeeds? is true' do
@@ -3580,8 +3580,20 @@ describe MergeRequest do
                  author: user)
         end
 
-        it 'contains author only' do
-          expect(subject.merge_participants).to eq([subject.author])
+        context 'author is not a project member' do
+          it 'is empty' do
+            expect(subject.merge_participants).to be_empty
+          end
+        end
+
+        context 'author is a project member' do
+          before do
+            subject.project.team.add_reporter(user)
+          end
+
+          it 'contains author only' do
+            expect(subject.merge_participants).to contain_exactly(subject.author)
+          end
         end
       end
 
@@ -3594,8 +3606,24 @@ describe MergeRequest do
                  merge_user: merge_user)
         end
 
-        it 'contains author and merge user' do
-          expect(subject.merge_participants).to eq([subject.author, merge_user])
+        before do
+          subject.project.team.add_reporter(subject.author)
+        end
+
+        context 'merge user is not a member' do
+          it 'contains author only' do
+            expect(subject.merge_participants).to contain_exactly(subject.author)
+          end
+        end
+
+        context 'both author and merge users are project members' do
+          before do
+            subject.project.team.add_reporter(merge_user)
+          end
+
+          it 'contains author and merge user' do
+            expect(subject.merge_participants).to contain_exactly(subject.author, merge_user)
+          end
         end
       end
     end

spec/requests/api/events_spec.rb

@@ -192,6 +192,19 @@ describe API::Events do
         end
       end
 
+      context 'when target users profile is private' do
+        it 'returns no events' do
+          user.update!(private_profile: true)
+          private_project.add_developer(non_member)
+
+          get api("/users/#{user.username}/events", non_member)
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to include_pagination_headers
+          expect(json_response).to eq([])
+        end
+      end
+
       context 'when scope is passed' do
         context 'when unauthenticated' do
           it 'returns no user events' do

spec/services/snippets/repository_validation_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Snippets::RepositoryValidationService do
+  describe '#execute' do
+    let_it_be(:user)    { create(:user) }
+    let_it_be(:snippet) { create(:personal_snippet, :empty_repo, author: user) }
+
+    let(:repository)    { snippet.repository }
+    let(:service)       { described_class.new(user, snippet) }
+
+    subject { service.execute }
+
+    before do
+      allow(repository).to receive(:branch_count).and_return(1)
+      allow(repository).to receive(:ls_files).and_return(['foo'])
+      allow(repository).to receive(:branch_names).and_return(['master'])
+    end
+
+    it 'returns error when the repository has more than one branch' do
+      allow(repository).to receive(:branch_count).and_return(2)
+
+      expect(subject).to be_error
+      expect(subject.message).to match /Repository has more than one branch/
+    end
+
+    it 'returns error when existing branch name is not the default one' do
+      allow(repository).to receive(:branch_names).and_return(['foo'])
+
+      expect(subject).to be_error
+      expect(subject.message).to match /Repository has an invalid default branch name/
+    end
+
+    it 'returns error when the repository has tags' do
+      allow(repository).to receive(:tag_count).and_return(1)
+
+      expect(subject).to be_error
+      expect(subject.message).to match /Repository has tags/
+    end
+
+    it 'returns error when the repository has more file than the limit' do
+      limit = Snippet.max_file_limit(user) + 1
+      files = Array.new(limit) { FFaker::Filesystem.file_name }
+      allow(repository).to receive(:ls_files).and_return(files)
+
+      expect(subject).to be_error
+      expect(subject.message).to match /Repository files count over the limit/
+    end
+
+    it 'returns error when the repository has no files' do
+      allow(repository).to receive(:ls_files).and_return([])
+
+      expect(subject).to be_error
+      expect(subject.message).to match /Repository must contain at least 1 file/
+    end
+
+    it 'returns error when the repository size is over the limit' do
+      expect_any_instance_of(Gitlab::RepositorySizeChecker).to receive(:above_size_limit?).and_return(true)
+
+      expect(subject).to be_error
+      expect(subject.message).to match /Repository size is above the limit/
+    end
+
+    it 'returns success when no validation errors are raised' do
+      expect(subject).to be_success
+    end
+  end
+end