root
/
gitlab-ce
mirror of https://gitlab.com/free-releases/gitlab-ce.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

Add latest changes from gitlab-org/security/gitlab@14-5-stable-ee

This commit is contained in:
GitLab Bot 2021-12-03 10:05:41 +00:00
parent 01a6adb2b4
commit e12f099f39
27 changed files with 346 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
app/models/concerns/bulk_member_access_load.rb
View File

@ -9,11 +9,15 @@ module BulkMemberAccessLoad
# Determine the maximum access level for a group of resources in bulk.
#
# Returns a Hash mapping resource ID -> maximum access level.
def max_member_access_for_resource_ids(resource_klass, resource_ids, memoization_index = self.id, &block)
def max_member_access_for_resource_ids(resource_klass, resource_ids, &block)
raise 'Block is mandatory' unless block_given?
memoization_index = self.id
memoization_class = self.class
resource_ids = resource_ids.uniq
access = load_access_hash(resource_klass, memoization_index)
memo_id = "#{memoization_class}:#{memoization_index}"
access = load_access_hash(resource_klass, memo_id)
# Look up only the IDs we need
resource_ids -= access.keys
@ -33,8 +37,8 @@ module BulkMemberAccessLoad
access
end
def merge_value_to_request_store(resource_klass, resource_id, memoization_index, value)
max_member_access_for_resource_ids(resource_klass, [resource_id], memoization_index) do
def merge_value_to_request_store(resource_klass, resource_id, value)
max_member_access_for_resource_ids(resource_klass, [resource_id]) do
{ resource_id => value }
end
end
@ -45,16 +49,13 @@ module BulkMemberAccessLoad
"max_member_access_for_#{klass.name.underscore.pluralize}:#{memoization_index}"
end
def load_access_hash(resource_klass, memoization_index)
key = max_member_access_for_resource_key(resource_klass, memoization_index)
def load_access_hash(resource_klass, memo_id)
return {} unless Gitlab::SafeRequestStore.active?
access = {}
if Gitlab::SafeRequestStore.active?
Gitlab::SafeRequestStore[key] ||= {}
access = Gitlab::SafeRequestStore[key]
end
key = max_member_access_for_resource_key(resource_klass, memo_id)
Gitlab::SafeRequestStore[key] ||= {}
access
Gitlab::SafeRequestStore[key]
end
end
end

1
app/models/concerns/diff_positionable_note.rb
View File

@ -12,6 +12,7 @@ module DiffPositionableNote
serialize :change_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize
validate :diff_refs_match_commit, if: :for_commit?
validates :position, json_schema: { filename: "position", hash_conversion: true }
end
%i(original_position position change_position).each do |meth|

9
app/models/preloaders/user_max_access_level_in_groups_preloader.rb
View File

@ -4,8 +4,6 @@ module Preloaders
# This class preloads the max access level (role) for the user within the given groups and
# stores the values in requests store.
class UserMaxAccessLevelInGroupsPreloader
include BulkMemberAccessLoad
def initialize(groups, user)
@groups = groups
@user = user
@ -27,8 +25,9 @@ module Preloaders
.group(:source_id)
.maximum(:access_level)
group_memberships.each do |group_id, max_access_level|
merge_value_to_request_store(User, @user.id, group_id, max_access_level)
@groups.each do |group|
access_level = group_memberships[group.id]
group.merge_value_to_request_store(User, @user.id, access_level) if access_level.present?
end
end
@ -41,7 +40,7 @@ module Preloaders
@groups.each do |group|
max_access_level = max_access_levels[group.id] || Gitlab::Access::NO_ACCESS
merge_value_to_request_store(User, @user.id, group.id, max_access_level)
group.merge_value_to_request_store(User, @user.id, max_access_level)
end
end

1
app/models/project.rb
View File

@ -36,6 +36,7 @@ class Project < ApplicationRecord
include Repositories::CanHousekeepRepository
include EachBatch
include GitlabRoutingHelper
include BulkMemberAccessLoad
extend Gitlab::Cache::RequestCache
extend Gitlab::Utils::Override

6
app/models/project_team.rb
View File

