Merge branch 'ci-setting-prevent-user-defined-ci-variables' into 'master'

Prevent user-defined variables from being used by non-maintainers

See merge request gitlab-org/gitlab!51682

app/models/ci/bridge.rb

@@ -210,7 +210,8 @@ module Ci
         project: downstream_project,
         source: :pipeline,
         target_revision: {
-          ref: target_ref || downstream_project.default_branch
+          ref: target_ref || downstream_project.default_branch,
+          variables_attributes: downstream_variables
         },
         execute_params: {
           ignore_skip_ci: true,
@@ -230,7 +231,8 @@ module Ci
           checkout_sha: parent_pipeline.sha,
           before: parent_pipeline.before_sha,
           source_sha: parent_pipeline.source_sha,
-          target_sha: parent_pipeline.target_sha
+          target_sha: parent_pipeline.target_sha,
+          variables_attributes: downstream_variables
         },
         execute_params: {
           ignore_skip_ci: true,

app/models/project.rb

@@ -412,6 +412,8 @@ class Project < ApplicationRecord
   delegate :default_git_depth, :default_git_depth=, to: :ci_cd_settings, prefix: :ci
   delegate :forward_deployment_enabled, :forward_deployment_enabled=, :forward_deployment_enabled?, to: :ci_cd_settings, prefix: :ci
   delegate :keep_latest_artifact, :keep_latest_artifact=, :keep_latest_artifact?, to: :ci_cd_settings, prefix: :ci
+  delegate :restrict_user_defined_variables, :restrict_user_defined_variables=, :restrict_user_defined_variables?,
+    to: :ci_cd_settings
   delegate :actual_limits, :actual_plan_name, to: :namespace, allow_nil: true
   delegate :allow_merge_on_skipped_pipeline, :allow_merge_on_skipped_pipeline?,
     :allow_merge_on_skipped_pipeline=, :has_confluence?, :allow_editing_commit_messages?,

app/policies/project_policy.rb

@@ -135,6 +135,10 @@ class ProjectPolicy < BasePolicy
     ::Feature.enabled?(:build_service_proxy, @subject)
   end
 
+  condition(:user_defined_variables_allowed) do
+    !@subject.restrict_user_defined_variables?
+  end
+
   with_scope :subject
   condition(:packages_disabled) { !@subject.packages_enabled }
 
@@ -616,6 +620,10 @@ class ProjectPolicy < BasePolicy
     enable :admin_resource_access_tokens
   end
 
+  rule { user_defined_variables_allowed | can?(:maintainer_access) }.policy do
+    enable :set_pipeline_variables
+  end
+
   private
 
   def user_is_user?

app/services/ci/create_downstream_pipeline_service.rb

@@ -33,9 +33,7 @@ module Ci
         pipeline_params.fetch(:target_revision))
 
       downstream_pipeline = service.execute(
-        pipeline_params.fetch(:source), **pipeline_params[:execute_params]) do |pipeline|
-          pipeline.variables.build(@bridge.downstream_variables)
-        end
+        pipeline_params.fetch(:source), **pipeline_params[:execute_params])
 
       downstream_pipeline.tap do |pipeline|
         update_bridge_status!(@bridge, pipeline)

app/services/ci/pipeline_trigger_service.rb

@@ -21,10 +21,10 @@ module Ci
       # this check is to not leak the presence of the project if user cannot read it
       return unless trigger.project == project
 
-      pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: params[:ref])
+      pipeline = Ci::CreatePipelineService
+        .new(project, trigger.owner, ref: params[:ref], variables_attributes: variables)
         .execute(:trigger, ignore_skip_ci: true) do |pipeline|
           pipeline.trigger_requests.build(trigger: trigger)
-          pipeline.variables.build(variables)
         end
 
       if pipeline.persisted?
@@ -44,7 +44,8 @@ module Ci
       # this check is to not leak the presence of the project if user cannot read it
       return unless can?(job.user, :read_project, project)
 
-      pipeline = Ci::CreatePipelineService.new(project, job.user, ref: params[:ref])
+      pipeline = Ci::CreatePipelineService
+        .new(project, job.user, ref: params[:ref], variables_attributes: variables)
         .execute(:pipeline, ignore_skip_ci: true) do |pipeline|
           source = job.sourced_pipelines.build(
             source_pipeline: job.pipeline,
@@ -53,7 +54,6 @@ module Ci
             project: project)
 
           pipeline.source_pipeline = source
