diff options
Diffstat (limited to 'app/models')
-rw-r--r-- | app/models/concerns/issuable.rb | 19 | ||||
-rw-r--r-- | app/models/concerns/sanitizable.rb | 9 | ||||
-rw-r--r-- | app/models/namespace_setting.rb | 13 |
3 files changed, 22 insertions, 19 deletions
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 9f0cd96a8f8..50696c7b5e1 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -92,10 +92,9 @@ module Issuable validates :author, presence: true validates :title, presence: true, length: { maximum: TITLE_LENGTH_MAX } - # we validate the description against DESCRIPTION_LENGTH_MAX only for Issuables being created - # to avoid breaking the existing Issuables which may have their descriptions longer - validates :description, length: { maximum: DESCRIPTION_LENGTH_MAX }, allow_blank: true, on: :create - validate :description_max_length_for_new_records_is_valid, on: :update + # we validate the description against DESCRIPTION_LENGTH_MAX only for Issuables being created and on updates if + # the description changes to avoid breaking the existing Issuables which may have their descriptions longer + validates :description, bytesize: { maximum: -> { DESCRIPTION_LENGTH_MAX } }, if: :validate_description_length? validate :validate_assignee_size_length, unless: :importing? before_validation :truncate_description_on_import! @@ -229,10 +228,14 @@ module Issuable private - def description_max_length_for_new_records_is_valid - if new_record? && description.length > Issuable::DESCRIPTION_LENGTH_MAX - errors.add(:description, :too_long, count: Issuable::DESCRIPTION_LENGTH_MAX) - end + def validate_description_length? + return false unless description_changed? + + previous_description = changes['description'].first + # previous_description will be nil for new records + return true if previous_description.blank? + + previous_description.bytesize <= DESCRIPTION_LENGTH_MAX end def truncate_description_on_import! diff --git a/app/models/concerns/sanitizable.rb b/app/models/concerns/sanitizable.rb index 05756beb404..653d7a4875d 100644 --- a/app/models/concerns/sanitizable.rb +++ b/app/models/concerns/sanitizable.rb @@ -45,6 +45,15 @@ module Sanitizable unless input.to_s == CGI.unescapeHTML(input.to_s) record.errors.add(attr, 'cannot contain escaped HTML entities') end + + # This method raises an exception on failure so perform this + # last if multiple errors should be returned. + Gitlab::Utils.check_path_traversal!(input.to_s) + + rescue Gitlab::Utils::DoubleEncodingError + record.errors.add(attr, 'cannot contain escaped components') + rescue Gitlab::Utils::PathTraversalAttackError + record.errors.add(attr, "cannot contain a path traversal component") end end end diff --git a/app/models/namespace_setting.rb b/app/models/namespace_setting.rb index 7f65fb3a378..aeb4d7a5694 100644 --- a/app/models/namespace_setting.rb +++ b/app/models/namespace_setting.rb @@ -14,10 +14,11 @@ class NamespaceSetting < ApplicationRecord validates :enabled_git_access_protocol, inclusion: { in: enabled_git_access_protocols.keys } - validate :default_branch_name_content validate :allow_mfa_for_group validate :allow_resource_access_token_creation_for_group + sanitizes! :default_branch_name + before_validation :normalize_default_branch_name chronic_duration_attr :runner_token_expiration_interval_human_readable, :runner_token_expiration_interval @@ -45,8 +46,6 @@ class NamespaceSetting < ApplicationRecord NAMESPACE_SETTINGS_PARAMS end - sanitizes! :default_branch_name - def prevent_sharing_groups_outside_hierarchy return super if namespace.root? @@ -85,14 +84,6 @@ class NamespaceSetting < ApplicationRecord self.default_branch_name = default_branch_name.presence end - def default_branch_name_content - return if default_branch_name.nil? - - if default_branch_name.blank? - errors.add(:default_branch_name, "can not be an empty string") - end - end - def allow_mfa_for_group if namespace&.subgroup? && allow_mfa_for_subgroups == false errors.add(:allow_mfa_for_subgroups, _('is not allowed since the group is not top-level group.')) |