From a1ba12d1deceabb3de951c800838efab4f2ccc76 Mon Sep 17 00:00:00 2001
From: Dmytro Biryukov <dbiryukov@gitlab.com>
Date: Wed, 15 Nov 2023 09:52:21 +0000
Subject: [PATCH] Exclude protected variables from multi-project pipeline
bridge triggers
---
app/models/ci/bridge.rb | 27 ++-
app/models/concerns/ci/contextable.rb | 19 ++
...s_from_multi_project_pipeline_triggers.yml | 8 +
lib/gitlab/ci/variables/builder.rb | 31 ++-
spec/models/ci/bridge_spec.rb | 197 ++++++++++++++++++
5 files changed, 275 insertions(+), 7 deletions(-)
create mode 100644 config/feature_flags/development/exclude_protected_variables_from_multi_project_pipeline_triggers.yml
diff --git a/app/models/ci/bridge.rb b/app/models/ci/bridge.rb
index cf6401dc1daaa..89f8520b33122 100644
--- a/app/models/ci/bridge.rb
+++ b/app/models/ci/bridge.rb
@@ -220,8 +220,19 @@ def downstream_variables
def variables
strong_memoize(:variables) do
+ bridge_variables =
+ if ::Feature.disabled?(:exclude_protected_variables_from_multi_project_pipeline_triggers, project) ||
+ (expose_protected_project_variables? && expose_protected_group_variables?)
+ scoped_variables
+ else
+ unprotected_scoped_variables(
+ expose_project_variables: expose_protected_project_variables?,
+ expose_group_variables: expose_protected_group_variables?
+ )
+ end
+
Gitlab::Ci::Variables::Collection.new
- .concat(scoped_variables)
+ .concat(bridge_variables)
.concat(pipeline.persisted_variables)
end
end
@@ -260,6 +271,20 @@ def expand_file_refs?
private
+ def expose_protected_group_variables?
+ return true if downstream_project.nil?
+ return true if project.group.present? && project.group == downstream_project.group
+
+ false
+ end
+
+ def expose_protected_project_variables?
+ return true if downstream_project.nil?
+ return true if project.id == downstream_project.id
+
+ false
+ end
+
def cross_project_params
{
project: downstream_project,
diff --git a/app/models/concerns/ci/contextable.rb b/app/models/concerns/ci/contextable.rb
index 88b7bb89b899e..dcbee5296376c 100644
--- a/app/models/concerns/ci/contextable.rb
+++ b/app/models/concerns/ci/contextable.rb
@@ -17,6 +17,25 @@ def scoped_variables(environment: expanded_environment_name, dependencies: true)
end
end
+ def unprotected_scoped_variables(
+ expose_project_variables:,
+ expose_group_variables:,
+ environment: expanded_environment_name,
+ dependencies: true)
+
+ track_duration do
+ pipeline
+ .variables_builder
+ .unprotected_scoped_variables(
+ self,
+ expose_project_variables: expose_project_variables,
+ expose_group_variables: expose_group_variables,
+ environment: environment,
+ dependencies: dependencies
+ )
+ end
+ end
+
def track_duration
start_time = ::Gitlab::Metrics::System.monotonic_time
result = yield
diff --git a/config/feature_flags/development/exclude_protected_variables_from_multi_project_pipeline_triggers.yml b/config/feature_flags/development/exclude_protected_variables_from_multi_project_pipeline_triggers.yml
new file mode 100644
index 0000000000000..57339a95f6b0d
--- /dev/null
+++ b/config/feature_flags/development/exclude_protected_variables_from_multi_project_pipeline_triggers.yml
@@ -0,0 +1,8 @@
+---
+name: exclude_protected_variables_from_multi_project_pipeline_triggers
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/135899
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/431266
+milestone: '16.6'
+type: development
+group: group::pipeline security
+default_enabled: true
\ No newline at end of file
diff --git a/lib/gitlab/ci/variables/builder.rb b/lib/gitlab/ci/variables/builder.rb
index c279af6acfc51..d0bd26e995b2c 100644
--- a/lib/gitlab/ci/variables/builder.rb
+++ b/lib/gitlab/ci/variables/builder.rb
@@ -34,6 +34,25 @@ def scoped_variables(job, environment:, dependencies:)
end
end
+ def unprotected_scoped_variables(job, expose_project_variables:, expose_group_variables:, environment:, dependencies:)
+ Gitlab::Ci::Variables::Collection.new.tap do |variables|
+ variables.concat(predefined_variables(job, environment))
+ variables.concat(project.predefined_variables)
+ variables.concat(pipeline_variables_builder.predefined_variables)
+ variables.concat(job.runner.predefined_variables) if job.runnable? && job.runner
+ variables.concat(kubernetes_variables(environment: environment, job: job))
+ variables.concat(job.yaml_variables)
+ variables.concat(user_variables(job.user))
+ variables.concat(job.dependency_variables) if dependencies
+ variables.concat(secret_instance_variables)
+ variables.concat(secret_group_variables(environment: environment, include_protected_vars: expose_group_variables))
+ variables.concat(secret_project_variables(environment: environment, include_protected_vars: expose_project_variables))
+ variables.concat(pipeline.variables)
+ variables.concat(pipeline_schedule_variables)
+ variables.concat(release_variables)
+ end
+ end
+
def config_variables
Gitlab::Ci::Variables::Collection.new.tap do |variables|
break variables unless project
@@ -91,21 +110,21 @@ def secret_instance_variables
end
end
- def secret_group_variables(environment:)
- strong_memoize_with(:secret_group_variables, environment) do
+ def secret_group_variables(environment:, include_protected_vars: protected_ref?)
+ strong_memoize_with(:secret_group_variables, environment, include_protected_vars) do
group_variables_builder
.secret_variables(
environment: environment,
- protected_ref: protected_ref?)
+ protected_ref: include_protected_vars)
end
end
- def secret_project_variables(environment:)
- strong_memoize_with(:secret_project_variables, environment) do
+ def secret_project_variables(environment:, include_protected_vars: protected_ref?)
+ strong_memoize_with(:secret_project_variables, environment, include_protected_vars) do
project_variables_builder
.secret_variables(
environment: environment,
- protected_ref: protected_ref?)
+ protected_ref: include_protected_vars)
end
end
diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb
index 1d0c3bb5deeca..775695b406c9a 100644
--- a/spec/models/ci/bridge_spec.rb
+++ b/spec/models/ci/bridge_spec.rb
@@ -595,6 +595,203 @@
end
end
+ describe 'variables expansion' do
+ let(:options) do
+ {
+ trigger: {
+ project: 'my/project',
+ branch: 'master',
+ forward: { yaml_variables: true,
+ pipeline_variables: true }.compact
+ }
+ }
+ end
+
+ let(:yaml_variables) do
+ [
+ {
+ key: 'EXPANDED_PROJECT_VAR6',
+ value: 'project value6 $PROJECT_PROTECTED_VAR'
+ },
+ {
+ key: 'EXPANDED_GROUP_VAR6',
+ value: 'group value6 $GROUP_PROTECTED_VAR'
+ },
+
+ {
+ key: 'VAR7',
+ value: 'value7 $VAR1',
+ raw: true
+ }
+ ]
+ end
+
+ let_it_be(:downstream_creator_user) { create(:user) }
+ let_it_be(:bridge_creator_user) { create(:user) }
+
+ let_it_be(:bridge_group) { create(:group) }
+ let_it_be(:downstream_group) { create(:group) }
+ let_it_be(:downstream_project) { create(:project, creator: downstream_creator_user, group: downstream_group) }
+ let_it_be(:project) { create(:project, :repository, :in_group, creator: bridge_creator_user, group: bridge_group) }
+ let(:bridge) { build(:ci_bridge, :playable, pipeline: pipeline, downstream: downstream_project) }
+ let!(:pipeline) { create(:ci_pipeline, project: project) }
+
+ let!(:ci_variable) do
+ create(:ci_variable,
+ project: project,
+ key: 'PROJECT_PROTECTED_VAR',
+ value: 'this is a secret',
+ protected: is_variable_protected?)
+ end
+
+ let!(:ci_group_variable) do
+ create(:ci_group_variable,
+ group: bridge_group,
+ key: 'GROUP_PROTECTED_VAR',
+ value: 'this is a secret',
+ protected: is_variable_protected?)
+ end
+
+ before do
+ bridge.yaml_variables = yaml_variables
+ allow(bridge.project).to receive(:protected_for?).and_return(true)
+ end
+
+ shared_examples 'expands variables from a project downstream' do
+ it do
+ vars = bridge.downstream_variables
+ expect(vars).to include({ key: 'EXPANDED_PROJECT_VAR6', value: 'project value6 this is a secret' })
+ end
+ end
+
+ shared_examples 'expands variables from a group downstream' do
+ it do
+ vars = bridge.downstream_variables
+ expect(vars).to include({ key: 'EXPANDED_GROUP_VAR6', value: 'group value6 this is a secret' })
+ end
+ end
+
+ shared_examples 'expands project and group variables downstream' do
+ it_behaves_like 'expands variables from a project downstream'
+
+ it_behaves_like 'expands variables from a group downstream'
+ end
+
+ shared_examples 'does not expand variables from a project downstream' do
+ it do
+ vars = bridge.downstream_variables
+ expect(vars).not_to include({ key: 'EXPANDED_PROJECT_VAR6', value: 'project value6 this is a secret' })
+ end
+ end
+
+ shared_examples 'does not expand variables from a group downstream' do
+ it do
+ vars = bridge.downstream_variables
+ expect(vars).not_to include({ key: 'EXPANDED_GROUP_VAR6', value: 'group value6 this is a secret' })
+ end
+ end
+
+ shared_examples 'feature flag is disabled' do
+ before do
+ stub_feature_flags(exclude_protected_variables_from_multi_project_pipeline_triggers: false)
+ end
+
+ it_behaves_like 'expands project and group variables downstream'
+ end
+
+ shared_examples 'does not expand project and group variables downstream' do
+ it_behaves_like 'does not expand variables from a project downstream'
+
+ it_behaves_like 'does not expand variables from a group downstream'
+ end
+
+ context 'when they are protected' do
+ let!(:is_variable_protected?) { true }
+
+ context 'and downstream project group is different from bridge group' do
+ it_behaves_like 'does not expand project and group variables downstream'
+
+ it_behaves_like 'feature flag is disabled'
+ end
+
+ context 'and there is no downstream project' do
+ let(:downstream_project) { nil }
+
+ it_behaves_like 'expands project and group variables downstream'
+
+ it_behaves_like 'feature flag is disabled'
+ end
+
+ context 'and downstream project equals bridge project' do
+ let(:downstream_project) { project }
+
+ it_behaves_like 'expands project and group variables downstream'
+
+ it_behaves_like 'feature flag is disabled'
+ end
+
+ context 'and downstream project group is equal to bridge project group' do
+ let_it_be(:downstream_project) { create(:project, creator: downstream_creator_user, group: bridge_group) }
+
+ it_behaves_like 'expands variables from a group downstream'
+
+ it_behaves_like 'does not expand variables from a project downstream'
+
+ it_behaves_like 'feature flag is disabled'
+ end
+
+ context 'and downstream project has no group' do
+ let_it_be(:downstream_project) { create(:project, creator: downstream_creator_user) }
+
+ it_behaves_like 'does not expand project and group variables downstream'
+
+ it_behaves_like 'feature flag is disabled'
+ end
+ end
+
+ context 'when they are not protected' do
+ let!(:is_variable_protected?) { false }
+
+ context 'and downstream project group is different from bridge group' do
+ it_behaves_like 'expands project and group variables downstream'
+
+ it_behaves_like 'feature flag is disabled'
+ end
+
+ context 'and there is no downstream project' do
+ let(:downstream_project) { nil }
+
+ it_behaves_like 'expands project and group variables downstream'
+
+ it_behaves_like 'feature flag is disabled'
+ end
+
+ context 'and downstream project equals bridge project' do
+ let(:downstream_project) { project }
+
+ it_behaves_like 'expands project and group variables downstream'
+
+ it_behaves_like 'feature flag is disabled'
+ end
+
+ context 'and downstream project group is equal to bridge project group' do
+ let_it_be(:downstream_project) { create(:project, creator: downstream_creator_user, group: bridge_group) }
+
+ it_behaves_like 'expands project and group variables downstream'
+
+ it_behaves_like 'feature flag is disabled'
+ end
+
+ context 'and downstream project has no group' do
+ let_it_be(:downstream_project) { create(:project, creator: downstream_creator_user) }
+
+ it_behaves_like 'expands project and group variables downstream'
+
+ it_behaves_like 'feature flag is disabled'
+ end
+ end
+ end
+
describe '#forward_pipeline_variables?' do
using RSpec::Parameterized::TableSyntax
--
GitLab