kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Add Maskable concern for CI variables

Browse source 
This adds a concern that abstracts the concept of masking a variable,
including the RegEx for validation.
This commit is contained in:
Matija Čupić 2019-02-26 14:03:42 +00:00 committed by Kamil Trzciński
parent c1d8f8a410
commit c9125c4786
24 changed files with 291 additions and 120 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
app/models/ci/group_variable.rb
View file

@ -5,6 +5,7 @@ module Ci
extend Gitlab::Ci::Model
include HasVariable
include Presentable
include Maskable
belongs_to :group, class_name: "::Group"

1
app/models/ci/variable.rb
View file

@ -5,6 +5,7 @@ module Ci
extend Gitlab::Ci::Model
include HasVariable
include Presentable
include Maskable
belongs_to :project

6
app/models/concerns/has_variable.rb
View file

@ -21,9 +21,9 @@ module HasVariable
def key=(new_key)
super(new_key.to_s.strip)
end
end
def to_runner_variable
{ key: key, value: value, public: false }
end
def to_runner_variable
{ key: key, value: value, public: false }
end
end

22
app/models/concerns/maskable.rb Normal file
View file

@ -0,0 +1,22 @@
# frozen_string_literal: true
module Maskable
extend ActiveSupport::Concern
# * Single line
# * No escape characters
# * No variables
# * No spaces
# * Minimal length of 8 characters
# * Absolutely no fun is allowed
REGEX = /\A\w{8,}\z/
included do
validates :masked, inclusion: { in: [true, false] }
validates :value, format: { with: REGEX }, if: :masked?
end
def to_runner_variable
super.merge(masked: masked?)
end
end

5
changelogs/unreleased/13784-simple-masking-of-protected-variables-in-logs.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Add support for masking CI variables.
merge_request: 25293
author:
type: added

21
db/migrate/20190218134158_add_masked_to_ci_variables.rb Normal file
View file

@ -0,0 +1,21 @@
# frozen_string_literal: true
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddMaskedToCiVariables < ActiveRecord::Migration[5.0]
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
def up
add_column_with_default :ci_variables, :masked, :boolean, default: false, allow_null: false
end
def down
remove_column :ci_variables, :masked
end
end

21
db/migrate/20190218134209_add_masked_to_ci_group_variables.rb Normal file
View file

@ -0,0 +1,21 @@
# frozen_string_literal: true
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddMaskedToCiGroupVariables < ActiveRecord::Migration[5.0]
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
def up
add_column_with_default :ci_group_variables, :masked, :boolean, default: false, allow_null: false
end
def down
remove_column :ci_group_variables, :masked
end
end

4
db/schema.rb
View file

@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20190204115450) do
ActiveRecord::Schema.define(version: 20190218134209) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@ -405,6 +405,7 @@ ActiveRecord::Schema.define(version: 20190204115450) do
t.boolean "protected", default: false, null: false
t.datetime_with_timezone "created_at", null: false
t.datetime_with_timezone "updated_at", null: false
t.boolean "masked", default: false, null: false
t.index ["group_id", "key"], name: "index_ci_group_variables_on_group_id_and_key", unique: true, using: :btree
end
@ -600,6 +601,7 @@ ActiveRecord::Schema.define(version: 20190204115450) do
t.integer "project_id", null: false
t.boolean "protected", default: false, null: false
t.string "environment_scope", default: "*", null: false
t.boolean "masked", default: false, null: false
t.index ["project_id", "key", "environment_scope"], name: "index_ci_variables_on_project_id_and_key_and_environment_scope", unique: true, using: :btree
end

10
lib/gitlab/ci/variables/collection/item.rb
View file

@ -5,12 +5,12 @@ module Gitlab
module Variables
class Collection
class Item
def initialize(key:, value:, public: true, file: false)
def initialize(key:, value:, public: true, file: false, masked: false)
raise ArgumentError, "`#{key}` must be of type String or nil value, while it was: #{value.class}" unless
value.is_a?(String) || value.nil?
@variable = {
key: key, value: value, public: public, file: file
key: key, value: value, public: public, file: file, masked: masked
}
end
@ -27,9 +27,13 @@ module Gitlab
# don't expose `file` attribute at all (stems from what the runner
# expects).
#
# If the `variable_masking` feature is enabled we expose the `masked`
# attribute, otherwise it's not exposed.
#
def to_runner_variable
@variable.reject do |hash_key, hash_value|
hash_key == :file && hash_value == false
(hash_key == :file && hash_value == false) ||
(hash_key == :masked && !Feature.enabled?(:variable_masking))
end
end

1
spec/factories/ci/group_variables.rb
View file

