Merge remote-tracking branch 'dev/14-6-stable-ee' into 14-6-stable-ee

CHANGELOG.md

@@ -2,6 +2,19 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
+## 14.6.5 (2022-02-25)
+
+### Security (8 changes)
+
+- [Limit commands_changes to certain keys](gitlab-org/security/gitlab@138c437f2819d62ce4750fb84399d8868c844b01) ([merge request](gitlab-org/security/gitlab!2227))
+- [Add runners_token prefix to Group and Project](gitlab-org/security/gitlab@682d4e9b63d3d36901638edc75c1b265460d42dc) ([merge request](gitlab-org/security/gitlab!2250))
+- [Anonymous user can enumerate all users through GraphQL endpoint](gitlab-org/security/gitlab@2b00a8036b291d3ad5de551a5e13c2a0a39d0234) ([merge request](gitlab-org/security/gitlab!2102))
+- [Check for unsafe characters in email addresses before sending](gitlab-org/security/gitlab@6bc653b3dadefb3d2c80823786d43e6b7f8c4620) ([merge request](gitlab-org/security/gitlab!2208))
+- [Warn when snippet contains unretrievable files](gitlab-org/security/gitlab@f9ae9515ec98ab934f4aa3a35af0aca806bbe21d) ([merge request](gitlab-org/security/gitlab!2203))
+- [Prevent DOS when rendering math markdown](gitlab-org/security/gitlab@fd6d496df6f4b5eb3da0b851f9ff8ebb1d68d3f2) ([merge request](gitlab-org/security/gitlab!2201))
+- [Check permission when creating members through service](gitlab-org/security/gitlab@948e5103285de2a6cdb5152ff2c13ae4db2f4cda) ([merge request](gitlab-org/security/gitlab!2211))
+- [Reset password field on page load](gitlab-org/security/gitlab@1417b463f2771a4b17e068dea9de3aa6c4540962) ([merge request](gitlab-org/security/gitlab!2194))
+
 ## 14.6.4 (2022-02-03)
 No changes.

GITALY_SERVER_VERSION

-14.6.4
\ No newline at end of file
+14.6.5
\ No newline at end of file

VERSION

-14.6.4-ee
\ No newline at end of file
+14.6.5-ee
\ No newline at end of file

app/assets/javascripts/mirrors/mirror_repos.js

@@ -6,6 +6,8 @@ import { __ } from '~/locale';
 import { hide } from '~/tooltips';
 import SSHMirror from './ssh_mirror';
