Commit e7f6eef2 authored by Allison Browne's avatar Allison Browne Committed by Sean McGivern

Restrict developer access to jobs when in debug mode

Keep users from viewing job traces if they are not developers on the
project and the job is using the env variable CI_DEBUG_TRACE=true
parent 03613793
......@@ -249,7 +249,6 @@ Rails/SaveBang:
- 'spec/features/profiles/personal_access_tokens_spec.rb'
- 'spec/features/projects/features_visibility_spec.rb'
- 'spec/features/projects/fork_spec.rb'
- 'spec/features/projects/jobs/permissions_spec.rb'
- 'spec/features/projects/jobs_spec.rb'
- 'spec/features/projects/members/user_requests_access_spec.rb'
- 'spec/features/projects/pages_lets_encrypt_spec.rb'
......
......@@ -13,6 +13,7 @@ import {
scrollDown,
scrollUp,
} from '~/lib/utils/scroll_utils';
import httpStatusCodes from '~/lib/utils/http_status';
export const init = ({ dispatch }, { endpoint, logState, pagePath }) => {
dispatch('setJobEndpoint', endpoint);
......@@ -187,7 +188,11 @@ export const fetchTrace = ({ dispatch, state }) =>
dispatch('startPollingTrace');
}
})
.catch(() => dispatch('receiveTraceError'));
.catch(e =>
e.response.status === httpStatusCodes.FORBIDDEN
? dispatch('receiveTraceUnauthorizedError')
: dispatch('receiveTraceError'),
);
export const startPollingTrace = ({ dispatch, commit }) => {
const traceTimeout = setTimeout(() => {
......@@ -209,6 +214,10 @@ export const receiveTraceError = ({ dispatch }) => {
dispatch('stopPollingTrace');
flash(__('An error occurred while fetching the job log.'));
};
export const receiveTraceUnauthorizedError = ({ dispatch }) => {
dispatch('stopPollingTrace');
flash(__('The current user is not authorized to access the job log.'));
};
/**
* When the user clicks a collapsible line in the job
* log, we commit a mutation to update the state
......
......@@ -6,6 +6,7 @@ class Projects::JobsController < Projects::ApplicationController
before_action :find_job_as_build, except: [:index, :play]
before_action :find_job_as_processable, only: [:play]
before_action :authorize_read_build_trace!, only: [:trace, :raw]
before_action :authorize_read_build!
before_action :authorize_update_build!,
except: [:index, :show, :status, :raw, :trace, :erase]
......@@ -157,6 +158,18 @@ class Projects::JobsController < Projects::ApplicationController
private
def authorize_read_build_trace!
return if can?(current_user, :read_build_trace, @build)
msg = _(
"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 must add you to the project with developer permissions or higher."
)
return access_denied!(msg) if @build.debug_mode?
access_denied!(_('The current user is not authorized to access the job log.'))
end
def authorize_update_build!
return access_denied! unless can?(current_user, :update_build, @build)
end
......
......@@ -1006,6 +1006,15 @@ module Ci
::Gitlab.com? ? 500_000 : 0
end
def debug_mode?
return false unless Feature.enabled?(:restrict_access_to_build_debug_mode, default_enabled: true)
# 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.any? { |variable| variable[:key] == 'CI_DEBUG_TRACE' && variable[:value].casecmp('true') == 0 }
end
protected
def run_status_commit_hooks!
......
......@@ -45,6 +45,21 @@ module Ci
@subject.pipeline.webide?
end
condition(:debug_mode, scope: :subject, score: 32) do
@subject.debug_mode?
end
condition(:project_read_build, scope: :subject) do
can?(:read_build, @subject.project)
end
condition(:project_update_build, scope: :subject) do
can?(:update_build, @subject.project)
end
rule { project_read_build }.enable :read_build_trace
rule { debug_mode & ~project_update_build }.prevent :read_build_trace
rule { ~protected_environment_access & (protected_ref | archived) }.policy do
prevent :update_build
prevent :update_commit_status
......
---
title: Restrict access to job page to developers only when use CI_DEBUG_TRACE is true
merge_request: 48932
author:
type: fixed
---
name: restrict_access_to_build_debug_mode
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48932
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/292661
milestone: '13.7'
type: development
group: group::continuous integration
default_enabled: true
......@@ -844,6 +844,50 @@ Before enabling this, you should ensure jobs are visible to
also [erase](../jobs/index.md#view-jobs-in-a-pipeline) all generated job logs
before making them visible again.
### Restricted access to debug logging
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/213159) in GitLab 13.7.
> - It's [deployed behind a feature flag](../../user/feature_flags.md), enabled by default.
> - It's enabled on GitLab.com.
> - It's recommended for production use.
> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-restricted-access-to-debug-logging). **(CORE ONLY)**
WARNING:
This feature might not be available to you. Check the **version history** note above for details.
With restricted access to debug logging, only users with
[developer or higher permissions](../../user/permissions.md#project-members-permissions)
can view job logs when debug logging is enabled with a variable in:
- The [`.gitlab-ci.yml` file](#gitlab-ciyml-defined-variables).
- The CI/CD variables set within the GitLab UI.
WARNING:
If you add `CI_DEBUG_TRACE` as a local variable to your runners, debug logs are visible
to all users with access to job logs. The permission levels are not checked by Runner,
so you should make use of the variable in GitLab only.
#### Enable or disable Restricted access to debug logging **(CORE ONLY)**
Restricted Access to Debug logging is under development but ready for production use.
It is deployed behind a feature flag that is **enabled by default**.
[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md)
can opt to disable it.
To enable it:
```ruby
Feature.enable(:restrict_access_to_build_debug_mode)
```
To disable it:
```ruby
Feature.disable(:restrict_access_to_build_debug_mode)
```
### Enable Debug logging
To enable debug logs (traces), set the `CI_DEBUG_TRACE` variable to `true`:
```yaml
......
......@@ -61,6 +61,7 @@ The following table depicts the various user permission levels in a project.
| View wiki pages | ✓ | ✓ | ✓ | ✓ | ✓ |
| See a list of jobs | ✓ (*3*) | ✓ | ✓ | ✓ | ✓ |
| See a job log | ✓ (*3*) | ✓ | ✓ | ✓ | ✓ |
| See a job with [debug logging](../ci/variables/README.md#debug-logging) | | | ✓ | ✓ | ✓ |
| Download and browse job artifacts | ✓ (*3*) | ✓ | ✓ | ✓ | ✓ |
| Create new issue | ✓ | ✓ | ✓ | ✓ | ✓ |
| See related issues | ✓ | ✓ | ✓ | ✓ | ✓ |
......
......@@ -271,6 +271,10 @@ module API
authorize! :read_build, user_project
end
def authorize_read_build_trace!(build)
authorize! :read_build_trace, build
end
def authorize_destroy_artifacts!
authorize! :destroy_artifacts, user_project
end
......
......@@ -76,6 +76,8 @@ module API
build = find_build!(params[:job_id])
authorize_read_build_trace!(build) if build
header 'Content-Disposition', "infile; filename=\"#{build.id}.log\""
content_type 'text/plain'
env['api.format'] = :binary
......
......@@ -27313,6 +27313,9 @@ msgstr ""
msgid "The current issue"
msgstr ""
msgid "The current user is not authorized to access the job log."
msgstr ""
msgid "The data source is connected, but there is no data to display. %{documentationLink}"
msgstr ""
......@@ -31690,6 +31693,9 @@ msgstr ""
msgid "You must disassociate %{domain} from all clusters it is attached to before deletion."
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 must add you to the project with developer permissions or higher."
msgstr ""
msgid "You must have maintainer access to force delete a lock"
msgstr ""
......
......@@ -161,11 +161,11 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do
context 'when requesting HTML' do
context 'when job exists' do
before do
get_show(id: job.id)
end
let(:extra_params) { { id: job.id } }
it 'has a job' do
get_show(**extra_params)
expect(response).to have_gitlab_http_status(:ok)
expect(assigns(:build).id).to eq(job.id)
end
......@@ -647,6 +647,46 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do
expect(json_response['total']).to be_present
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
before do
project.add_developer(user)
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
before do
project.add_guest(user)
sign_in(user)
end
it 'returns response forbidden' do
get_trace
expect(response).to have_gitlab_http_status(:forbidden)
end
context 'with restrict_access_to_build_debug_mode feature disabled' do
before do
stub_feature_flags(restrict_access_to_build_debug_mode: false)
end
it 'returns response forbidden' do
expect(response).to have_gitlab_http_status(:ok)
end
end
end
end
end
context 'when job has a trace' do
......@@ -1063,7 +1103,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do
}
end
context "when job has a trace artifact" do
context 'when job has a trace artifact' do
let(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
it "sets #{Gitlab::Workhorse::DETECT_HEADER} header" do
......@@ -1074,6 +1114,62 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do
expect(response.body).to eq(job.job_artifacts_trace.open.read)
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')
end
context 'with proper permissions for debug logging on a project' do
before do
project.add_developer(user)
sign_in(user)
end
it 'returns response ok' do
response = subject
expect(response).to have_gitlab_http_status(:ok)
end
context 'with restrict_access_to_build_debug_mode feature disabled' do
before do
stub_feature_flags(restrict_access_to_build_debug_mode: false)
end
it 'returns response ok' do
response = subject
expect(response).to have_gitlab_http_status(:ok)
end
end
end
context 'without proper permissions for debug logging on a project' do
before do
project.add_reporter(user)
sign_in(user)
end
it 'returns response forbidden' do
response = subject
expect(response).to have_gitlab_http_status(:forbidden)
end
context 'with restrict_access_to_build_debug_mode feature disabled' do
before do
stub_feature_flags(restrict_access_to_build_debug_mode: false)
end
it 'returns response ok' do
response = subject
expect(response).to have_gitlab_http_status(:ok)
end
end
end
end
end
context "when job has a trace file" do
......
......@@ -3,11 +3,13 @@
require 'spec_helper'
RSpec.describe 'Project Jobs Permissions' do
let(:user) { create(:user) }
let(:group) { create(:group, name: 'some group') }
let(:project) { create(:project, :repository, namespace: group) }
let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.sha, ref: 'master') }
let!(:job) { create(:ci_build, :running, :coverage, :trace_artifact, pipeline: pipeline) }
using RSpec::Parameterized::TableSyntax
let_it_be_with_reload(:group) { create(:group, name: 'some group') }
let_it_be_with_reload(:project) { create(:project, :repository, namespace: group) }
let_it_be_with_reload(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.sha, ref: 'master') }
let_it_be(:user) { create(:user) }
let_it_be(:job) { create(:ci_build, :running, :coverage, :trace_artifact, pipeline: pipeline) }
before do
sign_in(user)
......@@ -34,7 +36,7 @@ RSpec.describe 'Project Jobs Permissions' do
context 'when public access for jobs is disabled' do
before do
project.update(public_builds: false)
project.update!(public_builds: false)
end
context 'when user is a guest' do
......@@ -48,7 +50,7 @@ RSpec.describe 'Project Jobs Permissions' do
context 'when project is internal' do
before do
project.update(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
end
it_behaves_like 'recent job page details responds with status', 404
......@@ -58,12 +60,30 @@ RSpec.describe 'Project Jobs Permissions' do
context 'when public access for jobs is enabled' do
before do
project.update(public_builds: true)
project.update!(public_builds: true)
end
context 'when user is a guest' do
before do
project.add_guest(user)
end
it_behaves_like 'recent job page details responds with status', 200
it_behaves_like 'project jobs page responds with status', 200
end
context 'when user is a developer' do
before do
project.add_developer(user)
end
it_behaves_like 'recent job page details responds with status', 200
it_behaves_like 'project jobs page responds with status', 200
end
context 'when project is internal' do
before do
project.update(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
end
it_behaves_like 'recent job page details responds with status', 200 do
......@@ -89,7 +109,7 @@ RSpec.describe 'Project Jobs Permissions' do
describe 'artifacts page' do
context 'when recent job has artifacts available' do
before do
before_all do
archive = fixture_file_upload('spec/fixtures/ci_build_artifacts.zip')
create(:ci_job_artifact, :archive, file: archive, job: job)
......@@ -97,7 +117,7 @@ RSpec.describe 'Project Jobs Permissions' do
context 'when public access for jobs is disabled' do
before do
project.update(public_builds: false)
project.update!(public_builds: false)
end
context 'when user with guest role' do
......@@ -128,4 +148,124 @@ RSpec.describe 'Project Jobs Permissions' do
end
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
context 'when restrict_access_to_build_debug_mode feature not enabled' do
where(:public_builds, :user_project_role, :ci_debug_trace, :expected_status_code) do
true | 'developer' | true | 200
true | 'guest' | true | 200
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
stub_feature_flags(restrict_access_to_build_debug_mode: false)
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
end
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)
end
end
context 'when restrict_access_to_build_debug_mode feature not enabled' do
where(:public_builds, :user_project_role, :ci_debug_trace, :expected_status_code, :expected_msg) do
true | 'developer' | true | 200 | nil
true | 'guest' | true | 200 | nil
true | 'developer' | false | 200 | nil
true | 'guest' | false | 200 | nil
false | 'developer' | true | 200 | nil
false | 'guest' | true | 403 | 'The current user is not authorized to access the job log'
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
stub_feature_flags(restrict_access_to_build_debug_mode: false)
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)
end
end
end
end
end
end
......@@ -14,7 +14,7 @@ RSpec.describe Ci::Build do
status: 'success')
end
let(:build) { create(:ci_build, pipeline: pipeline) }
let_it_be(:build, refind: true) { create(:ci_build, pipeline: pipeline) }
it { is_expected.to belong_to(:runner) }
it { is_expected.to belong_to(:trigger_request) }
......@@ -307,8 +307,6 @@ RSpec.describe Ci::Build do
end
describe '.without_needs' do
let!(:build) { create(:ci_build) }
subject { described_class.without_needs }
context 'when no build_need is created' do
......@@ -2019,6 +2017,8 @@ RSpec.describe Ci::Build do
end
context 'when ci_build_metadata_config is disabled' do
let(:build) { create(:ci_build, pipeline: pipeline) }
before do
stub_feature_flags(ci_build_metadata_config: false)
end
......@@ -2755,7 +2755,11 @@ RSpec.describe Ci::Build do
pipeline.update!(tag: true)
end
it { is_expected.to include(tag_variable) }
it do
build.reload
expect(subject).to include(tag_variable)
end
end
context 'when CI variable is defined' do
......@@ -2989,8 +2993,11 @@ RSpec.describe Ci::Build do
end
context 'when pipeline variable overrides build variable' do
let(:build) do
create(:ci_build, pipeline: pipeline, yaml_variables: [{ key: 'MYVAR', value: 'myvar', public: true }])
end
before do
build.yaml_variables = [{ key: 'MYVAR', value: 'myvar', public: true }]
pipeline.variables.build(key: 'MYVAR', value: 'pipeline value')
end
......@@ -3306,9 +3313,12 @@ RSpec.describe Ci::Build do
end
context 'when overriding user-provided variables' do
let(:build) do
create(:ci_build, pipeline: pipeline, yaml_variables: [{ key: 'MY_VAR', value: 'myvar', public: true }])
end
before do
pipeline.variables.build(key: 'MY_VAR', value: 'pipeline value')
build.yaml_variables = [{ key: 'MY_VAR', value: 'myvar', public: true }]
end
it 'returns a hash including variable with higher precedence' do
......@@ -3665,7 +3675,7 @@ RSpec.describe Ci::Build do
end
it 'handles raised exception' do
expect { subject.drop! }.not_to raise_exception(Gitlab::Access::AccessDeniedError)
expect { subject.drop! }.not_to raise_error
end
it 'logs the error' do
......@@ -4723,4 +4733,78 @@ RSpec.describe Ci::Build do
expect(action).not_to have_received(:perform!)
end
end
describe '#debug_mode?' do
subject { build.debug_mode? }
context 'when feature is disabled' do
before do
stub_feature_flags(restrict_access_to_build_debug_mode: false)
end
it { is_expected.to eq false }
context 'when in variables' do
before do
create(:ci_instance_variable, key: 'CI_DEBUG_TRACE', value: 'true')
end
it { is_expected.to eq false }
end
end
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')
end
it { is_expected.to eq true }
end
context 'when in group variables' do
before do
create(:ci_group_variable, key: 'CI_DEBUG_TRACE', 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_TRACE', 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_TRACE', 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_TRACE', 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_TRACE, value: 'true' }])
end
it { is_expected.to eq true }
end
end
context 'when CI_DEBUG_TRACE is not in variables' do
it { is_expected.to eq false }
end
end
end
......@@ -3,6 +3,7 @@
require 'spec_helper'
RSpec.describe API::Jobs do
using RSpec::Parameterized::TableSyntax
include HttpIOHelpers
let_it_be(:project, reload: true) do
......@@ -717,6 +718,58 @@ RSpec.describe API::Jobs do
expect(response).to have_gitlab_http_status(:unauthorized)
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
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 trace to authorized users' do
expect(response).to have_gitlab_http_status(expected_status)
end
end
context 'with restrict_access_to_build_debug_mode feature disabled' do
before do
stub_feature_flags(restrict_access_to_build_debug_mode: false)
end
where(:public_builds, :user_project_role, :expected_status) do
true | 'developer' | :ok
true | 'guest' | :ok
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 trace to authorized users' do
expect(response).to have_gitlab_http_status(expected_status)
end
end
end
end
end
describe 'POST /projects/:id/jobs/:job_id/cancel' do
......
......@@ -67,7 +67,7 @@ module AccessMatchers
emulate_user(user, @membership)
visit(url)
[401, 404].include?(status_code) || current_path.in?([new_user_session_path, new_admin_session_path])
[401, 404, 403].include?(status_code) || current_path.in?([new_user_session_path, new_admin_session_path])
end
chain :of do |membership|
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment