Merge remote-tracking branch 'dev/12-10-stable' into 12-10-stable

CHANGELOG.md

@@ -2,6 +2,27 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 12.10.13 (2020-07-01)
+
+### Security (15 changes)
+
+- Do not show activity for users with private profiles.
+- Fix stored XSS in markdown renderer.
+- Upgrade swagger-ui to solve XSS issues.
+- Fix group deploy token API authorizations.
+- Check access when sending TODOs related to merge requests.
+- Change from hybrid to JSON cookies serializer.
+- Prevent XSS in group name validations.
+- Disable caching for wiki attachments.
+- Fix null byte error in upload path.
+- Update permissions for time tracking endpoints.
+- Update Kaminari gem.
+- Fix note author name rendering.
+- Sanitize bitbucket repo urls to mitigate XSS.
+- Stored XSS on the Error Tracking page.
+- Fix security issue when rendering issuable.
+
+
 ## 12.10.12 (2020-06-24)
 
 ### Fixed (1 change)

GITALY_SERVER_VERSION

-12.10.12
+12.10.13

Gemfile.lock

@@ -563,19 +563,19 @@ GEM
     json-schema (2.8.0)
       addressable (>= 2.4)
     jwt (2.1.0)
-    kaminari (1.0.1)
+    kaminari (1.2.1)
       activesupport (>= 4.1.0)
-      kaminari-actionview (= 1.0.1)
-      kaminari-activerecord (= 1.0.1)
-      kaminari-core (= 1.0.1)
-    kaminari-actionview (1.0.1)
+      kaminari-actionview (= 1.2.1)
+      kaminari-activerecord (= 1.2.1)
+      kaminari-core (= 1.2.1)
+    kaminari-actionview (1.2.1)
       actionview
-      kaminari-core (= 1.0.1)
-    kaminari-activerecord (1.0.1)
+      kaminari-core (= 1.2.1)
+    kaminari-activerecord (1.2.1)
       activerecord
-      kaminari-core (= 1.0.1)
-    kaminari-core (1.0.1)
-    kgio (2.11.2)
+      kaminari-core (= 1.2.1)
+    kaminari-core (1.2.1)
+    kgio (2.11.3)
     knapsack (1.17.0)
       rake
     kramdown (2.1.0)

VERSION

-12.10.12
+12.10.13

app/assets/javascripts/error_tracking/components/stacktrace_entry.vue

 <script>
-import { escape as esc } from 'lodash';
-import { GlTooltip } from '@gitlab/ui';
-import { __, sprintf } from '~/locale';
+import { GlTooltip, GlSprintf } from '@gitlab/ui';
 import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
 import FileIcon from '~/vue_shared/components/file_icon.vue';
 import Icon from '~/vue_shared/components/icon.vue';
