Add latest changes from gitlab-org/security/gitlab@14-10-stable-ee

5 files changed, 49 insertions, 7 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 16c9aa212d0..a8ad55fd5a4 100644
--- a/app/models/ci/build.rb
+++ b/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
diff --git a/doc/ci/caching/index.md b/doc/ci/caching/index.md
index c634491662d..777bbf6053f 100644
--- a/doc/ci/caching/index.md
+++ b/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:
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index fd87a388442..12e65974270 100644
--- a/spec/models/ci/build_spec.rb
+++ b/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
 
diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb
index 9e6bac41d59..a662c77e5a2 100644
--- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb
+++ b/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
diff --git a/spec/services/ci/create_pipeline_service/cache_spec.rb b/spec/services/ci/create_pipeline_service/cache_spec.rb
index ca85a8cce64..fe777bc50d9 100644
--- a/spec/services/ci/create_pipeline_service/cache_spec.rb
+++ b/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,