Add suffix to cache name to add isolation

Merge branch 'pedropombeiro/330047/use-protected-suffix-for-cache-name' into 'master'

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

Changelog: security

app/models/ci/build.rb

@@ -907,7 +907,10 @@ def cache
         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 14.10.
+
+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 @@
           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 @@
           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 @@
           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 @@
           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 @@
                                                        '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 @@
       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,