Merge remote-tracking branch 'dev/15-2-stable' into 15-2-stable

6 files changed, 76 insertions, 3 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b9d53d1ac30..e51329bd8f9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,13 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 15.2.3 (2022-08-22)
+
+### Security (2 changes)
+
+- [Validate if values to be saved in Redis can be converted to string](gitlab-org/security/gitlab@427c7818b229fd45b10cb5de9ea6cc7c451dd4da) ([merge request](gitlab-org/security/gitlab!2724))
+- [Fix CSS selector used in specs](gitlab-org/security/gitlab@47bb40d097e2b05ecdbeebf6bdbe6eb9b6db1c7b) ([merge request](gitlab-org/security/gitlab!2727))
+
 ## 15.2.2 (2022-08-01)
 
 ### Fixed (6 changes)
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index ee7380d66e8..462d212d88a 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-15.2.2
\ No newline at end of file
+15.2.3
\ No newline at end of file
diff --git a/VERSION b/VERSION
index ee7380d66e8..462d212d88a 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-15.2.2
\ No newline at end of file
+15.2.3
\ No newline at end of file
diff --git a/lib/gitlab/cache/import/caching.rb b/lib/gitlab/cache/import/caching.rb
index 4dbce0b05e1..4e7a7f326a5 100644
--- a/lib/gitlab/cache/import/caching.rb
+++ b/lib/gitlab/cache/import/caching.rb
@@ -65,6 +65,8 @@ module Gitlab
         # value - The value to set.
         # timeout - The time after which the cache key should expire.
         def self.write(raw_key, value, timeout: TIMEOUT)
+          validate_redis_value!(value)
+
           key = cache_key_for(raw_key)
 
           Redis::Cache.with do |redis|
@@ -99,6 +101,8 @@ module Gitlab
         # timeout - The time after which the cache key should expire.
         # @return - the incremented value
         def self.increment_by(raw_key, value, timeout: TIMEOUT)
+          validate_redis_value!(value)
+
           key = cache_key_for(raw_key)
 
           Redis::Cache.with do |redis|
@@ -113,6 +117,8 @@ module Gitlab
         # value - The value to add to the set.
         # timeout - The new timeout of the key.
         def self.set_add(raw_key, value, timeout: TIMEOUT)
+          validate_redis_value!(value)
+
           key = cache_key_for(raw_key)
 
           Redis::Cache.with do |redis|
@@ -128,6 +134,8 @@ module Gitlab
         # raw_key - The key of the set to check.
         # value - The value to check for.
         def self.set_includes?(raw_key, value)
+          validate_redis_value!(value)
+
           key = cache_key_for(raw_key)
 
           Redis::Cache.with do |redis|
@@ -157,6 +165,8 @@ module Gitlab
               mapping.each do |raw_key, value|
                 key = cache_key_for("#{key_prefix}#{raw_key}")
 
+                validate_redis_value!(value)
+
                 multi.set(key, value, ex: timeout)
               end
             end
@@ -186,6 +196,8 @@ module Gitlab
         #
         # Returns true when the key was overwritten, false otherwise.
         def self.write_if_greater(raw_key, value, timeout: TIMEOUT)
+          validate_redis_value!(value)
+
           key = cache_key_for(raw_key)
           val = Redis::Cache.with do |redis|
             redis
@@ -202,6 +214,8 @@ module Gitlab
         # value - The field value to add to the hash.
         # timeout - The new timeout of the key.
         def self.hash_add(raw_key, field, value, timeout: TIMEOUT)
+          validate_redis_value!(value)
+
           key = cache_key_for(raw_key)
 
           Redis::Cache.with do |redis|
@@ -226,6 +240,13 @@ module Gitlab
         def self.cache_key_for(raw_key)
           "#{Redis::Cache::CACHE_NAMESPACE}:#{raw_key}"
         end
+
+        def self.validate_redis_value!(value)
+          value_as_string = value.to_s
+          return if value_as_string.is_a?(String)
+
+          raise "Value '#{value_as_string}' of type '#{value_as_string.class}' for '#{value.inspect}' is not a String"
+        end
       end
     end
   end
diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb
index f6f9c7f0d3c..c7794b6c8b7 100644
--- a/spec/features/projects_spec.rb
+++ b/spec/features/projects_spec.rb
@@ -329,7 +329,7 @@ RSpec.describe 'Project' do
     it 'has working links to submodules' do
       click_link('645f6c4c')
 
-      expect(page).to have_selector('.qa-branches-select', text: '645f6c4c82fd3f5e06f67134450a570b795e55a6') # rubocop:disable QA/SelectorUsage
+      expect(page).to have_selector('[data-testid="branches-select"]', text: '645f6c4c82fd3f5e06f67134450a570b795e55a6')
     end
 
     context 'for signed commit on default branch', :js do
diff --git a/spec/lib/gitlab/cache/import/caching_spec.rb b/spec/lib/gitlab/cache/import/caching_spec.rb
index 946a7c604a1..6f9879da281 100644
--- a/spec/lib/gitlab/cache/import/caching_spec.rb
+++ b/spec/lib/gitlab/cache/import/caching_spec.rb
@@ -3,6 +3,17 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Cache::Import::Caching, :clean_gitlab_redis_cache do
+  shared_examples 'validated redis value' do
+    let(:value) { double('value', to_s: Object.new) }
+
+    it 'raise error if value.to_s does not return a String' do
+      value_as_string = value.to_s
+      message = /Value '#{value_as_string}' of type '#{value_as_string.class}' for '#{value.inspect}' is not a String/
+
+      expect { subject }.to raise_error(message)
+    end
+  end
+
   describe '.read' do
     it 'reads a value from the cache' do
       described_class.write('foo', 'bar')
@@ -56,6 +67,16 @@ RSpec.describe Gitlab::Cache::Import::Caching, :clean_gitlab_redis_cache do
       expect(described_class.write('foo', 10)).to eq(10)
       expect(described_class.read('foo')).to eq('10')
     end
+
+    it_behaves_like 'validated redis value' do
+      subject { described_class.write('foo', value) }
+    end
+  end
+
+  describe '.increment_by' do
+    it_behaves_like 'validated redis value' do
+      subject { described_class.increment_by('foo', value) }
+    end
   end
 
   describe '.increment' do
@@ -78,6 +99,10 @@ RSpec.describe Gitlab::Cache::Import::Caching, :clean_gitlab_redis_cache do
 
       expect(values).to eq(['10'])
     end
+
+    it_behaves_like 'validated redis value' do
+      subject { described_class.set_add('foo', value) }
+    end
   end
 
   describe '.set_includes?' do
@@ -96,6 +121,10 @@ RSpec.describe Gitlab::Cache::Import::Caching, :clean_gitlab_redis_cache do
 
       expect(described_class.set_includes?('foo', 10)).to eq(true)
     end
+
+    it_behaves_like 'validated redis value' do
+      subject { described_class.set_includes?('foo', value) }
+    end
   end
 
   describe '.values_from_set' do
@@ -120,6 +149,10 @@ RSpec.describe Gitlab::Cache::Import::Caching, :clean_gitlab_redis_cache do
 
       expect(values).to eq({ '1' => '1', '2' => '2' })
     end
+
+    it_behaves_like 'validated redis value' do
+      subject { described_class.hash_add('foo', 1, value) }
+    end
   end
 
   describe '.values_from_hash' do
@@ -160,6 +193,12 @@ RSpec.describe Gitlab::Cache::Import::Caching, :clean_gitlab_redis_cache do
         expect(found).to eq(value.to_s)
       end
     end
+
+    it_behaves_like 'validated redis value' do
+      let(:mapping) { { 'foo' => value, 'bar' => value } }
+
+      subject { described_class.write_multiple(mapping) }
+    end
   end
 
   describe '.expire' do
@@ -175,4 +214,10 @@ RSpec.describe Gitlab::Cache::Import::Caching, :clean_gitlab_redis_cache do
       expect(found_ttl).to be <= timeout
     end
   end
+
+  describe '.write_if_greater' do
+    it_behaves_like 'validated redis value' do
+      subject { described_class.write_if_greater('foo', value) }
+    end
+  end
 end