From a6f2213f8a704c5adc96de800eae71cf4a413ff0 Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Wed, 15 Oct 2025 11:06:40 +0000
Subject: [PATCH 1/6] Refactor UserDetail to use Sanitizable concern

Migrates UserDetail model from deprecated Sanitize.clean() to
the Sanitizable concern using modern Sanitize.fragment() API.
This aligns with patterns in NamespaceSetting and
ApplicationSetting models.

Changes:
- Use sanitizes! for 8 attributes (bluesky, discord, linkedin,
  mastodon, orcid, twitter, website_url, github)
- Keep custom sanitization for location and organization to
  preserve ampersand handling behavior
- Add validation for pre-escaped HTML, double-encoding, and
  path traversal attacks

The Sanitizable concern prevents double-encoding by using
CGI.unescapeHTML(), so ampersands now remain unencoded. Tests
updated to reflect this more correct behavior.

Changelog: other
---
 app/models/user_detail.rb       |  23 ++++-
 spec/models/user_detail_spec.rb | 143 ++++++++++++++++++++------------
 2 files changed, 108 insertions(+), 58 deletions(-)

diff --git a/app/models/user_detail.rb b/app/models/user_detail.rb
index 3e237dfb040b84..1fa2c022ededd6 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,11 @@ 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 for most attributes
+  sanitizes! :bluesky, :discord, :linkedin, :mastodon, :orcid, :twitter, :website_url, :github
+
+  # Keep custom sanitization for location and organization due to special &amp; handling
+  before_validation :sanitize_location_and_organization
   before_save :prevent_nil_fields
 
   # Exclude the hashed email_otp attribute
@@ -79,12 +84,12 @@ def serializable_hash(options = nil)
     options = options.try(:dup) || {}
     options[:except] = Array(options[:except]).dup
     options[:except].concat [:email_otp]
-
+    
     super
   end
-
+  
   private
-
+  
   def sanitize_attrs
     %i[bluesky discord linkedin mastodon orcid twitter website_url github].each do |attr|
       value = self[attr]
@@ -95,6 +100,16 @@ def sanitize_attrs
       self[attr] = Sanitize.clean(value).gsub('&amp;', '&') if value.present?
     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 &amp; to & replacement for these fields
+      self[attr] = Sanitize.fragment(value).gsub('&amp;', '&') if value.present?
+    end
+  end
+
 
   def prevent_nil_fields
     self.bluesky = '' if bluesky.nil?
diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb
index e73355a353b07c..3f0d5a2d09917f 100644
--- a/spec/models/user_detail_spec.rb
+++ b/spec/models/user_detail_spec.rb
@@ -539,62 +539,97 @@
     it_behaves_like 'prevents `nil` value', :website_url
   end
 