-          pipeline.variables.build(variables)
         end
 
       if pipeline.persisted?

app/services/ci/play_build_service.rb

@@ -5,6 +5,10 @@ module Ci
     def execute(build, job_variables_attributes = nil)
       raise Gitlab::Access::AccessDeniedError unless can?(current_user, :play_job, build)
 
+      if job_variables_attributes.present? && !can?(current_user, :set_pipeline_variables, project)
+        raise Gitlab::Access::AccessDeniedError
+      end
+
       # Try to enqueue the build, otherwise create a duplicate.
       #
       if build.enqueue

changelogs/unreleased/ci-setting-prevent-user-defined-ci-variables.yml

+---
+title: Prevent user-defined variables from being used by non-maintainers
+merge_request: 51682
+author:
+type: security

db/migrate/20201223114050_add_restrict_user_defined_variables_to_project_settings.rb

+# frozen_string_literal: true
+
+class AddRestrictUserDefinedVariablesToProjectSettings < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_column :project_ci_cd_settings, :restrict_user_defined_variables, :boolean, default: false, null: false
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_column :project_ci_cd_settings, :restrict_user_defined_variables
+    end
+  end
+end

db/schema_migrations/20201223114050

+35acb5bbabfd12f97c988776aafa6ff380e2cbe2267e856b8f439f7102a6fbf2
\ No newline at end of file

db/structure.sql

@@ -15572,7 +15572,8 @@ CREATE TABLE project_ci_cd_settings (
     forward_deployment_enabled boolean,
     merge_trains_enabled boolean DEFAULT false,
     auto_rollback_enabled boolean DEFAULT false NOT NULL,
-    keep_latest_artifact boolean DEFAULT true NOT NULL
+    keep_latest_artifact boolean DEFAULT true NOT NULL,
+    restrict_user_defined_variables boolean DEFAULT false NOT NULL
 );
 
 CREATE SEQUENCE project_ci_cd_settings_id_seq

doc/api/projects.md

@@ -160,6 +160,7 @@ When the user is authenticated and `simple` is not set this returns something li
     "shared_with_groups": [],
     "only_allow_merge_if_pipeline_succeeds": false,
     "allow_merge_on_skipped_pipeline": false,
+    "restrict_user_defined_variables": false,
     "only_allow_merge_if_all_discussions_are_resolved": false,
     "remove_source_branch_after_merge": false,
     "request_access_enabled": false,
@@ -253,6 +254,7 @@ When the user is authenticated and `simple` is not set this returns something li
     "shared_with_groups": [],
     "only_allow_merge_if_pipeline_succeeds": false,
     "allow_merge_on_skipped_pipeline": false,
+    "restrict_user_defined_variables": false,
     "only_allow_merge_if_all_discussions_are_resolved": false,
     "remove_source_branch_after_merge": false,
     "request_access_enabled": false,
@@ -418,6 +420,7 @@ GET /users/:user_id/projects
     "shared_with_groups": [],
     "only_allow_merge_if_pipeline_succeeds": false,
     "allow_merge_on_skipped_pipeline": false,
+    "restrict_user_defined_variables": false,
     "only_allow_merge_if_all_discussions_are_resolved": false,
     "remove_source_branch_after_merge": false,
     "request_access_enabled": false,
@@ -511,6 +514,7 @@ GET /users/:user_id/projects
     "shared_with_groups": [],
     "only_allow_merge_if_pipeline_succeeds": false,
     "allow_merge_on_skipped_pipeline": false,
+    "restrict_user_defined_variables": false,
     "only_allow_merge_if_all_discussions_are_resolved": false,
     "remove_source_branch_after_merge": false,
     "request_access_enabled": false,
@@ -640,6 +644,7 @@ Example response:
     "shared_with_groups": [],
     "only_allow_merge_if_pipeline_succeeds": false,
     "allow_merge_on_skipped_pipeline": false,
+    "restrict_user_defined_variables": false,
     "only_allow_merge_if_all_discussions_are_resolved": false,
     "remove_source_branch_after_merge": false,
     "request_access_enabled": false,
@@ -726,6 +731,7 @@ Example response:
     "shared_with_groups": [],
     "only_allow_merge_if_pipeline_succeeds": false,
     "allow_merge_on_skipped_pipeline": false,
