Add latest changes from gitlab-org/security/gitlab@15-5-stable-ee

6 files changed, 84 insertions, 69 deletions

diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb
index 2a8f7171f9c..01f7bb9e2cf 100644
--- a/app/controllers/projects/pipelines_controller.rb
+++ b/app/controllers/projects/pipelines_controller.rb
@@ -239,8 +239,7 @@ class Projects::PipelinesController < Projects::ApplicationController
   def config_variables
     respond_to do |format|
       format.json do
-        project = @project.uses_external_project_ci_config? ? @project.ci_config_external_project : @project
-        result = Ci::ListConfigVariablesService.new(project, current_user).execute(params[:sha])
+        result = Ci::ListConfigVariablesService.new(@project, current_user).execute(params[:sha])
 
         result.nil? ? head(:no_content) : render(json: result)
       end
diff --git a/app/services/ci/list_config_variables_service.rb b/app/services/ci/list_config_variables_service.rb
index c791a89b804..3890882b3d4 100644
--- a/app/services/ci/list_config_variables_service.rb
+++ b/app/services/ci/list_config_variables_service.rb
@@ -22,12 +22,13 @@ module Ci
     end
 
     def calculate_reactive_cache(sha)
-      config = project.ci_config_for(sha)
-      return {} unless config
+      config = ::Gitlab::Ci::ProjectConfig.new(project: project, sha: sha)
 
-      result = Gitlab::Ci::YamlProcessor.new(config, project: project,
-                                                     user: current_user,
-                                                     sha: sha).execute
+      return {} unless config.exists?
+
+      result = Gitlab::Ci::YamlProcessor.new(config.content, project: project,
+                                                             user: current_user,
+                                                             sha: sha).execute
 
       result.valid? ? result.variables_with_data : {}
     end
diff --git a/lib/api/ci/secure_files.rb b/lib/api/ci/secure_files.rb
index 68431df203b..511b6e06cd3 100644
--- a/lib/api/ci/secure_files.rb
+++ b/lib/api/ci/secure_files.rb
@@ -66,7 +66,7 @@ module API
           route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true
           post ':id/secure_files' do
             secure_file = user_project.secure_files.new(
-              name: params[:name]
+              name: Gitlab::Utils.check_path_traversal!(params[:name])
             )
 
             secure_file.file = params[:file]
diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb
index 6e2de0c4d57..b132c0b5a69 100644
--- a/spec/controllers/projects/pipelines_controller_spec.rb
+++ b/spec/controllers/projects/pipelines_controller_spec.rb
@@ -1360,11 +1360,12 @@ RSpec.describe Projects::PipelinesController do
   describe 'GET config_variables.json', :use_clean_rails_memory_store_caching do
     include ReactiveCachingHelpers
 
-    let(:result) { YAML.dump(ci_config) }
-    let(:service) { Ci::ListConfigVariablesService.new(project, user) }
+    let(:ci_config) { '' }
+    let(:files) {  { '.gitlab-ci.yml' => YAML.dump(ci_config) } }
+    let(:project)  { create(:project, :auto_devops_disabled, :custom_repo, files: files) }
+    let(:service)  { Ci::ListConfigVariablesService.new(project, user) }
 
     before do
-      stub_gitlab_ci_yml_for_sha(sha, result)
       allow(Ci::ListConfigVariablesService)
         .to receive(:new)
         .and_return(service)
@@ -1398,7 +1399,6 @@ RSpec.describe Projects::PipelinesController do
 
     context 'when sending an invalid sha' do
       let(:sha) { 'invalid-sha' }
-      let(:ci_config) { nil }
 
       before do
         synchronous_reactive_cache(service)
@@ -1460,11 +1460,11 @@ RSpec.describe Projects::PipelinesController do
     end
 
     context 'when project uses external project ci config' do
-      let(:other_project) { create(:project) }
+      let(:other_project) { create(:project, :custom_repo, files: other_project_files) }
+      let(:other_project_files) { { '.gitlab-ci.yml' => YAML.dump(other_project_ci_config) } }
       let(:sha) { 'master' }