-  describe '#sanitize_attrs' do
-    using RSpec::Parameterized::TableSyntax
-
-    subject { build(:user_detail, field => input).tap(&:validate) }
-
-    where(:field, :input, :expected) do
-      # HTML tags sanitization - all fields
-      :bluesky      | '<a href="//evil.com">did:plc:ewvi7nxzyoun6zhxrhs64oiz<a>'   | 'did:plc:ewvi7nxzyoun6zhxrhs64oiz'
-      :discord      | '<a href="//evil.com">1234567890987654321<a>'                | '1234567890987654321'
-      :linkedin     | '<a href="//evil.com">https://example.com<a>'                | 'https://example.com'
-      :mastodon     | '<a href="//evil.com">@robin@example.com<a>'                 | '@robin@example.com'
-      :orcid        | '<a href="//evil.com">1234-1234-1234-1234<a>'                | '1234-1234-1234-1234'
-      :twitter      | '<a href="//evil.com">https://example.com<a>'                | 'https://example.com'
-      :website_url  | '<a href="//evil.com">https://example.com<a>'                | 'https://example.com'
-      :github       | '<a href="//evil.com">https://example.com<a>'                | 'https://example.com'
-      :location     | '<a href="//evil.com">https://example.com<a>'                | 'https://example.com'
-      :organization | '<a href="//evil.com">https://example.com<a>'                | 'https://example.com'
-
-      # iframe scripts sanitization
-      :bluesky      | '<iframe src=javascript:alert()><iframe>'                    | ''
-      :discord      | '<iframe src=javascript:alert()><iframe>'                    | ''
-      :linkedin     | '<iframe src=javascript:alert()><iframe>'                    | ''
-      :mastodon     | '<iframe src=javascript:alert()><iframe>'                    | ''
-      :orcid        | '<iframe src=javascript:alert()>1234-1234-1234-1234<iframe>' | ''
-      :twitter      | '<iframe src=javascript:alert()><iframe>'                    | ''
-      :website_url  | '<iframe src=javascript:alert()><iframe>'                    | ''
-      :github       | '<iframe src=javascript:alert()><iframe>'                    | ''
-      :location     | '<iframe src=javascript:alert()><iframe>'                    | ''
-      :organization | '<iframe src=javascript:alert()><iframe>'                    | ''
-
-      # js scripts sanitization
-      :bluesky      | '<script>alert("Test")</script>'                             | ''
-      :discord      | '<script>alert("Test")</script>'                             | ''
-      :linkedin     | '<script>alert("Test")</script>'                             | ''
-      :mastodon     | '<script>alert("Test")</script>'                             | ''
-      :orcid        | '<script>alert("Test")1234-1234-1234-1234</script>'          | ''
-      :twitter      | '<script>alert("Test")</script>'                             | ''
-      :website_url  | '<script>alert("Test")</script>'                             | ''
-      :github       | '<script>alert("Test")</script>'                             | ''
-      :location     | '<script>alert("Test")</script>'                             | ''
-      :organization | '<script>alert("Test")</script>'                             | ''
-
-      # HTML entities encoding - fields that encode & to &amp;
-      :linkedin     | 'test&attr'                                                  | 'test&amp;attr'
-      :twitter      | 'test&attr'                                                  | 'test&amp;attr'
-      :website_url  | 'http://example.com?test&attr'                               | 'http://example.com?test&amp;attr'
-      :github       | 'test&attr'                                                  | 'test&amp;attr'
-
-      # HTML entities NOT encoded - location, organization preserve &
-      :location     | 'test&attr'                                                  | 'test&attr'
-      :organization | 'test&attr'                                                  | 'test&attr'
+  describe 'sanitization' do
+    shared_examples 'sanitizes html' do |attr|
+      it 'sanitizes html tags' do
+        details = build_stubbed(:user_detail, attr => '<a href="//evil.com">https://example.com<a>')
+        details.valid?
+        expect(details[attr]).to eq('https://example.com')
+      end
+
+      it 'sanitizes iframe scripts' do
+        details = build_stubbed(:user_detail, attr => '<iframe src=javascript:alert()><iframe>')
+        details.valid?
+        expect(details[attr]).to eq('')
+      end
+
+      it 'sanitizes js scripts' do
+        details = build_stubbed(:user_detail, attr => '<script>alert("Test")</script>')
+        details.valid?
+        expect(details[attr]).to eq('')
+      end
+    end
+
+    shared_examples 'does not encode HTML entities' do |attr|
+      it "does not encode HTML entities for #{attr}" do
+        details = build_stubbed(:user_detail, attr => 'test&attr')
+        details.valid?
+        expect(details[attr]).to eq('test&attr')
+      end
+    end
+
+    %i[linkedin twitter website_url bluesky discord mastodon orcid github].each do |attr|
+      it_behaves_like 'sanitizes html', attr
+      it_behaves_like 'does not encode HTML entities', attr
+    end
+
+    %i[location organization].each do |attr|
+      it_behaves_like 'sanitizes html', attr
+      it_behaves_like 'does not encode HTML entities', attr
+    end
+
+    it 'sanitizes on validation' do
+      details = build(:user_detail)
+
+      expect(details)
+        .to receive(:sanitize_location_and_organization)
+        .at_least(:once)
+        .and_call_original
+
+      details.valid?
+    end
+  end
+
+  describe 'Sanitizable concern validations' do
+    let_it_be(:user_detail) { build(:user_detail) }
+
+    shared_examples 'sanitizable validation' do |attr|
+      context 'when input contains pre-escaped HTML entities' do
+        before do
+          user_detail.send(:"#{attr}=", '&lt;script&gt;alert(1)&lt;/script&gt;')
+        end
+
+        it 'is not valid' do
+          expect(user_detail).not_to be_valid
+          expect(user_detail.errors[attr]).to include('cannot contain escaped HTML entities')
+        end
+      end
+
+      context 'when input contains double-encoded data' do
+        before do
+          user_detail.send(:"#{attr}=", '%2526lt%253Bscript%2526gt%253B')
+        end
+
+        it 'is not valid' do
+          expect(user_detail).not_to be_valid
+          expect(user_detail.errors[attr]).to include('cannot contain escaped components')
+        end
+      end
+
+      context 'when input contains path traversal attempt' do
+        before do
+          user_detail.send(:"#{attr}=", 'main../../../../../../api/v4/projects/1')
+        end
+
+        it 'is not valid' do
+          expect(user_detail).not_to be_valid
+          expect(user_detail.errors[attr]).to include('cannot contain a path traversal component')
+        end
+      end
     end
 
-    with_them do
-      it { is_expected.to be_valid }
-      it { is_expected.to have_attributes field => expected }
+    %i[bluesky discord linkedin mastodon orcid twitter website_url github].each do |attr|
+      it_behaves_like 'sanitizable validation', attr
     end
   end
 
-- 
GitLab


From 0ee1266a57c4b782379608a94c118ca0e7b1cc0c Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Tue, 18 Nov 2025 08:36:42 +0000
Subject: [PATCH 2/6] refactor: Apply suggestion from @mhamda

- Add feature flag for breaking API change with gradual rollout support,
see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/208943#note_2852182718
- Revert to original sanitization `before_validation`
---
 app/models/user_detail.rb                     | 13 ++-
 .../validate_sanitizable_user_details.yml     |  8 ++
 spec/models/user_detail_spec.rb               | 87 +++++++++++++++++++
 3 files changed, 104 insertions(+), 4 deletions(-)
 create mode 100644 config/feature_flags/development/validate_sanitizable_user_details.yml