+    "restrict_user_defined_variables": false,
     "only_allow_merge_if_all_discussions_are_resolved": false,
     "remove_source_branch_after_merge": false,
     "request_access_enabled": false,
@@ -881,6 +887,7 @@ GET /projects/:id
   "repository_storage": "default",
   "only_allow_merge_if_pipeline_succeeds": false,
   "allow_merge_on_skipped_pipeline": false,
+  "restrict_user_defined_variables": false,
   "only_allow_merge_if_all_discussions_are_resolved": false,
   "remove_source_branch_after_merge": false,
   "printing_merge_requests_link_enabled": true,
@@ -1234,6 +1241,7 @@ PUT /projects/:id
 | `packages_enabled`                                          | boolean        | **{dotted-circle}** No | Enable or disable packages repository feature. |
 | `pages_access_level`                                        | string         | **{dotted-circle}** No | One of `disabled`, `private`, `enabled`, or `public`. |
 | `requirements_access_level`                                 | string         | **{dotted-circle}** No | One of `disabled`, `private`, `enabled` or `public` |
+| `restrict_user_defined_variables`                           | boolean        | **{dotted-circle}** No | Allow only maintainers to pass user-defined variables when triggering a pipeline. For example when the pipeline is triggered in the UI, with the API, or by a trigger token. |
 | `path`                                                      | string         | **{dotted-circle}** No | Custom repository name for the project. By default generated based on name. |
 | `public_builds`                                             | boolean        | **{dotted-circle}** No | If `true`, jobs can be viewed by non-project members. |
 | `remove_source_branch_after_merge`                          | boolean        | **{dotted-circle}** No | Enable `Delete source branch` option by default for all new merge requests. |
@@ -1356,6 +1364,7 @@ Example responses:
     "shared_with_groups": [],
     "only_allow_merge_if_pipeline_succeeds": false,
     "allow_merge_on_skipped_pipeline": false,
+    "restrict_user_defined_variables": false,
     "only_allow_merge_if_all_discussions_are_resolved": false,
     "remove_source_branch_after_merge": false,
     "request_access_enabled": false,
@@ -1449,6 +1458,7 @@ Example response:
   "shared_with_groups": [],
   "only_allow_merge_if_pipeline_succeeds": false,
   "allow_merge_on_skipped_pipeline": false,
+  "restrict_user_defined_variables": false,
   "only_allow_merge_if_all_discussions_are_resolved": false,
   "remove_source_branch_after_merge": false,
   "request_access_enabled": false,
@@ -1540,6 +1550,7 @@ Example response:
   "shared_with_groups": [],
   "only_allow_merge_if_pipeline_succeeds": false,
   "allow_merge_on_skipped_pipeline": false,
+  "restrict_user_defined_variables": false,
   "only_allow_merge_if_all_discussions_are_resolved": false,
   "remove_source_branch_after_merge": false,
   "request_access_enabled": false,
@@ -1725,6 +1736,7 @@ Example response:
   "shared_with_groups": [],
   "only_allow_merge_if_pipeline_succeeds": false,
   "allow_merge_on_skipped_pipeline": false,
+  "restrict_user_defined_variables": false,
   "only_allow_merge_if_all_discussions_are_resolved": false,
   "remove_source_branch_after_merge": false,
   "request_access_enabled": false,
@@ -1837,6 +1849,7 @@ Example response:
   "shared_with_groups": [],
   "only_allow_merge_if_pipeline_succeeds": false,
   "allow_merge_on_skipped_pipeline": false,
+  "restrict_user_defined_variables": false,
   "only_allow_merge_if_all_discussions_are_resolved": false,
   "remove_source_branch_after_merge": false,
   "request_access_enabled": false,
@@ -2397,6 +2410,7 @@ Example response:
   "shared_with_groups": [],
   "only_allow_merge_if_pipeline_succeeds": false,
   "allow_merge_on_skipped_pipeline": null,
+  "restrict_user_defined_variables": false,
   "request_access_enabled": true,
   "only_allow_merge_if_all_discussions_are_resolved": false,
   "remove_source_branch_after_merge": true,

doc/ci/variables/README.md

@@ -594,7 +594,35 @@ WARNING:
 Variables with multi-line values are not supported due to
 limitations with the Auto DevOps scripting environment.
 