@ -1,8 +1,6 @@
# frozen_string_literal: true
class ProjectTeam
include BulkMemberAccessLoad
attr_accessor :project
def initialize(project)
@ -171,7 +169,7 @@ class ProjectTeam
#
# Returns a Hash mapping user ID -> maximum access level.
def max_member_access_for_user_ids(user_ids)
max_member_access_for_resource_ids(User, user_ids, project.id) do |user_ids|
project.max_member_access_for_resource_ids(User, user_ids) do |user_ids|
project.project_authorizations
.where(user: user_ids)
.group(:user_id)
@ -180,7 +178,7 @@ class ProjectTeam
end
def write_member_access_for_user_id(user_id, project_access_level)
merge_value_to_request_store(User, user_id, project.id, project_access_level)
project.merge_value_to_request_store(User, user_id, project_access_level)
end
def max_member_access(user_id)

4
app/validators/json_schema_validator.rb
View File

@ -24,8 +24,10 @@ class JsonSchemaValidator < ActiveModel::EachValidator
end
def validate_each(record, attribute, value)
value = value.to_h.stringify_keys if options[:hash_conversion] == true
unless valid_schema?(value)
record.errors.add(attribute, "must be a valid json schema")
record.errors.add(attribute, _("must be a valid json schema"))
end
end

151
app/validators/json_schemas/position.json Normal file
View File

@ -0,0 +1,151 @@
{
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Gitlab::Diff::Position",
"type": "object",
"additionalProperties": false,
"properties": {
"base_sha": {
"oneOf": [
{ "type": "null" },
{ "type": "string", "maxLength": 40 }
]
},
"start_sha": {
"oneOf": [
{ "type": "null" },
{ "type": "string", "maxLength": 40 }
]
},
"head_sha": {
"oneOf": [
{ "type": "null" },
{ "type": "string", "maxLength": 40 }
]
},
"file_identifier_hash": {
"oneOf": [
{ "type": "null" },
{ "type": "string", "maxLength": 40 }
]
},
"old_path": {
"oneOf": [
{ "type": "null" },
{ "type": "string", "maxLength": 1000 }
]
},
"new_path": {
"oneOf": [
{ "type": "null" },
{ "type": "string", "maxLength": 1000 }
]
},
"position_type": {
"oneOf": [
{ "type": "null" },
{ "type": "string", "maxLength": 10 }
]
},
"old_line": {
"oneOf": [
{ "type": "null" },
{ "type": "integer" }
]
},
"new_line": {
"oneOf": [
{ "type": "null" },
{ "type": "integer" }
]
},
"line_range": {
"oneOf": [
{ "type": "null" },
{
"type": "object",
"additionalProperties": false,
"properties": {
"start": {
"type": "object",
"additionalProperties": false,
"properties": {
"line_code": { "type": "string", "maxLength": 100 },
"type": {
"oneOf": [
{ "type": "null" },
{ "type": "string", "maxLength": 100 }
]
},
"old_line": {
"oneOf": [
{ "type": "null" },
{ "type": "integer" }
]
},
"new_line": {
"oneOf": [
{ "type": "null" },
{ "type": "integer" }
]
}
}
},
"end": {
"type": "object",
"additionalProperties": false,
"properties": {
"line_code": { "type": "string", "maxLength": 100 },
"type": {
"oneOf": [
{ "type": "null" },
{ "type": "string", "maxLength": 100 }
]
},
"old_line": {
"oneOf": [
{ "type": "null" },
{ "type": "integer" }
]
},
"new_line": {
"oneOf": [
{ "type": "null" },
{ "type": "integer" }
]
}
}
}
}
}
]
},
"width": {
"oneOf": [
{ "type": "null" },
{ "type": "integer" },
{ "type": "string", "maxLength": 10 }
]
},
"height": {
"oneOf": [
{ "type": "null" },
{ "type": "integer" },
{ "type": "string", "maxLength": 10 }
]
},
"x": {
"oneOf": [
{ "type": "null" },
{ "type": "integer" },
{ "type": "string", "maxLength": 10 }
]
},
"y": {
"oneOf": [
{ "type": "null" },
{ "type": "integer" },
{ "type": "string", "maxLength": 10 }
]
}
}
}

12
lib/api/lint.rb
View File

@ -4,6 +4,16 @@ module API
class Lint < ::API::Base
feature_category :pipeline_authoring
helpers do
def can_lint_ci?
signup_unrestricted = Gitlab::CurrentSettings.signup_enabled? && !Gitlab::CurrentSettings.signup_limited?
internal_user = current_user.present? && !current_user.external?
is_developer = current_user.present? && current_user.projects.any? { |p| p.team.member?(current_user, Gitlab::Access::DEVELOPER) }
signup_unrestricted || internal_user || is_developer
end
end
namespace :ci do
desc 'Validation of .gitlab-ci.yml content'
params do
@ -12,7 +22,7 @@ module API
optional :include_jobs, type: Boolean, desc: 'Whether or not to include CI jobs in the response'
end
post '/lint' do
unauthorized! if (Gitlab::CurrentSettings.signup_disabled? || Gitlab::CurrentSettings.signup_limited?) && current_user.nil?
unauthorized! unless can_lint_ci?
result = Gitlab::Ci::Lint.new(project: nil, current_user: current_user)
.validate(params[:content], dry_run: false)