diff --git a/app/models/user_detail.rb b/app/models/user_detail.rb
index 1fa2c022ededd6..5720c5d7ac084f 100644
--- a/app/models/user_detail.rb
+++ b/app/models/user_detail.rb
@@ -72,11 +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
 
-  # Use Sanitizable concern for most attributes
-  sanitizes! :bluesky, :discord, :linkedin, :mastodon, :orcid, :twitter, :website_url, :github
+  # 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) }
 
-  # Keep custom sanitization for location and organization due to special &amp; handling
-  before_validation :sanitize_location_and_organization
   before_save :prevent_nil_fields
 
   # Exclude the hashed email_otp attribute
diff --git a/config/feature_flags/development/validate_sanitizable_user_details.yml b/config/feature_flags/development/validate_sanitizable_user_details.yml
new file mode 100644
index 00000000000000..aedf385e134028
--- /dev/null
+++ b/config/feature_flags/development/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::api
+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 3f0d5a2d09917f..28c4e529f9ea58 100644
--- a/spec/models/user_detail_spec.rb
+++ b/spec/models/user_detail_spec.rb
@@ -568,6 +568,44 @@
       end
     end
 
+    context 'when feature flag :validate_sanitizable_user_details is disabled' do
+      before do
+        stub_feature_flags(validate_sanitizable_user_details: false)
+      end
+
+      %i[linkedin twitter website_url bluesky discord mastodon orcid github].each do |attr|
+        it_behaves_like 'sanitizes html', attr
+        it_behaves_like 'does not encode HTML entities', attr
+      end
+
+      %i[location organization].each do |attr|
+        it_behaves_like 'sanitizes html', attr
+        it_behaves_like 'does not encode HTML entities', attr
+      end
+
+      it 'sanitizes on validation' do
+        details = build(:user_detail)
+
+        expect(details)
+          .to receive(:sanitize_attrs)
+          .at_least(:once)
+          .and_call_original
+
+        details.valid?
+      end
+
+      it 'does not raise validation errors for malicious input' do
+        details = build(:user_detail,
+          twitter: '&lt;a href="evil.com"&gt;Click me&lt;/a&gt;',
+          website_url: 'http://example.com/../../../etc/passwd')
+
+        expect(details).to be_valid
+        # Pre-escaped HTML is unescaped first, then sanitized (tags removed, content kept)
+        expect(details.twitter).to eq('Click me')
+        expect(details.website_url).to eq('http://example.com/../../../etc/passwd')
+      end
+    end
+
     %i[linkedin twitter website_url bluesky discord mastodon orcid github].each do |attr|
       it_behaves_like 'sanitizes html', attr
       it_behaves_like 'does not encode HTML entities', attr
@@ -631,6 +669,55 @@
     %i[bluesky discord linkedin mastodon orcid twitter website_url github].each do |attr|
       it_behaves_like 'sanitizable validation', attr
     end
+
+    describe 'legitimate edge cases' do
+      it 'allows properly encoded URLs' do
+        user_detail = build(:user_detail,
+          website_url: 'https://example.com/search?q=hello%20world',
+          twitter: 'https://twitter.com/user')
+
+        expect(user_detail).to be_valid
+      end
+
+      it 'allows special characters in social profile URLs' do
+        user_detail = build(:user_detail,
+          linkedin: 'https://linkedin.com/in/user-name',
+          twitter: '@user_name',
+          discord: '1234567890')
+
+        expect(user_detail).to be_valid
+      end
+
+      it 'allows ampersands in URLs' do
+        user_detail = build(:user_detail,
+          website_url: 'https://example.com?foo=bar&baz=qux')
+
+        expect(user_detail).to be_valid
+      end
+
+      it 'allows valid GitHub and website URLs' do
+        user_detail = build(:user_detail,
+          github: 'https://github.com/user/repo',
+          website_url: 'https://example.com/path/to/page')
+
+        expect(user_detail).to be_valid
+      end
+    end
+
+    context 'when validate_sanitizable_user_details feature flag is disabled' do
+      before do
+        stub_feature_flags(validate_sanitizable_user_details: false)
+      end
+
+      it 'does not apply Sanitizable validations' do
+        user_detail = build(:user_detail,
+          twitter: '&lt;script&gt;alert(1)&lt;/script&gt;',
+          linkedin: '%2526lt%253Bscript%2526gt%253B',
+          github: 'main../../../../../../api/v4/projects/1')
+
+        expect(user_detail).to be_valid
+      end
+    end
   end
 
   describe '#email_otp' do
-- 
GitLab


From f676b391d7feef4db774d5b86d09ff3672e5fe00 Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Fri, 21 Nov 2025 09:02:55 +0000
Subject: [PATCH 3/6] ci: Fix pipeline

- https://gitlab.com/gitlab-community/gitlab-org/gitlab/-/jobs/12141208298#L75
- https://gitlab.com/gitlab-community/gitlab-org/gitlab/-/jobs/12169841722
---
 .gitleaksignore                               |  1 +
 .../validate_sanitizable_user_details.yml     |  0
 spec/models/user_detail_spec.rb               | 27 +++++++++----------
 3 files changed, 13 insertions(+), 15 deletions(-)
 rename config/feature_flags/{development => gitlab_com_derisk}/validate_sanitizable_user_details.yml (100%)

