Add latest changes from gitlab-org/security/gitlab@16-4-stable-ee

10 files changed, 276 insertions, 13 deletions

diff --git a/app/services/ci/list_config_variables_service.rb b/app/services/ci/list_config_variables_service.rb
index 1020e98f463..e028d7252ae 100644
--- a/app/services/ci/list_config_variables_service.rb
+++ b/app/services/ci/list_config_variables_service.rb
@@ -32,7 +32,8 @@ module Ci
         config.content,
         project: project,
         user: current_user,
-        sha: sha
+        sha: sha,
+        verify_project_sha: true
       ).execute
 
       result.valid? ? result.root_variables_with_prefill_data : {}
diff --git a/lib/gitlab/checks/tag_check.rb b/lib/gitlab/checks/tag_check.rb
index 5c43ca946b5..4505bcb5411 100644
--- a/lib/gitlab/checks/tag_check.rb
+++ b/lib/gitlab/checks/tag_check.rb
@@ -39,6 +39,10 @@ module Gitlab
       def prohibited_tag_checks
         return if deletion?
 
+        unless Gitlab::GitRefValidator.validate(tag_name)
+          raise GitAccess::ForbiddenError, ERROR_MESSAGES[:prohibited_tag_name]
+        end
+
         if tag_name.start_with?("refs/tags/") # rubocop: disable Style/GuardClause
           raise GitAccess::ForbiddenError, ERROR_MESSAGES[:prohibited_tag_name]
         end
diff --git a/lib/gitlab/ci/lint.rb b/lib/gitlab/ci/lint.rb
index e0112a1b1c2..54861e2769e 100644
--- a/lib/gitlab/ci/lint.rb
+++ b/lib/gitlab/ci/lint.rb
@@ -28,6 +28,9 @@ module Gitlab
       def initialize(project:, current_user:, sha: nil)
         @project = project
         @current_user = current_user
+        # If the `sha` is not provided, the default is the project's head commit (or nil). In such case, we
+        # don't need to call `YamlProcessor.verify_project_sha!`, which prevents redundant calls to Gitaly.
+        @verify_project_sha = sha.present?
         @sha = sha || project&.repository&.commit&.sha
       end
 
@@ -77,6 +80,7 @@ module Gitlab
           Gitlab::Ci::YamlProcessor.new(content, project: @project,
                                                  user: @current_user,
                                                  sha: @sha,
+                                                 verify_project_sha: @verify_project_sha,
                                                  logger: logger).execute
         end
       end
diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb
index 3a0173d1548..289f41b4ec7 100644
--- a/lib/gitlab/ci/yaml_processor.rb
+++ b/lib/gitlab/ci/yaml_processor.rb
@@ -8,6 +8,8 @@
 module Gitlab
   module Ci
     class YamlProcessor
+      include Gitlab::Utils::StrongMemoize
+
       ValidationError = Class.new(StandardError)
 
       def initialize(config_content, opts = {})
@@ -28,6 +30,8 @@ module Gitlab
           return Result.new(errors: ['Please provide content of .gitlab-ci.yml'])
         end
 
+        verify_project_sha! if verify_project_sha?
+
         @ci_config = Gitlab::Ci::Config.new(@config_content, **@opts)
 
         unless @ci_config.valid?
@@ -47,6 +51,15 @@ module Gitlab
         @opts[:project]
       end
 
+      def sha
+        @opts[:sha]
+      end
+
+      def verify_project_sha?
+        @opts.delete(:verify_project_sha) || false
+      end
+      strong_memoize_attr :verify_project_sha?
+
       def run_logical_validations!
         @stages = @ci_config.stages
         @jobs = @ci_config.normalized_jobs
@@ -191,6 +204,24 @@ module Gitlab
       def error!(message)
         raise ValidationError, message
       end