+const PASSWORD_FIELD_SELECTOR = '.js-mirror-password-field';
+
 export default class MirrorRepos {
   constructor(container) {
     this.$container = $(container);
@@ -27,7 +29,6 @@ export default class MirrorRepos {
     this.$passwordGroup = $('.js-password-group', this.$container);
     this.$password = $('.js-password', this.$passwordGroup);
     this.$authMethod = $('.js-auth-method', this.$form);
-
     this.$keepDivergentRefsInput.on('change', () => this.updateKeepDivergentRefs());
     this.$authMethod.on('change', () => this.togglePassword());
     this.$password.on('input.updateUrl', () => this.debouncedUpdateUrl());
@@ -35,6 +36,13 @@ export default class MirrorRepos {
     this.initMirrorSSH();
     this.updateProtectedBranches();
     this.updateKeepDivergentRefs();
+    MirrorRepos.resetPasswordField();
+  }
+
+  static resetPasswordField() {
+    if (document.querySelector(PASSWORD_FIELD_SELECTOR)) {
+      document.querySelector(PASSWORD_FIELD_SELECTOR).value = '';
+    }
   }
   initMirrorSSH() {

app/assets/javascripts/snippets/components/show.vue

 <script>
-import { GlLoadingIcon } from '@gitlab/ui';
+import { GlAlert, GlLoadingIcon } from '@gitlab/ui';
 import eventHub from '~/blob/components/eventhub';
 import {
   SNIPPET_MARK_VIEW_APP_START,
@@ -23,6 +23,7 @@ export default {
     EmbedDropdown,
     SnippetHeader,
     SnippetTitle,
+    GlAlert,
     GlLoadingIcon,
     SnippetBlob,
     CloneDropdownButton,
@@ -35,6 +36,9 @@ export default {
     canBeCloned() {
       return Boolean(this.snippet.sshUrlToRepo || this.snippet.httpUrlToRepo);
     },
+    hasUnretrievableBlobs() {
+      return this.snippet.hasUnretrievableBlobs;
+    },
   },
   beforeCreate() {
     performanceMarkAndMeasure({ mark: SNIPPET_MARK_VIEW_APP_START });
@@ -66,6 +70,13 @@ export default {
           data-qa-selector="clone_button"
         />
       </div>
+      <gl-alert v-if="hasUnretrievableBlobs" variant="danger" class="gl-mb-3" :dismissible="false">
+        {{
+          __(
+            'WARNING: This snippet contains hidden files which might be used to mask malicious behavior. Exercise caution if cloning and executing code from this snippet.',
+          )
+        }}
+      </gl-alert>
       <snippet-blob
         v-for="blob in blobs"
         :key="blob.path"

app/assets/javascripts/snippets/mixins/snippets.js

@@ -17,6 +17,7 @@ export const getSnippetMixin = {
         // Set `snippet.blobs` since some child components are coupled to this.
         if (!isEmpty(res)) {
+          res.hasUnretrievableBlobs = res.blobs?.hasUnretrievableBlobs || false;
           // It's possible for us to not get any blobs in a response.
           // In this case, we should default to current blobs.
           res.blobs = res.blobs ? res.blobs.nodes : blobsDefault;

app/graphql/queries/snippet/snippet.query.graphql

@@ -15,6 +15,7 @@ query GetSnippetQuery($ids: [SnippetID!]) {
       sshUrlToRepo
       blobs {
         __typename
+        hasUnretrievableBlobs
         nodes {
           __typename
           binary

app/graphql/queries/snippet/snippet_blob_content.query.graphql

@@ -12,6 +12,7 @@ query SnippetBlobContent($ids: [ID!], $rich: Boolean!, $paths: [String!]) {
           richData @include(if: $rich)
           plainData @skip(if: $rich)
         }
+        hasUnretrievableBlobs
       }
     }
   }

app/graphql/resolvers/snippets/blobs_resolver.rb

@@ -19,18 +19,18 @@ module Resolvers
       def resolve(paths: [])
         return [snippet.blob] if snippet.empty_repo?
-        if paths.empty?
-          snippet.blobs
-        else
-          snippet.repository.blobs_at(transformed_blob_paths(paths))
-        end
-      end
-
-      private
-
-      def transformed_blob_paths(paths)
-        ref = snippet.default_branch
-        paths.map { |path| [ref, path] }
+        paths = snippet.all_files if paths.empty?
+        blobs = snippet.blobs(paths)
+
+        # TODO: Some blobs, e.g. those with non-utf8 filenames, are returned as nil from the
+        # repository. We need to provide a flag to notify the user of this until we come up with a
+        # way to retrieve and display these blobs. We will be exploring a more holistic solution for
+        # this general problem of making all blobs retrievable as part
+        # of https://gitlab.com/gitlab-org/gitlab/-/issues/323082, at which point this attribute may
+        # be removed.
+        context[:unretrievable_blobs?] = blobs.size < paths.size
+
+        blobs
       end
     end
   end

app/graphql/resolvers/users_resolver.rb

@@ -29,7 +29,7 @@ module Resolvers
               description: 'Return only admin users.'
     def resolve(ids: nil, usernames: nil, sort: nil, search: nil, admins: nil)
-      authorize!
+      authorize!(usernames)
       ::UsersFinder.new(context[:current_user], finder_params(ids, usernames, sort, search, admins)).execute
     end
@@ -46,8 +46,11 @@ module Resolvers
       super
     end
-    def authorize!
-      Ability.allowed?(context[:current_user], :read_users_list) || raise_resource_not_available_error!
+    def authorize!(usernames)
+      authorized = Ability.allowed?(context[:current_user], :read_users_list)
+      authorized &&= usernames.present? if context[:current_user].blank?
+
+      raise_resource_not_available_error! unless authorized
     end
     private

app/graphql/types/snippets/blob_connection_type.rb

+# frozen_string_literal: true
+
+module Types
+  module Snippets
+    # rubocop: disable Graphql/AuthorizeTypes
+    class BlobConnectionType < GraphQL::Types::Relay::BaseConnection
+      field :has_unretrievable_blobs, GraphQL::Types::Boolean, null: false,
+            description: 'Indicates if the snippet has unretrievable blobs.',
+            resolver_method: :unretrievable_blobs?
+
+      def unretrievable_blobs?
+        !!context[:unretrievable_blobs?]
+      end
+    end
+  end
+end

app/graphql/types/snippets/blob_type.rb

@@ -8,6 +8,8 @@ module Types
       description 'Represents the snippet blob'
       present_using SnippetBlobPresenter
+      connection_type_class(Types::Snippets::BlobConnectionType)
+
       field :rich_data, GraphQL::Types::String,
             description: 'Blob highlighted data.',
             null: true

app/models/concerns/token_authenticatable_strategies/encrypted.rb

@@ -5,16 +5,18 @@ module TokenAuthenticatableStrategies
     def find_token_authenticatable(token, unscoped = false)
       return if token.blank?
-      if required?
-        find_by_encrypted_token(token, unscoped)
-      elsif optional?
-        find_by_encrypted_token(token, unscoped) ||
-          find_by_plaintext_token(token, unscoped)
-      elsif migrating?
-        find_by_plaintext_token(token, unscoped)
-      else
-        raise ArgumentError, _("Unknown encryption strategy: %{encrypted_strategy}!") % { encrypted_strategy: encrypted_strategy }
-      end
+      instance = if required?
+                   find_by_encrypted_token(token, unscoped)
+                 elsif optional?
+                   find_by_encrypted_token(token, unscoped) ||
+                     find_by_plaintext_token(token, unscoped)
+                 elsif migrating?
+                   find_by_plaintext_token(token, unscoped)
+                 else
+                   raise ArgumentError, _("Unknown encryption strategy: %{encrypted_strategy}!") % { encrypted_strategy: encrypted_strategy }
+                 end
+
+      instance if instance && matches_prefix?(instance, token)
     end
     def ensure_token(instance)
@@ -41,9 +43,7 @@ module TokenAuthenticatableStrategies
     def get_token(instance)
       return insecure_strategy.get_token(instance) if migrating?
-      encrypted_token = instance.read_attribute(encrypted_field)
-      token = EncryptionHelper.decrypt_token(encrypted_token)
-      token || (insecure_strategy.get_token(instance) if optional?)
+      get_encrypted_token(instance)
     end
     def set_token(instance, token)
@@ -69,6 +69,12 @@ module TokenAuthenticatableStrategies
     protected
+    def get_encrypted_token(instance)
+      encrypted_token = instance.read_attribute(encrypted_field)
+      token = EncryptionHelper.decrypt_token(encrypted_token)
+      token || (insecure_strategy.get_token(instance) if optional?)
+    end
+
     def encrypted_strategy
       value = options[:encrypted]
       value = value.call if value.is_a?(Proc)
@@ -95,14 +101,22 @@ module TokenAuthenticatableStrategies
         .new(klass, token_field, options)
     end
+    def matches_prefix?(instance, token)
+      prefix = options[:prefix]
+      prefix = prefix.call(instance) if prefix.is_a?(Proc)
+      prefix = '' unless prefix.is_a?(String)
+
+      token.start_with?(prefix)
+    end
+
     def token_set?(instance)
-      raw_token = instance.read_attribute(encrypted_field)
+      token = get_encrypted_token(instance)
       unless required?
-        raw_token ||= insecure_strategy.get_token(instance)
+        token ||= insecure_strategy.get_token(instance)
       end
-      raw_token.present?
+      token.present? && matches_prefix?(instance, token)
     end
     def encrypted_field

app/models/group.rb

@@ -18,6 +18,13 @@ class Group < Namespace
   include EachBatch
   include BulkMemberAccessLoad
+  extend ::Gitlab::Utils::Override
+
+  # Prefix for runners_token which can be used to invalidate existing tokens.
+  # The value chosen here is GR (for Gitlab Runner) combined with the rotation
+  # date (20220225) decimal to hex encoded.
+  RUNNERS_TOKEN_PREFIX = 'GR1348941'
+
   def self.sti_name
     'Group'
   end
@@ -108,7 +115,9 @@ class Group < Namespace
                       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 }
+  add_authentication_token_field :runners_token,
+                                 encrypted: -> { Feature.enabled?(:groups_tokens_optional_encryption, default_enabled: true) ? :optional : :required },
+                                 prefix: ->(instance) { instance.runners_token_prefix }
   after_create :post_create_hook
   after_destroy :post_destroy_hook
@@ -652,6 +661,15 @@ class Group < Namespace
     ensure_runners_token!
   end
+  def runners_token_prefix
+    Feature.enabled?(:groups_runners_token_prefix, self, default_enabled: :yaml) ? RUNNERS_TOKEN_PREFIX : ''
+  end
+
+  override :format_runners_token
+  def format_runners_token(token)
+    "#{runners_token_prefix}#{token}"
+  end
+
   def project_creation_level
     super || ::Gitlab::CurrentSettings.default_project_creation
   end

app/models/note.rb

@@ -46,7 +46,7 @@ class Note < ApplicationRecord
   attr_accessor :user_visible_reference_count
   # Attribute used to store the attributes that have been changed by quick actions.
-  attr_accessor :commands_changes
+  attr_writer :commands_changes
   # Attribute used to determine whether keep_around_commits will be skipped for diff notes.
   attr_accessor :skip_keep_around_commits
@@ -612,6 +612,41 @@ class Note < ApplicationRecord
     change_position.line_range["end"] || change_position.line_range["start"]
   end
+  def commands_changes
+    @commands_changes&.slice(
+      :due_date,
+      :label_ids,
+      :remove_label_ids,
+      :add_label_ids,
+      :canonical_issue_id,
+      :clone_with_notes,
+      :confidential,
+      :create_merge_request,
+      :add_contacts,
+      :remove_contacts,
+      :assignee_ids,
+      :milestone_id,
+      :time_estimate,
+      :spend_time,
+      :discussion_locked,
+      :merge,
+      :rebase,
+      :wip_event,
+      :target_branch,
+      :reviewer_ids,
+      :health_status,
+      :promote_to_epic,
+      :weight,
+      :emoji_award,
+      :todo_event,
+      :subscription_event,
+      :state_event,
+      :title,
+      :tag_message,
+      :tag_name
+    )
+  end
+
   private
   def system_note_viewable_by?(user)

app/models/project.rb

@@ -73,6 +73,11 @@ class Project < ApplicationRecord
   GL_REPOSITORY_TYPES = [Gitlab::GlRepository::PROJECT, Gitlab::GlRepository::WIKI, Gitlab::GlRepository::DESIGN].freeze
+  # Prefix for runners_token which can be used to invalidate existing tokens.
+  # The value chosen here is GR (for Gitlab Runner) combined with the rotation
+  # date (20220225) decimal to hex encoded.
+  RUNNERS_TOKEN_PREFIX = 'GR1348941'
+
   cache_markdown_field :description, pipeline: :description
   default_value_for :packages_enabled, true
@@ -93,7 +98,9 @@ class Project < ApplicationRecord
   default_value_for :autoclose_referenced_issues, true
   default_value_for(:ci_config_path) { Gitlab::CurrentSettings.default_ci_config_path }
-  add_authentication_token_field :runners_token, encrypted: -> { Feature.enabled?(:projects_tokens_optional_encryption, default_enabled: true) ? :optional : :required }
+  add_authentication_token_field :runners_token,
+                                 encrypted: -> { Feature.enabled?(:projects_tokens_optional_encryption, default_enabled: true) ? :optional : :required },
+                                 prefix: ->(instance) { instance.runners_token_prefix }
   before_validation :mark_remote_mirrors_for_removal, if: -> { RemoteMirror.table_exists? }
@@ -1843,6 +1850,15 @@ class Project < ApplicationRecord
     ensure_runners_token!
   end
+  def runners_token_prefix
+    Feature.enabled?(:projects_runners_token_prefix, self, default_enabled: :yaml) ? RUNNERS_TOKEN_PREFIX : ''
+  end
+
+  override :format_runners_token
+  def format_runners_token(token)
+    "#{runners_token_prefix}#{token}"
+  end
+
   def pages_deployed?
     pages_metadatum&.deployed?
   end

app/models/snippet.rb

@@ -237,15 +237,19 @@ class Snippet < ApplicationRecord
     end
   end
+  def all_files
+    list_files(default_branch)
+  end
+
   def blob
     @blob ||= Blob.decorate(SnippetBlob.new(self), self)
   end
-  def blobs
+  def blobs(paths = [])
     return [] unless repository_exists?
-    files = list_files(default_branch)
-    items = files.map { |file| [default_branch, file] }
+    paths = all_files if paths.empty?
+    items = paths.map { |path| [default_branch, path] }
     repository.blobs_at(items).compact
   end

app/services/members/create_service.rb

@@ -19,6 +19,8 @@ module Members
     end
     def execute
+      raise Gitlab::Access::AccessDeniedError unless can?(current_user, create_member_permission(source), source)
+
       validate_invite_source!
       validate_invitable!
@@ -144,6 +146,17 @@ module Members
     def formatted_errors
       errors.to_sentence
     end
+
+    def create_member_permission(source)
+      case source
+      when Group
+        :admin_group_member
+      when Project
+        :admin_project_member
+      else
+        raise "Unknown source type: #{source.class}!"
+      end
+    end
   end
 end

app/views/projects/mirrors/_authentication_method.html.haml

@@ -13,4 +13,4 @@
 .form-group
   .well-password-auth.collapse.js-well-password-auth
     = f.label :password, _("Password"), class: "label-bold"
-    = f.password_field :password, class: 'form-control gl-form-input qa-password', autocomplete: 'new-password'
+    = f.password_field :password, class: 'form-control gl-form-input qa-password js-mirror-password-field', autocomplete: 'off'

config/feature_flags/development/groups_runners_token_prefix.yml

+---
+name: groups_runners_token_prefix
+introduced_by_url: 
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/353805
+milestone: '14.9'
+type: development
+group: group::database
+default_enabled: true