-      let(:service) { ::Ci::ListConfigVariablesService.new(other_project, user) }
 
-      let(:ci_config) do
+      let(:other_project_ci_config) do
         {
           variables: {
             KEY1: { value: 'val 1', description: 'description 1' }
@@ -1477,13 +1477,12 @@ RSpec.describe Projects::PipelinesController do
       end
 
       before do
-        project.update!(ci_config_path: ".gitlab-ci.yml@#{other_project.full_path}")
+        other_project.add_developer(user)
+        project.update!(ci_config_path: ".gitlab-ci.yml@#{other_project.full_path}:master")
         synchronous_reactive_cache(service)
       end
 
       it 'returns other project config variables' do
-        expect(::Ci::ListConfigVariablesService).to receive(:new).with(other_project, anything).and_return(service)
-
         get_config_variables
 
         expect(response).to have_gitlab_http_status(:ok)
@@ -1493,13 +1492,6 @@ RSpec.describe Projects::PipelinesController do
 
     private
 
-    def stub_gitlab_ci_yml_for_sha(sha, result)
-      allow_any_instance_of(Repository)
-          .to receive(:gitlab_ci_yml_for)
-          .with(sha, '.gitlab-ci.yml')
-          .and_return(result)
-    end
-
     def get_config_variables
       get :config_variables, params: { namespace_id: project.namespace,
                                        project_id: project,
diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb
index f1f22dfadc2..0b8116d5e20 100644
--- a/spec/requests/api/ci/secure_files_spec.rb
+++ b/spec/requests/api/ci/secure_files_spec.rb
@@ -341,6 +341,15 @@ RSpec.describe API::Ci::SecureFiles do
 
         expect(response).to have_gitlab_http_status(:payload_too_large)
       end
+
+      it 'returns an error when and invalid file name is supplied' do
+        params = file_params.merge(name: '../../upload-keystore.jks')
+        expect do
+          post api("/projects/#{project.id}/secure_files", maintainer), params: params
+        end.not_to change { project.secure_files.count }
+
+        expect(response).to have_gitlab_http_status(:internal_server_error)
+      end
     end
 
     context 'authenticated user with read permissions' do
diff --git a/spec/services/ci/list_config_variables_service_spec.rb b/spec/services/ci/list_config_variables_service_spec.rb
index 4953b18bfcc..5b865914d1b 100644
--- a/spec/services/ci/list_config_variables_service_spec.rb
+++ b/spec/services/ci/list_config_variables_service_spec.rb
@@ -5,19 +5,16 @@ require 'spec_helper'
 RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_caching do
   include ReactiveCachingHelpers
 
-  let(:project) { create(:project, :repository) }
+  let(:ci_config) { {} }
+  let(:files) { { '.gitlab-ci.yml' => YAML.dump(ci_config) } }
+  let(:project) { create(:project, :custom_repo, :auto_devops_disabled, files: files) }
   let(:user) { project.creator }
+  let(:sha) { project.default_branch }
   let(:service) { described_class.new(project, user) }
-  let(:result) { YAML.dump(ci_config) }
 
-  subject { service.execute(sha) }
-
-  before do
-    stub_gitlab_ci_yml_for_sha(sha, result)
-  end
+  subject(:result) { service.execute(sha) }
 
   context 'when sending a valid sha' do
-    let(:sha) { 'master' }
     let(:ci_config) do
       {
         variables: {
@@ -38,15 +35,14 @@ RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_cac
     end
 
     it 'returns variable list' do
-      expect(subject['KEY1']).to eq({ value: 'val 1', description: 'description 1' })
-      expect(subject['KEY2']).to eq({ value: 'val 2', description: '' })
-      expect(subject['KEY3']).to eq({ value: 'val 3' })
-      expect(subject['KEY4']).to eq({ value: 'val 4' })
+      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' })
     end
   end
 
   context 'when config has includes' do
-    let(:sha) { 'master' }
     let(:ci_config) do
       {
         include: [{ local: 'other_file.yml' }],
@@ -60,24 +56,56 @@ RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_cac
       }
     end
 
-    before do
-      allow_next_instance_of(Repository) do |repository|
-        allow(repository).to receive(:blob_data_at).with(sha, 'other_file.yml') do
-          <<~HEREDOC
-            variables:
-              KEY2:
-                value: 'val 2'
-                description: 'description 2'
-          HEREDOC
-        end
-      end
+    let(:other_file) do
+      {
+        variables: {
+          KEY2: { value: 'val 2', description: 'description 2' }
+        }
+      }
+    end
+
+    let(:files) { { '.gitlab-ci.yml' => YAML.dump(ci_config), 'other_file.yml' => YAML.dump(other_file) } }
 
+    before do
       synchronous_reactive_cache(service)
     end
 
     it 'returns variable list' do
-      expect(subject['KEY1']).to eq({ value: 'val 1', description: 'description 1' })
-      expect(subject['KEY2']).to eq({ value: 'val 2', description: 'description 2' })
+      expect(result['KEY1']).to eq({ value: 'val 1', description: 'description 1' })
+      expect(result['KEY2']).to eq({ value: 'val 2', description: 'description 2' })
+    end
+  end
+
+  context 'when project CI config is external' do
+    let(:other_project_ci_config) do
+      {
+        variables: { KEY1: { value: 'val 1', description: 'description 1' } },
+        test: { script: 'echo' }
+      }
+    end
+
+    let(:other_project_files) { { '.gitlab-ci.yml' => YAML.dump(other_project_ci_config) } }
+    let(:other_project) { create(:project, :custom_repo, files: other_project_files) }
+
+    before do
+      project.update!(ci_config_path: ".gitlab-ci.yml@#{other_project.full_path}:master")
+      synchronous_reactive_cache(service)
+    end
+
+    context 'when the user has access to the external project' do
+      before do
+        other_project.add_developer(user)
+      end
+
+      it 'returns variable list' do
+        expect(result['KEY1']).to eq({ value: 'val 1', description: 'description 1' })
+      end
+    end
+
+    context 'when the user has no access to the external project' do
+      it 'returns empty json' do
+        expect(result).to eq({})
+      end
     end
   end
 
@@ -90,12 +118,11 @@ RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_cac
     end
 
     it 'returns empty json' do
-      expect(subject).to eq({})
+      expect(result).to eq({})
     end
   end
 
   context 'when sending an invalid config' do
-    let(:sha) { 'master' }
     let(:ci_config) do
       {
         variables: {
@@ -113,13 +140,11 @@ RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_cac
     end
 
     it 'returns empty result' do
-      expect(subject).to eq({})
+      expect(result).to eq({})
     end
   end
 
   context 'when reading from cache' do
-    let(:sha) { 'master' }
-    let(:ci_config) { {} }
     let(:reactive_cache_params) { [sha] }
     let(:return_value) { { 'KEY1' => { value: 'val 1', description: 'description 1' } } }
 
@@ -128,13 +153,11 @@ RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_cac
     end
 
     it 'returns variable list' do
-      expect(subject).to eq(return_value)
+      expect(result).to eq(return_value)
     end
   end
 
   context 'when the cache is empty' do
-    let(:sha) { 'master' }
-    let(:ci_config) { {} }
     let(:reactive_cache_params) { [sha] }
 
     it 'returns nil and enquques the worker to fill cache' do
@@ -142,16 +165,7 @@ RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_cac
         .to receive(:perform_async)
         .with(service.class, service.id, *reactive_cache_params)
 
-      expect(subject).to be_nil
+      expect(result).to be_nil
     end
   end
-
-  private
-
-  def stub_gitlab_ci_yml_for_sha(sha, result)
-    allow_any_instance_of(Repository)
-        .to receive(:gitlab_ci_yml_for)
-        .with(sha, '.gitlab-ci.yml')
-        .and_return(result)
-  end
 end