+
+      def verify_project_sha!
+        return unless project && sha && project.repository_exists? && project.commit(sha)
+
+        unless project_ref_contains_sha?
+          error!('Could not validate configuration. Config originates from external project')
+        end
+      end
+
+      def project_ref_contains_sha?
+        # A 5-minute cache TTL is sufficient to prevent Gitaly load issues while also mitigating rare
+        # use cases concerning stale data. For example, when an external commit gets merged into the
+        # project, there may be at most a 5-minute window where the `sha` is still considered external.
+        Rails.cache.fetch(['project', project.id, 'ref/contains/sha', sha], expires_in: 5.minutes) do
+          repo = project.repository
+          repo.branch_names_contains(sha, limit: 1).any? || repo.tag_names_contains(sha, limit: 1).any?
+        end
+      end
     end
   end
 end
diff --git a/spec/lib/gitlab/checks/tag_check_spec.rb b/spec/lib/gitlab/checks/tag_check_spec.rb
index e75b0459337..60d3eb4bfb3 100644
--- a/spec/lib/gitlab/checks/tag_check_spec.rb
+++ b/spec/lib/gitlab/checks/tag_check_spec.rb
@@ -15,6 +15,12 @@ RSpec.describe Gitlab::Checks::TagCheck, feature_category: :source_code_manageme
     end
 
     context "prohibited tags check" do
+      it 'prohibits tags name that include refs/heads at the head' do
+        allow(subject).to receive(:tag_name).and_return("refs/heads/foo")
+
+        expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "You cannot create a tag with a prohibited pattern.")
+      end
+
       it "prohibits tag names that include refs/tags/ at the head" do
         allow(subject).to receive(:tag_name).and_return("refs/tags/foo")
 
diff --git a/spec/lib/gitlab/ci/lint_spec.rb b/spec/lib/gitlab/ci/lint_spec.rb
index b238e9161eb..4196aad2db4 100644
--- a/spec/lib/gitlab/ci/lint_spec.rb
+++ b/spec/lib/gitlab/ci/lint_spec.rb
@@ -6,8 +6,9 @@ RSpec.describe Gitlab::Ci::Lint, feature_category: :pipeline_composition do
   let_it_be(:project) { create(:project, :repository) }
   let_it_be(:user) { create(:user) }
 
-  let(:lint) { described_class.new(project: project, current_user: user) }
+  let(:sha) { nil }
   let(:ref) { project.default_branch }
+  let(:lint) { described_class.new(project: project, current_user: user, sha: sha) }
 
   describe '#validate' do
     subject { lint.validate(content, dry_run: dry_run, ref: ref) }
@@ -250,6 +251,59 @@ RSpec.describe Gitlab::Ci::Lint, feature_category: :pipeline_composition do
 
           subject
         end
+
+        context 'when sha is provided' do
+          let(:sha) { project.commit.sha }
+
+          it 'runs YamlProcessor with verify_project_sha: true' do
+            expect(Gitlab::Ci::YamlProcessor)
+              .to receive(:new)
+              .with(content, a_hash_including(verify_project_sha: true))
+              .and_call_original
+
+            subject
+          end
+
+          it_behaves_like 'content is valid'
+
+          context 'when the sha is invalid' do
+            let(:sha) { 'invalid-sha' }
+
+            it_behaves_like 'content is valid'
+          end
+
+          context 'when the sha is from a fork' do
+            include_context 'when a project repository contains a forked commit'
+
+            let(:sha) { forked_commit_sha }
+
+            context 'when a project ref contains the sha' do
+              before do
+                mock_branch_contains_forked_commit_sha
+              end
+
+              it_behaves_like 'content is valid'
+            end
+
+            context 'when a project ref does not contain the sha' do
+              it 'returns an error' do
+                expect(subject).not_to be_valid
+                expect(subject.errors).to include(/Could not validate configuration/)
+              end
+            end
+          end
+        end
+
+        context 'when sha is not provided' do
+          it 'runs YamlProcessor with verify_project_sha: false' do
+            expect(Gitlab::Ci::YamlProcessor)
+              .to receive(:new)
+              .with(content, a_hash_including(verify_project_sha: false))
+              .and_call_original
+
+            subject
+          end
+        end
       end
 
       context 'when using dry run mode' do
diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb
index c09c0b31e97..5cfd8d9b9fb 100644
--- a/spec/lib/gitlab/ci/yaml_processor_spec.rb
+++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -3607,6 +3607,88 @@ module Gitlab
           end
         end
       end
+
+      describe 'verify project sha', :use_clean_rails_redis_caching do
+        include_context 'when a project repository contains a forked commit'
+
+        let(:config) { YAML.dump(job: { script: 'echo' }) }
+        let(:verify_project_sha) { true }
+        let(:sha) { forked_commit_sha }
+
+        let(:processor) { described_class.new(config, project: project, sha: sha, verify_project_sha: verify_project_sha) }
+
+        subject { processor.execute }
+
+        shared_examples 'when the processor is executed twice consecutively' do |branch_names_contains_sha = false|
+          it 'calls Gitaly only once for each ref type' do
+            expect(repository).to receive(:branch_names_contains).once.and_call_original
+            expect(repository).to receive(:tag_names_contains).once.and_call_original unless branch_names_contains_sha
+
+            2.times { processor.execute }
+          end
+        end
+
+        context 'when a project branch contains the forked commit sha' do
+          before_all do
+            repository.add_branch(project.owner, 'branch1', forked_commit_sha)
+          end
+
+          after(:all) do
+            repository.rm_branch(project.owner, 'branch1')
+          end
+
+          it { is_expected.to be_valid }
+
+          it_behaves_like 'when the processor is executed twice consecutively', true
+        end
+
+        context 'when a project tag contains the forked commit sha' do
+          before_all do
+            repository.add_tag(project.owner, 'tag1', forked_commit_sha)
+          end
+
+          after(:all) do
+            repository.rm_tag(project.owner, 'tag1')
+          end
+
+          it { is_expected.to be_valid }
+
+          it_behaves_like 'when the processor is executed twice consecutively'
+        end
+
+        context 'when a project ref does not contain the forked commit sha' do
+          it 'returns an error' do
+            is_expected.not_to be_valid
+            expect(subject.errors).to include(/Could not validate configuration/)
+          end
+
+          it_behaves_like 'when the processor is executed twice consecutively'
+        end
+
+        context 'when verify_project_sha option is false' do
+          let(:verify_project_sha) { false }
+
+          it { is_expected.to be_valid }
+        end
+
+        context 'when project is not provided' do
+          let(:project) { nil }
+
+          it { is_expected.to be_valid }
+        end
+
+        context 'when sha is not provided' do
+          let(:sha) { nil }
+
+          it { is_expected.to be_valid }
+        end
+
+        context 'when sha is invalid' do
+          let(:sha) { 'invalid-sha' }
+
+          it { is_expected.to be_valid }
+        end
+      end
     end
   end
 end
diff --git a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb
index 803614d90a5..36b75e5338a 100644
--- a/spec/models/blob_viewer/gitlab_ci_yml_spec.rb
+++ b/spec/models/blob_viewer/gitlab_ci_yml_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe BlobViewer::GitlabCiYml do
+RSpec.describe BlobViewer::GitlabCiYml, feature_category: :source_code_management do
   include FakeBlobHelpers
   include RepoHelpers
 
@@ -13,18 +13,20 @@ RSpec.describe BlobViewer::GitlabCiYml do
   let(:blob) { fake_blob(path: '.gitlab-ci.yml', data: data) }
   let(:sha) { sample_commit.id }
 
-  subject { described_class.new(blob) }
+  subject(:blob_viewer) { described_class.new(blob) }
 
   describe '#validation_message' do
+    subject(:validation_message) { blob_viewer.validation_message(project: project, sha: sha, user: user) }
+
     it 'calls prepare! on the viewer' do
-      expect(subject).to receive(:prepare!)
+      expect(blob_viewer).to receive(:prepare!)
 
-      subject.validation_message(project: project, sha: sha, user: user)
+      validation_message
     end
 
     context 'when the configuration is valid' do
       it 'returns nil' do
-        expect(subject.validation_message(project: project, sha: sha, user: user)).to be_nil
+        expect(validation_message).to be_nil
       end
     end
 
