Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2023-07-31 17:34:04 +0300
committerGitLab Bot <gitlab-bot@gitlab.com>2023-07-31 17:34:24 +0300
commit3c93d74713f5a845429b4c19b046f57cc8ea325c (patch)
tree82a692612482b6a1369986e390c7d78958ddf9f0
parentf5fe9b63037d428aecb04c375579ef022ba98e1d (diff)
Add latest changes from gitlab-org/security/gitlab@16-2-stable-ee
-rw-r--r--.rubocop_todo/rspec/missing_feature_category.yml1
-rw-r--r--app/assets/javascripts/diffs/store/mutations.js2
-rw-r--r--app/assets/javascripts/diffs/utils/diff_file.js6
-rw-r--r--app/assets/javascripts/notes/components/diff_with_note.vue3
-rw-r--r--app/assets/javascripts/notes/components/noteable_discussion.vue2
-rw-r--r--app/controllers/projects/pipeline_schedules_controller.rb10
-rw-r--r--app/policies/ci/pipeline_schedule_policy.rb24
-rw-r--r--app/services/discussions/capture_diff_note_positions_service.rb2
-rw-r--r--lib/banzai/filter/autolink_filter.rb15
-rw-r--r--lib/gitlab/harbor/query.rb2
-rw-r--r--lib/gitlab/path_regex.rb2
-rw-r--r--package.json2
-rw-r--r--spec/controllers/projects/pipeline_schedules_controller_spec.rb69
-rw-r--r--spec/factories/diff_position.rb4
-rw-r--r--spec/features/merge_request/user_views_comment_on_diff_file_spec.rb49
-rw-r--r--spec/lib/banzai/filter/autolink_filter_spec.rb16
-rw-r--r--spec/lib/banzai/filter/references/project_reference_filter_spec.rb1
-rw-r--r--spec/lib/gitlab/harbor/query_spec.rb23
-rw-r--r--spec/policies/ci/pipeline_schedule_policy_spec.rb185
-rw-r--r--spec/services/ci/pipeline_schedules/update_service_spec.rb4
-rw-r--r--yarn.lock8
21 files changed, 337 insertions, 93 deletions
diff --git a/.rubocop_todo/rspec/missing_feature_category.yml b/.rubocop_todo/rspec/missing_feature_category.yml
index 0a559874217..cdd58e0c046 100644
--- a/.rubocop_todo/rspec/missing_feature_category.yml
+++ b/.rubocop_todo/rspec/missing_feature_category.yml
@@ -5088,7 +5088,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/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index 4855ca87e91..f90e0a24d0e 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -167,7 +167,7 @@ export default {
originalStartLineCode,
...(discussion.line_codes || []),
];
- const fileHash = discussion.diff_file.file_hash;
+ const fileHash = discussion.diff_file?.file_hash;
const lineCheck = (line) =>
discussionLineCodes.some(
(discussionLineCode) =>
diff --git a/app/assets/javascripts/diffs/utils/diff_file.js b/app/assets/javascripts/diffs/utils/diff_file.js
index f2a3224d332..98e1c1cc849 100644
--- a/app/assets/javascripts/diffs/utils/diff_file.js
+++ b/app/assets/javascripts/diffs/utils/diff_file.js
@@ -77,7 +77,7 @@ export function prepareRawDiffFile({ file, allFiles, meta = false, index = -1 })
}
export function collapsedType(file) {
- const isManual = typeof file.viewer?.manuallyCollapsed === 'boolean';
+ const isManual = typeof file?.viewer?.manuallyCollapsed === 'boolean';
return isManual ? DIFF_FILE_MANUAL_COLLAPSE : DIFF_FILE_AUTOMATIC_COLLAPSE;
}
@@ -85,8 +85,8 @@ export function collapsedType(file) {
export function isCollapsed(file) {
const type = collapsedType(file);
const collapsedStates = {
- [DIFF_FILE_AUTOMATIC_COLLAPSE]: file.viewer?.automaticallyCollapsed || false,
- [DIFF_FILE_MANUAL_COLLAPSE]: file.viewer?.manuallyCollapsed,
+ [DIFF_FILE_AUTOMATIC_COLLAPSE]: file?.viewer?.automaticallyCollapsed || false,
+ [DIFF_FILE_MANUAL_COLLAPSE]: file?.viewer?.manuallyCollapsed,
};
return collapsedStates[type];
diff --git a/app/assets/javascripts/notes/components/diff_with_note.vue b/app/assets/javascripts/notes/components/diff_with_note.vue
index db32079e6b9..b1a2ab77fa8 100644
--- a/app/assets/javascripts/notes/components/diff_with_note.vue
+++ b/app/assets/javascripts/notes/components/diff_with_note.vue
@@ -41,7 +41,7 @@ export default {
return getDiffMode(this.discussion.diff_file);
},
diffViewerMode() {
- return this.discussion.diff_file.viewer.name;
+ return this.discussion.diff_file?.viewer.name;
},
fileDiffRefs() {
return this.discussion.diff_file.diff_refs;
@@ -96,6 +96,7 @@ export default {
<template>
<div :class="{ 'text-file': isTextFile }" class="diff-file file-holder">
<diff-file-header
+ v-if="discussion.diff_file"
:discussion-path="discussion.discussion_path"
:diff-file="discussion.diff_file"
:can-current-user-fork="false"
diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue
index a5939e1023c..7e79edfea15 100644
--- a/app/assets/javascripts/notes/components/noteable_discussion.vue
+++ b/app/assets/javascripts/notes/components/noteable_discussion.vue
@@ -169,7 +169,7 @@ export default {
return !this.discussionResolved ? this.discussion.resolve_with_issue_path : '';
},
canShowReplyActions() {
- if (this.shouldRenderDiffs && !this.discussion.diff_file.diff_refs) {
+ if (this.shouldRenderDiffs && !this.discussion.diff_file?.diff_refs) {
return false;
}
diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb
index 4fd307b5105..96c9aa89953 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
@@ -113,6 +112,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/app/services/discussions/capture_diff_note_positions_service.rb b/app/services/discussions/capture_diff_note_positions_service.rb
index 3684a3f679a..f9b31e0f2f1 100644
--- a/app/services/discussions/capture_diff_note_positions_service.rb
+++ b/app/services/discussions/capture_diff_note_positions_service.rb
@@ -26,7 +26,7 @@ module Discussions
active_diff_discussions = merge_request.notes.new_diff_notes.discussions.select do |discussion|
discussion.active?(merge_request.diff_refs)
end
- paths = active_diff_discussions.flat_map { |n| n.diff_file.paths }
+ paths = active_diff_discussions.flat_map { |n| n.diff_file&.paths }
[active_diff_discussions, paths]
end
diff --git a/lib/banzai/filter/autolink_filter.rb b/lib/banzai/filter/autolink_filter.rb
index 336d60055e2..bbddaa37380 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 `(?<!` is unsupported in `re2`, see https://github.com/google/re2/wiki/Syntax
LINK_PATTERN = %r{([a-z][a-z0-9\+\.-]+://[^\s>]+)(?<!\?|!|\.|,|:)}.freeze
+ ENTITY_UNTRUSTED = '((?:&[\w#]+;)+)\z'
+ ENTITY_UNTRUSTED_REGEX = Gitlab::UntrustedRegexp.new(ENTITY_UNTRUSTED, multiline: false)
+
# Text matching LINK_PATTERN inside these elements will not be linked
IGNORE_PARENTS = %w(a code kbd pre script style).to_set
@@ -85,10 +90,14 @@ module Banzai
# Remove any trailing HTML entities and store them for appending
# outside the link element. The entity must be marked HTML safe in
# order to be output literally rather than escaped.
- match.gsub!(/((?:&[\w#]+;)+)\z/, '')
- dropped = (Regexp.last_match(1) || '').html_safe
+ dropped = ''
+ match = ENTITY_UNTRUSTED_REGEX.replace_gsub(match) do |entities|
+ dropped = entities[1].html_safe
+
+ ''
+ end
- # To match the behaviour of Rinku, if the matched link ends with a
+ # To match the behavior of Rinku, if the matched link ends with a
# closing part of a matched pair of punctuation, we remove that trailing
# character unless there are an equal number of closing and opening
# characters in the link.
diff --git a/lib/gitlab/harbor/query.rb b/lib/gitlab/harbor/query.rb
index fcd984b01ce..fc0ac539e07 100644
--- a/lib/gitlab/harbor/query.rb
+++ b/lib/gitlab/harbor/query.rb
@@ -25,7 +25,7 @@ module Gitlab
message: 'params invalid'
}, allow_blank: true
validates :search, format: {
- with: /\A([a-z\_]*=[a-zA-Z0-9\- :]*,*)*\z/,
+ with: /\A(name=[a-zA-Z0-9\-:]+(?:,name=[a-zA-Z0-9\-:]+)*)\z/,
message: 'params invalid'
}, allow_blank: true
diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb
index e112423f167..8afcf682d5d 100644
--- a/lib/gitlab/path_regex.rb
+++ b/lib/gitlab/path_regex.rb
@@ -131,7 +131,7 @@ module Gitlab
# `NAMESPACE_FORMAT_REGEX`, with the negative lookbehind assertion removed. This means that the client-side validation
# will pass for usernames ending in `.atom` and `.git`, but will be caught by the server-side validation.
PATH_START_CHAR = '[a-zA-Z0-9_\.]'
- PATH_REGEX_STR = PATH_START_CHAR + '[a-zA-Z0-9_\-\.]*'
+ PATH_REGEX_STR = PATH_START_CHAR + '[a-zA-Z0-9_\-\.]' + "{0,#{Namespace::URL_MAX_LENGTH - 1}}"
NAMESPACE_FORMAT_REGEX_JS = PATH_REGEX_STR + '[a-zA-Z0-9_\-]|[a-zA-Z0-9_]'
NO_SUFFIX_REGEX = /(?<!\.git|\.atom)/.freeze
diff --git a/package.json b/package.json
index 7677d108133..ac838559367 100644
--- a/package.json
+++ b/package.json
@@ -60,7 +60,7 @@
"@gitlab/svgs": "3.55.0",
"@gitlab/ui": "64.20.1",
"@gitlab/visual-review-tools": "1.7.3",
- "@gitlab/web-ide": "0.0.1-dev-20230713160749",
+ "@gitlab/web-ide": "0.0.1-dev-20230713160749-patch-1",
"@mattiasbuelens/web-streams-adapter": "^0.1.0",
"@popperjs/core": "^2.11.2",
"@rails/actioncable": "7.0.6",
diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
index 486062fe52b..e15f07a4e22 100644
--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
@@ -4,6 +4,7 @@ require 'spec_helper'
RSpec.describe Projects::PipelineSchedulesController, feature_category: :continuous_integration do
include AccessMatchersForController
+ using RSpec::Parameterized::TableSyntax
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :public, :repository) }
@@ -45,6 +46,43 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu
end
end
+ shared_examples 'protecting ref' do
+ where(:branch_access_levels, :tag_access_level, :maintainer_accessible, :developer_accessible) do
+ [:no_one_can_push, :no_one_can_merge] | :no_one_can_create | \
+ :be_denied_for | :be_denied_for
+ [:maintainers_can_push, :maintainers_can_merge] | :maintainers_can_create | \
+ :be_allowed_for | :be_denied_for
+ [:developers_can_push, :developers_can_merge] | :developers_can_create | \
+ :be_allowed_for | :be_allowed_for
+ end
+
+ with_them do
+ context 'when branch is protected' do
+ let(:ref_prefix) { 'heads' }
+ let(:ref_name) { 'master' }
+
+ before do
+ create(:protected_branch, *branch_access_levels, name: ref_name, project: project)
+ end
+
+ it { expect { go }.to try(maintainer_accessible, :maintainer).of(project) }
+ it { expect { go }.to try(developer_accessible, :developer).of(project) }
+ end
+
+ context 'when tag is protected' do
+ let(:ref_prefix) { 'tags' }
+ let(:ref_name) { 'v1.0.0' }
+
+ before do
+ create(:protected_tag, tag_access_level, name: ref_name, project: project)
+ end
+
+ it { expect { go }.to try(maintainer_accessible, :maintainer).of(project) }
+ it { expect { go }.to try(developer_accessible, :developer).of(project) }
+ end
+ end
+ end
+
describe 'GET #index' do
render_views
@@ -159,7 +197,9 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu
end
describe 'security' do
- let(:schedule) { attributes_for(:ci_pipeline_schedule) }
+ let(:schedule) { attributes_for(:ci_pipeline_schedule, ref: "refs/#{ref_prefix}/#{ref_name}") }
+ let(:ref_prefix) { 'heads' }
+ let(:ref_name) { "master" }
it 'is allowed for admin when admin mode enabled', :enable_admin_mode do
expect { go }.to be_allowed_for(:admin)
@@ -178,6 +218,8 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu
it { expect { go }.to be_denied_for(:user) }
it { expect { go }.to be_denied_for(:external) }
it { expect { go }.to be_denied_for(:visitor) }
+
+ it_behaves_like 'protecting ref'
end
def go
@@ -438,7 +480,7 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu
end
describe 'POST #play', :clean_gitlab_redis_rate_limiting do
- let(:ref) { 'master' }
+ let(:ref_name) { 'master' }
before do
project.add_developer(user)
@@ -454,7 +496,7 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu
it 'does not allow pipeline to be executed' do
expect(RunPipelineScheduleWorker).not_to receive(:perform_async)
- post :play, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id }
+ go
expect(response).to have_gitlab_http_status(:not_found)
end
@@ -464,16 +506,14 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu
it 'executes a new pipeline' do
expect(RunPipelineScheduleWorker).to receive(:perform_async).with(pipeline_schedule.id, user.id).and_return('job-123')
- post :play, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id }
+ go
expect(flash[:notice]).to start_with 'Successfully scheduled a pipeline to run'
expect(response).to have_gitlab_http_status(:found)
end
it 'prevents users from scheduling the same pipeline repeatedly' do
- 2.times do
- post :play, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id }
- end
+ 2.times { go }
expect(flash.to_a.size).to eq(2)
expect(flash[:alert]).to eq _('You cannot play this scheduled pipeline at the moment. Please wait a minute.')
@@ -481,17 +521,14 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu
end
end
- context 'when a developer attempts to schedule a protected ref' do
- it 'does not allow pipeline to be executed' do
- create(:protected_branch, project: project, name: ref)
- protected_schedule = create(:ci_pipeline_schedule, project: project, ref: ref)
-
- expect(RunPipelineScheduleWorker).not_to receive(:perform_async)
+ describe 'security' do
+ let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, ref: "refs/#{ref_prefix}/#{ref_name}") }
- post :play, params: { namespace_id: project.namespace.to_param, project_id: project, id: protected_schedule.id }
+ it_behaves_like 'protecting ref'
+ end
- expect(response).to have_gitlab_http_status(:not_found)
- end
+ def go
+ post :play, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id }
end
end
diff --git a/spec/factories/diff_position.rb b/spec/factories/diff_position.rb
index bd248452de8..87875e0fa4d 100644
--- a/spec/factories/diff_position.rb
+++ b/spec/factories/diff_position.rb
@@ -54,6 +54,10 @@ FactoryBot.define do
end
end
+ factory :file_diff_position do
+ position_type { 'file' }
+ end
+
factory :image_diff_position do
position_type { 'image' }
x { 1 }
diff --git a/spec/features/merge_request/user_views_comment_on_diff_file_spec.rb b/spec/features/merge_request/user_views_comment_on_diff_file_spec.rb
new file mode 100644
index 00000000000..26dbf5605ec
--- /dev/null
+++ b/spec/features/merge_request/user_views_comment_on_diff_file_spec.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'User views comment on a diff file', :js, feature_category: :code_review_workflow do
+ include MergeRequestDiffHelpers
+ include RepoHelpers
+
+ let_it_be(:project) { create(:project, :repository) }
+ let_it_be(:merge_request) do
+ create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test')
+ end
+
+ let_it_be(:user) { create(:user) }
+
+ before_all do
+ project.add_maintainer(user)
+ end
+
+ before do
+ sign_in(user)
+
+ visit(diffs_project_merge_request_path(project, merge_request))
+ end
+
+ context 'with invalid start_sha position' do
+ before do
+ diff_refs = Gitlab::Diff::DiffRefs.new(
+ base_sha: merge_request.diff_refs.base_sha,
+ start_sha: 'fakesha',
+ head_sha: merge_request.diff_refs.head_sha
+ )
+ position = build(:file_diff_position, file: 'files/ruby/popen.rb', diff_refs: diff_refs)
+ create(:diff_note_on_merge_request, noteable: merge_request, project: project, position: position)
+ end
+
+ it 'renders diffs' do
+ visit diffs_project_merge_request_path(project, merge_request)
+
+ expect(page).to have_selector('.diff-file')
+ end
+
+ it 'renders discussion on overview tab' do
+ visit project_merge_request_path(project, merge_request)
+
+ expect(page).to have_selector('.note-discussion')
+ end
+ end
+end
diff --git a/spec/lib/banzai/filter/autolink_filter_spec.rb b/spec/lib/banzai/filter/autolink_filter_spec.rb
index c8b5a9ffa0b..fe642855f3b 100644
--- a/spec/lib/banzai/filter/autolink_filter_spec.rb
+++ b/spec/lib/banzai/filter/autolink_filter_spec.rb
@@ -226,6 +226,22 @@ RSpec.describe Banzai::Filter::AutolinkFilter, feature_category: :team_planning
end
end
+ it 'protects against malicious backtracking' do
+ doc = "http://#{'&' * 1_000_000}x"
+
+ expect do
+ Timeout.timeout(30.seconds) { filter(doc) }
+ end.not_to raise_error
+ end
+
+ it 'does not timeout with excessively long scheme' do
+ doc = "#{'h' * 1_000_000}://example.com"
+
+ expect do
+ Timeout.timeout(30.seconds) { filter(doc) }
+ end.not_to raise_error
+ end
+
# Rinku does not escape these characters in HTML attributes, but content_tag
# does. We don't care about that difference for these specs, though.
def unescape(html)
diff --git a/spec/lib/banzai/filter/references/project_reference_filter_spec.rb b/spec/lib/banzai/filter/references/project_reference_filter_spec.rb
index 49c71d08364..b6d6ff2309a 100644
--- a/spec/lib/banzai/filter/references/project_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/references/project_reference_filter_spec.rb
@@ -39,6 +39,7 @@ RSpec.describe Banzai::Filter::References::ProjectReferenceFilter, feature_categ
it_behaves_like 'fails fast', 'A' * 50000
it_behaves_like 'fails fast', '/a' * 50000
+ it_behaves_like 'fails fast', "mailto:#{'a-' * 499_000}@aaaaaaaa..aaaaaaaa.example.com"
end
it 'allows references with text after the > character' do
diff --git a/spec/lib/gitlab/harbor/query_spec.rb b/spec/lib/gitlab/harbor/query_spec.rb
index dcb9a16b27b..fe05643e04a 100644
--- a/spec/lib/gitlab/harbor/query_spec.rb
+++ b/spec/lib/gitlab/harbor/query_spec.rb
@@ -3,6 +3,8 @@
require 'spec_helper'
RSpec.describe Gitlab::Harbor::Query do
+ using RSpec::Parameterized::TableSyntax
+
let_it_be(:harbor_integration) { create(:harbor_integration) }
let(:params) { {} }
@@ -111,19 +113,20 @@ RSpec.describe Gitlab::Harbor::Query do
end
context 'search' do
- context 'with valid search' do
- let(:params) { { search: 'name=desc' } }
-
- it 'initialize successfully' do
- expect(query.valid?).to eq(true)
- end
+ where(:search_param, :is_valid) do
+ "name=desc" | true
+ "name=value1,name=value-2" | true
+ "name=value1,name=value_2" | false
+ "name=desc,key=value" | false
+ "name=value1, name=value2" | false
+ "name" | false
end
- context 'with invalid search' do
- let(:params) { { search: 'blabla' } }
+ with_them do
+ let(:params) { { search: search_param } }
- it 'initialize failed' do
- expect(query.valid?).to eq(false)
+ it "validates according to the regex" do
+ expect(query.valid?).to eq(is_valid)
end
end
end
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 '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
-
- it 'does not include ability to play pipeline schedule' do
- expect(policy).to be_disallowed :play_pipeline_schedule
+ 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
+
+ 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
+
+ :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
-
- it 'includes ability to update pipeline' do
- expect(policy).to be_allowed :play_pipeline_schedule
- end
- end
-
- context 'when no one can create the tag' do
- let(:tag) { 'v1.0.0' }
-
- before do
- pipeline_schedule.update!(ref: tag)
-
- create(:protected_tag, :no_one_can_create, name: pipeline_schedule.ref, project: project)
- end
-
- it 'does not include ability to play pipeline schedule' do
- expect(policy).to be_disallowed :play_pipeline_schedule
- end
- end
-
- 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
-
- it 'includes ability to play pipeline schedule' do
- expect(policy).to be_allowed :play_pipeline_schedule
+ 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
+
+ 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
+
+ :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
+
+ :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
+
+ with_them do
+ before do
+ create(:protected_tag, access_level, name: tag_ref_name, project: project)
+ project.add_role(user, project_role)
+ end
+
+ context 'for create_pipeline_schedule' do
+ subject(:policy) { described_class.new(user, new_tag_pipeline_schedule) }
+
+ let(:new_tag_pipeline_schedule) { project.pipeline_schedules.new(ref: tag_ref) }
+
+ 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 c31a652ed93..5c1354bd5aa 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
let_it_be(:pipeline_schedule_variable) do
create(:ci_pipeline_schedule_variable,
diff --git a/yarn.lock b/yarn.lock
index 2282ff0ddfc..123d6a0bda1 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -1151,10 +1151,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-20230713160749":
- version "0.0.1-dev-20230713160749"
- resolved "https://registry.yarnpkg.com/@gitlab/web-ide/-/web-ide-0.0.1-dev-20230713160749.tgz#427aed5e9fa6be1d1f58dde282500759df204b6c"
- integrity sha512-QySxWa5dzuZw1XGtkFDuTX9CF/kvBMGoiM9PySH2cvx6mGvC9dphis06eeliNKkNZG3arjwNSC6/8JRkg96B5A==
+"@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.20.0":
version "3.20.0"