@@ -11,6 +9,7 @@ export default {
     ClipboardButton,
     FileIcon,
     Icon,
+    GlSprintf,
   },
   directives: {
     GlTooltip,
@@ -57,36 +56,6 @@ export default {
     collapseIcon() {
       return this.isExpanded ? 'chevron-down' : 'chevron-right';
     },
-    errorFnText() {
-      return this.errorFn
-        ? sprintf(
-            __(`%{spanStart}in%{spanEnd} %{errorFn}`),
-            {
-              errorFn: `<strong>${esc(this.errorFn)}</strong>`,
-              spanStart: `<span class="text-tertiary">`,
-              spanEnd: `</span>`,
-            },
-            false,
-          )
-        : '';
-    },
-    errorPositionText() {
-      return this.errorLine
-        ? sprintf(
-            __(`%{spanStart}at line%{spanEnd} %{errorLine}%{errorColumn}`),
-            {
-              errorLine: `<strong>${this.errorLine}</strong>`,
-              errorColumn: this.errorColumn ? `:<strong>${this.errorColumn}</strong>` : ``,
-              spanStart: `<span class="text-tertiary">`,
-              spanEnd: `</span>`,
-            },
-            false,
-          )
-        : '';
-    },
-    errorInfo() {
-      return `${this.errorFnText} ${this.errorPositionText}`;
-    },
   },
   methods: {
     isHighlighted(lineNum) {
@@ -132,7 +101,27 @@ export default {
           :text="filePath"
           css-class="btn-default btn-transparent btn-clipboard position-static"
         />
-        <span v-html="errorInfo"></span>
+
+        <gl-sprintf v-if="errorFn" :message="__('%{spanStart}in%{spanEnd} %{errorFn}')">
+          <template #span="{content}">
+            <span class="gl-text-gray-400">{{ content }}&nbsp;</span>
+          </template>
+          <template #errorFn>
+            <strong>{{ errorFn }}&nbsp;</strong>
+          </template>
+        </gl-sprintf>
+
+        <gl-sprintf :message="__('%{spanStart}at line%{spanEnd} %{errorLine}%{errorColumn}')">
+          <template #span="{content}">
+            <span class="gl-text-gray-400">{{ content }}&nbsp;</span>
+          </template>
+          <template #errorLine>
+            <strong>{{ errorLine }}</strong>
+          </template>
+          <template #errorColumn>
+            <strong v-if="errorColumn">:{{ errorColumn }}</strong>
+          </template>
+        </gl-sprintf>
       </div>
     </div>
 

app/assets/javascripts/issuables_list/components/issuable.vue

@@ -4,7 +4,7 @@
  * any changes done to the haml need to be reflected here.
  */
 import { escape, isNumber } from 'lodash';
-import { GlLink, GlTooltipDirective as GlTooltip } from '@gitlab/ui';
+import { GlLink, GlTooltipDirective as GlTooltip, GlSprintf } from '@gitlab/ui';
 import {
   dateInWords,
   formatDate,
@@ -20,10 +20,14 @@ import Icon from '~/vue_shared/components/icon.vue';
 import IssueAssignees from '~/vue_shared/components/issue/issue_assignees.vue';
 
 export default {
+  i18n: {
+    openedAgo: __('opened %{timeAgoString} by %{user}'),
+  },
   components: {
     Icon,
     IssueAssignees,
     GlLink,
+    GlSprintf,
   },
   directives: {
     GlTooltip,
@@ -98,23 +102,21 @@ export default {
       }
       return __('Milestone');
     },
-    openedAgoByString() {
-      const { author, created_at } = this.issuable;
+    issuableAuthor() {
+      return this.issuable.author;
+    },
+    issuableCreatedAt() {
+      return getTimeago().format(this.issuable.created_at);
+    },
+    popoverDataAttrs() {
+      const { id, username, name, avatar_url } = this.issuableAuthor;
 
-      return sprintf(
-        __('opened %{timeAgoString} by %{user}'),
-        {
-          timeAgoString: escape(getTimeago().format(created_at)),
-          user: `<a href="${escape(author.web_url)}"
-            data-user-id=${escape(author.id)}
-            data-username=${escape(author.username)}
-            data-name=${escape(author.name)}
-            data-avatar-url="${escape(author.avatar_url)}">
-            ${escape(author.name)}
-          </a>`,
-        },
-        false,
-      );
+      return {
+        'data-user-id': id,
+        'data-username': username,
+        'data-name': name,
+        'data-avatar-url': avatar_url,
+      };
     },
     referencePath() {
       return this.issuable.references.relative;
@@ -160,7 +162,7 @@ export default {
   mounted() {
     // TODO: Refactor user popover to use its own component instead of
     // spawning event listeners on Vue-rendered elements.
-    initUserPopovers([this.$refs.openedAgoByContainer.querySelector('a')]);
+    initUserPopovers([this.$refs.openedAgoByContainer.$el]);
   },
   methods: {
     labelStyle(label) {
@@ -221,17 +223,30 @@ export default {
             ></i>
             <gl-link :href="issuable.web_url">{{ issuable.title }}</gl-link>
           </span>
-          <span v-if="issuable.has_tasks" class="ml-1 task-status d-none d-sm-inline-block">{{
-            issuable.task_status
-          }}</span>
+          <span v-if="issuable.has_tasks" class="ml-1 task-status d-none d-sm-inline-block">
+            {{ issuable.task_status }}
+          </span>
         </div>
 
         <div class="issuable-info">
           <span class="js-ref-path">{{ referencePath }}</span>
 
-          <span class="d-none d-sm-inline-block mr-1">
+          <span data-testid="openedByMessage" class="d-none d-sm-inline-block mr-1">
             &middot;
-            <span ref="openedAgoByContainer" v-html="openedAgoByString"></span>
+            <gl-sprintf :message="$options.i18n.openedAgo">
+              <template #timeAgoString>
+                <span>{{ issuableCreatedAt }}</span>
+              </template>
+              <template #user>
+                <gl-link
+                  ref="openedAgoByContainer"
+                  v-bind="popoverDataAttrs"
+                  :href="issuableAuthor.web_url"
+                >
+                  {{ issuableAuthor.name }}
+                </gl-link>
+              </template>
+            </gl-sprintf>
           </span>
 
           <gl-link

app/controllers/groups/application_controller.rb

@@ -34,6 +34,18 @@ class Groups::ApplicationController < ApplicationController
     end
   end
 
+  def authorize_create_deploy_token!
+    unless can?(current_user, :create_deploy_token, group)
+      return render_404
+    end
+  end
+
+  def authorize_destroy_deploy_token!
+    unless can?(current_user, :destroy_deploy_token, group)
+      return render_404
+    end
+  end
+
   def authorize_admin_group_member!
     unless can?(current_user, :admin_group_member, group)
       return render_403

app/controllers/groups/deploy_tokens_controller.rb

 # frozen_string_literal: true
 
 class Groups::DeployTokensController < Groups::ApplicationController
-  before_action :authorize_admin_group!
+  before_action :authorize_destroy_deploy_token!
 
   def revoke
     @token = @group.deploy_tokens.find(params[:id])

app/controllers/groups/settings/repository_controller.rb

@@ -4,7 +4,7 @@ module Groups
   module Settings
     class RepositoryController < Groups::ApplicationController
       skip_cross_project_access_check :show
-      before_action :authorize_admin_group!
+      before_action :authorize_create_deploy_token!
       before_action :define_deploy_token_variables
       before_action do
         push_frontend_feature_flag(:ajax_new_deploy_token, @group)

app/controllers/projects/wikis_controller.rb

@@ -45,7 +45,7 @@ class Projects::WikisController < Projects::ApplicationController
 
       render 'show'
     elsif file_blob
-      send_blob(@project_wiki.repository, file_blob, allow_caching: @project.public?)
+      send_blob(@project_wiki.repository, file_blob)
     elsif show_create_form?
       # Assign a title to the WikiPage unless `id` is a randomly generated slug from #new
       title = params[:id] unless params[:random_title].present?

app/finders/events_finder.rb

@@ -33,6 +33,8 @@ class EventsFinder
   end
 
   def execute
+    return Event.none if cannot_access_private_profile?
+
     events = get_events
 
     events = by_current_user_access(events)
@@ -102,6 +104,10 @@ class EventsFinder
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
+  def cannot_access_private_profile?
+    source.is_a?(User) && !Ability.allowed?(current_user, :read_user_profile, source)
+  end
+
   def sort(events)
     return events unless params[:sort]
 

app/models/group.rb

@@ -70,9 +70,12 @@ class Group < Namespace
   validates :variables, variable_duplicates: true
 
   validates :two_factor_grace_period, presence: true, numericality: { greater_than_or_equal_to: 0 }
+
   validates :name,
-    format: { with: Gitlab::Regex.group_name_regex,
-              message: Gitlab::Regex.group_name_regex_message }, if: :name_changed?
+            html_safety: true,
+            format: { with: Gitlab::Regex.group_name_regex,
+                      message: Gitlab::Regex.group_name_regex_message },
+            if: :name_changed?
 
   add_authentication_token_field :runners_token, encrypted: -> { Feature.enabled?(:groups_tokens_optional_encryption, default_enabled: true) ? :optional : :required }
 

app/models/merge_request.rb

@@ -513,7 +513,7 @@ class MergeRequest < ApplicationRecord
       participants << merge_user
     end
 
-    participants
+    participants.select { |participant| Ability.allowed?(participant, :read_merge_request, self) }
   end
 
   def first_commit

app/policies/group_policy.rb

@@ -109,9 +109,7 @@ class GroupPolicy < BasePolicy
     enable :create_cluster
     enable :update_cluster
     enable :admin_cluster
-    enable :destroy_deploy_token
     enable :read_deploy_token
-    enable :create_deploy_token
     enable :admin_wiki
   end
 
@@ -123,6 +121,8 @@ class GroupPolicy < BasePolicy
 
     enable :set_note_created_at
     enable :set_emails_disabled
+    enable :create_deploy_token
+    enable :destroy_deploy_token
   end
 
   rule { can?(:read_nested_project_resources) }.policy do

app/validators/html_safety_validator.rb

+# frozen_string_literal: true
+
+# HtmlSafetyValidator
+#
+# Validates that a value does not contain HTML
+# or other unsafe content that could lead to XSS.
+# Relies on Rails HTML Sanitizer:
+# https://github.com/rails/rails-html-sanitizer
+#
+# Example:
+#
+#   class Group < ActiveRecord::Base
+#     validates :name, presence: true, html_safety: true
+#   end
+#
+class HtmlSafetyValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    return if value.blank? || safe_value?(value)
+
+    record.errors.add(attribute, self.class.error_message)
+  end
+
+  def self.error_message
+    _("cannot contain HTML/XML tags, including any word between angle brackets (<,>).")
+  end
+
+  private
+
+  # The `FullSanitizer` encodes ampersands as the HTML entity name.
+  # This isn't particularly necessary for preventing XSS so the ampersand
+  # is pre-encoded to avoid it being flagged in the comparison.
+  def safe_value?(text)
+    pre_encoded_text = text.gsub('&', '&amp;')
+    Rails::Html::FullSanitizer.new.sanitize(pre_encoded_text) == pre_encoded_text
+  end
+end

app/views/import/bitbucket_server/status.html.haml

@@ -54,7 +54,7 @@
       - @repos.each do |repo|
         %tr{ id: "repo_#{repo.project_key}___#{repo.slug}", data: { project: repo.project_key, repository: repo.slug } }
           %td
-            = link_to repo.browse_url, repo.browse_url, target: '_blank', rel: 'noopener noreferrer'
+            = sanitize(link_to(repo.browse_url, repo.browse_url, target: '_blank', rel: 'noopener noreferrer'), attributes: %w(href target rel))
           %td.import-target
             %fieldset.row
             .input-group
@@ -75,7 +75,7 @@
       - @incompatible_repos.each do |repo|
         %tr{ id: "repo_#{repo.project_key}___#{repo.slug}" }
           %td
-            = link_to repo.browse_url, repo.browse_url, target: '_blank', rel: 'noopener noreferrer'
+            = sanitize(link_to(repo.browse_url, repo.browse_url, target: '_blank', rel: 'noopener noreferrer'), attributes: %w(href target rel))
           %td.import-target
           %td.import-actions-job-status
             = label_tag 'Incompatible Project', nil, class: 'label badge-danger'

app/views/shared/notes/_note.html.haml

@@ -32,7 +32,7 @@
         .note-header-info
           %a{ href: user_path(note.author) }
             %span.note-header-author-name.bold
-              = sanitize(note.author.name)
+              = note.author.name
             = user_status(note.author)
             %span.note-headline-light
               = note.author.to_reference

config/initializers/cookies_serializer.rb

 # Be sure to restart your server when you modify this file.
 
 Rails.application.config.action_dispatch.use_cookies_with_metadata = false
-Rails.application.config.action_dispatch.cookies_serializer = :hybrid
+Rails.application.config.action_dispatch.cookies_serializer =
+  Gitlab::Utils.to_boolean(ENV['USE_UNSAFE_HYBRID_COOKIES']) ? :hybrid : :json

doc/api/deploy_tokens.md

@@ -136,7 +136,8 @@ curl --request DELETE --header "PRIVATE-TOKEN: <your_access_token>" "https://git
 
 ## Group deploy tokens
 
-These endpoints require group maintainer access or higher.
+Group maintainers and owners can list group deploy
+tokens. Only group owners can create and delete group deploy tokens.
 
 ### List group deploy tokens
 

doc/user/permissions.md

@@ -239,6 +239,8 @@ group.
 | Edit epic comments (posted by any user) **(ULTIMATE)** |       |          |           | ✓ (2)      | ✓ (2) |
 | Edit group                                             |       |          |           |            | ✓     |
 | Manage group level CI/CD variables                     |       |          |           |            | ✓     |
+| List group deploy tokens                               |       |          |           | ✓          | ✓     |
+| Create/Delete group deploy tokens                      |       |          |           |            | ✓     |
 | Manage group members                                   |       |          |           |            | ✓     |
 | Remove group                                           |       |          |           |            | ✓     |
 | Delete group epic **(ULTIMATE)**                       |       |          |           |            | ✓     |

lib/api/time_tracking_endpoints.rb

@@ -14,8 +14,8 @@ module API
           "#{issuable_name}_iid".to_sym
         end
 
-        def update_issuable_key
-          "update_#{issuable_name}".to_sym
+        def admin_issuable_key
+          "admin_#{issuable_name}".to_sym
         end
 
         def read_issuable_key
@@ -60,7 +60,7 @@ module API
         requires :duration, type: String, desc: 'The duration to be parsed'
       end
       post ":id/#{issuable_collection_name}/:#{issuable_key}/time_estimate" do
-        authorize! update_issuable_key, load_issuable
+        authorize! admin_issuable_key, load_issuable
 
         status :ok
         update_issuable(time_estimate: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration)))
@@ -71,7 +71,7 @@ module API
         requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}"
       end
       post ":id/#{issuable_collection_name}/:#{issuable_key}/reset_time_estimate" do
-        authorize! update_issuable_key, load_issuable
+        authorize! admin_issuable_key, load_issuable
 
         status :ok
         update_issuable(time_estimate: 0)
@@ -83,7 +83,7 @@ module API
         requires :duration, type: String, desc: 'The duration to be parsed'
       end
       post ":id/#{issuable_collection_name}/:#{issuable_key}/add_spent_time" do
-        authorize! update_issuable_key, load_issuable
+        authorize! admin_issuable_key, load_issuable
 
         update_issuable(spend_time: {
           duration: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration)),
@@ -96,7 +96,7 @@ module API
         requires issuable_key, type: Integer, desc: "The ID of a project #{issuable_name}"
       end
       post ":id/#{issuable_collection_name}/:#{issuable_key}/reset_spent_time" do
-        authorize! update_issuable_key, load_issuable
+        authorize! admin_issuable_key, load_issuable
 
         status :ok
         update_issuable(spend_time: { duration: :reset, user_id: current_user.id })

lib/banzai/filter/abstract_reference_filter.rb

@@ -253,7 +253,7 @@ module Banzai
         object_parent_type = parent.is_a?(Group) ? :group : :project
 
         {
-          original:             text,
+          original:             escape_html_entities(text),
           link:                 link_content,
           link_reference:       link_reference,
           object_parent_type => parent.id,

lib/banzai/filter/base_relative_link_filter.rb

@@ -38,7 +38,7 @@ module Banzai
       private
 
       def unescape_and_scrub_uri(uri)
-        Addressable::URI.unescape(uri).scrub
+        Addressable::URI.unescape(uri).scrub.delete("\0")
       end
     end
   end

lib/gitlab/markdown_cache.rb

@@ -3,7 +3,7 @@
 module Gitlab
   module MarkdownCache
     # Increment this number every time the renderer changes its output
-    CACHE_COMMONMARK_VERSION        = 20
+    CACHE_COMMONMARK_VERSION        = 23
     CACHE_COMMONMARK_VERSION_START  = 10
 
     BaseError = Class.new(StandardError)

locale/gitlab.pot

@@ -24308,6 +24308,9 @@ msgstr ""
 msgid "cannot block others"
 msgstr ""
 
+msgid "cannot contain HTML/XML tags, including any word between angle brackets (<,>)."
+msgstr ""
+
 msgid "cannot include leading slash or directory traversal."
 msgstr ""
 

package.json

@@ -115,7 +115,7 @@
     "stickyfilljs": "^2.1.0",
     "style-loader": "^1.1.3",
     "svg4everybody": "2.1.9",
-    "swagger-ui-dist": "^3.24.3",
+    "swagger-ui-dist": "^3.26.2",
     "three": "^0.84.0",
     "three-orbit-controls": "^82.1.0",
     "three-stl-loader": "^1.0.4",

spec/controllers/projects/wikis_controller_spec.rb

@@ -141,43 +141,19 @@ describe Projects::WikisController do
     context 'when page is a file' do
       include WikiHelpers
 
-      let(:id) { upload_file_to_wiki(project, user, file_name) }
+      where(:file_name) { ['dk.png', 'unsanitized.svg', 'git-cheat-sheet.pdf'] }
 
-      context 'when file is an image' do
-        let(:file_name) { 'dk.png' }
+      with_them do
+        let(:id) { upload_file_to_wiki(project, user, file_name) }
 
-        it 'delivers the image' do
+        it 'delivers the file with the correct headers' do
           subject
 
           expect(response.headers['Content-Disposition']).to match(/^inline/)
-          expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
+          expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq('true')
+          expect(response.cache_control[:public]).to be(false)
+          expect(response.cache_control[:extras]).to include('no-store')
         end
-
-        context 'when file is a svg' do
-          let(:file_name) { 'unsanitized.svg' }
-
-          it 'delivers the image' do
-            subject
-
-            expect(response.headers['Content-Disposition']).to match(/^inline/)
-            expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
-          end
-        end
-
-        it_behaves_like 'project cache control headers'
-      end
-
-      context 'when file is a pdf' do
-        let(:file_name) { 'git-cheat-sheet.pdf' }
-
-        it 'sets the content type to sets the content response headers' do
-          subject
-
-          expect(response.headers['Content-Disposition']).to match(/^inline/)
-          expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq "true"
-        end
-
-        it_behaves_like 'project cache control headers'
       end
     end
   end

spec/features/snippets/notes_on_personal_snippets_spec.rb

@@ -5,15 +5,17 @@ require 'spec_helper'
 describe 'Comments on personal snippets', :js do
   include NoteInteractionHelpers
 
-  let!(:user)    { create(:user) }
-  let!(:snippet) { create(:personal_snippet, :public) }
+  let_it_be(:snippet) { create(:personal_snippet, :public) }
+  let_it_be(:other_note) { create(:note_on_personal_snippet) }
+
+  let(:user_name) { 'Test User' }
+  let!(:user) { create(:user, name: user_name) }
   let!(:snippet_notes) do
     [
       create(:note_on_personal_snippet, noteable: snippet, author: user),
       create(:note_on_personal_snippet, noteable: snippet)
     ]
   end
-  let!(:other_note) { create(:note_on_personal_snippet) }
 
   before do
     stub_feature_flags(snippets_vue: false)
@@ -56,6 +58,26 @@ describe 'Comments on personal snippets', :js do
         expect(page).to show_user_status(status)
       end
     end
+
+    it 'shows the author name' do
+      visit snippet_path(snippet)
+
+      within("#note_#{snippet_notes[0].id}") do
+        expect(page).to have_content(user_name)
+      end
+    end
+
+    context 'when the author name contains HTML' do
+      let(:user_name) { '<h1><a href="https://bad.link/malicious.exe" class="evil">Fake Content<img class="fake-icon" src="image.png"></a></h1>' }
+
+      it 'renders the name as plain text' do
+        visit snippet_path(snippet)
+
+        content = find("#note_#{snippet_notes[0].id} .note-header-author-name").text
+
+        expect(content).to eq user_name
+      end
+    end
   end
 
   context 'when submitting a note' do

spec/finders/events_finder_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 describe EventsFinder do
   let_it_be(:user) { create(:user) }
+  let(:private_user) { create(:user, private_profile: true) }
   let(:other_user) { create(:user) }
 
   let(:project1) { create(:project, :private, creator_id: user.id, namespace: user.namespace) }
@@ -57,6 +58,12 @@ describe EventsFinder do
 
       expect(events).to be_empty
     end
+
+    it 'returns nothing when the target profile is private' do
+      events = described_class.new(source: private_user, current_user: other_user).execute
+
+      expect(events).to be_empty
+    end
   end
 
   describe 'wiki events feature flag' do

spec/frontend/error_tracking/components/stacktrace_entry_spec.js

 import { shallowMount } from '@vue/test-utils';
+import { GlSprintf } from '@gitlab/ui';
 import StackTraceEntry from '~/error_tracking/components/stacktrace_entry.vue';
 import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
 import FileIcon from '~/vue_shared/components/file_icon.vue';
 import Icon from '~/vue_shared/components/icon.vue';
+import { trimText } from 'helpers/text_helper';
 
 describe('Stacktrace Entry', () => {
   let wrapper;
@@ -21,6 +23,9 @@ describe('Stacktrace Entry', () => {
         errorLine: 24,
         ...props,
       },
+      stubs: {
+        GlSprintf,
+      },
     });
   }
 
@@ -53,7 +58,7 @@ describe('Stacktrace Entry', () => {
       const extraInfo = { errorLine: 34, errorFn: 'errorFn', errorColumn: 77 };
       mountComponent({ expanded: false, lines: [], ...extraInfo });
       expect(wrapper.find(Icon).exists()).toBe(false);
-      expect(findFileHeaderContent()).toContain(
+      expect(trimText(findFileHeaderContent())).toContain(
         `in ${extraInfo.errorFn} at line ${extraInfo.errorLine}:${extraInfo.errorColumn}`,
       );
     });
@@ -61,17 +66,17 @@ describe('Stacktrace Entry', () => {
     it('should render only lineNo:columnNO when there is no errorFn ', () => {
       const extraInfo = { errorLine: 34, errorFn: null, errorColumn: 77 };
       mountComponent({ expanded: false, lines: [], ...extraInfo });
-      expect(findFileHeaderContent()).not.toContain(`in ${extraInfo.errorFn}`);
-      expect(findFileHeaderContent()).toContain(`${extraInfo.errorLine}:${extraInfo.errorColumn}`);
+      const fileHeaderContent = trimText(findFileHeaderContent());
+      expect(fileHeaderContent).not.toContain(`in ${extraInfo.errorFn}`);
+      expect(fileHeaderContent).toContain(`${extraInfo.errorLine}:${extraInfo.errorColumn}`);
     });
 
     it('should render only lineNo when there is no errorColumn ', () => {
       const extraInfo = { errorLine: 34, errorFn: 'errorFn', errorColumn: null };
       mountComponent({ expanded: false, lines: [], ...extraInfo });
-      expect(findFileHeaderContent()).toContain(
-        `in ${extraInfo.errorFn} at line ${extraInfo.errorLine}`,
-      );
-      expect(findFileHeaderContent()).not.toContain(`:${extraInfo.errorColumn}`);
+      const fileHeaderContent = trimText(findFileHeaderContent());
+      expect(fileHeaderContent).toContain(`in ${extraInfo.errorFn} at line ${extraInfo.errorLine}`);
+      expect(fileHeaderContent).not.toContain(`:${extraInfo.errorColumn}`);
     });
   });
 });

spec/frontend/issuables_list/components/issuable_spec.js

 import { shallowMount } from '@vue/test-utils';
-import { GlLink } from '@gitlab/ui';
+import { GlSprintf } from '@gitlab/ui';
 import { TEST_HOST } from 'helpers/test_constants';
 import { trimText } from 'helpers/text_helper';
 import initUserPopovers from '~/user_popovers';
@@ -44,6 +44,10 @@ describe('Issuable component', () => {
         baseUrl: TEST_BASE_URL,
         ...props,
       },
+      stubs: {
+        'gl-sprintf': GlSprintf,
+        'gl-link': '<a><slot></slot></a>',
+      },
     });
   };
 
@@ -66,12 +70,12 @@ describe('Issuable component', () => {
 
   const findConfidentialIcon = () => wrapper.find('.fa-eye-slash');
   const findTaskStatus = () => wrapper.find('.task-status');
-  const findOpenedAgoContainer = () => wrapper.find({ ref: 'openedAgoByContainer' });
+  const findOpenedAgoContainer = () => wrapper.find('[data-testid="openedByMessage"]');
   const findMilestone = () => wrapper.find('.js-milestone');
   const findMilestoneTooltip = () => findMilestone().attributes('title');
   const findDueDate = () => wrapper.find('.js-due-date');
   const findLabelContainer = () => wrapper.find('.js-labels');
-  const findLabelLinks = () => findLabelContainer().findAll(GlLink);
+  const findLabelLinks = () => findLabelContainer().findAll('a');
   const findWeight = () => wrapper.find('.js-weight');
   const findAssignees = () => wrapper.find(IssueAssignees);
   const findMergeRequestsCount = () => wrapper.find('.js-merge-requests');
@@ -86,7 +90,7 @@ describe('Issuable component', () => {
 
       factory();
 
-      expect(initUserPopovers).toHaveBeenCalledWith([findOpenedAgoContainer().find('a').element]);
+      expect(initUserPopovers).toHaveBeenCalledWith([wrapper.vm.$refs.openedAgoByContainer.$el]);
     });
   });
 
@@ -135,7 +139,7 @@ describe('Issuable component', () => {
     });
 
     it('renders fuzzy opened date and author', () => {
-      expect(trimText(findOpenedAgoContainer().text())).toEqual(
+      expect(trimText(findOpenedAgoContainer().text())).toContain(
         `opened 1 month ago by ${TEST_USER_NAME}`,
       );
     });

spec/initializers/cookies_serializer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Cookies serializer initializer' do
+  def load_initializer
+    load Rails.root.join('config/initializers/cookies_serializer.rb')
+  end
+
+  subject { Rails.application.config.action_dispatch.cookies_serializer }
+
+  it 'uses JSON serializer by default' do
+    load_initializer
+
+    expect(subject).to eq(:json)
+  end
+
+  it 'uses the unsafe hybrid serializer when the environment variables is set' do
+    stub_env('USE_UNSAFE_HYBRID_COOKIES', 'true')
+
+    load_initializer
+
+    expect(subject).to eq(:hybrid)
+  end
+end

spec/lib/banzai/filter/abstract_reference_filter_spec.rb

@@ -20,6 +20,18 @@ describe Banzai::Filter::AbstractReferenceFilter do
     end
   end
 
+  describe '#data_attributes_for' do
+    let_it_be(:issue) { create(:issue, project: project) }
+
+    it 'is not an XSS vector' do
+      allow(described_class).to receive(:object_class).and_return(Issue)
+
+      data_attributes = filter.data_attributes_for('xss <img onerror=alert(1) src=x>', project, issue, link_content: true)
+
+      expect(data_attributes[:original]).to eq('xss <img onerror=alert(1) src=x>')
+    end
+  end
+
   describe '#parent_per_reference' do
     it 'returns a Hash containing projects grouped per parent paths' do
       expect(filter).to receive(:references_per_parent)

spec/lib/banzai/filter/upload_link_filter_spec.rb

@@ -212,6 +212,7 @@ describe Banzai::Filter::UploadLinkFilter do
       'invalid UTF-8 byte sequences' | '%FF'
       'garbled path'                 | 'open(/var/tmp/):%20/location%0Afrom:%20/test'
       'whitespace'                   | "d18213acd3732630991986120e167e3d/Landscape_8.jpg\nand more"
+      'null byte'                    | "%00"
     end
 
     with_them do

spec/lib/banzai/pipeline/full_pipeline_spec.rb

@@ -24,7 +24,7 @@ describe Banzai::Pipeline::FullPipeline do
     it 'escapes the data-original attribute on a reference' do
       markdown = %Q{[">bad things](#{issue.to_reference})}
       result = described_class.to_html(markdown, project: project)
-      expect(result).to include(%{data-original='\">bad things'})
+      expect(result).to include(%{data-original='\">bad things'})
     end
   end
 

spec/models/merge_request_spec.rb

@@ -3458,7 +3458,7 @@ describe MergeRequest do
 
   describe '#merge_participants' do
     it 'contains author' do
-      expect(subject.merge_participants).to eq([subject.author])
+      expect(subject.merge_participants).to contain_exactly(subject.author)
     end
 
     describe 'when merge_when_pipeline_succeeds? is true' do
@@ -3472,8 +3472,20 @@ describe MergeRequest do
                  author: user)
         end
 
-        it 'contains author only' do
-          expect(subject.merge_participants).to eq([subject.author])
+        context 'author is not a project member' do
+          it 'is empty' do
+            expect(subject.merge_participants).to be_empty
+          end
+        end
+
+        context 'author is a project member' do
+          before do
+            subject.project.team.add_reporter(user)
+          end
+
+          it 'contains author only' do
+            expect(subject.merge_participants).to contain_exactly(subject.author)
+          end
         end
       end
 
@@ -3486,8 +3498,24 @@ describe MergeRequest do
                  merge_user: merge_user)
         end
 
-        it 'contains author and merge user' do
-          expect(subject.merge_participants).to eq([subject.author, merge_user])
+        before do
+          subject.project.team.add_reporter(subject.author)
+        end
+
+        context 'merge user is not a member' do
+          it 'contains author only' do
+            expect(subject.merge_participants).to contain_exactly(subject.author)
+          end
+        end
+
+        context 'both author and merge users are project members' do
+          before do
+            subject.project.team.add_reporter(merge_user)
+          end
+
+          it 'contains author and merge user' do
+            expect(subject.merge_participants).to contain_exactly(subject.author, merge_user)
+          end
         end
       end
     end

spec/requests/api/deploy_tokens_spec.rb

@@ -204,7 +204,7 @@ describe API::DeployTokens do
   end
 
   context 'deploy token creation' do
-    shared_examples 'creating a deploy token' do |entity, unauthenticated_response|
+    shared_examples 'creating a deploy token' do |entity, unauthenticated_response, authorized_role|
       let(:expires_time) { 1.year.from_now }
       let(:params) do
         {
@@ -231,9 +231,9 @@ describe API::DeployTokens do
         it { is_expected.to have_gitlab_http_status(:forbidden) }
       end
 
-      context 'when authenticated as maintainer' do
+      context "when authenticated as #{authorized_role}" do
         before do
-          send(entity).add_maintainer(user)
+          send(entity).send("add_#{authorized_role}", user)
         end
 
         it 'creates the deploy token' do
@@ -282,7 +282,7 @@ describe API::DeployTokens do
         response
       end
 
-      it_behaves_like 'creating a deploy token', :project, :not_found
+      it_behaves_like 'creating a deploy token', :project, :not_found, :maintainer
     end
 
     describe 'POST /groups/:id/deploy_tokens' do
@@ -291,7 +291,17 @@ describe API::DeployTokens do
         response
       end
 
-      it_behaves_like 'creating a deploy token', :group, :forbidden
+      it_behaves_like 'creating a deploy token', :group, :forbidden, :owner
+
+      context 'when authenticated as maintainer' do
+        before do
+          group.add_maintainer(user)
+        end
+
+        let(:params) { { name: 'test', scopes: ['read_repository'] } }
+
+        it { is_expected.to have_gitlab_http_status(:forbidden) }
+      end
     end
   end
 
@@ -320,6 +330,14 @@ describe API::DeployTokens do
         group.add_maintainer(user)
       end
 
+      it { is_expected.to have_gitlab_http_status(:forbidden) }
+    end
+
+    context 'when authenticated as owner' do
+      before do
+        group.add_owner(user)
+      end
+
       it 'calls the deploy token destroy service' do
         expect(::Groups::DeployTokens::DestroyService).to receive(:new)
           .with(group, user, token_id: group_deploy_token.id)

spec/requests/api/events_spec.rb

@@ -192,6 +192,19 @@ describe API::Events do
         end
       end
 
+      context 'when target users profile is private' do
+        it 'returns no events' do
+          user.update!(private_profile: true)
+          private_project.add_developer(non_member)
+
+          get api("/users/#{user.username}/events", non_member)
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to include_pagination_headers
+          expect(json_response).to eq([])
+        end
+      end
+
       context 'when scope is passed' do
         context 'when unauthenticated' do
           it 'returns no user events' do

spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb

@@ -4,6 +4,16 @@ RSpec.shared_examples 'an unauthorized API user' do
   it { is_expected.to eq(403) }
 end
 
+RSpec.shared_examples 'API user with insufficient permissions' do
+  context 'with non member that is the author' do
+    before do
+      issuable.update!(author: non_member) # an external author can't admin issuable
+    end
+
+    it_behaves_like 'an unauthorized API user'
+  end
+end
+
 RSpec.shared_examples 'time tracking endpoints' do |issuable_name|
   let(:non_member) { create(:user) }
 
@@ -14,6 +24,7 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name|
       subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", non_member), params: { duration: '1w' }) }
 
       it_behaves_like 'an unauthorized API user'
+      it_behaves_like 'API user with insufficient permissions'
     end
 
     it "sets the time estimate for #{issuable_name}" do
@@ -53,6 +64,7 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name|
       subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/reset_time_estimate", non_member)) }
 
       it_behaves_like 'an unauthorized API user'
+      it_behaves_like 'API user with insufficient permissions'
     end
 
     it "resets the time estimate for #{issuable_name}" do
@@ -70,6 +82,7 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name|
       end
 
       it_behaves_like 'an unauthorized API user'
+      it_behaves_like 'API user with insufficient permissions'
     end
 
     it "add spent time for #{issuable_name}" do
@@ -119,6 +132,7 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name|
       subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/reset_spent_time", non_member)) }
 
       it_behaves_like 'an unauthorized API user'
+      it_behaves_like 'API user with insufficient permissions'
     end
 
     it "resets spent time for #{issuable_name}" do

spec/validators/html_safety_validator_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe HtmlSafetyValidator do
+  let(:validator) { described_class.new(attributes: [:name]) }
+  let(:group) { build(:group) }
+
+  def validate(value)
+    validator.validate_each(group, :name, value)
+  end
+
+  it 'adds an error when a script is included in the name' do
+    validate('My group <script>evil_script</script>')
+
+    expect(group.errors[:name]).to eq([HtmlSafetyValidator.error_message])
+  end
+
+  it 'does not add an error when an ampersand is included in the name' do
+    validate('Group with 1 & 2')
+
+    expect(group.errors).to be_empty
+  end
+end

yarn.lock

@@ -10876,10 +10876,10 @@ svg4everybody@2.1.9:
   resolved "https://registry.yarnpkg.com/svg4everybody/-/svg4everybody-2.1.9.tgz#5bd9f6defc133859a044646d4743fabc28db7e2d"
   integrity sha1-W9n23vwTOFmgRGRtR0P6vCjbfi0=
 
-swagger-ui-dist@^3.24.3:
-  version "3.24.3"
-  resolved "https://registry.yarnpkg.com/swagger-ui-dist/-/swagger-ui-dist-3.24.3.tgz#99754d11b0ddd314a1a50db850acb415e4b0a0c6"
-  integrity sha512-kB8qobP42Xazaym7sD9g5mZuRL4416VIIYZMqPEIskkzKqbPLQGEiHA3ga31bdzyzFLgr6Z797+6X1Am6zYpbg==
+swagger-ui-dist@^3.26.2:
+  version "3.26.2"
+  resolved "https://registry.yarnpkg.com/swagger-ui-dist/-/swagger-ui-dist-3.26.2.tgz#22c700906c8911b1c9956da6c3fca371dba6219f"
+  integrity sha512-cpR3A9uEs95gGQSaIXgiTpnetIifTF1u2a0fWrnVl+HyLpCdHVgOy7FGlVD1iVkts7AE5GOiGjA7VyDNiRaNgw==
 
 symbol-observable@^1.0.2:
   version "1.2.0"