Add suffix to cache name to add isolation

Merge branch 'security-pedropombeiro/330047/use-protected-suffix-for-cache-name-2-14-10' into '14-10-stable-ee'

See merge request gitlab-org/security/gitlab!2426

Changelog: security

app/models/ci/build.rb

@@ -911,7 +911,10 @@ module Ci
         end
       end
 
-      cache
+      type_suffix = pipeline.protected_ref? ? 'protected' : 'non_protected'
+      cache.map do |entry|
+        entry.merge(key: "#{entry[:key]}-#{type_suffix}")
+      end
     end
 
     def credentials

doc/ci/caching/index.md

@@ -31,6 +31,7 @@ can't link to files outside it.
 - Subsequent pipelines can use the cache.
 - Subsequent jobs in the same pipeline can use the cache, if the dependencies are identical.
 - Different projects cannot share the cache.
+- Protected and non-protected branches do not share the cache.
 
 ### Artifacts
 
@@ -446,6 +447,22 @@ is stored on the machine where GitLab Runner is installed. The location also dep
 If you use cache and artifacts to store the same path in your jobs, the cache might
 be overwritten because caches are restored before artifacts.
 
+### Segregation of caches between protected and non-protected branches
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/330047) in GitLab 15.0.
+
+A suffix is added to the cache key, with the exception of the [fallback cache key](#use-a-fallback-cache-key).
+This is done in order to prevent cache poisoning that might occur through manipulation of the cache in a non-protected
+branch. Any subsequent protected-branch jobs would then potentially use a poisoned cache from the preceding job.
+
+As an example, assuming that `cache.key` is set to `$CI_COMMIT_REF_SLUG`, and that we have two branches `main`
+and `feature`, then the following table represents the resulting cache keys:
+
+| Branch name | Cache key |
+|-------------|-----------|
+| `main`      | `main-protected` |
+| `feature`   | `feature-non_protected` |
+
 ### How archiving and extracting works
 
 This example shows two jobs in two consecutive stages:

spec/models/ci/build_spec.rb

@@ -1048,7 +1048,27 @@ RSpec.describe Ci::Build do
           allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(1)
         end
 
-        it { is_expected.to match([a_hash_including(key: "key-1"), a_hash_including(key: "key2-1")]) }
+        it { is_expected.to match([a_hash_including(key: 'key-1-non_protected'), a_hash_including(key: 'key2-1-non_protected')]) }
+
+        context 'when pipeline is on a protected ref' do
+          before do
+            allow(build.pipeline).to receive(:protected_ref?).and_return(true)
+          end
+
+          it do
+            is_expected.to all(a_hash_including(key: a_string_matching(/-protected$/)))
+          end
+        end
+
+        context 'when pipeline is not on a protected ref' do
+          before do
+            allow(build.pipeline).to receive(:protected_ref?).and_return(false)
+          end
+
+          it do
+            is_expected.to all(a_hash_including(key: a_string_matching(/-non_protected$/)))
+          end
+        end
       end
 
       context 'when project has jobs_cache_index' do
@@ -1056,7 +1076,7 @@ RSpec.describe Ci::Build do
           allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(1)
         end
 
-        it { is_expected.to be_an(Array).and all(include(key: "key-1")) }
+        it { is_expected.to be_an(Array).and all(include(key: a_string_matching(/^key-1-(?>protected|non_protected)/))) }
       end
 
       context 'when project does not have jobs_cache_index' do
@@ -1064,7 +1084,9 @@ RSpec.describe Ci::Build do
           allow_any_instance_of(Project).to receive(:jobs_cache_index).and_return(nil)
         end
 
-        it { is_expected.to eq(options[:cache]) }
+        it do
+          is_expected.to eq(options[:cache].map { |entry| entry.merge(key: "#{entry[:key]}-non_protected") })
+        end
       end
     end
 

spec/requests/api/ci/runner/jobs_request_post_spec.rb

@@ -191,7 +191,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
           end
 
           let(:expected_cache) do
-            [{ 'key' => 'cache_key',
+            [{ 'key' => a_string_matching(/^cache_key-(?>protected|non_protected)$/),
                'untracked' => false,
                'paths' => ['vendor/*'],
                'policy' => 'pull-push',
@@ -225,7 +225,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
                                                        'alias' => nil, 'command' => nil, 'ports' => [], 'variables' => [{ 'key' => 'MYSQL_ROOT_PASSWORD', 'value' => 'root123.' }] }])
             expect(json_response['steps']).to eq(expected_steps)
             expect(json_response['artifacts']).to eq(expected_artifacts)
-            expect(json_response['cache']).to eq(expected_cache)
+            expect(json_response['cache']).to match(expected_cache)
             expect(json_response['variables']).to include(*expected_variables)
             expect(json_response['features']).to match(expected_features)
           end

spec/services/ci/create_pipeline_service/cache_spec.rb

@@ -33,7 +33,7 @@ RSpec.describe Ci::CreatePipelineService do
 
       it 'uses the provided key' do
         expected = {
-          key: 'a-key',
+          key: a_string_matching(/^a-key-(?>protected|non_protected)$/),
           paths: ['logs/', 'binaries/'],
           policy: 'pull-push',
           untracked: true,