@ -2,6 +2,7 @@ FactoryBot.define do
factory :ci_group_variable, class: Ci::GroupVariable do
sequence(:key) { |n| "VARIABLE_#{n}" }
value 'VARIABLE_VALUE'
masked false
trait(:protected) do
protected true

1
spec/factories/ci/variables.rb
View file

@ -2,6 +2,7 @@ FactoryBot.define do
factory :ci_variable, class: Ci::Variable do
sequence(:key) { |n| "VARIABLE_#{n}" }
value 'VARIABLE_VALUE'
masked false
trait(:protected) do
protected true

2
spec/features/group_variables_spec.rb
View file

@ -3,7 +3,7 @@ require 'spec_helper'
describe 'Group variables', :js do
let(:user) { create(:user) }
let(:group) { create(:group) }
let!(:variable) { create(:ci_group_variable, key: 'test_key', value: 'test value', group: group) }
let!(:variable) { create(:ci_group_variable, key: 'test_key', value: 'test_value', group: group) }
let(:page_path) { group_settings_ci_cd_path(group) }
before do

2
spec/features/project_variables_spec.rb
View file

@ -3,7 +3,7 @@ require 'spec_helper'
describe 'Project variables', :js do
let(:user) { create(:user) }
let(:project) { create(:project) }
let(:variable) { create(:ci_variable, key: 'test_key', value: 'test value') }
let(:variable) { create(:ci_variable, key: 'test_key', value: 'test_value') }
let(:page_path) { project_settings_ci_cd_path(project) }
before do

20
spec/lib/gitlab/ci/variables/collection/item_spec.rb
View file

@ -6,7 +6,7 @@ describe Gitlab::Ci::Variables::Collection::Item do
let(:expected_value) { variable_value }
let(:variable) do
{ key: variable_key, value: variable_value, public: true }
{ key: variable_key, value: variable_value, public: true, masked: false }
end
describe '.new' do
@ -88,7 +88,7 @@ describe Gitlab::Ci::Variables::Collection::Item do
resource = described_class.fabricate(variable)
expect(resource).to be_a(described_class)
expect(resource).to eq(key: 'CI_VAR', value: '123', public: false)
expect(resource).to eq(key: 'CI_VAR', value: '123', public: false, masked: false)
end
it 'supports using another collection item' do
@ -134,7 +134,21 @@ describe Gitlab::Ci::Variables::Collection::Item do
.to_runner_variable
expect(runner_variable)
.to eq(key: 'VAR', value: 'value', public: true, file: true)
.to eq(key: 'VAR', value: 'value', public: true, file: true, masked: false)
end
end
context 'when variable masking is disabled' do
before do
stub_feature_flags(variable_masking: false)
end
it 'does not expose the masked field to the runner' do
runner_variable = described_class
.new(key: 'VAR', value: 'value', masked: true)
.to_runner_variable
expect(runner_variable).to eq(key: 'VAR', value: 'value', public: true)
end
end
end

4
spec/lib/gitlab/ci/variables/collection_spec.rb
View file

@ -3,7 +3,7 @@ require 'spec_helper'
describe Gitlab::Ci::Variables::Collection do
describe '.new' do
it 'can be initialized with an array' do
variable = { key: 'VAR', value: 'value', public: true }
variable = { key: 'VAR', value: 'value', public: true, masked: false }
collection = described_class.new([variable])
@ -93,7 +93,7 @@ describe Gitlab::Ci::Variables::Collection do
collection = described_class.new([{ key: 'TEST', value: '1' }])
expect(collection.to_runner_variables)
.to eq [{ key: 'TEST', value: '1', public: true }]
.to eq [{ key: 'TEST', value: '1', public: true, masked: false }]
end
end

176
spec/models/ci/build_spec.rb
View file