@@ -32,7 +34,29 @@ RSpec.describe BlobViewer::GitlabCiYml do
       let(:data) { 'oof' }
 
       it 'returns the error message' do
-        expect(subject.validation_message(project: project, sha: sha, user: user)).to eq('Invalid configuration format')
+        expect(validation_message).to eq('Invalid configuration format')
+      end
+    end
+
+    context 'when the sha is from a fork' do
+      include_context 'when a project repository contains a forked commit'
+
+      let(:sha) { forked_commit_sha }
+
+      context 'when a project ref contains the sha' do
+        before do
+          mock_branch_contains_forked_commit_sha
+        end
+
+        it 'returns nil' do
+          expect(validation_message).to be_nil
+        end
+      end
+
+      context 'when a project ref does not contain the sha' do
+        it 'returns an error' do
+          expect(validation_message).to match(/Could not validate configuration/)
+        end
       end
     end
   end
diff --git a/spec/services/ci/list_config_variables_service_spec.rb b/spec/services/ci/list_config_variables_service_spec.rb
index 07c9085b83a..ccc8b77f6b2 100644
--- a/spec/services/ci/list_config_variables_service_spec.rb
+++ b/spec/services/ci/list_config_variables_service_spec.rb
@@ -32,15 +32,50 @@ RSpec.describe Ci::ListConfigVariablesService,
       }
     end
 
+    let(:expected_result) do
+      {
+        'KEY1' => { value: 'val 1', description: 'description 1' },
+        'KEY2' => { value: 'val 2', description: '' },
+        'KEY3' => { value: 'val 3' },
+        'KEY4' => { value: 'val 4' }
+      }
+    end
+
     before do
       synchronous_reactive_cache(service)
     end
 
-    it 'returns variable list' do
-      expect(result['KEY1']).to eq({ value: 'val 1', description: 'description 1' })
-      expect(result['KEY2']).to eq({ value: 'val 2', description: '' })
-      expect(result['KEY3']).to eq({ value: 'val 3' })
-      expect(result['KEY4']).to eq({ value: 'val 4' })
+    it 'returns variables list' do
+      expect(result).to eq(expected_result)
+    end
+
+    context 'when the ref is a sha from a fork' do
+      include_context 'when a project repository contains a forked commit'
+
+      before do
+        allow_next_instance_of(Gitlab::Ci::ProjectConfig) do |instance|
+          allow(instance).to receive(:exists?).and_return(true)
+          allow(instance).to receive(:content).and_return(YAML.dump(ci_config))
+        end
+      end
+
+      let(:ref) { forked_commit_sha }
+
+      context 'when a project ref contains the sha' do
+        before do
+          mock_branch_contains_forked_commit_sha
+        end
+
+        it 'returns variables list' do
+          expect(result).to eq(expected_result)
+        end
+      end
+
+      context 'when a project ref does not contain the sha' do
+        it 'returns empty response' do
+          expect(result).to eq({})
+        end
+      end
     end
   end
 
diff --git a/spec/support/shared_contexts/ci/yaml_processor_shared_context.rb b/spec/support/shared_contexts/ci/yaml_processor_shared_context.rb
new file mode 100644
index 00000000000..3b4d6fcef6f
--- /dev/null
+++ b/spec/support/shared_contexts/ci/yaml_processor_shared_context.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+RSpec.shared_context 'when a project repository contains a forked commit' do
+  include ProjectForksHelper
+
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:repository) { project.repository }
+  let_it_be(:forked_project) { fork_project(project, project.owner, repository: true) }
+
+  let_it_be(:forked_commit_sha) do
+    forked_project.repository.create_file(project.owner, 'file.txt', 'file', message: 'test', branch_name: 'master')
+  end
+
+  before_all do
+    # Creating this MR moves the forked commit to the project repository
+    create(:merge_request, source_project: forked_project, target_project: project)
+  end
+
+  def mock_branch_contains_forked_commit_sha
+    allow(repository).to receive(:branch_names_contains).with(forked_commit_sha, limit: 1).and_return(['branch1'])
+  end
+end