diff --git a/.gitleaksignore b/.gitleaksignore index 80e43253ad5991b22f6cbec9a28826fd2db8bf59..48fd24b38b0e93aa4b0abefa5c6c7c493d990034 100644 --- a/.gitleaksignore +++ b/.gitleaksignore @@ -19,3 +19,5 @@ ee/spec/lib/security/secret_detection/partner_tokens/gcp_client_spec.rb:gcp-api- ee/spec/lib/security/secret_detection/partner_tokens/gcp_client_spec.rb:gcp-api-key:22 ee/spec/lib/security/secret_detection/partner_tokens/postman_client_spec.rb:postman-api-token:153 ee/spec/workers/security/secret_detection/partner_token_verification_worker_spec.rb:generic-api-key:56 +spec/models/user_detail_spec.rb:discord-client-id:654 +spec/models/user_detail_spec.rb:discord-client-id:683 \ No newline at end of file diff --git a/app/models/user_detail.rb b/app/models/user_detail.rb index 3e237dfb040b8484bc239d346c215bc6ac8fb614..dadb67646692b27208251443f329354f0655df51 100644 --- a/app/models/user_detail.rb +++ b/app/models/user_detail.rb @@ -2,6 +2,7 @@ class UserDetail < ApplicationRecord extend ::Gitlab::Utils::Override + include Sanitizable belongs_to :user belongs_to :bot_namespace, class_name: 'Namespace', optional: true, inverse_of: :bot_user_details @@ -71,7 +72,16 @@ class UserDetail < ApplicationRecord validates :onboarding_status, json_schema: { filename: 'user_detail_onboarding_status' } validates :github, length: { maximum: DEFAULT_FIELD_LENGTH }, allow_blank: true - before_validation :sanitize_attrs + # Use Sanitizable concern when feature flag is enabled + sanitizes! :bluesky, :discord, :linkedin, :mastodon, :orcid, :twitter, :website_url, :github, + if: -> { Feature.enabled?(:validate_sanitizable_user_details, user) } + + # New sanitization for location/organization (when feature flag is enabled) + before_validation :sanitize_location_and_organization, if: -> { Feature.enabled?(:validate_sanitizable_user_details, user) } + + # Legacy sanitization (when feature flag is disabled) + before_validation :sanitize_attrs, if: -> { Feature.disabled?(:validate_sanitizable_user_details, user) } + before_save :prevent_nil_fields # Exclude the hashed email_otp attribute @@ -96,6 +106,15 @@ def sanitize_attrs end end + def sanitize_location_and_organization + %i[location organization].each do |attr| + value = self[attr] + # Use Sanitize.fragment (modern) instead of deprecated Sanitize.clean + # Apply special & to & replacement for these fields + self[attr] = Sanitize.fragment(value).gsub('&', '&') if value.present? + end + end + def prevent_nil_fields self.bluesky = '' if bluesky.nil? self.bio = '' if bio.nil? diff --git a/config/feature_flags/gitlab_com_derisk/validate_sanitizable_user_details.yml b/config/feature_flags/gitlab_com_derisk/validate_sanitizable_user_details.yml new file mode 100644 index 0000000000000000000000000000000000000000..a1d3ae0960c851e478cd665d110253d7c63c5e40 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/validate_sanitizable_user_details.yml @@ -0,0 +1,8 @@ +--- +name: validate_sanitizable_user_details +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/208943 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/581152 +milestone: '18.8' +type: gitlab_com_derisk +group: group::authentication +default_enabled: false \ No newline at end of file diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb index e73355a353b07c5529ee9c4990fa396656c7151b..ec7e320c47cded1de20d692601654db210a24ec0 100644 --- a/spec/models/user_detail_spec.rb +++ b/spec/models/user_detail_spec.rb @@ -539,62 +539,153 @@ it_behaves_like 'prevents `nil` value', :website_url end - describe '#sanitize_attrs' do + describe 'sanitization' do using RSpec::Parameterized::TableSyntax subject { build(:user_detail, field => input).tap(&:validate) } - where(:field, :input, :expected) do - # HTML tags sanitization - all fields - :bluesky | 'did:plc:ewvi7nxzyoun6zhxrhs64oiz' | 'did:plc:ewvi7nxzyoun6zhxrhs64oiz' - :discord | '1234567890987654321' | '1234567890987654321' - :linkedin | 'https://example.com' | 'https://example.com' - :mastodon | '@robin@example.com' | '@robin@example.com' - :orcid | '1234-1234-1234-1234' | '1234-1234-1234-1234' - :twitter | 'https://example.com' | 'https://example.com' - :website_url | 'https://example.com' | 'https://example.com' - :github | 'https://example.com' | 'https://example.com' - :location | 'https://example.com' | 'https://example.com' - :organization | 'https://example.com' | 'https://example.com' - - # iframe scripts sanitization - :bluesky | '