@ -2114,55 +2114,55 @@ describe Ci::Build do
context 'returns variables' do
let(:predefined_variables) do
[
{ key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true },
{ key: 'CI_PIPELINE_URL', value: project.web_url + "/pipelines/#{pipeline.id}", public: true },
{ key: 'CI_JOB_ID', value: build.id.to_s, public: true },
{ key: 'CI_JOB_URL', value: project.web_url + "/-/jobs/#{build.id}", public: true },
{ key: 'CI_JOB_TOKEN', value: 'my-token', public: false },
{ key: 'CI_BUILD_ID', value: build.id.to_s, public: true },
{ key: 'CI_BUILD_TOKEN', value: 'my-token', public: false },
{ key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true },
{ key: 'CI_REGISTRY_PASSWORD', value: 'my-token', public: false },
{ key: 'CI_REPOSITORY_URL', value: build.repo_url, public: false },
{ key: 'CI', value: 'true', public: true },
{ key: 'GITLAB_CI', value: 'true', public: true },
{ key: 'GITLAB_FEATURES', value: project.licensed_features.join(','), public: true },
{ key: 'CI_SERVER_NAME', value: 'GitLab', public: true },
{ key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true },
{ key: 'CI_SERVER_VERSION_MAJOR', value: Gitlab.version_info.major.to_s, public: true },
{ key: 'CI_SERVER_VERSION_MINOR', value: Gitlab.version_info.minor.to_s, public: true },
{ key: 'CI_SERVER_VERSION_PATCH', value: Gitlab.version_info.patch.to_s, public: true },
{ key: 'CI_SERVER_REVISION', value: Gitlab.revision, public: true },
{ key: 'CI_JOB_NAME', value: 'test', public: true },
{ key: 'CI_JOB_STAGE', value: 'test', public: true },
{ key: 'CI_COMMIT_SHA', value: build.sha, public: true },
{ key: 'CI_COMMIT_SHORT_SHA', value: build.short_sha, public: true },
{ key: 'CI_COMMIT_BEFORE_SHA', value: build.before_sha, public: true },
{ key: 'CI_COMMIT_REF_NAME', value: build.ref, public: true },
{ key: 'CI_COMMIT_REF_SLUG', value: build.ref_slug, public: true },
{ key: 'CI_NODE_TOTAL', value: '1', public: true },
{ key: 'CI_BUILD_REF', value: build.sha, public: true },
{ key: 'CI_BUILD_BEFORE_SHA', value: build.before_sha, public: true },
{ key: 'CI_BUILD_REF_NAME', value: build.ref, public: true },
{ key: 'CI_BUILD_REF_SLUG', value: build.ref_slug, public: true },
{ key: 'CI_BUILD_NAME', value: 'test', public: true },
{ key: 'CI_BUILD_STAGE', value: 'test', public: true },
{ key: 'CI_PROJECT_ID', value: project.id.to_s, public: true },
{ key: 'CI_PROJECT_NAME', value: project.path, public: true },
{ key: 'CI_PROJECT_PATH', value: project.full_path, public: true },
{ key: 'CI_PROJECT_PATH_SLUG', value: project.full_path_slug, public: true },
{ key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true },
{ key: 'CI_PROJECT_URL', value: project.web_url, public: true },
{ key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true },
{ key: 'CI_PAGES_DOMAIN', value: Gitlab.config.pages.host, public: true },
{ key: 'CI_PAGES_URL', value: project.pages_url, public: true },
{ key: 'CI_API_V4_URL', value: 'http://localhost/api/v4', public: true },
{ key: 'CI_PIPELINE_IID', value: pipeline.iid.to_s, public: true },
{ key: 'CI_CONFIG_PATH', value: pipeline.ci_yaml_file_path, public: true },
{ key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true },
{ key: 'CI_COMMIT_MESSAGE', value: pipeline.git_commit_message, public: true },
{ key: 'CI_COMMIT_TITLE', value: pipeline.git_commit_title, public: true },
{ key: 'CI_COMMIT_DESCRIPTION', value: pipeline.git_commit_description, public: true }
{ key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true, masked: false },
{ key: 'CI_PIPELINE_URL', value: project.web_url + "/pipelines/#{pipeline.id}", public: true, masked: false },
{ key: 'CI_JOB_ID', value: build.id.to_s, public: true, masked: false },
{ key: 'CI_JOB_URL', value: project.web_url + "/-/jobs/#{build.id}", public: true, masked: false },
{ key: 'CI_JOB_TOKEN', value: 'my-token', public: false, masked: false },
{ key: 'CI_BUILD_ID', value: build.id.to_s, public: true, masked: false },
{ key: 'CI_BUILD_TOKEN', value: 'my-token', public: false, masked: false },
{ key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true, masked: false },
{ key: 'CI_REGISTRY_PASSWORD', value: 'my-token', public: false, masked: false },
{ key: 'CI_REPOSITORY_URL', value: build.repo_url, public: false, masked: false },
{ key: 'CI', value: 'true', public: true, masked: false },
{ key: 'GITLAB_CI', value: 'true', public: true, masked: false },
{ key: 'GITLAB_FEATURES', value: project.licensed_features.join(','), public: true, masked: false },
{ key: 'CI_SERVER_NAME', value: 'GitLab', public: true, masked: false },
{ key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true, masked: false },
{ key: 'CI_SERVER_VERSION_MAJOR', value: Gitlab.version_info.major.to_s, public: true, masked: false },
{ key: 'CI_SERVER_VERSION_MINOR', value: Gitlab.version_info.minor.to_s, public: true, masked: false },
{ key: 'CI_SERVER_VERSION_PATCH', value: Gitlab.version_info.patch.to_s, public: true, masked: false },
{ key: 'CI_SERVER_REVISION', value: Gitlab.revision, public: true, masked: false },
{ key: 'CI_JOB_NAME', value: 'test', public: true, masked: false },
{ key: 'CI_JOB_STAGE', value: 'test', public: true, masked: false },
{ key: 'CI_COMMIT_SHA', value: build.sha, public: true, masked: false },
{ key: 'CI_COMMIT_SHORT_SHA', value: build.short_sha, public: true, masked: false },
{ key: 'CI_COMMIT_BEFORE_SHA', value: build.before_sha, public: true, masked: false },
{ key: 'CI_COMMIT_REF_NAME', value: build.ref, public: true, masked: false },
{ key: 'CI_COMMIT_REF_SLUG', value: build.ref_slug, public: true, masked: false },
{ key: 'CI_NODE_TOTAL', value: '1', public: true, masked: false },
{ key: 'CI_BUILD_REF', value: build.sha, public: true, masked: false },
{ key: 'CI_BUILD_BEFORE_SHA', value: build.before_sha, public: true, masked: false },
{ key: 'CI_BUILD_REF_NAME', value: build.ref, public: true, masked: false },
{ key: 'CI_BUILD_REF_SLUG', value: build.ref_slug, public: true, masked: false },
{ key: 'CI_BUILD_NAME', value: 'test', public: true, masked: false },
{ key: 'CI_BUILD_STAGE', value: 'test', public: true, masked: false },
{ key: 'CI_PROJECT_ID', value: project.id.to_s, public: true, masked: false },
{ key: 'CI_PROJECT_NAME', value: project.path, public: true, masked: false },
{ key: 'CI_PROJECT_PATH', value: project.full_path, public: true, masked: false },
{ key: 'CI_PROJECT_PATH_SLUG', value: project.full_path_slug, public: true, masked: false },
{ key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true, masked: false },
{ key: 'CI_PROJECT_URL', value: project.web_url, public: true, masked: false },
{ key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true, masked: false },
{ key: 'CI_PAGES_DOMAIN', value: Gitlab.config.pages.host, public: true, masked: false },
{ key: 'CI_PAGES_URL', value: project.pages_url, public: true, masked: false },
{ key: 'CI_API_V4_URL', value: 'http://localhost/api/v4', public: true, masked: false },
{ key: 'CI_PIPELINE_IID', value: pipeline.iid.to_s, public: true, masked: false },
{ key: 'CI_CONFIG_PATH', value: pipeline.ci_yaml_file_path, public: true, masked: false },
{ key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true, masked: false },
{ key: 'CI_COMMIT_MESSAGE', value: pipeline.git_commit_message, public: true, masked: false },
{ key: 'CI_COMMIT_TITLE', value: pipeline.git_commit_title, public: true, masked: false },
{ key: 'CI_COMMIT_DESCRIPTION', value: pipeline.git_commit_description, public: true, masked: false }
]
end
@ -2175,10 +2175,10 @@ describe Ci::Build do
describe 'variables ordering' do
context 'when variables hierarchy is stubbed' do
let(:build_pre_var) { { key: 'build', value: 'value', public: true } }
let(:project_pre_var) { { key: 'project', value: 'value', public: true } }
let(:pipeline_pre_var) { { key: 'pipeline', value: 'value', public: true } }
let(:build_yaml_var) { { key: 'yaml', value: 'value', public: true } }
let(:build_pre_var) { { key: 'build', value: 'value', public: true, masked: false } }
let(:project_pre_var) { { key: 'project', value: 'value', public: true, masked: false } }
let(:pipeline_pre_var) { { key: 'pipeline', value: 'value', public: true, masked: false } }
let(:build_yaml_var) { { key: 'yaml', value: 'value', public: true, masked: false } }
before do
allow(build).to receive(:predefined_variables) { [build_pre_var] }
@ -2200,7 +2200,7 @@ describe Ci::Build do
project_pre_var,
pipeline_pre_var,
build_yaml_var,
{ key: 'secret', value: 'value', public: false }])
{ key: 'secret', value: 'value', public: false, masked: false }])
end
end
@ -2233,10 +2233,10 @@ describe Ci::Build do
context 'when build has user' do
let(:user_variables) do
[
{ key: 'GITLAB_USER_ID', value: user.id.to_s, public: true },
{ key: 'GITLAB_USER_EMAIL', value: user.email, public: true },
{ key: 'GITLAB_USER_LOGIN', value: user.username, public: true },
{ key: 'GITLAB_USER_NAME', value: user.name, public: true }
{ key: 'GITLAB_USER_ID', value: user.id.to_s, public: true, masked: false },
{ key: 'GITLAB_USER_EMAIL', value: user.email, public: true, masked: false },
{ key: 'GITLAB_USER_LOGIN', value: user.username, public: true, masked: false },
{ key: 'GITLAB_USER_NAME', value: user.name, public: true, masked: false }
]
end
@ -2250,8 +2250,8 @@ describe Ci::Build do
context 'when build has an environment' do
let(:environment_variables) do
[
{ key: 'CI_ENVIRONMENT_NAME', value: 'production', public: true },
{ key: 'CI_ENVIRONMENT_SLUG', value: 'prod-slug', public: true }
{ key: 'CI_ENVIRONMENT_NAME', value: 'production', public: true, masked: false },
{ key: 'CI_ENVIRONMENT_SLUG', value: 'prod-slug', public: true, masked: false }
]
end
@ -2286,7 +2286,7 @@ describe Ci::Build do
before do
environment_variables <<
{ key: 'CI_ENVIRONMENT_URL', value: url, public: true }
{ key: 'CI_ENVIRONMENT_URL', value: url, public: true, masked: false }
end
context 'when the URL was set from the job' do
@ -2323,7 +2323,7 @@ describe Ci::Build do
end
let(:manual_variable) do
{ key: 'CI_JOB_MANUAL', value: 'true', public: true }
{ key: 'CI_JOB_MANUAL', value: 'true', public: true, masked: false }
end
it { is_expected.to include(manual_variable) }
@ -2331,7 +2331,7 @@ describe Ci::Build do
context 'when build is for tag' do
let(:tag_variable) do
{ key: 'CI_COMMIT_TAG', value: 'master', public: true }
{ key: 'CI_COMMIT_TAG', value: 'master', public: true, masked: false }
end
before do
@ -2343,7 +2343,7 @@ describe Ci::Build do
context 'when CI variable is defined' do
let(:ci_variable) do
{ key: 'SECRET_KEY', value: 'secret_value', public: false }
{ key: 'SECRET_KEY', value: 'secret_value', public: false, masked: false }
end
before do
@ -2358,7 +2358,7 @@ describe Ci::Build do
let(:ref) { Gitlab::Git::BRANCH_REF_PREFIX + build.ref }
let(:protected_variable) do
{ key: 'PROTECTED_KEY', value: 'protected_value', public: false }
{ key: 'PROTECTED_KEY', value: 'protected_value', public: false, masked: false }
end
before do
@ -2390,7 +2390,7 @@ describe Ci::Build do
context 'when group CI variable is defined' do
let(:ci_variable) do
{ key: 'SECRET_KEY', value: 'secret_value', public: false }
{ key: 'SECRET_KEY', value: 'secret_value', public: false, masked: false }
end
before do
@ -2405,7 +2405,7 @@ describe Ci::Build do
let(:ref) { Gitlab::Git::BRANCH_REF_PREFIX + build.ref }
let(:protected_variable) do
{ key: 'PROTECTED_KEY', value: 'protected_value', public: false }
{ key: 'PROTECTED_KEY', value: 'protected_value', public: false, masked: false }
end
before do
@ -2444,11 +2444,11 @@ describe Ci::Build do
let(:trigger_request) { create(:ci_trigger_request, pipeline: pipeline, trigger: trigger) }
let(:user_trigger_variable) do
{ key: 'TRIGGER_KEY_1', value: 'TRIGGER_VALUE_1', public: false }
{ key: 'TRIGGER_KEY_1', value: 'TRIGGER_VALUE_1', public: false, masked: false }
end
let(:predefined_trigger_variable) do
{ key: 'CI_PIPELINE_TRIGGERED', value: 'true', public: true }
{ key: 'CI_PIPELINE_TRIGGERED', value: 'true', public: true, masked: false }
end
before do
@ -2480,7 +2480,7 @@ describe Ci::Build do
context 'when pipeline has a variable' do
let!(:pipeline_variable) { create(:ci_pipeline_variable, pipeline: pipeline) }
it { is_expected.to include(pipeline_variable.to_runner_variable) }
it { is_expected.to include(key: pipeline_variable.key, value: pipeline_variable.value, public: false, masked: false) }
end
context 'when a job was triggered by a pipeline schedule' do
@ -2497,16 +2497,16 @@ describe Ci::Build do
pipeline_schedule.reload
end
it { is_expected.to include(pipeline_schedule_variable.to_runner_variable) }
it { is_expected.to include(key: pipeline_schedule_variable.key, value: pipeline_schedule_variable.value, public: false, masked: false) }
end
context 'when container registry is enabled' do
let(:container_registry_enabled) { true }
let(:ci_registry) do
{ key: 'CI_REGISTRY', value: 'registry.example.com', public: true }
{ key: 'CI_REGISTRY', value: 'registry.example.com', public: true, masked: false }
end
let(:ci_registry_image) do
{ key: 'CI_REGISTRY_IMAGE', value: project.container_registry_url, public: true }
{ key: 'CI_REGISTRY_IMAGE', value: project.container_registry_url, public: true, masked: false }
end
context 'and is disabled for project' do
@ -2535,13 +2535,13 @@ describe Ci::Build do
build.update(runner: runner)
end
it { is_expected.to include({ key: 'CI_RUNNER_ID', value: runner.id.to_s, public: true }) }
it { is_expected.to include({ key: 'CI_RUNNER_DESCRIPTION', value: 'description', public: true }) }
it { is_expected.to include({ key: 'CI_RUNNER_TAGS', value: 'docker, linux', public: true }) }
it { is_expected.to include({ key: 'CI_RUNNER_ID', value: runner.id.to_s, public: true, masked: false }) }
it { is_expected.to include({ key: 'CI_RUNNER_DESCRIPTION', value: 'description', public: true, masked: false }) }
it { is_expected.to include({ key: 'CI_RUNNER_TAGS', value: 'docker, linux', public: true, masked: false }) }
end
context 'when build is for a deployment' do
let(:deployment_variable) { { key: 'KUBERNETES_TOKEN', value: 'TOKEN', public: false } }
let(:deployment_variable) { { key: 'KUBERNETES_TOKEN', value: 'TOKEN', public: false, masked: false } }
before do
build.environment = 'production'
@ -2555,7 +2555,7 @@ describe Ci::Build do
end
context 'when project has custom CI config path' do
let(:ci_config_path) { { key: 'CI_CONFIG_PATH', value: 'custom', public: true } }
let(:ci_config_path) { { key: 'CI_CONFIG_PATH', value: 'custom', public: true, masked: false } }
before do
project.update(ci_config_path: 'custom')
@ -2572,7 +2572,7 @@ describe Ci::Build do
it "includes AUTO_DEVOPS_DOMAIN" do
is_expected.to include(
{ key: 'AUTO_DEVOPS_DOMAIN', value: 'example.com', public: true })
{ key: 'AUTO_DEVOPS_DOMAIN', value: 'example.com', public: true, masked: false })
end
end
@ -2583,7 +2583,7 @@ describe Ci::Build do
it "includes AUTO_DEVOPS_DOMAIN" do
is_expected.not_to include(
{ key: 'AUTO_DEVOPS_DOMAIN', value: 'example.com', public: true })
{ key: 'AUTO_DEVOPS_DOMAIN', value: 'example.com', public: true, masked: false })
end
end
end
@ -2598,9 +2598,9 @@ describe Ci::Build do
variables = subject.reverse.uniq { |variable| variable[:key] }.reverse
expect(variables)
.not_to include(key: 'MYVAR', value: 'myvar', public: true)
.not_to include(key: 'MYVAR', value: 'myvar', public: true, masked: false)
expect(variables)
.to include(key: 'MYVAR', value: 'pipeline value', public: false)
.to include(key: 'MYVAR', value: 'pipeline value', public: false, masked: false)
end
end
@ -2616,13 +2616,13 @@ describe Ci::Build do
it 'includes CI_NODE_INDEX' do
is_expected.to include(
{ key: 'CI_NODE_INDEX', value: index.to_s, public: true }
{ key: 'CI_NODE_INDEX', value: index.to_s, public: true, masked: false }
)
end
it 'includes correct CI_NODE_TOTAL' do
is_expected.to include(
{ key: 'CI_NODE_TOTAL', value: total.to_s, public: true }
{ key: 'CI_NODE_TOTAL', value: total.to_s, public: true, masked: false }
)
end
end
@ -2641,7 +2641,7 @@ describe Ci::Build do
it 'returns static predefined variables' do
expect(build.variables.size).to be >= 28
expect(build.variables)
.to include(key: 'CI_COMMIT_REF_NAME', value: 'feature', public: true)
.to include(key: 'CI_COMMIT_REF_NAME', value: 'feature', public: true, masked: false)
expect(build).not_to be_persisted
end
end
@ -2651,8 +2651,8 @@ describe Ci::Build do
let(:deploy_token_variables) do
[
{ key: 'CI_DEPLOY_USER', value: deploy_token.username, public: true },
{ key: 'CI_DEPLOY_PASSWORD', value: deploy_token.token, public: false }
{ key: 'CI_DEPLOY_USER', value: deploy_token.username, public: true, masked: false },
{ key: 'CI_DEPLOY_PASSWORD', value: deploy_token.token, public: false, masked: false }
]
end
@ -2711,7 +2711,7 @@ describe Ci::Build do
end
expect(variables)
.to include(key: 'CI_COMMIT_REF_NAME', value: 'feature', public: true)
.to include(key: 'CI_COMMIT_REF_NAME', value: 'feature', public: true, masked: false)
end
it 'does not return prohibited variables' do

