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

6 files changed, 120 insertions, 221 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index f8b3777841d..1e70dd171ed 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -74,6 +74,7 @@ module Ci
     delegate :trigger_short_token, to: :trigger_request, allow_nil: true
     delegate :ensure_persistent_ref, to: :pipeline
     delegate :enable_debug_trace!, to: :metadata
+    delegate :debug_trace_enabled?, to: :metadata
 
     serialize :options # rubocop:disable Cop/ActiveRecordSerialize
     serialize :yaml_variables, Gitlab::Serializer::Ci::Variables # rubocop:disable Cop/ActiveRecordSerialize
@@ -1069,11 +1070,10 @@ module Ci
     end
 
     def debug_mode?
-      # TODO: Have `debug_mode?` check against data on sent back from runner
-      # to capture all the ways that variables can be set.
-      # See (https://gitlab.com/gitlab-org/gitlab/-/issues/290955)
-      variables['CI_DEBUG_TRACE']&.value&.casecmp('true') == 0 ||
-        variables['CI_DEBUG_SERVICES']&.value&.casecmp('true') == 0
+      # perform the check on both sides in case the runner version is old
+      debug_trace_enabled? ||
+        Gitlab::Utils.to_boolean(variables['CI_DEBUG_SERVICES']&.value, default: false) ||
+        Gitlab::Utils.to_boolean(variables['CI_DEBUG_TRACE']&.value, default: false)
     end
 
     def drop_with_exit_code!(failure_reason, exit_code)
diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb
index 32cd10d9805..c707b5dc39d 100644
--- a/spec/controllers/projects/artifacts_controller_spec.rb
+++ b/spec/controllers/projects/artifacts_controller_spec.rb
@@ -244,7 +244,9 @@ RSpec.describe Projects::ArtifactsController, feature_category: :build_artifacts
       let(:job) { create(:ci_build, :success, :trace_artifact, pipeline: pipeline) }
 
       before do
-        create(:ci_job_variable, key: 'CI_DEBUG_TRACE', value: 'true', job: job)
+        allow_next_found_instance_of(Ci::Build) do |build|
+          allow(build).to receive(:debug_mode?).and_return(true)
+        end
       end
 
       context 'when the user does not have update_build permissions' do
diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb
index 8fb9623c21a..2d047957430 100644
--- a/spec/controllers/projects/jobs_controller_spec.rb
+++ b/spec/controllers/projects/jobs_controller_spec.rb
@@ -629,40 +629,12 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu
         expect(json_response['lines'].count).to be_positive
       end
 
-      context 'when CI_DEBUG_TRACE enabled' do
-        let!(:variable) { create(:ci_instance_variable, key: 'CI_DEBUG_TRACE', value: 'true') }
-
-        context 'with proper permissions on a project' do
-          let(:user) { developer }
-
-          before do
-            sign_in(user)
-          end
-
-          it 'returns response ok' do
-            get_trace
-
-            expect(response).to have_gitlab_http_status(:ok)
-          end
-        end
-
-        context 'without proper permissions for debug logging' do
-          let(:user) { guest }
-
-          before do
-            sign_in(user)
-          end
-
-          it 'returns response forbidden' do
-            get_trace
-
-            expect(response).to have_gitlab_http_status(:forbidden)
+      context 'when debug_mode? is enabled' do
+        before do
+          allow_next_found_instance_of(Ci::Build) do |build|
+            allow(build).to receive(:debug_mode?).and_return(true)
           end
         end
-      end
-
-      context 'when CI_DEBUG_SERVICES enabled' do
-        let!(:variable) { create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES', value: 'true') }
 
         context 'with proper permissions on a project' do
           let(:user) { developer }
@@ -1243,10 +1215,10 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu
       context 'when CI_DEBUG_TRACE and/or CI_DEBUG_SERVICES are enabled' do
         using RSpec::Parameterized::TableSyntax
         where(:ci_debug_trace, :ci_debug_services) do
-          'true'  | 'true'
-          'true'  | 'false'
-          'false' | 'true'
-          'false' | 'false'
+          true  | true
+          true  | false
+          false | true
+          false | false
         end
 
         with_them do
@@ -1279,7 +1251,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state, featu
             it 'returns response forbidden if dev mode enabled' do
               response = subject
 
-              if ci_debug_trace == 'true' || ci_debug_services == 'true'
+              if ci_debug_trace || ci_debug_services
                 expect(response).to have_gitlab_http_status(:forbidden)
               else
                 expect(response).to have_gitlab_http_status(:ok)
diff --git a/spec/features/projects/jobs/permissions_spec.rb b/spec/features/projects/jobs/permissions_spec.rb
index c3c0043a6ef..dce86c9f0a4 100644
--- a/spec/features/projects/jobs/permissions_spec.rb
+++ b/spec/features/projects/jobs/permissions_spec.rb
@@ -149,109 +149,44 @@ RSpec.describe 'Project Jobs Permissions', feature_category: :projects do
     end
   end
 
-  context 'with CI_DEBUG_TRACE' do
-    let_it_be(:ci_instance_variable) { create(:ci_instance_variable, key: 'CI_DEBUG_TRACE') }
-
-    describe 'trace endpoint' do
-      let_it_be(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
-
-      where(:public_builds, :user_project_role, :ci_debug_trace, :expected_status_code) do
-        true         | 'developer'      | true  | 200
-        true         | 'guest'          | true  | 403
-        true         | 'developer'      | false | 200
-        true         | 'guest'          | false | 200
-        false        | 'developer'      | true  | 200
-        false        | 'guest'          | true  | 403
-        false        | 'developer'      | false | 200
-        false        | 'guest'          | false | 403
-      end
-
-      with_them do
-        before do
-          ci_instance_variable.update!(value: ci_debug_trace)
-          project.update!(public_builds: public_builds)
-          project.add_role(user, user_project_role)
-        end
-
-        it 'renders trace to authorized users' do
-          visit trace_project_job_path(project, job)
-
-          expect(status_code).to eq(expected_status_code)
-        end
-      end
+  describe 'debug_mode' do
+    let(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
+
+    where(:public_builds, :user_project_role, :debug_mode, :expected_status_code, :expected_msg) do
+      true         | 'developer'      | true  | 200 | ''
+      true         | 'guest'          | true  | 403 | 'You must have developer or higher permissions'
+      true         | nil              | true  | 404 | 'Page Not Found Make sure the address is correct'
+      true         | 'developer'      | false | 200 | ''
+      true         | 'guest'          | false | 200 | ''
+      true         | nil              | false | 404 | 'Page Not Found Make sure the address is correct'
+      false        | 'developer'      | true  | 200 | ''
+      false        | 'guest'          | true  | 403 | 'You must have developer or higher permissions'
+      false        | nil              | true  | 404 | 'Page Not Found Make sure the address is correct'
+      false        | 'developer'      | false | 200 | ''
+      false        | 'guest'          | false | 403 | 'The current user is not authorized to access the job log'
+      false        | nil              | false | 404 | 'Page Not Found Make sure the address is correct'
     end
 
-    describe 'raw page' do
-      let_it_be(:job) { create(:ci_build, :running, :coverage, :trace_artifact, pipeline: pipeline) }
-
-      where(:public_builds, :user_project_role, :ci_debug_trace, :expected_status_code, :expected_msg) do
-        true         | 'developer'      | true  | 200 | nil
-        true         | 'guest'          | true  | 403 | 'You must have developer or higher permissions'
-        true         | 'developer'      | false | 200 | nil
-        true         | 'guest'          | false | 200 | nil
-        false        | 'developer'      | true  | 200 | nil
-        false        | 'guest'          | true  | 403 | 'You must have developer or higher permissions'
-        false        | 'developer'      | false | 200 | nil
-        false        | 'guest'          | false | 403 | 'The current user is not authorized to access the job log'
-      end
-
-      with_them do
-        before do
-          ci_instance_variable.update!(value: ci_debug_trace)
-          project.update!(public_builds: public_builds)
-          project.add_role(user, user_project_role)
-        end
-
-        it 'renders raw trace to authorized users' do
-          visit raw_project_job_path(project, job)
-
-          expect(status_code).to eq(expected_status_code)
-          expect(page).to have_content(expected_msg)
+    with_them do
+      before do
+        project.update!(public_builds: public_builds)
+        user_project_role && project.add_role(user, user_project_role)
+        allow_next_found_instance_of(Ci::Build) do |build|
+          allow(build).to receive(:debug_mode?).and_return(debug_mode)
         end
       end
-    end
-  end
-
-  context 'with CI_DEBUG_SERVICES' do
-    let_it_be(:ci_instance_variable) { create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES') }
-
-    describe 'trace endpoint and raw page' do
-      let_it_be(:job) { create(:ci_build, :running, :coverage, :trace_artifact, pipeline: pipeline) }
-
-      where(:public_builds, :user_project_role, :ci_debug_services, :expected_status_code, :expected_msg) do
-        true         | 'developer'      | true  | 200 | nil
-        true         | 'guest'          | true  | 403 | 'You must have developer or higher permissions'
-        true         | nil              | true  | 404 | 'Page Not Found Make sure the address is correct'
-        true         | 'developer'      | false | 200 | nil
-        true         | 'guest'          | false | 200 | nil
-        true         | nil              | false | 404 | 'Page Not Found Make sure the address is correct'
-        false        | 'developer'      | true  | 200 | nil
-        false        | 'guest'          | true  | 403 | 'You must have developer or higher permissions'
-        false        | nil              | true  | 404 | 'Page Not Found Make sure the address is correct'
-        false        | 'developer'      | false | 200 | nil
-        false        | 'guest'          | false | 403 | 'The current user is not authorized to access the job log'
-        false        | nil              | false | 404 | 'Page Not Found Make sure the address is correct'
-      end
 
-      with_them do
-        before do
-          ci_instance_variable.update!(value: ci_debug_services)
-          project.update!(public_builds: public_builds)
-          user_project_role && project.add_role(user, user_project_role)
-        end
-
-        it 'renders trace to authorized users' do
-          visit trace_project_job_path(project, job)
+      it 'renders trace to authorized users' do
+        visit trace_project_job_path(project, job)
 
-          expect(status_code).to eq(expected_status_code)
-        end
+        expect(status_code).to eq(expected_status_code)
+      end
 
-        it 'renders raw trace to authorized users' do
-          visit raw_project_job_path(project, job)
+      it 'renders raw trace to authorized users' do
+        visit raw_project_job_path(project, job)
 
-          expect(status_code).to eq(expected_status_code)
-          expect(page).to have_content(expected_msg)
-        end
+        expect(status_code).to eq(expected_status_code)
+        expect(page).to have_content(expected_msg)
       end
     end
   end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 2b3dc97e06d..80d6693e08e 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -5162,52 +5162,42 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def
     subject { build.debug_mode? }
 
     context 'when CI_DEBUG_TRACE=true is in variables' do
-      context 'when in instance variables' do
-        before do
-          create(:ci_instance_variable, key: 'CI_DEBUG_TRACE', value: 'true')
+      ['true', 1, 'y'].each do |value|
+        it 'reflects instance variables' do
+          create(:ci_instance_variable, key: 'CI_DEBUG_TRACE', value: value)
+
+          is_expected.to eq true
         end
 
-        it { is_expected.to eq true }
-      end
+        it 'reflects group variables' do
+          create(:ci_group_variable, key: 'CI_DEBUG_TRACE', value: value, group: project.group)
 
-      context 'when in group variables' do
-        before do
-          create(:ci_group_variable, key: 'CI_DEBUG_TRACE', value: 'true', group: project.group)
+          is_expected.to eq true
         end
 
-        it { is_expected.to eq true }
-      end
+        it 'reflects pipeline variables' do
+          create(:ci_pipeline_variable, key: 'CI_DEBUG_TRACE', value: value, pipeline: pipeline)
 
-      context 'when in pipeline variables' do
-        before do
-          create(:ci_pipeline_variable, key: 'CI_DEBUG_TRACE', value: 'true', pipeline: pipeline)
+          is_expected.to eq true
         end
 
-        it { is_expected.to eq true }
-      end
+        it 'reflects project variables' do
+          create(:ci_variable, key: 'CI_DEBUG_TRACE', value: value, project: project)
 
-      context 'when in project variables' do
-        before do
-          create(:ci_variable, key: 'CI_DEBUG_TRACE', value: 'true', project: project)
+          is_expected.to eq true
         end
 
-        it { is_expected.to eq true }
-      end
+        it 'reflects job variables' do
+          create(:ci_job_variable, key: 'CI_DEBUG_TRACE', value: value, job: build)
 
-      context 'when in job variables' do
-        before do
-          create(:ci_job_variable, key: 'CI_DEBUG_TRACE', value: 'true', job: build)
+          is_expected.to eq true
         end
 
-        it { is_expected.to eq true }
-      end
+        it 'when in yaml variables' do
+          build.update!(yaml_variables: [{ key: :CI_DEBUG_TRACE, value: value.to_s }])
 
-      context 'when in yaml variables' do
-        before do
-          build.update!(yaml_variables: [{ key: :CI_DEBUG_TRACE, value: 'true' }])
+          is_expected.to eq true
         end
-
-        it { is_expected.to eq true }
       end
     end
 
@@ -5216,58 +5206,64 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration, factory_def
     end
 
     context 'when CI_DEBUG_SERVICES=true is in variables' do
-      context 'when in instance variables' do
-        before do
-          create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES', value: 'true')
+      ['true', 1, 'y'].each do |value|
+        it 'reflects instance variables' do
+          create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES', value: value)
+
+          is_expected.to eq true
         end
 
-        it { is_expected.to eq true }
-      end
+        it 'reflects group variables' do
+          create(:ci_group_variable, key: 'CI_DEBUG_SERVICES', value: value, group: project.group)
 
-      context 'when in group variables' do
-        before do
-          create(:ci_group_variable, key: 'CI_DEBUG_SERVICES', value: 'true', group: project.group)
+          is_expected.to eq true
         end
 
-        it { is_expected.to eq true }
-      end
+        it 'reflects pipeline variables' do
+          create(:ci_pipeline_variable, key: 'CI_DEBUG_SERVICES', value: value, pipeline: pipeline)
 
-      context 'when in pipeline variables' do
-        before do
-          create(:ci_pipeline_variable, key: 'CI_DEBUG_SERVICES', value: 'true', pipeline: pipeline)
+          is_expected.to eq true
         end
 
-        it { is_expected.to eq true }
-      end
+        it 'reflects project variables' do
+          create(:ci_variable, key: 'CI_DEBUG_SERVICES', value: value, project: project)
 
-      context 'when in project variables' do
-        before do
-          create(:ci_variable, key: 'CI_DEBUG_SERVICES', value: 'true', project: project)
+          is_expected.to eq true
         end
 
-        it { is_expected.to eq true }
-      end
+        it 'reflects job variables' do
+          create(:ci_job_variable, key: 'CI_DEBUG_SERVICES', value: value, job: build)
 
-      context 'when in job variables' do
-        before do
-          create(:ci_job_variable, key: 'CI_DEBUG_SERVICES', value: 'true', job: build)
+          is_expected.to eq true
         end
 
-        it { is_expected.to eq true }
-      end
+        it 'when in yaml variables' do
+          build.update!(yaml_variables: [{ key: :CI_DEBUG_SERVICES, value: value.to_s }])
 
-      context 'when in yaml variables' do
-        before do
-          build.update!(yaml_variables: [{ key: :CI_DEBUG_SERVICES, value: 'true' }])
+          is_expected.to eq true
         end
-
-        it { is_expected.to eq true }
       end
     end
 
     context 'when CI_DEBUG_SERVICES is not in variables' do
       it { is_expected.to eq false }
     end
+
+    context 'when metadata has debug_trace_enabled true' do
+      before do
+        build.metadata.update!(debug_trace_enabled: true)
+      end
+
+      it { is_expected.to eq true }
+    end
+
+    context 'when metadata has debug_trace_enabled false' do
+      before do
+        build.metadata.update!(debug_trace_enabled: false)
+      end
+
+      it { is_expected.to eq false }
+    end
   end
 
   describe '#drop_with_exit_code!' do
diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb
index 10dd9c3b556..8b3ec59b785 100644
--- a/spec/requests/api/ci/jobs_spec.rb
+++ b/spec/requests/api/ci/jobs_spec.rb
@@ -750,11 +750,7 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do
       end
     end
 
-    context 'when ci_debug_trace is set to true' do
-      before_all do
-        create(:ci_instance_variable, key: 'CI_DEBUG_TRACE', value: true)
-      end
-
+    shared_examples_for "additional access criteria" do
       where(:public_builds, :user_project_role, :expected_status) do
         true         | 'developer'     | :ok
         true         | 'guest'         | :forbidden
@@ -776,30 +772,28 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do
       end
     end
 
-    context 'when ci_debug_services is set to true' do
-      before_all do
-        create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES', value: true)
+    describe 'when metadata debug_trace_enabled is set to true' do
+      before do
+        job.metadata.update!(debug_trace_enabled: true)
       end
 
-      where(:public_builds, :user_project_role, :expected_status) do
-        true         | 'developer'     | :ok
-        true         | 'guest'         | :forbidden
-        false        | 'developer'     | :ok
-        false        | 'guest'         | :forbidden
-      end
+      it_behaves_like "additional access criteria"
+    end
 
-      with_them do
-        before do
-          project.update!(public_builds: public_builds)
-          project.add_role(user, user_project_role)
+    context 'when ci_debug_trace is set to true' do
+      before_all do
+        create(:ci_instance_variable, key: 'CI_DEBUG_TRACE', value: true)
+      end
 
-          get api("/projects/#{project.id}/jobs/#{job.id}/trace", api_user)
-        end
+      it_behaves_like "additional access criteria"
+    end
 
-        it 'renders successfully to authorized users' do
-          expect(response).to have_gitlab_http_status(expected_status)
-        end
+    context 'when ci_debug_services is set to true' do
+      before_all do
+        create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES', value: true)
       end
+
+      it_behaves_like "additional access criteria"
     end
   end