diff --git a/.gitleaksignore b/.gitleaksignore
index 80e43253ad5991..0232964b7a4a0b 100644
--- a/.gitleaksignore
+++ b/.gitleaksignore
@@ -19,3 +19,4 @@ 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
diff --git a/config/feature_flags/development/validate_sanitizable_user_details.yml b/config/feature_flags/gitlab_com_derisk/validate_sanitizable_user_details.yml
similarity index 100%
rename from config/feature_flags/development/validate_sanitizable_user_details.yml
rename to config/feature_flags/gitlab_com_derisk/validate_sanitizable_user_details.yml
diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb
index 28c4e529f9ea58..1fd707d9586d5c 100644
--- a/spec/models/user_detail_spec.rb
+++ b/spec/models/user_detail_spec.rb
@@ -568,6 +568,14 @@
       end
     end
 
+    shared_examples 'encodes HTML entities for legacy sanitization' do |attr|
+      it "encodes HTML entities for #{attr}" do
+        details = build_stubbed(:user_detail, attr => 'test&attr')
+        details.valid?
+        expect(details[attr]).to eq('test&amp;attr')
+      end
+    end
+
     context 'when feature flag :validate_sanitizable_user_details is disabled' do
       before do
         stub_feature_flags(validate_sanitizable_user_details: false)
@@ -575,7 +583,7 @@
 
       %i[linkedin twitter website_url bluesky discord mastodon orcid github].each do |attr|
         it_behaves_like 'sanitizes html', attr
-        it_behaves_like 'does not encode HTML entities', attr
+        it_behaves_like 'encodes HTML entities for legacy sanitization', attr
       end
 
       %i[location organization].each do |attr|
@@ -600,8 +608,8 @@
           website_url: 'http://example.com/../../../etc/passwd')
 
         expect(details).to be_valid
-        # Pre-escaped HTML is unescaped first, then sanitized (tags removed, content kept)
-        expect(details.twitter).to eq('Click me')
+        # Pre-escaped HTML stays escaped with legacy sanitization
+        expect(details.twitter).to eq('&lt;a href="evil.com"&gt;Click me&lt;/a&gt;')
         expect(details.website_url).to eq('http://example.com/../../../etc/passwd')
       end
     end
@@ -615,17 +623,6 @@
       it_behaves_like 'sanitizes html', attr
       it_behaves_like 'does not encode HTML entities', attr
     end
-
-    it 'sanitizes on validation' do
-      details = build(:user_detail)
-
-      expect(details)
-        .to receive(:sanitize_location_and_organization)
-        .at_least(:once)
-        .and_call_original
-
-      details.valid?
-    end
   end
 
   describe 'Sanitizable concern validations' do
@@ -683,7 +680,7 @@
         user_detail = build(:user_detail,
           linkedin: 'https://linkedin.com/in/user-name',
           twitter: '@user_name',
-          discord: '1234567890')
+          discord: '123456789012345678')
 
         expect(user_detail).to be_valid
       end
-- 
GitLab


From 105508b8224a28ce42443dfc71cbeaf8f2e031df Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Tue, 2 Dec 2025 07:38:41 +0100
Subject: [PATCH 4/6] refactor: Apply suggestion from @habdul-razak

- https://gitlab.com/gitlab-org/gitlab/-/merge_requests/208943#note_2925312519
---
 .../gitlab_com_derisk/validate_sanitizable_user_details.yml     | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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
index aedf385e134028..a1d3ae0960c851 100644
--- a/config/feature_flags/gitlab_com_derisk/validate_sanitizable_user_details.yml
+++ b/config/feature_flags/gitlab_com_derisk/validate_sanitizable_user_details.yml
@@ -4,5 +4,5 @@ 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::api
+group: group::authentication
 default_enabled: false
\ No newline at end of file
-- 
GitLab


From b00456d7d88dcc92b439e8cc74e40e73812484a8 Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Wed, 3 Dec 2025 14:30:29 +0000
Subject: [PATCH 5/6] refactor: Apply suggestion from @habdul-razak

- https://gitlab.com/gitlab-org/gitlab/-/merge_requests/208943#note_2928642361
---
 spec/requests/api/users_spec.rb | 81 +++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index f108e73c9791dc..30839306c606d3 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -1940,6 +1940,87 @@ def update_password(user, admin, password = User.random_password)
       end
     end
 