2
lib/banzai/filter/front_matter_filter.rb
View File

@ -9,7 +9,7 @@ module Banzai
html.sub(Gitlab::FrontMatter::PATTERN) do |_match|
lang = $~[:lang].presence || lang_mapping[$~[:delim]]
["```#{lang}:frontmatter", $~[:front_matter], "```", "\n"].join("\n")
["```#{lang}:frontmatter", $~[:front_matter].strip!, "```", "\n"].join("\n")
end
end
end

2
lib/gitlab/current_settings.rb
View File

@ -8,7 +8,7 @@ module Gitlab
end
def signup_limited?
domain_allowlist.present? || email_restrictions_enabled? || require_admin_approval_after_user_signup?
domain_allowlist.present? || email_restrictions_enabled? || require_admin_approval_after_user_signup? || user_default_external?
end
def current_application_settings

1
lib/gitlab/diff/lines_unfolder.rb
View File

@ -57,6 +57,7 @@ module Gitlab
next false unless @position.unfoldable?
next false if @diff_file.new_file? || @diff_file.deleted_file?
next false unless @position.old_line
next false unless @position.old_line.is_a?(Integer)
# Invalid position (MR import scenario)
next false if @position.old_line > @blob.lines.size
next false if @diff_file.diff_lines.empty?

10
lib/gitlab/front_matter.rb
View File

@ -11,13 +11,11 @@ module Gitlab
DELIM = Regexp.union(DELIM_LANG.keys)
PATTERN = %r{
\A(?:[^\r\n]*coding:[^\r\n]*)? # optional encoding line
\A(?:[^\r\n]*coding:[^\r\n]*\R)? # optional encoding line
\s*
^(?<delim>#{DELIM})[ \t]*(?<lang>\S*) # opening front matter marker (optional language specifier)
\s*
^(?<front_matter>.*?) # front matter block content (not greedy)
\s*
^(\k<delim> | \.{3}) # closing front matter marker
^(?<delim>#{DELIM})[ \t]*(?<lang>\S*)\R # opening front matter marker (optional language specifier)
(?<front_matter>.*?) # front matter block content (not greedy)
^(\k<delim> | \.{3}) # closing front matter marker
\s*
}mx.freeze
end

2
lib/gitlab/wiki_pages/front_matter_parser.rb
View File

@ -54,7 +54,7 @@ module Gitlab
def initialize(delim = nil, lang = '', text = nil)
@lang = lang.downcase.presence || Gitlab::FrontMatter::DELIM_LANG[delim]
@text = text
@text = text&.strip!
end
def data

3
locale/gitlab.pot
View File

@ -41577,6 +41577,9 @@ msgstr ""
msgid "must be a valid IPv4 or IPv6 address"
msgstr ""
msgid "must be a valid json schema"
msgstr ""
msgid "must be after start"
msgstr ""

8
spec/factories/diff_position.rb
View File

@ -43,8 +43,12 @@ FactoryBot.define do
trait :multi_line do
line_range do
{
start_line_code: Gitlab::Git.diff_line_code(file, 10, 10),
end_line_code: Gitlab::Git.diff_line_code(file, 12, 13)
start: {
line_code: Gitlab::Git.diff_line_code(file, 10, 10)
},
end: {
line_code: Gitlab::Git.diff_line_code(file, 12, 13)
}
}
end
end

4
spec/frontend/diffs/store/utils_spec.js
View File

@ -138,7 +138,7 @@ describe('DiffsStoreUtils', () => {
old_line: 1,
},
linePosition: LINE_POSITION_LEFT,
lineRange: { start_line_code: 'abc_1_1', end_line_code: 'abc_2_2' },
lineRange: { start: { line_code: 'abc_1_1' }, end: { line_code: 'abc_2_2' } },
};
const position = JSON.stringify({
@ -608,7 +608,7 @@ describe('DiffsStoreUtils', () => {
// When multi line comments are fully implemented `line_code` will be
// included in all requests. Until then we need to ensure the logic does
// not change when it is included only in the "comparison" argument.
const lineRange = { start_line_code: 'abc_1_1', end_line_code: 'abc_1_2' };
const lineRange = { start: { line_code: 'abc_1_1' }, end: { line_code: 'abc_1_2' } };
it('returns true when the discussion is up to date', () => {
expect(

16
spec/lib/banzai/filter/front_matter_filter_spec.rb
View File

@ -139,4 +139,20 @@ RSpec.describe Banzai::Filter::FrontMatterFilter do
end
end
end
it 'fails fast for strings with many spaces' do
content = "coding:" + " " * 50_000 + ";"
expect do
Timeout.timeout(3.seconds) { filter(content) }
end.not_to raise_error
end
it 'fails fast for strings with many newlines' do
content = "coding:\n" + ";;;" + "\n" * 10_000 + "x"
expect do
Timeout.timeout(3.seconds) { filter(content) }
end.not_to raise_error
end
end

10
spec/lib/gitlab/current_settings_spec.rb
View File

@ -51,9 +51,17 @@ RSpec.describe Gitlab::CurrentSettings do
it { is_expected.to be_truthy }
end
context 'when new users are set to external' do
before do
create(:application_setting, user_default_external: true)
end
it { is_expected.to be_truthy }
end
context 'when there are no restrictions' do
before do
create(:application_setting, domain_allowlist: [], email_restrictions_enabled: false, require_admin_approval_after_user_signup: false)
create(:application_setting, domain_allowlist: [], email_restrictions_enabled: false, require_admin_approval_after_user_signup: false, user_default_external: false)
end
it { is_expected.to be_falsey }

6
spec/lib/gitlab/diff/formatters/text_formatter_spec.rb
View File

@ -47,14 +47,14 @@ RSpec.describe Gitlab::Diff::Formatters::TextFormatter do
describe "#==" do
it "is false when the line_range changes" do
formatter_1 = described_class.new(base.merge(line_range: { start_line_code: "foo", end_line_code: "bar" }))
formatter_2 = described_class.new(base.merge(line_range: { start_line_code: "foo", end_line_code: "baz" }))
formatter_1 = described_class.new(base.merge(line_range: { "start": { "line_code" => "foo" }, "end": { "line_code" => "bar" } }))
formatter_2 = described_class.new(base.merge(line_range: { "start": { "line_code" => "foo" }, "end": { "line_code" => "baz" } }))
expect(formatter_1).not_to eq(formatter_2)
end
it "is true when the line_range doesn't change" do
attrs = base.merge({ line_range: { start_line_code: "foo", end_line_code: "baz" } })
attrs = base.merge({ line_range: { start: { line_code: "foo" }, end: { line_code: "baz" } } })
formatter_1 = described_class.new(attrs)
formatter_2 = described_class.new(attrs)

10
spec/lib/gitlab/diff/lines_unfolder_spec.rb
View File

@ -215,6 +215,16 @@ RSpec.describe Gitlab::Diff::LinesUnfolder do
build(:text_diff_position, old_line: 43, new_line: 40)
end
context 'old_line is an invalid number' do
let(:position) do
build(:text_diff_position, old_line: "foo", new_line: 40)
end
it 'fails gracefully' do
expect(subject.unfolded_diff_lines).to be_nil
end
end
context 'blob lines' do
let(:expected_blob_lines) do
[[40, 40, " \"config-opts\": [ \"--disable-introspection\" ],"],

24
spec/lib/gitlab/diff/position_tracer/line_strategy_spec.rb
View File

@ -295,8 +295,12 @@ RSpec.describe Gitlab::Diff::PositionTracer::LineStrategy, :clean_gitlab_redis_c
new_path: file_name,
new_line: 2,
line_range: {
"start_line_code" => 1,
"end_line_code" => 2
"start" => {
"line_code" => 1
},
"end" => {
"line_code" => 2
}
}
)
end
@ -575,8 +579,12 @@ RSpec.describe Gitlab::Diff::PositionTracer::LineStrategy, :clean_gitlab_redis_c
new_path: file_name,
new_line: 2,
line_range: {
"start_line_code" => 1,
"end_line_code" => 2
"start" => {
"line_code" => 1
},
"end" => {
"line_code" => 2
}
}
)
end
@ -588,8 +596,12 @@ RSpec.describe Gitlab::Diff::PositionTracer::LineStrategy, :clean_gitlab_redis_c
old_line: nil,
new_line: 2,
line_range: {
"start_line_code" => 1,
"end_line_code" => 2
"start" => {
"line_code" => 1
},
"end" => {
"line_code" => 2
}
}
)
end

2
spec/lib/gitlab/wiki_pages/front_matter_parser_spec.rb
View File

@ -118,7 +118,7 @@ RSpec.describe Gitlab::WikiPages::FrontMatterParser do
MD
end
it { is_expected.to have_attributes(reason: :not_mapping) }
it { is_expected.to have_attributes(reason: :no_match) }
end
context 'there is a string in the YAML block' do

3
spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb
View File

@ -13,7 +13,8 @@ RSpec.describe Preloaders::UserMaxAccessLevelInGroupsPreloader do
shared_examples 'executes N max member permission queries to the DB' do
it 'executes the specified max membership queries' do
expect { groups.each { |group| user.can?(:read_group, group) } }.to make_queries_matching(max_query_regex, expected_query_count)
expect { groups.each { |group| user.can?(:read_group, group) } }
.to make_queries_matching(max_query_regex, expected_query_count)
end
it 'caches the correct access_level for each group' do

29
spec/requests/api/lint_spec.rb
View File

@ -26,6 +26,35 @@ RSpec.describe API::Lint do
expect(response).to have_gitlab_http_status(:ok)
end
end
context 'when authenticated as external user' do
let(:project) { create(:project) }
let(:api_user) { create(:user, :external) }
context 'when reporter in a project' do
before do
project.add_reporter(api_user)
end
it 'returns authorization failure' do
post api('/ci/lint', api_user), params: { content: 'content' }
expect(response).to have_gitlab_http_status(:unauthorized)
end
end
context 'when developer in a project' do
before do
project.add_developer(api_user)
end
it 'returns authorization success' do
post api('/ci/lint', api_user), params: { content: 'content' }
expect(response).to have_gitlab_http_status(:ok)
end
end
end
end
context 'when signup is enabled and not limited' do

33
spec/support/shared_examples/models/diff_positionable_note_shared_examples.rb
View File

@ -71,5 +71,38 @@ RSpec.shared_examples 'a valid diff positionable note' do |factory_on_commit|
end
end
end
describe 'schema validation' do
where(:position_attrs) do
[
{ old_path: SecureRandom.alphanumeric(1001) },
{ new_path: SecureRandom.alphanumeric(1001) },
{ old_line: "foo" }, # this should be an integer
{ new_line: "foo" }, # this should be an integer
{ line_range: { "foo": "bar" } },
{ line_range: { "line_code": SecureRandom.alphanumeric(101) } },
{ line_range: { "type": SecureRandom.alphanumeric(101) } },
{ line_range: { "old_line": "foo" } },
{ line_range: { "new_line": "foo" } }
]
end
with_them do
let(:position) do
Gitlab::Diff::Position.new(
{
old_path: "files/ruby/popen.rb",
new_path: "files/ruby/popen.rb",
old_line: nil,
new_line: 14,
line_range: nil,
diff_refs: diff_refs
}.merge(position_attrs)
)
end
it { is_expected.to be_invalid }
end
end
end
end

12
spec/support/shared_examples/requests/api/diff_discussions_shared_examples.rb
View File

@ -29,10 +29,14 @@ RSpec.shared_examples 'diff discussions API' do |parent_type, noteable_type, id_
describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions" do
it "creates a new diff note" do
line_range = {
"start_line_code" => Gitlab::Git.diff_line_code(diff_note.position.file_path, 1, 1),
"end_line_code" => Gitlab::Git.diff_line_code(diff_note.position.file_path, 2, 2),
"start_line_type" => diff_note.position.type,
"end_line_type" => diff_note.position.type
"start" => {
"line_code" => Gitlab::Git.diff_line_code(diff_note.position.file_path, 1, 1),
"type" => diff_note.position.type
},
"end" => {
"line_code" => Gitlab::Git.diff_line_code(diff_note.position.file_path, 2, 2),
"type" => diff_note.position.type
}
}
position = diff_note.position.to_h.merge({ line_range: line_range })

12
spec/validators/json_schema_validator_spec.rb
View File

@ -46,5 +46,17 @@ RSpec.describe JsonSchemaValidator do
expect { subject }.to raise_error(described_class::FilenameError)
end
end
describe 'hash_conversion option' do
context 'when hash_conversion is enabled' do
let(:validator) { described_class.new(attributes: [:data], filename: "build_report_result_data", hash_conversion: true) }
it 'returns no errors' do
subject
expect(build_report_result.errors).to be_empty
end
end
end
end
end