diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-04-29 14:53:03 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-04-29 14:53:03 +0000 |
commit | ae079c98fea47e8101415cf7c29679bda33c0d45 (patch) | |
tree | ff78059a1b5bbbcf4b2c1e9c4a49e1e9ccb08c26 | |
parent | d65459a8217dfbc248b058d7b72dbcdc74305141 (diff) | |
download | gitlab-ce-ae079c98fea47e8101415cf7c29679bda33c0d45.tar.gz |
Add latest changes from gitlab-org/security/gitlab@14-10-stable-ee
-rw-r--r-- | app/models/ci/build.rb | 5 | ||||
-rw-r--r-- | doc/ci/caching/index.md | 17 | ||||
-rw-r--r-- | spec/models/ci/build_spec.rb | 28 | ||||
-rw-r--r-- | spec/requests/api/ci/runner/jobs_request_post_spec.rb | 4 | ||||
-rw-r--r-- | spec/services/ci/create_pipeline_service/cache_spec.rb | 2 |
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, |