1
spec/models/ci/group_variable_spec.rb
View file

@ -5,6 +5,7 @@ describe Ci::GroupVariable do
it { is_expected.to include_module(HasVariable) }
it { is_expected.to include_module(Presentable) }
it { is_expected.to include_module(Maskable) }
it { is_expected.to validate_uniqueness_of(:key).scoped_to(:group_id).with_message(/\(\w+\) has already been taken/) }
describe '.unprotected' do

1
spec/models/ci/variable_spec.rb
View file

@ -6,6 +6,7 @@ describe Ci::Variable do
describe 'validations' do
it { is_expected.to include_module(HasVariable) }
it { is_expected.to include_module(Presentable) }
it { is_expected.to include_module(Maskable) }
it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id, :environment_scope).with_message(/\(\w+\) has already been taken/) }
end

2
spec/models/concerns/has_variable_spec.rb
View file

@ -57,7 +57,7 @@ describe HasVariable do
describe '#to_runner_variable' do
it 'returns a hash for the runner' do
expect(subject.to_runner_variable)
.to eq(key: subject.key, value: subject.value, public: false)
.to include(key: subject.key, value: subject.value, public: false)
end
end
end

76
spec/models/concerns/maskable_spec.rb Normal file
View file

@ -0,0 +1,76 @@
# frozen_string_literal: true
require 'spec_helper'
describe Maskable do
let(:variable) { build(:ci_variable) }
describe 'masked value validations' do
subject { variable }
context 'when variable is masked' do
before do
subject.masked = true
end
it { is_expected.not_to allow_value('hello').for(:value) }
it { is_expected.not_to allow_value('hello world').for(:value) }
it { is_expected.not_to allow_value('hello$VARIABLEworld').for(:value) }
it { is_expected.not_to allow_value('hello\rworld').for(:value) }
it { is_expected.to allow_value('helloworld').for(:value) }
end
context 'when variable is not masked' do
before do
subject.masked = false
end
it { is_expected.to allow_value('hello').for(:value) }
it { is_expected.to allow_value('hello world').for(:value) }
it { is_expected.to allow_value('hello$VARIABLEworld').for(:value) }
it { is_expected.to allow_value('hello\rworld').for(:value) }
it { is_expected.to allow_value('helloworld').for(:value) }
end
end
describe 'REGEX' do
subject { Maskable::REGEX }
it 'does not match strings shorter than 8 letters' do
expect(subject.match?('hello')).to eq(false)
end
it 'does not match strings with spaces' do
expect(subject.match?('hello world')).to eq(false)
end
it 'does not match strings with shell variables' do
expect(subject.match?('hello$VARIABLEworld')).to eq(false)
end
it 'does not match strings with escape characters' do
expect(subject.match?('hello\rworld')).to eq(false)
end
it 'does not match strings that span more than one line' do
string = <<~EOS
hello
world
EOS
expect(subject.match?(string)).to eq(false)
end
it 'matches valid strings' do
expect(subject.match?('helloworld')).to eq(true)
end
end
describe '#to_runner_variable' do
subject { variable.to_runner_variable }
it 'exposes the masked attribute' do
expect(subject).to include(:masked)
end
end
end