+    context 'when updating user_details with sanitizable fields' do
+      using RSpec::Parameterized::TableSyntax
+
+      let_it_be(:user_for_details) { create(:user) }
+      let(:details_path) { "/users/#{user_for_details.id}" }
+
+      subject(:put_request_response) { put api(details_path, admin, admin_mode: true), params: params }
+
+      shared_examples 'successful user_details update' do
+        with_them do
+          it 'responds :ok and sets the field correctly' do
+            put_request_response
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response).to include(expected_attributes)
+          end
+        end
+      end
+
+      shared_examples 'rejected user_details update' do
+        with_them do
+          it 'responds :bad_request with validation error' do
+            put_request_response
+
+            expect(response).to have_gitlab_http_status(:bad_request)
+            expect(json_response['message'].values.flatten).to include(match(expected_error))
+          end
+        end
+      end
+
+      context 'with valid input' do
+        where(:params, :expected_attributes) do
+          { linkedin: 'https://linkedin.com/in/username' }  | { 'linkedin' => 'https://linkedin.com/in/username' }
+          { website_url: 'https://example.com?a=1&b=2' }    | { 'website_url' => 'https://example.com?a=1&b=2' }
+          { twitter: '<script>alert("xss")</script>@user' } | { 'twitter' => '@user' }
+        end
+
+        it_behaves_like 'successful user_details update'
+      end
+
+      context 'with invalid input' do
+        where(:params, :expected_error) do
+          { linkedin: '&lt;script&gt;alert(1)&lt;/script&gt;' } | /cannot contain escaped HTML entities/
+          { website_url: '%2526lt%253Bscript%2526gt%253B' }     | /cannot contain escaped components/
+          { github: 'main../../../../../../etc/passwd' }        | /cannot contain a path traversal component/
+        end
+
+        it_behaves_like 'rejected user_details update'
+      end
+
+      context 'when feature flag :validate_sanitizable_user_details is disabled' do
+        before do
+          stub_feature_flags(validate_sanitizable_user_details: false)
+        end
+
+        context 'with valid input' do
+          where(:params, :expected_attributes) do
+            { linkedin: 'https://linkedin.com/in/username' }  | { 'linkedin' => 'https://linkedin.com/in/username' }
+            { website_url: 'https://example.com?a=1&b=2' }    | { 'website_url' => 'https://example.com?a=1&b=2' }
+            { twitter: '<script>alert("xss")</script>@user' } | { 'twitter' => '@user' }
+
+            # Valid inputs when the feature flag :validate_sanitizable_user_details is enabled
+            { linkedin: '&lt;script&gt;alert(1)&lt;/script&gt;' } | { 'linkedin' => '&lt;script&gt;alert(1)&lt;/script&gt;' }
+            { website_url: '%2526lt%253Bscript%2526gt%253B' }     | { 'website_url' => '%2526lt%253Bscript%2526gt%253B' }
+            { github: 'main../../../../../../etc/passwd' }        | { 'github' => 'main../../../../../../etc/passwd' }
+          end
+
+          it_behaves_like 'successful user_details update'
+        end
+
+        context 'with input that was already invalid before this MR' do
+          where(:params, :expected_error) do
+            { linkedin: 'a' * 501 }            | /is too long/
+            { website_url: 'not-a-valid-url' } | /is not a valid URL/
+          end
+
+          it_behaves_like 'rejected user_details update'
+        end
+      end
+    end
+
     it 'updates user with avatar' do
       workhorse_form_with_file(
         api(path, admin, admin_mode: true),
-- 
GitLab


From 7ffbb834bf6071994bd15527b26bd51665d55855 Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Fri, 12 Dec 2025 14:45:49 +0000
Subject: [PATCH 6/6] test: Refactor user detail tests after rebase

---
 .gitleaksignore                 |   1 +
 app/models/user_detail.rb       |   9 +-
 spec/models/user_detail_spec.rb | 276 ++++++++++++++------------------
 3 files changed, 129 insertions(+), 157 deletions(-)

diff --git a/.gitleaksignore b/.gitleaksignore
index 0232964b7a4a0b..48fd24b38b0e93 100644
--- a/.gitleaksignore
+++ b/.gitleaksignore
@@ -20,3 +20,4 @@ ee/spec/lib/security/secret_detection/partner_tokens/gcp_client_spec.rb:gcp-api-
 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 5720c5d7ac084f..dadb67646692b2 100644
--- a/app/models/user_detail.rb
+++ b/app/models/user_detail.rb
@@ -89,12 +89,12 @@ def serializable_hash(options = nil)
     options = options.try(:dup) || {}
     options[:except] = Array(options[:except]).dup
     options[:except].concat [:email_otp]
-    
+
     super
   end
-  
+
   private
-  
+
   def sanitize_attrs
     %i[bluesky discord linkedin mastodon orcid twitter website_url github].each do |attr|
       value = self[attr]
@@ -105,7 +105,7 @@ def sanitize_attrs
       self[attr] = Sanitize.clean(value).gsub('&amp;', '&') if value.present?
     end
   end
-  
+
   def sanitize_location_and_organization
     %i[location organization].each do |attr|
       value = self[attr]
@@ -115,7 +115,6 @@ def sanitize_location_and_organization
     end
   end
 
-
   def prevent_nil_fields
     self.bluesky = '' if bluesky.nil?
     self.bio = '' if bio.nil?
diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb
index 1fd707d9586d5c..ec7e320c47cded 100644
--- a/spec/models/user_detail_spec.rb
+++ b/spec/models/user_detail_spec.rb
@@ -540,179 +540,151 @@
   end
 
   describe 'sanitization' do
-    shared_examples 'sanitizes html' do |attr|
-      it 'sanitizes html tags' do
-        details = build_stubbed(:user_detail, attr => '<a href="//evil.com">https://example.com<a>')
-        details.valid?
-        expect(details[attr]).to eq('https://example.com')
-      end
-
-      it 'sanitizes iframe scripts' do
-        details = build_stubbed(:user_detail, attr => '<iframe src=javascript:alert()><iframe>')
-        details.valid?
-        expect(details[attr]).to eq('')
-      end
-
-      it 'sanitizes js scripts' do
-        details = build_stubbed(:user_detail, attr => '<script>alert("Test")</script>')
-        details.valid?
-        expect(details[attr]).to eq('')
+    using RSpec::Parameterized::TableSyntax
+
+    subject { build(:user_detail, field => input).tap(&:validate) }
+
+    shared_examples 'standard sanitization tests' do
+      # rubocop:disable Layout/LineLength -- Ignore long lines to keep single line table format
+      where(:field, :input, :expected) do
+        # HTML tags sanitization - all fields
+        :bluesky      | '<a href="//evil.com">did:plc:ewvi7nxzyoun6zhxrhs64oiz<a>'   | 'did:plc:ewvi7nxzyoun6zhxrhs64oiz'
+        :discord      | '<a href="//evil.com">1234567890987654321<a>'                | '1234567890987654321'
+        :linkedin     | '<a href="//evil.com">https://example.com<a>'                | 'https://example.com'
+        :mastodon     | '<a href="//evil.com">@robin@example.com<a>'                 | '@robin@example.com'
+        :orcid        | '<a href="//evil.com">1234-1234-1234-1234<a>'                | '1234-1234-1234-1234'
+        :twitter      | '<a href="//evil.com">https://example.com<a>'                | 'https://example.com'
+        :website_url  | '<a href="//evil.com">https://example.com<a>'                | 'https://example.com'
+        :github       | '<a href="//evil.com">https://example.com<a>'                | 'https://example.com'
+        :location     | '<a href="//evil.com">https://example.com<a>'                | 'https://example.com'
+        :organization | '<a href="//evil.com">https://example.com<a>'                | 'https://example.com'
+
+        # iframe scripts sanitization
+        :bluesky      | '<iframe src=javascript:alert()><iframe>'                    | ''
+        :discord      | '<iframe src=javascript:alert()><iframe>'                    | ''
+        :linkedin     | '<iframe src=javascript:alert()><iframe>'                    | ''
+        :mastodon     | '<iframe src=javascript:alert()><iframe>'                    | ''
+        :orcid        | '<iframe src=javascript:alert()>1234-1234-1234-1234<iframe>' | ''
+        :twitter      | '<iframe src=javascript:alert()><iframe>'                    | ''
+        :website_url  | '<iframe src=javascript:alert()><iframe>'                    | ''
+        :github       | '<iframe src=javascript:alert()><iframe>'                    | ''
+        :location     | '<iframe src=javascript:alert()><iframe>'                    | ''
+        :organization | '<iframe src=javascript:alert()><iframe>'                    | ''
+
+        # js scripts sanitization
+        :bluesky      | '<script>alert("Test")</script>'                             | ''
+        :discord      | '<script>alert("Test")</script>'                             | ''
+        :linkedin     | '<script>alert("Test")</script>'                             | ''
+        :mastodon     | '<script>alert("Test")</script>'                             | ''
+        :orcid        | '<script>alert("Test")1234-1234-1234-1234</script>'          | ''
+        :twitter      | '<script>alert("Test")</script>'                             | ''
+        :website_url  | '<script>alert("Test")</script>'                             | ''
+        :github       | '<script>alert("Test")</script>'                             | ''
+        :location     | '<script>alert("Test")</script>'                             | ''
+        :organization | '<script>alert("Test")</script>'                             | ''
+      end
+      # rubocop:enable Layout/LineLength
+
+      with_them do
+        it { is_expected.to be_valid }
+        it { is_expected.to have_attributes field => expected }
       end
     end
 
-    shared_examples 'does not encode HTML entities' do |attr|
-      it "does not encode HTML entities for #{attr}" do
-        details = build_stubbed(:user_detail, attr => 'test&attr')
-        details.valid?
-        expect(details[attr]).to eq('test&attr')
-      end
-    end
+    it_behaves_like 'standard sanitization tests'
 
-    shared_examples 'encodes HTML entities for legacy sanitization' do |attr|
-      it "encodes HTML entities for #{attr}" do
-        details = build_stubbed(:user_detail, attr => 'test&attr')
-        details.valid?
-        expect(details[attr]).to eq('test&amp;attr')
-      end
-    end
+    context 'with valid field inputs' do
+      where(:field, :input, :expected) do
+        # HTML entities NOT encoding - fields that encode & to &amp;
+        :linkedin     | 'test&attr'                                  | 'test&attr'
+        :twitter      | 'test&attr'                                  | 'test&attr'
+        :website_url  | 'http://example.com?test&attr'               | 'http://example.com?test&attr'
+        :github       | 'test&attr'                                  | 'test&attr'
 
-    context 'when feature flag :validate_sanitizable_user_details is disabled' do
-      before do
-        stub_feature_flags(validate_sanitizable_user_details: false)
-      end
-
-      %i[linkedin twitter website_url bluesky discord mastodon orcid github].each do |attr|
-        it_behaves_like 'sanitizes html', attr
-        it_behaves_like 'encodes HTML entities for legacy sanitization', attr
-      end
+        # HTML entities NOT encoded - location, organization preserve &
+        :location     | 'test&attr'                                  | 'test&attr'
+        :organization | 'test&attr'                                  | 'test&attr'
 
-      %i[location organization].each do |attr|
-        it_behaves_like 'sanitizes html', attr
-        it_behaves_like 'does not encode HTML entities', attr
+        # Legitimate edge cases
+        :website_url  | 'https://example.com/search?q=hello%20world' | 'https://example.com/search?q=hello%20world'
+        :twitter      | 'https://twitter.com/user'                   | 'https://twitter.com/user'
+        :linkedin     | 'https://linkedin.com/in/user-name'          | 'https://linkedin.com/in/user-name'
+        :twitter      | '@user_name'                                 | '@user_name'
+        :discord      | '123456789012345678'                         | '123456789012345678'
+        :website_url  | 'https://example.com?foo=bar&baz=qux'        | 'https://example.com?foo=bar&baz=qux'
+        :github       | 'https://github.com/user/repo'               | 'https://github.com/user/repo'
+        :website_url  | 'https://example.com/path/to/page'           | 'https://example.com/path/to/page'
       end
 
-      it 'sanitizes on validation' do
-        details = build(:user_detail)
-
-        expect(details)
-          .to receive(:sanitize_attrs)
-          .at_least(:once)
-          .and_call_original
-
-        details.valid?
+      with_them do
+        it { is_expected.to be_valid }
+        it { is_expected.to have_attributes field => expected }
       end
-
-      it 'does not raise validation errors for malicious input' do
-        details = build(:user_detail,
-          twitter: '&lt;a href="evil.com"&gt;Click me&lt;/a&gt;',
-          website_url: 'http://example.com/../../../etc/passwd')
-
-        expect(details).to be_valid
-        # Pre-escaped HTML stays escaped with legacy sanitization
-        expect(details.twitter).to eq('&lt;a href="evil.com"&gt;Click me&lt;/a&gt;')
-        expect(details.website_url).to eq('http://example.com/../../../etc/passwd')
-      end
-    end
-
-    %i[linkedin twitter website_url bluesky discord mastodon orcid github].each do |attr|
-      it_behaves_like 'sanitizes html', attr
-      it_behaves_like 'does not encode HTML entities', attr
     end
 
-    %i[location organization].each do |attr|
-      it_behaves_like 'sanitizes html', attr
-      it_behaves_like 'does not encode HTML entities', attr
-    end
-  end
-
-  describe 'Sanitizable concern validations' do
-    let_it_be(:user_detail) { build(:user_detail) }
-
-    shared_examples 'sanitizable validation' do |attr|
-      context 'when input contains pre-escaped HTML entities' do
-        before do
-          user_detail.send(:"#{attr}=", '&lt;script&gt;alert(1)&lt;/script&gt;')
-        end
-
-        it 'is not valid' do
-          expect(user_detail).not_to be_valid
-          expect(user_detail.errors[attr]).to include('cannot contain escaped HTML entities')
-        end
+    context 'with invalid field inputs' do
+      where(:field, :input, :error_message) do
+        :bluesky     | '&lt;script&gt;alert(1)&lt;/script&gt;'   | 'cannot contain escaped HTML entities'
+        :discord     | '&lt;script&gt;alert(1)&lt;/script&gt;'   | 'cannot contain escaped HTML entities'
+        :linkedin    | '&lt;script&gt;alert(1)&lt;/script&gt;'   | 'cannot contain escaped HTML entities'
+        :mastodon    | '&lt;script&gt;alert(1)&lt;/script&gt;'   | 'cannot contain escaped HTML entities'
+        :orcid       | '&lt;script&gt;alert(1)&lt;/script&gt;'   | 'cannot contain escaped HTML entities'
+        :twitter     | '&lt;script&gt;alert(1)&lt;/script&gt;'   | 'cannot contain escaped HTML entities'
+        :website_url | '&lt;script&gt;alert(1)&lt;/script&gt;'   | 'cannot contain escaped HTML entities'
+        :github      | '&lt;script&gt;alert(1)&lt;/script&gt;'   | 'cannot contain escaped HTML entities'
+
+        :bluesky     | '%2526lt%253Bscript%2526gt%253B'          | 'cannot contain escaped components'
+        :discord     | '%2526lt%253Bscript%2526gt%253B'          | 'cannot contain escaped components'
+        :linkedin    | '%2526lt%253Bscript%2526gt%253B'          | 'cannot contain escaped components'
+        :mastodon    | '%2526lt%253Bscript%2526gt%253B'          | 'cannot contain escaped components'
+        :orcid       | '%2526lt%253Bscript%2526gt%253B'          | 'cannot contain escaped components'
+        :twitter     | '%2526lt%253Bscript%2526gt%253B'          | 'cannot contain escaped components'
+        :website_url | '%2526lt%253Bscript%2526gt%253B'          | 'cannot contain escaped components'
+        :github      | '%2526lt%253Bscript%2526gt%253B'          | 'cannot contain escaped components'
+
+        :bluesky     | 'main../../../../../../api/v4/projects/1' | 'cannot contain a path traversal component'
+        :discord     | 'main../../../../../../api/v4/projects/1' | 'cannot contain a path traversal component'
+        :linkedin    | 'main../../../../../../api/v4/projects/1' | 'cannot contain a path traversal component'
+        :mastodon    | 'main../../../../../../api/v4/projects/1' | 'cannot contain a path traversal component'
+        :orcid       | 'main../../../../../../api/v4/projects/1' | 'cannot contain a path traversal component'
+        :twitter     | 'main../../../../../../api/v4/projects/1' | 'cannot contain a path traversal component'
+        :website_url | 'main../../../../../../api/v4/projects/1' | 'cannot contain a path traversal component'
+        :github      | 'main../../../../../../api/v4/projects/1' | 'cannot contain a path traversal component'
+      end
+
+      with_them do
+        it { is_expected.not_to be_valid }
+        it { is_expected.to have_attributes errors: hash_including(field => array_including(error_message)) }
       end
-
-      context 'when input contains double-encoded data' do
-        before do
-          user_detail.send(:"#{attr}=", '%2526lt%253Bscript%2526gt%253B')
-        end
-
-        it 'is not valid' do
-          expect(user_detail).not_to be_valid
-          expect(user_detail.errors[attr]).to include('cannot contain escaped components')
-        end
-      end
-
-      context 'when input contains path traversal attempt' do
-        before do
-          user_detail.send(:"#{attr}=", 'main../../../../../../api/v4/projects/1')
-        end
-
-        it 'is not valid' do
-          expect(user_detail).not_to be_valid
-          expect(user_detail.errors[attr]).to include('cannot contain a path traversal component')
-        end
-      end
-    end
-
-    %i[bluesky discord linkedin mastodon orcid twitter website_url github].each do |attr|
-      it_behaves_like 'sanitizable validation', attr
     end
 
-    describe 'legitimate edge cases' do
-      it 'allows properly encoded URLs' do
-        user_detail = build(:user_detail,
-          website_url: 'https://example.com/search?q=hello%20world',
-          twitter: 'https://twitter.com/user')
-
-        expect(user_detail).to be_valid
-      end
-
-      it 'allows special characters in social profile URLs' do
-        user_detail = build(:user_detail,
-          linkedin: 'https://linkedin.com/in/user-name',
-          twitter: '@user_name',
-          discord: '123456789012345678')
-
-        expect(user_detail).to be_valid
+    context 'when feature flag :validate_sanitizable_user_details is disabled' do
+      before do
+        stub_feature_flags(validate_sanitizable_user_details: false)
       end
 
-      it 'allows ampersands in URLs' do
-        user_detail = build(:user_detail,
-          website_url: 'https://example.com?foo=bar&baz=qux')
-
-        expect(user_detail).to be_valid
-      end
+      it_behaves_like 'standard sanitization tests'
 
-      it 'allows valid GitHub and website URLs' do
-        user_detail = build(:user_detail,
-          github: 'https://github.com/user/repo',
-          website_url: 'https://example.com/path/to/page')
+      where(:field, :input, :expected) do
+        # HTML entities encoding - fields that encode & to &amp;
+        :linkedin     | 'test&attr'                                 | 'test&amp;attr'
+        :twitter      | 'test&attr'                                 | 'test&amp;attr'
+        :website_url  | 'http://example.com?test&attr'              | 'http://example.com?test&amp;attr'
+        :github       | 'test&attr'                                 | 'test&amp;attr'
 
-        expect(user_detail).to be_valid
-      end
-    end
+        # HTML entities NOT encoded - location, organization preserve &
+        :location     | 'test&attr'                                 | 'test&attr'
+        :organization | 'test&attr'                                 | 'test&attr'
 
-    context 'when validate_sanitizable_user_details feature flag is disabled' do
-      before do
-        stub_feature_flags(validate_sanitizable_user_details: false)
+        # Does not apply Sanitizable validations
+        :twitter      | '&lt;script&gt;alert(1)&lt;/script&gt;'     | '&lt;script&gt;alert(1)&lt;/script&gt;'
+        :linkedin     | '%2526lt%253Bscript%2526gt%253B'            | '%2526lt%253Bscript%2526gt%253B'
+        :github       | 'main../../../../../../api/v4/projects/1'   | 'main../../../../../../api/v4/projects/1'
       end
 
-      it 'does not apply Sanitizable validations' do
-        user_detail = build(:user_detail,
-          twitter: '&lt;script&gt;alert(1)&lt;/script&gt;',
-          linkedin: '%2526lt%253Bscript%2526gt%253B',
-          github: 'main../../../../../../api/v4/projects/1')
-
-        expect(user_detail).to be_valid
+      with_them do
+        it { is_expected.to be_valid }
+        it { is_expected.to have_attributes field => expected }
       end
     end
   end
-- 
GitLab