From 7b1c7e980459210bea3f967cbc6b1c797c1ff658 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 29 Apr 2022 08:18:56 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@14-10-stable-ee --- lib/gitlab/ci/pipeline/chain/helpers.rb | 11 +- lib/gitlab/ci/pipeline/chain/validate/abilities.rb | 2 +- lib/gitlab/git_access_wiki.rb | 14 ++- spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb | 25 ++++ spec/lib/gitlab/git_access_wiki_spec.rb | 138 +++++++++++++++------ 5 files changed, 143 insertions(+), 47 deletions(-) diff --git a/lib/gitlab/ci/pipeline/chain/helpers.rb b/lib/gitlab/ci/pipeline/chain/helpers.rb index 09158bf8bfd..343a189f773 100644 --- a/lib/gitlab/ci/pipeline/chain/helpers.rb +++ b/lib/gitlab/ci/pipeline/chain/helpers.rb @@ -6,25 +6,28 @@ module Gitlab module Chain module Helpers def error(message, config_error: false, drop_reason: nil) + sanitized_message = ActionController::Base.helpers.sanitize(message, tags: []) + if config_error drop_reason = :config_error - pipeline.yaml_errors = message + pipeline.yaml_errors = sanitized_message end - pipeline.add_error_message(message) + pipeline.add_error_message(sanitized_message) drop_pipeline!(drop_reason) # TODO: consider not to rely on AR errors directly as they can be # polluted with other unrelated errors (e.g. state machine) # https://gitlab.com/gitlab-org/gitlab/-/issues/220823 - pipeline.errors.add(:base, message) + pipeline.errors.add(:base, sanitized_message) pipeline.errors.full_messages end def warning(message) - pipeline.add_warning_message(message) + sanitized_message = ActionController::Base.helpers.sanitize(message, tags: []) + pipeline.add_warning_message(sanitized_message) end private diff --git a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb index 1c1f7abb6f6..035167f1a74 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb @@ -23,7 +23,7 @@ module Gitlab end unless allowed_to_write_ref? - error("You do not have sufficient permission to run a pipeline on '#{command.ref}'. Please select a different branch or contact your administrator for assistance. Learn more".html_safe) + error("You do not have sufficient permission to run a pipeline on '#{command.ref}'. Please select a different branch or contact your administrator for assistance.") end end diff --git a/lib/gitlab/git_access_wiki.rb b/lib/gitlab/git_access_wiki.rb index f8f61511265..fdd7e8a8c4a 100644 --- a/lib/gitlab/git_access_wiki.rb +++ b/lib/gitlab/git_access_wiki.rb @@ -31,7 +31,8 @@ module Gitlab def check_download_access! super - raise ForbiddenError, download_forbidden_message if deploy_token && !deploy_token.can?(:download_wiki_code, container) + raise ForbiddenError, download_forbidden_message if build_cannot_download? + raise ForbiddenError, download_forbidden_message if deploy_token_cannot_download? end override :check_change_access! @@ -52,6 +53,17 @@ module Gitlab def not_found_message error_message(:not_found) end + + private + + # when accessing via the CI_JOB_TOKEN + def build_cannot_download? + build_can_download_code? && !user_access.can_do_action?(download_ability) + end + + def deploy_token_cannot_download? + deploy_token && !deploy_token.can?(download_ability, container) + end end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb index bcea6462790..96ada90b4e1 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/helpers_spec.rb @@ -22,6 +22,19 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Helpers do let(:command) { double(save_incompleted: true) } let(:message) { 'message' } + describe '.warning' do + context 'when the warning includes malicious HTML' do + let(:message) { '
gimme your password
' } + let(:sanitized_message) { 'gimme your password' } + + it 'sanitizes' do + subject.warning(message) + + expect(pipeline.warning_messages[0].content).to include(sanitized_message) + end + end + end + describe '.error' do shared_examples 'error function' do specify do @@ -36,6 +49,18 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Helpers do end end + context 'when the error includes malicious HTML' do + let(:message) { '
gimme your password
' } + let(:sanitized_message) { 'gimme your password' } + + it 'sanitizes the error and removes the HTML tags' do + subject.error(message, config_error: true, drop_reason: :config_error) + + expect(pipeline.yaml_errors).to eq(sanitized_message) + expect(pipeline.errors[:base]).to include(sanitized_message) + end + end + context 'when given a drop reason' do context 'when config error is true' do context 'sets the yaml error and overrides the drop reason' do diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb index 27175dc8c44..de3e674c3a7 100644 --- a/spec/lib/gitlab/git_access_wiki_spec.rb +++ b/spec/lib/gitlab/git_access_wiki_spec.rb @@ -4,8 +4,9 @@ require 'spec_helper' RSpec.describe Gitlab::GitAccessWiki do let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project, :wiki_repo) } - let_it_be(:wiki) { create(:project_wiki, project: project) } + let_it_be_with_reload(:project) { create(:project, :wiki_repo) } + + let(:wiki) { create(:project_wiki, project: project) } let(:changes) { ['6f6d7e7ed 570e7b2ab refs/heads/master'] } let(:authentication_abilities) { %i[read_project download_code push_code] } @@ -17,6 +18,61 @@ RSpec.describe Gitlab::GitAccessWiki do redirected_path: redirected_path) end + RSpec.shared_examples 'wiki access by level' do + where(:project_visibility, :project_member?, :wiki_access_level, :wiki_repo?, :expected_behavior) do + [ + # Private project - is a project member + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::ENABLED, true, :no_error], + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::PRIVATE, true, :no_error], + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::DISABLED, true, :forbidden_wiki], + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::ENABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::DISABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, true, ProjectFeature::PRIVATE, false, :not_found_wiki], + # Private project - is NOT a project member + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::ENABLED, true, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::PRIVATE, true, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::DISABLED, true, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::ENABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::DISABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PRIVATE, false, ProjectFeature::PRIVATE, false, :not_found_wiki], + # Public project - is a project member + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::ENABLED, true, :no_error], + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::PRIVATE, true, :no_error], + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::DISABLED, true, :forbidden_wiki], + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::ENABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::DISABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PUBLIC, true, ProjectFeature::PRIVATE, false, :not_found_wiki], + # Public project - is NOT a project member + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::ENABLED, true, :no_error], + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::PRIVATE, true, :forbidden_wiki], + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::DISABLED, true, :forbidden_wiki], + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::ENABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::DISABLED, false, :not_found_wiki], + [Gitlab::VisibilityLevel::PUBLIC, false, ProjectFeature::PRIVATE, false, :not_found_wiki] + ] + end + + with_them do + before do + project.update!(visibility_level: project_visibility) + project.add_developer(user) if project_member? + project.project_feature.update_attribute(:wiki_access_level, wiki_access_level) + allow(wiki.repository).to receive(:exists?).and_return(wiki_repo?) + end + + it 'provides access by level' do + case expected_behavior + when :no_error + expect { subject }.not_to raise_error + when :forbidden_wiki + expect { subject }.to raise_wiki_forbidden + when :not_found_wiki + expect { subject }.to raise_wiki_not_found + end + end + end + end + describe '#push_access_check' do subject { access.check('git-receive-pack', changes) } @@ -28,56 +84,26 @@ RSpec.describe Gitlab::GitAccessWiki do it { expect { subject }.not_to raise_error } context 'when in a read-only GitLab instance' do - let(:message) { "You can't push code to a read-only GitLab instance." } - before do allow(Gitlab::Database).to receive(:read_only?) { true } end - it_behaves_like 'forbidden git access' + it_behaves_like 'forbidden git access' do + let(:message) { "You can't push code to a read-only GitLab instance." } + end end end context 'the user cannot :create_wiki' do - it_behaves_like 'not-found git access' do - let(:message) { 'The wiki you were looking for could not be found.' } - end + it { expect { subject }.to raise_wiki_not_found } end end describe '#check_download_access!' do subject { access.check('git-upload-pack', Gitlab::GitAccess::ANY) } - context 'the user can :download_wiki_code' do - before do - project.add_developer(user) - end - - context 'when wiki feature is disabled' do - before do - project.project_feature.update_attribute(:wiki_access_level, ProjectFeature::DISABLED) - end - - it_behaves_like 'forbidden git access' do - let(:message) { include('wiki') } - end - end - - context 'when the repository does not exist' do - before do - allow(wiki.repository).to receive(:exists?).and_return(false) - end - - it_behaves_like 'not-found git access' do - let(:message) { include('for this wiki') } - end - end - end - - context 'the user cannot :download_wiki_code' do - it_behaves_like 'not-found git access' do - let(:message) { include('wiki') } - end + context 'when actor is a user' do + it_behaves_like 'wiki access by level' end context 'when the actor is a deploy token' do @@ -99,10 +125,40 @@ RSpec.describe Gitlab::GitAccessWiki do context 'when the wiki is disabled' do let(:wiki_access_level) { ProjectFeature::DISABLED } - it_behaves_like 'forbidden git access' do - let(:message) { 'You are not allowed to download files from this wiki.' } - end + it { expect { subject }.to raise_wiki_forbidden } end end + + describe 'when actor is a user provided by build via CI_JOB_TOKEN' do + let(:protocol) { 'http' } + let(:authentication_abilities) { [:build_download_code] } + let(:auth_result_type) { :build } + + before do + project.project_feature.update_attribute(:wiki_access_level, wiki_access_level) + end + + subject { access.check('git-upload-pack', changes) } + + it_behaves_like 'wiki access by level' + end + end + + RSpec::Matchers.define :raise_wiki_not_found do + match do |actual| + expect { actual.call }.to raise_error(Gitlab::GitAccess::NotFoundError, include('wiki')) + end + def supports_block_expectations? + true + end + end + + RSpec::Matchers.define :raise_wiki_forbidden do + match do |actual| + expect { subject }.to raise_error(Gitlab::GitAccess::ForbiddenError, include('wiki')) + end + def supports_block_expectations? + true + end end end -- cgit v1.2.3