diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 2256471047da48ceae87a842e3246dd891040d63..25b83aed78a456cd45c3573d9af248e65b64116b 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -38,9 +38,9 @@ def authorize_read_build_trace! if build.debug_mode? access_denied!( _('You must have developer or higher permissions in the associated project to view job logs when debug trace ' \ - "is enabled. To disable debug trace, set the 'CI_DEBUG_TRACE' variable to 'false' in your pipeline " \ - 'configuration or CI/CD settings. If you need to view this job log, a project maintainer or owner must add ' \ - 'you to the project with developer permissions or higher.') + "is enabled. To disable debug trace, set the 'CI_DEBUG_TRACE' and 'CI_DEBUG_SERVICES' variables to 'false' " \ + 'in your pipeline configuration or CI/CD settings. If you must view this job log, a project maintainer ' \ + 'or owner must add you to the project with developer permissions or higher.') ) else access_denied!(_('The current user is not authorized to access the job log.')) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 1bacd65b5747d4cb3a4e1c47b65e952e73c23752..13279b19a3a62bfad5c3d2820e77e8726c32ddc4 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -1040,7 +1040,8 @@ 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_TRACE']&.value&.casecmp('true') == 0 || + variables['CI_DEBUG_SERVICES']&.value&.casecmp('true') == 0 end def drop_with_exit_code!(failure_reason, exit_code) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 133b3505e1b8fb9b142bb2c071cc615f5de9066b..13a4c6d1fb0ce9ff21397e21bf29e52861c42aeb 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -46729,7 +46729,7 @@ msgstr "" msgid "You must be logged in to search across all of GitLab" msgstr "" -msgid "You must have developer or higher permissions in the associated project to view job logs when debug trace is enabled. To disable debug trace, set the 'CI_DEBUG_TRACE' variable to 'false' in your pipeline configuration or CI/CD settings. If you need to view this job log, a project maintainer or owner must add you to the project with developer permissions or higher." +msgid "You must have developer or higher permissions in the associated project to view job logs when debug trace is enabled. To disable debug trace, set the 'CI_DEBUG_TRACE' and 'CI_DEBUG_SERVICES' variables to 'false' in your pipeline configuration or CI/CD settings. If you must view this job log, a project maintainer or owner must add you to the project with developer permissions or higher." msgstr "" msgid "You must have maintainer access to force delete a lock" diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index 808e67eff3dfce72ce34ceb290b67bba2a5adef1..d6165c66b35e226a9aba934106201c88168359ed 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -228,8 +228,9 @@ def download_artifact(extra_params = {}) expect(response).to have_gitlab_http_status(:forbidden) expect(response.body).to include( 'You must have developer or higher permissions in the associated project to view job logs when debug trace is enabled. ' \ - 'To disable debug trace, set the 'CI_DEBUG_TRACE' variable to 'false' in your pipeline configuration or CI/CD settings. ' \ - 'If you need to view this job log, a project maintainer or owner must add you to the project with developer permissions or higher.' + 'To disable debug trace, set the 'CI_DEBUG_TRACE' and 'CI_DEBUG_SERVICES' variables to 'false' ' \ + 'in your pipeline configuration or CI/CD settings. If you must view this job log, a project maintainer or owner must ' \ + 'add you to the project with developer permissions or higher.' ) end end diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 58627c4dc746feab3f3886706f054e1616e14bc1..3dc89365530353ba7d8c2e169e49d343eda427fc 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -660,6 +660,38 @@ def get_show(**extra_params) 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 } + + 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) + end + end + end end context 'when job has a live trace' do @@ -1184,36 +1216,51 @@ def post_erase expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq "true" end - context 'when CI_DEBUG_TRACE enabled' do - before do - create(:ci_instance_variable, key: 'CI_DEBUG_TRACE', value: 'true') + 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' end - context 'with proper permissions for debug logging on a project' do - let(:user) { developer } - + with_them do before do - sign_in(user) + create(:ci_instance_variable, key: 'CI_DEBUG_TRACE', value: ci_debug_trace) + create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES', value: ci_debug_services) end - it 'returns response ok' do - response = subject + context 'with proper permissions for debug logging on a project' do + let(:user) { developer } - expect(response).to have_gitlab_http_status(:ok) - end - end + before do + sign_in(user) + end - context 'without proper permissions for debug logging on a project' do - let(:user) { reporter } + it 'returns response ok' do + response = subject - before do - sign_in(user) + expect(response).to have_gitlab_http_status(:ok) + end end - it 'returns response forbidden' do - response = subject + context 'without proper permissions for debug logging on a project' do + let(:user) { reporter } - expect(response).to have_gitlab_http_status(:forbidden) + before do + sign_in(user) + end + + it 'returns response forbidden if dev mode enabled' do + response = subject + + if ci_debug_trace == 'true' || ci_debug_services == 'true' + expect(response).to have_gitlab_http_status(:forbidden) + else + expect(response).to have_gitlab_http_status(:ok) + end + end end end end diff --git a/spec/features/projects/jobs/permissions_spec.rb b/spec/features/projects/jobs/permissions_spec.rb index b6019944071c2292d13eeddc8c1aeef38e3efb15..740d009d6b87aefdc02e520d9493205a0750062d 100644 --- a/spec/features/projects/jobs/permissions_spec.rb +++ b/spec/features/projects/jobs/permissions_spec.rb @@ -211,4 +211,48 @@ 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) + + expect(status_code).to eq(expected_status_code) + 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) + end + end + end + end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 9c83c1df10c7842c040731a1b5f80d9a86b9fd80..b3313c09ef6f857469055d299e5dbbc366156c40 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -5095,6 +5095,60 @@ def run_job_without_exception context 'when CI_DEBUG_TRACE is not in variables' do it { is_expected.to eq false } 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') + end + + it { is_expected.to eq true } + end + + context 'when in group variables' do + before do + create(:ci_group_variable, key: 'CI_DEBUG_SERVICES', value: 'true', group: project.group) + end + + it { is_expected.to eq true } + end + + context 'when in pipeline variables' do + before do + create(:ci_pipeline_variable, key: 'CI_DEBUG_SERVICES', value: 'true', pipeline: pipeline) + end + + it { is_expected.to eq true } + end + + context 'when in project variables' do + before do + create(:ci_variable, key: 'CI_DEBUG_SERVICES', value: 'true', project: project) + end + + it { is_expected.to eq true } + end + + context 'when in job variables' do + before do + create(:ci_job_variable, key: 'CI_DEBUG_SERVICES', value: 'true', job: build) + end + + it { is_expected.to eq true } + end + + context 'when in yaml variables' do + before do + build.update!(yaml_variables: [{ key: :CI_DEBUG_SERVICES, value: '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 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 0e17db516f4573c601301a618a2f64b3e7f496ea..c1b7461f444c815b197a950c8dffc9301a191108 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -606,6 +606,32 @@ def go end 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) + end + + where(:public_builds, :user_project_role, :expected_status) do + true | 'developer' | :ok + true | 'guest' | :forbidden + false | 'developer' | :ok + false | 'guest' | :forbidden + end + + with_them do + before do + project.update!(public_builds: public_builds) + project.add_role(user, user_project_role) + + get api("/projects/#{project.id}/jobs/#{job.id}/trace", api_user) + end + + it 'renders successfully to authorized users' do + expect(response).to have_gitlab_http_status(expected_status) + end + end + end end describe 'POST /projects/:id/jobs/:job_id/cancel' do