From 438a27b4b268ff093187fc5ee3261c96cc611402 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 31 Jul 2023 14:33:03 +0000 Subject: [PATCH] Add latest changes from gitlab-org/security/gitlab@16-0-stable-ee --- .../rspec/missing_feature_category.yml | 1 - .../projects/pipeline_schedules_controller.rb | 10 +- app/policies/ci/pipeline_schedule_policy.rb | 24 ++- lib/banzai/filter/autolink_filter.rb | 15 +- lib/gitlab/path_regex.rb | 2 +- package.json | 2 +- .../pipeline_schedules_controller_spec.rb | 69 ++++++-- .../lib/banzai/filter/autolink_filter_spec.rb | 16 ++ .../project_reference_filter_spec.rb | 1 + .../ci/pipeline_schedule_policy_spec.rb | 167 ++++++++++++++---- .../pipeline_schedules/update_service_spec.rb | 4 +- yarn.lock | 8 +- 12 files changed, 253 insertions(+), 66 deletions(-) diff --git a/.rubocop_todo/rspec/missing_feature_category.yml b/.rubocop_todo/rspec/missing_feature_category.yml index 58d02eb3641..e06fb034c96 100644 --- a/.rubocop_todo/rspec/missing_feature_category.yml +++ b/.rubocop_todo/rspec/missing_feature_category.yml @@ -5126,7 +5126,6 @@ RSpec/MissingFeatureCategory: - 'spec/policies/ci/bridge_policy_spec.rb' - 'spec/policies/ci/build_policy_spec.rb' - 'spec/policies/ci/pipeline_policy_spec.rb' - - 'spec/policies/ci/pipeline_schedule_policy_spec.rb' - 'spec/policies/ci/trigger_policy_spec.rb' - 'spec/policies/clusters/agent_policy_spec.rb' - 'spec/policies/clusters/agent_token_policy_spec.rb' diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb index fb332fec3b5..cd495627d66 100644 --- a/app/controllers/projects/pipeline_schedules_controller.rb +++ b/app/controllers/projects/pipeline_schedules_controller.rb @@ -21,7 +21,6 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController end def new - @schedule = project.pipeline_schedules.new end def create @@ -102,6 +101,15 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController variables_attributes: [:id, :variable_type, :key, :secret_value, :_destroy]) end + def new_schedule + # We need the `ref` here for `authorize_create_pipeline_schedule!` + @schedule ||= project.pipeline_schedules.new(ref: params.dig(:schedule, :ref)) + end + + def authorize_create_pipeline_schedule! + return access_denied! unless can?(current_user, :create_pipeline_schedule, new_schedule) + end + def authorize_play_pipeline_schedule! return access_denied! unless can?(current_user, :play_pipeline_schedule, schedule) end diff --git a/app/policies/ci/pipeline_schedule_policy.rb b/app/policies/ci/pipeline_schedule_policy.rb index 7b0d484f9f7..cbc60c4a30a 100644 --- a/app/policies/ci/pipeline_schedule_policy.rb +++ b/app/policies/ci/pipeline_schedule_policy.rb @@ -5,7 +5,18 @@ module Ci alias_method :pipeline_schedule, :subject condition(:protected_ref) do - ref_protected?(@user, @subject.project, @subject.project.repository.tag_exists?(@subject.ref), @subject.ref) + if full_ref?(@subject.ref) + is_tag = Gitlab::Git.tag_ref?(@subject.ref) + ref_name = Gitlab::Git.ref_name(@subject.ref) + else + # NOTE: this block should not be removed + # until the full ref validation is in place + # and all old refs are updated and validated + is_tag = @subject.project.repository.tag_exists?(@subject.ref) + ref_name = @subject.ref + end + + ref_protected?(@user, @subject.project, is_tag, ref_name) end condition(:owner_of_schedule) do @@ -31,6 +42,15 @@ module Ci enable :take_ownership_pipeline_schedule end - rule { protected_ref }.prevent :play_pipeline_schedule + rule { protected_ref }.policy do + prevent :play_pipeline_schedule + prevent :create_pipeline_schedule + end + + private + + def full_ref?(ref) + Gitlab::Git.tag_ref?(ref) || Gitlab::Git.branch_ref?(ref) + end end end diff --git a/lib/banzai/filter/autolink_filter.rb b/lib/banzai/filter/autolink_filter.rb index a86c1bb2892..f2460471cc7 100644 --- a/lib/banzai/filter/autolink_filter.rb +++ b/lib/banzai/filter/autolink_filter.rb @@ -34,8 +34,13 @@ module Banzai # https://github.com/vmg/rinku/blob/v2.0.1/ext/rinku/autolink.c#L65 # # Rubular: http://rubular.com/r/nrL3r9yUiq + # Note that it's not possible to use Gitlab::UntrustedRegexp for LINK_PATTERN, + # as `(?]+)(? character' do diff --git a/spec/policies/ci/pipeline_schedule_policy_spec.rb b/spec/policies/ci/pipeline_schedule_policy_spec.rb index 7025eda1ba1..8fc5c6ca296 100644 --- a/spec/policies/ci/pipeline_schedule_policy_spec.rb +++ b/spec/policies/ci/pipeline_schedule_policy_spec.rb @@ -2,10 +2,13 @@ require 'spec_helper' -RSpec.describe Ci::PipelineSchedulePolicy, :models, :clean_gitlab_redis_cache do +RSpec.describe Ci::PipelineSchedulePolicy, :models, :clean_gitlab_redis_cache, feature_category: :continuous_integration do + using RSpec::Parameterized::TableSyntax + let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project, :repository) } - let_it_be(:pipeline_schedule, reload: true) { create(:ci_pipeline_schedule, :nightly, project: project) } + let_it_be_with_reload(:project) { create(:project, :repository, create_tag: tag_ref_name) } + let_it_be_with_reload(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project) } + let_it_be(:tag_ref_name) { "v1.0.0" } let(:policy) do described_class.new(user, pipeline_schedule) @@ -13,51 +16,143 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models, :clean_gitlab_redis_cache do describe 'rules' do describe 'rules for protected ref' do - before do - project.add_developer(user) - end + context 'for branch' do + %w[refs/heads/master master].each do |branch_ref| + context "with #{branch_ref}" do + let_it_be(:branch_ref_name) { "master" } + let_it_be(:branch_pipeline_schedule) do + create(:ci_pipeline_schedule, :nightly, project: project, ref: branch_ref) + end - context 'when no one can push or merge to the branch' do - before do - create(:protected_branch, :no_one_can_push, name: pipeline_schedule.ref, project: project) - end + where(:push_access_level, :merge_access_level, :project_role, :accessible) do + :no_one_can_push | :no_one_can_merge | :owner | :be_disallowed + :no_one_can_push | :no_one_can_merge | :maintainer | :be_disallowed + :no_one_can_push | :no_one_can_merge | :developer | :be_disallowed + :no_one_can_push | :no_one_can_merge | :reporter | :be_disallowed + :no_one_can_push | :no_one_can_merge | :guest | :be_disallowed - it 'does not include ability to play pipeline schedule' do - expect(policy).to be_disallowed :play_pipeline_schedule + :maintainers_can_push | :no_one_can_merge | :owner | :be_allowed + :maintainers_can_push | :no_one_can_merge | :maintainer | :be_allowed + :maintainers_can_push | :no_one_can_merge | :developer | :be_disallowed + :maintainers_can_push | :no_one_can_merge | :reporter | :be_disallowed + :maintainers_can_push | :no_one_can_merge | :guest | :be_disallowed + + :developers_can_push | :no_one_can_merge | :owner | :be_allowed + :developers_can_push | :no_one_can_merge | :maintainer | :be_allowed + :developers_can_push | :no_one_can_merge | :developer | :be_allowed + :developers_can_push | :no_one_can_merge | :reporter | :be_disallowed + :developers_can_push | :no_one_can_merge | :guest | :be_disallowed + + :no_one_can_push | :maintainers_can_merge | :owner | :be_allowed + :no_one_can_push | :maintainers_can_merge | :maintainer | :be_allowed + :no_one_can_push | :maintainers_can_merge | :developer | :be_disallowed + :no_one_can_push | :maintainers_can_merge | :reporter | :be_disallowed + :no_one_can_push | :maintainers_can_merge | :guest | :be_disallowed + + :maintainers_can_push | :maintainers_can_merge | :owner | :be_allowed + :maintainers_can_push | :maintainers_can_merge | :maintainer | :be_allowed + :maintainers_can_push | :maintainers_can_merge | :developer | :be_disallowed + :maintainers_can_push | :maintainers_can_merge | :reporter | :be_disallowed + :maintainers_can_push | :maintainers_can_merge | :guest | :be_disallowed + + :developers_can_push | :maintainers_can_merge | :owner | :be_allowed + :developers_can_push | :maintainers_can_merge | :maintainer | :be_allowed + :developers_can_push | :maintainers_can_merge | :developer | :be_allowed + :developers_can_push | :maintainers_can_merge | :reporter | :be_disallowed + :developers_can_push | :maintainers_can_merge | :guest | :be_disallowed + + :no_one_can_push | :developers_can_merge | :owner | :be_allowed + :no_one_can_push | :developers_can_merge | :maintainer | :be_allowed + :no_one_can_push | :developers_can_merge | :developer | :be_allowed + :no_one_can_push | :developers_can_merge | :reporter | :be_disallowed + :no_one_can_push | :developers_can_merge | :guest | :be_disallowed + + :maintainers_can_push | :developers_can_merge | :owner | :be_allowed + :maintainers_can_push | :developers_can_merge | :maintainer | :be_allowed + :maintainers_can_push | :developers_can_merge | :developer | :be_allowed + :maintainers_can_push | :developers_can_merge | :reporter | :be_disallowed + :maintainers_can_push | :developers_can_merge | :guest | :be_disallowed + + :developers_can_push | :developers_can_merge | :owner | :be_allowed + :developers_can_push | :developers_can_merge | :maintainer | :be_allowed + :developers_can_push | :developers_can_merge | :developer | :be_allowed + :developers_can_push | :developers_can_merge | :reporter | :be_disallowed + :developers_can_push | :developers_can_merge | :guest | :be_disallowed + end + + with_them do + before do + create(:protected_branch, push_access_level, merge_access_level, name: branch_ref_name, + project: project) + project.add_role(user, project_role) + end + + context 'for create_pipeline_schedule' do + subject(:policy) { described_class.new(user, new_branch_pipeline_schedule) } + + let(:new_branch_pipeline_schedule) { project.pipeline_schedules.new(ref: branch_ref) } + + it { expect(policy).to try(accessible, :create_pipeline_schedule) } + end + + context 'for play_pipeline_schedule' do + subject(:policy) { described_class.new(user, branch_pipeline_schedule) } + + it { expect(policy).to try(accessible, :play_pipeline_schedule) } + end + end + end end end - context 'when developers can push to the branch' do - before do - create(:protected_branch, :developers_can_merge, name: pipeline_schedule.ref, project: project) - end + context 'for tag' do + %w[refs/tags/v1.0.0 v1.0.0].each do |tag_ref| + context "with #{tag_ref}" do + let_it_be(:tag_pipeline_schedule) do + create(:ci_pipeline_schedule, :nightly, project: project, ref: tag_ref) + end - it 'includes ability to update pipeline' do - expect(policy).to be_allowed :play_pipeline_schedule - end - end + where(:access_level, :project_role, :accessible) do + :no_one_can_create | :owner | :be_disallowed + :no_one_can_create | :maintainer | :be_disallowed + :no_one_can_create | :developer | :be_disallowed + :no_one_can_create | :reporter | :be_disallowed + :no_one_can_create | :guest | :be_disallowed - context 'when no one can create the tag' do - let(:tag) { 'v1.0.0' } + :maintainers_can_create | :owner | :be_allowed + :maintainers_can_create | :maintainer | :be_allowed + :maintainers_can_create | :developer | :be_disallowed + :maintainers_can_create | :reporter | :be_disallowed + :maintainers_can_create | :guest | :be_disallowed - before do - pipeline_schedule.update!(ref: tag) + :developers_can_create | :owner | :be_allowed + :developers_can_create | :maintainer | :be_allowed + :developers_can_create | :developer | :be_allowed + :developers_can_create | :reporter | :be_disallowed + :developers_can_create | :guest | :be_disallowed + end - create(:protected_tag, :no_one_can_create, name: pipeline_schedule.ref, project: project) - end + with_them do + before do + create(:protected_tag, access_level, name: tag_ref_name, project: project) + project.add_role(user, project_role) + end - it 'does not include ability to play pipeline schedule' do - expect(policy).to be_disallowed :play_pipeline_schedule - end - end + context 'for create_pipeline_schedule' do + subject(:policy) { described_class.new(user, new_tag_pipeline_schedule) } - context 'when no one can create the tag but it is not a tag' do - before do - create(:protected_tag, :no_one_can_create, name: pipeline_schedule.ref, project: project) - end + let(:new_tag_pipeline_schedule) { project.pipeline_schedules.new(ref: tag_ref) } - it 'includes ability to play pipeline schedule' do - expect(policy).to be_allowed :play_pipeline_schedule + it { expect(policy).to try(accessible, :create_pipeline_schedule) } + end + + context 'for play_pipeline_schedule' do + subject(:policy) { described_class.new(user, tag_pipeline_schedule) } + + it { expect(policy).to try(accessible, :play_pipeline_schedule) } + end + end + end end end end diff --git a/spec/services/ci/pipeline_schedules/update_service_spec.rb b/spec/services/ci/pipeline_schedules/update_service_spec.rb index 838f49f6dea..63c0c996a1a 100644 --- a/spec/services/ci/pipeline_schedules/update_service_spec.rb +++ b/spec/services/ci/pipeline_schedules/update_service_spec.rb @@ -6,7 +6,9 @@ RSpec.describe Ci::PipelineSchedules::UpdateService, feature_category: :continuo let_it_be(:user) { create(:user) } let_it_be(:reporter) { create(:user) } let_it_be(:project) { create(:project, :public, :repository) } - let_it_be(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } + let_it_be(:pipeline_schedule) do + create(:ci_pipeline_schedule, project: project, owner: user, ref: 'master') + end before_all do project.add_maintainer(user) diff --git a/yarn.lock b/yarn.lock index 9c9a98d4184..16d2c2c61a9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1134,10 +1134,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/visual-review-tools/-/visual-review-tools-1.7.3.tgz#9ea641146436da388ffbad25d7f2abe0df52c235" integrity sha512-NMV++7Ew1FSBDN1xiZaauU9tfeSfgDHcOLpn+8bGpP+O5orUPm2Eu66R5eC5gkjBPaXosNAxNWtriee+aFk4+g== -"@gitlab/web-ide@0.0.1-dev-20230511143809": - version "0.0.1-dev-20230511143809" - resolved "https://registry.yarnpkg.com/@gitlab/web-ide/-/web-ide-0.0.1-dev-20230511143809.tgz#c13dfb4d1edab2e020d4a102d4ec18048917490f" - integrity sha512-caP5WSaTuIhPrPGUWyvPT4np6swkKQHM1Pa9HiBnGhiOhhQ1+3X/+J9EoZXUhnhwiBzS7sp32Uyttam4am/sTA== +"@gitlab/web-ide@0.0.1-dev-20230713160749-patch-1": + version "0.0.1-dev-20230713160749-patch-1" + resolved "https://registry.yarnpkg.com/@gitlab/web-ide/-/web-ide-0.0.1-dev-20230713160749-patch-1.tgz#6420b55aae444533f9a4bd6269503d98a72aaa2e" + integrity sha512-Dh8XQyPwDY6fkd/A+hTHCqrD23u5qnlaxKu5myyxDEgBNGgu4SGblFU9B6NHNm8eGUZk6Cs5MuMk+NUvWRKbmA== "@graphql-eslint/eslint-plugin@3.18.0": version "3.18.0"