diff options
| -rw-r--r-- | config/initializers/rdoc_segfault_patch.rb | 21 | ||||
| -rw-r--r-- | lib/banzai/filter/syntax_highlight_filter.rb | 2 | ||||
| -rw-r--r-- | lib/gitlab/import_export/members_mapper.rb | 65 | ||||
| -rw-r--r-- | spec/initializers/rdoc_segfault_patch_spec.rb | 24 | ||||
| -rw-r--r-- | spec/lib/banzai/filter/syntax_highlight_filter_spec.rb | 6 | ||||
| -rw-r--r-- | spec/lib/gitlab/import_export/members_mapper_spec.rb | 53 |
6 files changed, 147 insertions, 24 deletions
diff --git a/config/initializers/rdoc_segfault_patch.rb b/config/initializers/rdoc_segfault_patch.rb new file mode 100644 index 00000000000..2494d7ef421 --- /dev/null +++ b/config/initializers/rdoc_segfault_patch.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +# Monkey patch of RDoc to prevent Ruby segfault due to +# stack buffer overflow Ruby bug - +# https://bugs.ruby-lang.org/issues/16376 +# +# Safe to remove once GitLab upgrades to Ruby 3.0 +# or once the fix is backported to 2.7.x and +# GitLab upgrades. +# https://gitlab.com/gitlab-org/gitlab/-/issues/351179 +class RDoc::Markup::ToHtml + def parseable?(_) + false + end +end + +class RDoc::Markup::Verbatim + def ruby? + false + end +end diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb index 07f82c98666..bcd9f39d1dc 100644 --- a/lib/banzai/filter/syntax_highlight_filter.rb +++ b/lib/banzai/filter/syntax_highlight_filter.rb @@ -56,7 +56,7 @@ module Banzai retry end - sourcepos_attr = sourcepos ? "data-sourcepos=\"#{sourcepos}\"" : '' + sourcepos_attr = sourcepos ? "data-sourcepos=\"#{escape_once(sourcepos)}\"" : '' highlighted = %(<div class="gl-relative markdown-code-block js-markdown-code"><pre #{sourcepos_attr} class="#{css_classes}" lang="#{language}" diff --git a/lib/gitlab/import_export/members_mapper.rb b/lib/gitlab/import_export/members_mapper.rb index dd7ec361dd8..d3b1bb6a57d 100644 --- a/lib/gitlab/import_export/members_mapper.rb +++ b/lib/gitlab/import_export/members_mapper.rb @@ -19,9 +19,8 @@ module Gitlab @exported_members.inject(missing_keys_tracking_hash) do |hash, member| if member['user'] old_user_id = member['user']['id'] - old_user_email = member.dig('user', 'public_email') || member.dig('user', 'email') - existing_user = User.find_by(find_user_query(old_user_email)) if old_user_email - hash[old_user_id] = existing_user.id if existing_user && add_team_member(member, existing_user) + existing_user_id = existing_users_email_map[get_email(member)] + hash[old_user_id] = existing_user_id if existing_user_id && add_team_member(member, existing_user_id) else add_team_member(member) end @@ -72,11 +71,45 @@ module Gitlab member&.user == @user && member.access_level >= highest_access_level end - def add_team_member(member, existing_user = nil) - return true if existing_user && @importable.members.exists?(user_id: existing_user.id) + # Returns {email => user_id} hash where user_id is an ID at current instance + def existing_users_email_map + @existing_users_email_map ||= begin + emails = @exported_members.map { |member| get_email(member) } + + User.by_user_email(emails).pluck(:email, :id).to_h + end + end + + # Returns {user_id => email} hash where user_id is an ID at source "old" instance + def exported_members_email_map + @exported_members_email_map ||= begin + result = {} + @exported_members.each do |member| + email = get_email(member) + + next unless email + + result[member.dig('user', 'id')] = email + end + + result + end + end + + def get_email(member_data) + return unless member_data['user'] + + member_data.dig('user', 'public_email') || member_data.dig('user', 'email') + end + + def add_team_member(member, existing_user_id = nil) + return true if existing_user_id && @importable.members.exists?(user_id: existing_user_id) - member['user'] = existing_user member_hash = member_hash(member) + if existing_user_id + member_hash.delete('user') + member_hash['user_id'] = existing_user_id + end member = relation_class.create(member_hash) @@ -92,11 +125,19 @@ module Gitlab end def member_hash(member) - parsed_hash(member).merge( + result = parsed_hash(member).merge( 'source_id' => @importable.id, 'importing' => true, 'access_level' => [member['access_level'], highest_access_level].min ).except('user_id') + + if result['created_by_id'] + created_by_email = exported_members_email_map[result['created_by_id']] + + result['created_by_id'] = existing_users_email_map[created_by_email] + end + + result end def parsed_hash(member) @@ -104,14 +145,6 @@ module Gitlab relation_class: relation_class) end - def find_user_query(email) - user_arel[:email].eq(email) - end - - def user_arel - @user_arel ||= User.arel_table - end - def relation_class case @importable when ::Project @@ -143,7 +176,7 @@ module Gitlab def base_log_params(member_hash) { - user_id: member_hash['user']&.id, + user_id: member_hash['user_id'], access_level: member_hash['access_level'], importable_type: @importable.class.to_s, importable_id: @importable.id, diff --git a/spec/initializers/rdoc_segfault_patch_spec.rb b/spec/initializers/rdoc_segfault_patch_spec.rb new file mode 100644 index 00000000000..f9630295052 --- /dev/null +++ b/spec/initializers/rdoc_segfault_patch_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +RSpec.describe 'RDoc segfault patch fix' do + describe 'RDoc::Markup::ToHtml' do + describe '#parseable?' do + it 'returns false' do + to_html = RDoc::Markup::ToHtml.new( nil) + + expect(to_html.parseable?('"def foo; end"')).to eq(false) + end + end + end + + describe 'RDoc::Markup::Verbatim' do + describe 'ruby?' do + it 'returns false' do + verbatim = RDoc::Markup::Verbatim.new('def foo; end') + verbatim.format = :ruby + + expect(verbatim.ruby?).to eq(false) + end + end + end +end diff --git a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb index aee4bd93207..16c958ec10b 100644 --- a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb +++ b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb @@ -132,6 +132,12 @@ RSpec.describe Banzai::Filter::SyntaxHighlightFilter do expect(result.to_html.delete("\n")).to eq('<div class="gl-relative markdown-code-block js-markdown-code"><pre data-sourcepos="1:1-3:3" class="code highlight js-syntax-highlight language-plaintext" lang="plaintext" v-pre="true"><code><span id="LC1" class="line" lang="plaintext">This is a test</span></code></pre><copy-code></copy-code></div>') end + + it "escape sourcepos metadata to prevent XSS" do + result = filter('<pre data-sourcepos=""%22 href="x"></pre><base href=http://unsafe-website.com/><pre x=""><code></code></pre>') + + expect(result.to_html.delete("\n")).to eq('<div class="gl-relative markdown-code-block js-markdown-code"><pre data-sourcepos=\'"%22 href="x"></pre><base href=http://unsafe-website.com/><pre x="\' class="code highlight js-syntax-highlight language-plaintext" lang="plaintext" v-pre="true"><code></code></pre><copy-code></copy-code></div>') + end end context "when Rouge lexing fails" do diff --git a/spec/lib/gitlab/import_export/members_mapper_spec.rb b/spec/lib/gitlab/import_export/members_mapper_spec.rb index 8d9bff9c610..87ca899a87d 100644 --- a/spec/lib/gitlab/import_export/members_mapper_spec.rb +++ b/spec/lib/gitlab/import_export/members_mapper_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do "notification_level" => 3, "created_at" => "2016-03-11T10:21:44.822Z", "updated_at" => "2016-03-11T10:21:44.822Z", - "created_by_id" => nil, + "created_by_id" => 1, "invite_email" => nil, "invite_token" => nil, "invite_accepted_at" => nil, @@ -38,10 +38,24 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do "notification_level" => 3, "created_at" => "2016-03-11T10:21:44.822Z", "updated_at" => "2016-03-11T10:21:44.822Z", - "created_by_id" => 1, + "created_by_id" => 2, "invite_email" => 'invite@test.com', "invite_token" => 'token', "invite_accepted_at" => nil + }, + { + "id" => 3, + "access_level" => 40, + "source_id" => 14, + "source_type" => source_type, + "user_id" => nil, + "notification_level" => 3, + "created_at" => "2016-03-11T10:21:44.822Z", + "updated_at" => "2016-03-11T10:21:44.822Z", + "created_by_id" => nil, + "invite_email" => 'invite2@test.com', + "invite_token" => 'token', + "invite_accepted_at" => nil }] end @@ -68,12 +82,37 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do expect(member_class.find_by_invite_email('invite@test.com')).not_to be_nil end - it 'removes old user_id from member_hash to avoid conflict with user key' do + it 'maps created_by_id to user on new instance' do + expect(member_class) + .to receive(:create) + .once + .with(hash_including('user_id' => user2.id, 'created_by_id' => nil)) + .and_call_original + expect(member_class) + .to receive(:create) + .once + .with(hash_including('invite_email' => 'invite@test.com', 'created_by_id' => nil)) + .and_call_original + expect(member_class) + .to receive(:create) + .once + .with(hash_including('invite_email' => 'invite2@test.com', 'created_by_id' => nil)) + .and_call_original + + members_mapper.map + end + + it 'replaced user_id with user_id from new instance' do + expect(member_class) + .to receive(:create) + .once + .with(hash_including('user_id' => user2.id)) + .and_call_original expect(member_class) .to receive(:create) - .twice - .with(hash_excluding('user_id')) - .and_call_original + .twice + .with(hash_excluding('user_id')) + .and_call_original members_mapper.map end @@ -99,7 +138,7 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do end expect(logger).to receive(:info).with(hash_including(expected_log_params.call(user2.id))).once - expect(logger).to receive(:info).with(hash_including(expected_log_params.call(nil))).once + expect(logger).to receive(:info).with(hash_including(expected_log_params.call(nil))).twice members_mapper.map end |