4
spec/requests/api/group_variables_spec.rb
View file

@ -87,12 +87,12 @@ describe API::GroupVariables do
it 'creates variable' do
expect do
post api("/groups/#{group.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'VALUE_2', protected: true }
post api("/groups/#{group.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'PROTECTED_VALUE_2', protected: true }
end.to change {group.variables.count}.by(1)
expect(response).to have_gitlab_http_status(201)
expect(json_response['key']).to eq('TEST_VARIABLE_2')
expect(json_response['value']).to eq('VALUE_2')
expect(json_response['value']).to eq('PROTECTED_VALUE_2')
expect(json_response['protected']).to be_truthy
end

18
spec/requests/api/runner_spec.rb
View file

@ -436,9 +436,9 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
end
let(:expected_variables) do
[{ 'key' => 'CI_JOB_NAME', 'value' => 'spinach', 'public' => true },
{ 'key' => 'CI_JOB_STAGE', 'value' => 'test', 'public' => true },
{ 'key' => 'DB_NAME', 'value' => 'postgres', 'public' => true }]
[{ 'key' => 'CI_JOB_NAME', 'value' => 'spinach', 'public' => true, 'masked' => false },
{ 'key' => 'CI_JOB_STAGE', 'value' => 'test', 'public' => true, 'masked' => false },
{ 'key' => 'DB_NAME', 'value' => 'postgres', 'public' => true, 'masked' => false }]
end
let(:expected_artifacts) do
@ -740,12 +740,12 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
context 'when triggered job is available' do
let(:expected_variables) do
[{ 'key' => 'CI_JOB_NAME', 'value' => 'spinach', 'public' => true },
{ 'key' => 'CI_JOB_STAGE', 'value' => 'test', 'public' => true },
{ 'key' => 'CI_PIPELINE_TRIGGERED', 'value' => 'true', 'public' => true },
{ 'key' => 'DB_NAME', 'value' => 'postgres', 'public' => true },
{ 'key' => 'SECRET_KEY', 'value' => 'secret_value', 'public' => false },
{ 'key' => 'TRIGGER_KEY_1', 'value' => 'TRIGGER_VALUE_1', 'public' => false }]
[{ 'key' => 'CI_JOB_NAME', 'value' => 'spinach', 'public' => true, 'masked' => false },
{ 'key' => 'CI_JOB_STAGE', 'value' => 'test', 'public' => true, 'masked' => false },
{ 'key' => 'CI_PIPELINE_TRIGGERED', 'value' => 'true', 'public' => true, 'masked' => false },
{ 'key' => 'DB_NAME', 'value' => 'postgres', 'public' => true, 'masked' => false },
{ 'key' => 'SECRET_KEY', 'value' => 'secret_value', 'public' => false, 'masked' => false },
{ 'key' => 'TRIGGER_KEY_1', 'value' => 'TRIGGER_VALUE_1', 'public' => false, 'masked' => false }]
end
let(:trigger) { create(:ci_trigger, project: project) }

4
spec/requests/api/variables_spec.rb
View file

@ -73,12 +73,12 @@ describe API::Variables do
context 'authorized user with proper permissions' do
it 'creates variable' do
expect do
post api("/projects/#{project.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'VALUE_2', protected: true }
post api("/projects/#{project.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'PROTECTED_VALUE_2', protected: true }
end.to change {project.variables.count}.by(1)
expect(response).to have_gitlab_http_status(201)
expect(json_response['key']).to eq('TEST_VARIABLE_2')
expect(json_response['value']).to eq('VALUE_2')
expect(json_response['value']).to eq('PROTECTED_VALUE_2')
expect(json_response['protected']).to be_truthy
end

8
spec/support/features/variable_list_shared_examples.rb
View file

@ -8,7 +8,7 @@ shared_examples 'variable list' do
it 'adds new CI variable' do
page.within('.js-ci-variable-list-section .js-row:last-child') do
find('.js-ci-variable-input-key').set('key')
find('.js-ci-variable-input-value').set('key value')
find('.js-ci-variable-input-value').set('key_value')
end
click_button('Save variables')
@ -19,7 +19,7 @@ shared_examples 'variable list' do
# We check the first row because it re-sorts to alphabetical order on refresh
page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do
expect(find('.js-ci-variable-input-key').value).to eq('key')
expect(find('.js-ci-variable-input-value', visible: false).value).to eq('key value')
expect(find('.js-ci-variable-input-value', visible: false).value).to eq('key_value')
end
end
@ -44,7 +44,7 @@ shared_examples 'variable list' do
it 'adds new protected variable' do
page.within('.js-ci-variable-list-section .js-row:last-child') do
find('.js-ci-variable-input-key').set('key')
find('.js-ci-variable-input-value').set('key value')
find('.js-ci-variable-input-value').set('key_value')
find('.ci-variable-protected-item .js-project-feature-toggle').click
expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('true')
@ -58,7 +58,7 @@ shared_examples 'variable list' do
# We check the first row because it re-sorts to alphabetical order on refresh
page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do
expect(find('.js-ci-variable-input-key').value).to eq('key')
expect(find('.js-ci-variable-input-value', visible: false).value).to eq('key value')
expect(find('.js-ci-variable-input-value', visible: false).value).to eq('key_value')
expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('true')
end
end