-### Override a variable by manually running a pipeline
+### When you can override variables
+
+You can override the value of a variable when:
+
+1. [Manually running](#override-a-variable-by-manually-running-a-pipeline) pipelines in the UI.
+1. Manually creating pipelines [via API](../../api/pipelines.md#create-a-new-pipeline).
+1. Manually playing a job via the UI.
+1. Using [push options](../../user/project/push_options.md#push-options-for-gitlab-cicd).
+1. Manually triggering pipelines with [the API](../triggers/README.md#making-use-of-trigger-variables).
+1. Passing variables to a [downstream pipeline](../multi_project_pipelines.md#passing-variables-to-a-downstream-pipeline).
+
+These pipeline variables declared in these events take [priority over other variables](#priority-of-environment-variables).
+
+#### Restrict who can override variables
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/295234) in GitLab 13.8.
+
+To allow only users with Maintainer role to set these variables, you can use
+[the API](../../api/projects.md#edit-project) to enable the project setting `restrict_user_defined_variables`.
+When a user without Maintainer role tries to run a pipeline with overridden
+variables, an `Insufficient permissions to set pipeline variables` error occurs.
+
+The setting is `disabled` by default.
+
+If you [store your CI configurations in a different repository](../../ci/pipelines/settings.md#custom-ci-configuration-path),
+use this setting for strict control over all aspects of the environment
+the pipeline runs in.
+
+#### Override a variable by manually running a pipeline
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/44059) in GitLab 10.8.
 

lib/api/entities/project.rb

@@ -100,6 +100,7 @@ module API
       end
       expose :only_allow_merge_if_pipeline_succeeds
       expose :allow_merge_on_skipped_pipeline
+      expose :restrict_user_defined_variables
       expose :request_access_enabled
       expose :only_allow_merge_if_all_discussions_are_resolved
       expose :remove_source_branch_after_merge

lib/api/helpers/projects_helpers.rb

@@ -87,6 +87,7 @@ module API
 
       params :optional_update_params_ce do
         optional :ci_forward_deployment_enabled, type: Boolean, desc: 'Skip older deployment jobs that are still pending'
+        optional :restrict_user_defined_variables, type: Boolean, desc: 'Restrict use of user-defined variables when triggering a pipeline'
       end
 
       params :optional_update_params_ee do
@@ -141,6 +142,7 @@ module API
           :repository_access_level,
           :request_access_enabled,
           :resolve_outdated_diff_discussions,
+          :restrict_user_defined_variables,
           :shared_runners_enabled,
           :snippets_access_level,
           :tag_list,

lib/gitlab/ci/pipeline/chain/build.rb

@@ -5,6 +5,9 @@ module Gitlab
     module Pipeline
       module Chain
         class Build < Chain::Base
+          include Gitlab::Allowable
+          include Chain::Helpers
+
           def perform!
             @pipeline.assign_attributes(
               source: @command.source,
@@ -20,13 +23,34 @@ module Gitlab
               pipeline_schedule: @command.schedule,
               merge_request: @command.merge_request,
               external_pull_request: @command.external_pull_request,
-              variables_attributes: Array(@command.variables_attributes),
-              locked: @command.project.latest_pipeline_locked
+              locked: @command.project.latest_pipeline_locked,
+              variables_attributes: variables_attributes
             )
           end
 
           def break?
-            false
+            @pipeline.errors.any?
+          end
+
+          private
+
+          def variables_attributes
+            variables = Array(@command.variables_attributes)
+
+            # We allow parent pipelines to pass variables to child pipelines since
+            # these variables are coming from internal configurations. We will check
+            # permissions to :set_pipeline_variables when those are injected upstream,
+            # to the parent pipeline.
+            # In other scenarios (e.g. multi-project pipelines or run pipeline via UI)
+            # the variables are provided from the outside and those should be guarded.
+            return variables if @command.creates_child_pipeline?
+
+            if variables.present? && !can?(@command.current_user, :set_pipeline_variables, @command.project)
+              error("Insufficient permissions to set pipeline variables")
+              variables = []
+            end
+
+            variables
           end
         end
       end

lib/gitlab/ci/pipeline/chain/command.rb

@@ -79,6 +79,10 @@ module Gitlab
             bridge&.parent_pipeline
           end
 
+          def creates_child_pipeline?
+            bridge&.triggers_child_pipeline?
+          end
+
           def metrics
             @metrics ||= ::Gitlab::Ci::Pipeline::Metrics.new
           end

spec/factories/projects.rb

@@ -45,6 +45,7 @@ FactoryBot.define do
       import_correlation_id { nil }
       import_last_error { nil }
       forward_deployment_enabled { nil }
+      restrict_user_defined_variables { nil }
     end
 
     before(:create) do |project, evaluator|
@@ -84,6 +85,7 @@ FactoryBot.define do
       project.merge_pipelines_enabled = evaluator.merge_pipelines_enabled unless evaluator.merge_pipelines_enabled.nil?
       project.merge_trains_enabled = evaluator.merge_trains_enabled unless evaluator.merge_trains_enabled.nil?
       project.ci_keep_latest_artifact = evaluator.ci_keep_latest_artifact unless evaluator.ci_keep_latest_artifact.nil?
+      project.restrict_user_defined_variables = evaluator.restrict_user_defined_variables unless evaluator.restrict_user_defined_variables.nil?
 
       if evaluator.import_status
         import_state = project.import_state || project.build_import_state

spec/lib/gitlab/ci/pipeline/chain/build_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Ci::Pipeline::Chain::Build do
-  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:project, reload: true) { create(:project, :repository) }
   let_it_be(:user) { create(:user, developer_projects: [project]) }
   let(:pipeline) { Ci::Pipeline.new }
 
@@ -29,29 +29,96 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Build do
 
   let(:step) { described_class.new(pipeline, command) }
 
-  before do
-    stub_ci_pipeline_yaml_file(gitlab_ci_yaml)
+  shared_examples 'builds pipeline' do
+    it 'builds a pipeline with the expected attributes' do
+      step.perform!
+
+      expect(pipeline.sha).not_to be_empty
+      expect(pipeline.sha).to eq project.commit.id
+      expect(pipeline.ref).to eq 'master'
+      expect(pipeline.tag).to be false
+      expect(pipeline.user).to eq user
+      expect(pipeline.project).to eq project
+    end
   end
 
-  it 'never breaks the chain' do
-    step.perform!
+  shared_examples 'breaks the chain' do
+    it 'returns true' do
+      step.perform!
 
-    expect(step.break?).to be false
+      expect(step.break?).to be true
+    end
   end
 
-  it 'fills pipeline object with data' do
+  shared_examples 'does not break the chain' do
+    it 'returns false' do
+      step.perform!
+
+      expect(step.break?).to be false
+    end
+  end
+
+  before do
+    stub_ci_pipeline_yaml_file(gitlab_ci_yaml)
+  end
+
+  it_behaves_like 'does not break the chain'
+  it_behaves_like 'builds pipeline'
+
+  it 'sets pipeline variables' do
     step.perform!
 
-    expect(pipeline.sha).not_to be_empty
-    expect(pipeline.sha).to eq project.commit.id
-    expect(pipeline.ref).to eq 'master'
-    expect(pipeline.tag).to be false
-    expect(pipeline.user).to eq user
-    expect(pipeline.project).to eq project
     expect(pipeline.variables.map { |var| var.slice(:key, :secret_value) })
       .to eq variables_attributes.map(&:with_indifferent_access)
   end
 
+  context 'when project setting restrict_user_defined_variables is enabled' do
+    before do
+      project.update!(restrict_user_defined_variables: true)
+    end
+
+    context 'when user is developer' do
+      it_behaves_like 'breaks the chain'
+      it_behaves_like 'builds pipeline'
+
+      it 'returns an error on variables_attributes', :aggregate_failures do
+        step.perform!
+
+        expect(pipeline.errors.full_messages).to eq(['Insufficient permissions to set pipeline variables'])
+        expect(pipeline.variables).to be_empty
+      end
+
+      context 'when variables_attributes is not specified' do
+        let(:variables_attributes) { nil }
+
+        it_behaves_like 'does not break the chain'
+        it_behaves_like 'builds pipeline'
+
+        it 'assigns empty variables' do
+          step.perform!
+
+          expect(pipeline.variables).to be_empty
+        end
+      end
+    end
+
+    context 'when user is maintainer' do
+      before do
+        project.add_maintainer(user)
+      end
+
+      it_behaves_like 'does not break the chain'
+      it_behaves_like 'builds pipeline'
+
+      it 'assigns variables_attributes' do
+        step.perform!
+
+        expect(pipeline.variables.map { |var| var.slice(:key, :secret_value) })
+          .to eq variables_attributes.map(&:with_indifferent_access)
+      end
+    end
+  end
+
   it 'returns a valid pipeline' do
     step.perform!
 

spec/lib/gitlab/ci/pipeline/chain/command_spec.rb

@@ -295,4 +295,30 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Command do
       it { is_expected.to eq(false) }
     end
   end
+
+  describe '#creates_child_pipeline?' do
+    let(:command) { described_class.new(bridge: bridge) }
+
+    subject { command.creates_child_pipeline? }
+
+    context 'when bridge is present' do
+      context 'when bridge triggers a child pipeline' do
+        let(:bridge) { double(:bridge, triggers_child_pipeline?: true) }
+
+        it { is_expected.to be_truthy }
+      end
+
+      context 'when bridge triggers a multi-project pipeline' do
+        let(:bridge) { double(:bridge, triggers_child_pipeline?: false) }
+
+        it { is_expected.to be_falsey }
+      end
+    end
+
+    context 'when bridge is not present' do
+      let(:bridge) { nil }
+
+      it { is_expected.to be_falsey }
+    end
+  end
 end

spec/policies/project_policy_spec.rb

@@ -401,6 +401,48 @@ RSpec.describe ProjectPolicy do
     end
   end
 
+  describe 'set_pipeline_variables' do
+    context 'when user is developer' do
+      let(:current_user) { developer }
+
+      context 'when project allows user defined variables' do
+        before do
+          project.update!(restrict_user_defined_variables: false)
+        end
+
+        it { is_expected.to be_allowed(:set_pipeline_variables) }
+      end
+
+      context 'when project restricts use of user defined variables' do
+        before do
+          project.update!(restrict_user_defined_variables: true)
+        end
+
+        it { is_expected.not_to be_allowed(:set_pipeline_variables) }
+      end
+    end
+
+    context 'when user is maintainer' do
+      let(:current_user) { maintainer }
+
+      context 'when project allows user defined variables' do
+        before do
+          project.update!(restrict_user_defined_variables: false)
+        end
+
+        it { is_expected.to be_allowed(:set_pipeline_variables) }
+      end
+
+      context 'when project restricts use of user defined variables' do
+        before do
+          project.update!(restrict_user_defined_variables: true)
+        end
+
+        it { is_expected.to be_allowed(:set_pipeline_variables) }
+      end
+    end
+  end
+
   context 'support bot' do
     let(:current_user) { User.support_bot }
 

spec/requests/api/projects_spec.rb

@@ -1583,6 +1583,7 @@ RSpec.describe API::Projects do
         expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access)
         expect(json_response['only_allow_merge_if_pipeline_succeeds']).to eq(project.only_allow_merge_if_pipeline_succeeds)
         expect(json_response['allow_merge_on_skipped_pipeline']).to eq(project.allow_merge_on_skipped_pipeline)
+        expect(json_response['restrict_user_defined_variables']).to eq(project.restrict_user_defined_variables?)
         expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved)
         expect(json_response['operations_access_level']).to be_present
       end
@@ -1654,6 +1655,7 @@ RSpec.describe API::Projects do
         expect(json_response['shared_with_groups'][0]).to have_key('expires_at')
         expect(json_response['only_allow_merge_if_pipeline_succeeds']).to eq(project.only_allow_merge_if_pipeline_succeeds)
         expect(json_response['allow_merge_on_skipped_pipeline']).to eq(project.allow_merge_on_skipped_pipeline)
+        expect(json_response['restrict_user_defined_variables']).to eq(project.restrict_user_defined_variables?)
         expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved)
         expect(json_response['ci_default_git_depth']).to eq(project.ci_default_git_depth)
         expect(json_response['ci_forward_deployment_enabled']).to eq(project.ci_forward_deployment_enabled)
@@ -2597,6 +2599,18 @@ RSpec.describe API::Projects do
         expect(response).to have_gitlab_http_status(:bad_request)
       end
 
+      it 'updates restrict_user_defined_variables', :aggregate_failures do
+        project_param = { restrict_user_defined_variables: true }
+
+        put api("/projects/#{project3.id}", user), params: project_param
+
+        expect(response).to have_gitlab_http_status(:ok)
+
+        project_param.each_pair do |k, v|
+          expect(json_response[k.to_s]).to eq(v)
+        end
+      end
+
       it 'updates avatar' do
         project_param = {
           avatar: fixture_file_upload('spec/fixtures/banana_sample.gif',

spec/services/ci/create_downstream_pipeline_service_spec.rb

@@ -371,6 +371,26 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
             expect { service.execute(bridge) }.to change { Ci::Pipeline.count }.by(1)
           end
         end
+
+        context 'when downstream project does not allow user-defined variables for child pipelines' do
+          before do
+            bridge.yaml_variables = [{ key: 'BRIDGE', value: '$PIPELINE_VARIABLE-var', public: true }]
+
+            upstream_pipeline.project.update!(restrict_user_defined_variables: true)
+          end
+
+          it 'creates a new pipeline allowing variables to be passed downstream' do
+            expect { service.execute(bridge) }.to change { Ci::Pipeline.count }.by(1)
+          end
+
+          it 'passes variables downstream from the bridge' do
+            pipeline = service.execute(bridge)
+
+            pipeline.variables.map(&:key).tap do |variables|
+              expect(variables).to include 'BRIDGE'
+            end
+          end
+        end
       end
     end
 
@@ -460,6 +480,33 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
             expect(variable.value).to eq 'my-value-var'
           end
         end
+
+        context 'when downstream project does not allow user-defined variables for multi-project pipelines' do
+          before do
+            downstream_project.update!(restrict_user_defined_variables: true)
+          end
+
+          it 'does not create a new pipeline' do
+            expect { service.execute(bridge) }
+              .not_to change { Ci::Pipeline.count }
+          end
+
+          it 'ignores variables passed downstream from the bridge' do
+            pipeline = service.execute(bridge)
+
+            pipeline.variables.map(&:key).tap do |variables|
+              expect(variables).not_to include 'BRIDGE'
+            end
+          end
+
+          it 'sets errors', :aggregate_failures do
+            service.execute(bridge)
+
+            expect(bridge.reload).to be_failed
+            expect(bridge.failure_reason).to eq('downstream_pipeline_creation_failed')
+            expect(bridge.options[:downstream_errors]).to eq(['Insufficient permissions to set pipeline variables'])
+          end
+        end
       end
     end
 

spec/services/ci/pipeline_trigger_service_spec.rb

@@ -3,14 +3,16 @@
 require 'spec_helper'
 
 RSpec.describe Ci::PipelineTriggerService do
-  let(:project) { create(:project, :repository) }
+  include AfterNextHelpers
+
+  let_it_be(:project) { create(:project, :repository) }
 
   before do
     stub_ci_pipeline_to_return_yaml_file
   end
 
   describe '#execute' do
-    let(:user) { create(:user) }
+    let_it_be(:user) { create(:user) }
     let(:result) { described_class.new(project, user, params).execute }
 
     before do
@@ -29,8 +31,8 @@ RSpec.describe Ci::PipelineTriggerService do
         end
       end
 
-      context 'when params have an existsed trigger token' do
-        context 'when params have an existsed ref' do
+      context 'when params have an existing trigger token' do
+        context 'when params have an existing ref' do
           let(:params) { { token: trigger.token, ref: 'master', variables: nil } }
 
           it 'triggers a pipeline' do
@@ -45,9 +47,7 @@ RSpec.describe Ci::PipelineTriggerService do
 
           context 'when commit message has [ci skip]' do
             before do
-              allow_next_instance_of(Ci::Pipeline) do |instance|
-                allow(instance).to receive(:git_commit_message) { '[ci skip]' }
-              end
+              allow_next(Ci::Pipeline).to receive(:git_commit_message) { '[ci skip]' }
             end
 
             it 'ignores [ci skip] and create as general' do

spec/services/ci/play_build_service_spec.rb

@@ -72,6 +72,31 @@ RSpec.describe Ci::PlayBuildService, '#execute' do
 
         expect(build.reload.job_variables.map(&:key)).to contain_exactly('first', 'second')
       end
+
+      context 'when user defined variables are restricted' do
+        before do
+          project.update!(restrict_user_defined_variables: true)
+        end
+
+        context 'when user is maintainer' do
+          before do
+            project.add_maintainer(user)
+          end
+
+          it 'assigns the variables to the build' do
+            service.execute(build, job_variables)
+
+            expect(build.reload.job_variables.map(&:key)).to contain_exactly('first', 'second')
+          end
+        end
+
+        context 'when user is developer' do
+          it 'raises an error' do
+            expect { service.execute(build, job_variables) }
+              .to raise_error Gitlab::Access::AccessDeniedError
+          end
+        end
+      end
     end
   end