From 686ffdc2e9eb13c2785816aeadafc5a9ee5b0aa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 7 Mar 2019 17:08:17 +0000 Subject: [PATCH] Resolve "Mask the existing variables" --- app/models/ci/build.rb | 8 ++++---- app/models/clusters/kubernetes_namespace.rb | 2 +- app/models/clusters/platforms/kubernetes.rb | 2 +- app/models/project_services/kubernetes_service.rb | 2 +- .../unreleased/58010-mask-the-existing-variables.yml | 5 +++++ spec/models/ci/build_spec.rb | 8 ++++---- spec/models/clusters/kubernetes_namespace_spec.rb | 2 +- spec/models/clusters/platforms/kubernetes_spec.rb | 10 +++++----- .../models/project_services/kubernetes_service_spec.rb | 2 +- spec/models/project_spec.rb | 4 ++-- 10 files changed, 25 insertions(+), 20 deletions(-) create mode 100644 changelogs/unreleased/58010-mask-the-existing-variables.yml diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index a64c6051f95..a629db82c19 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -426,11 +426,11 @@ module Ci .concat(pipeline.persisted_variables) .append(key: 'CI_JOB_ID', value: id.to_s) .append(key: 'CI_JOB_URL', value: Gitlab::Routing.url_helpers.project_job_url(project, self)) - .append(key: 'CI_JOB_TOKEN', value: token.to_s, public: false) + .append(key: 'CI_JOB_TOKEN', value: token.to_s, public: false, masked: true) .append(key: 'CI_BUILD_ID', value: id.to_s) - .append(key: 'CI_BUILD_TOKEN', value: token.to_s, public: false) + .append(key: 'CI_BUILD_TOKEN', value: token.to_s, public: false, masked: true) .append(key: 'CI_REGISTRY_USER', value: CI_REGISTRY_USER) - .append(key: 'CI_REGISTRY_PASSWORD', value: token.to_s, public: false) + .append(key: 'CI_REGISTRY_PASSWORD', value: token.to_s, public: false, masked: true) .append(key: 'CI_REPOSITORY_URL', value: repo_url.to_s, public: false) .concat(deploy_token_variables) end @@ -454,7 +454,7 @@ module Ci break variables unless gitlab_deploy_token variables.append(key: 'CI_DEPLOY_USER', value: gitlab_deploy_token.username) - variables.append(key: 'CI_DEPLOY_PASSWORD', value: gitlab_deploy_token.token, public: false) + variables.append(key: 'CI_DEPLOY_PASSWORD', value: gitlab_deploy_token.token, public: false, masked: true) end end diff --git a/app/models/clusters/kubernetes_namespace.rb b/app/models/clusters/kubernetes_namespace.rb index 73da6cb37d7..7fc75e00cd0 100644 --- a/app/models/clusters/kubernetes_namespace.rb +++ b/app/models/clusters/kubernetes_namespace.rb @@ -37,7 +37,7 @@ module Clusters variables .append(key: 'KUBE_SERVICE_ACCOUNT', value: service_account_name.to_s) .append(key: 'KUBE_NAMESPACE', value: namespace.to_s) - .append(key: 'KUBE_TOKEN', value: service_account_token.to_s, public: false) + .append(key: 'KUBE_TOKEN', value: service_account_token.to_s, public: false, masked: true) .append(key: 'KUBECONFIG', value: kubeconfig, public: false, file: true) end end diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index 814fc591408..fb2221b601f 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -95,7 +95,7 @@ module Clusters # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/22433 variables .append(key: 'KUBE_URL', value: api_url) - .append(key: 'KUBE_TOKEN', value: token, public: false) + .append(key: 'KUBE_TOKEN', value: token, public: false, masked: true) .append(key: 'KUBE_NAMESPACE', value: actual_namespace) .append(key: 'KUBECONFIG', value: kubeconfig, public: false, file: true) end diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb index f69edd60003..4cf3a7f3d84 100644 --- a/app/models/project_services/kubernetes_service.rb +++ b/app/models/project_services/kubernetes_service.rb @@ -113,7 +113,7 @@ class KubernetesService < DeploymentService Gitlab::Ci::Variables::Collection.new.tap do |variables| variables .append(key: 'KUBE_URL', value: api_url) - .append(key: 'KUBE_TOKEN', value: token, public: false) + .append(key: 'KUBE_TOKEN', value: token, public: false, masked: true) .append(key: 'KUBE_NAMESPACE', value: actual_namespace) .append(key: 'KUBECONFIG', value: kubeconfig, public: false, file: true) diff --git a/changelogs/unreleased/58010-mask-the-existing-variables.yml b/changelogs/unreleased/58010-mask-the-existing-variables.yml new file mode 100644 index 00000000000..cc5fdb29686 --- /dev/null +++ b/changelogs/unreleased/58010-mask-the-existing-variables.yml @@ -0,0 +1,5 @@ +--- +title: Mask all TOKEN and PASSWORD CI variables. +merge_request: 25868 +author: +type: changed diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index fc75d3e23fb..dd40b968bc1 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2118,11 +2118,11 @@ describe Ci::Build do { 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_JOB_TOKEN', value: 'my-token', public: false, masked: true }, { 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_BUILD_TOKEN', value: 'my-token', public: false, masked: true }, { 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_REGISTRY_PASSWORD', value: 'my-token', public: false, masked: true }, { 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 }, @@ -2652,7 +2652,7 @@ describe Ci::Build do let(:deploy_token_variables) do [ { key: 'CI_DEPLOY_USER', value: deploy_token.username, public: true, masked: false }, - { key: 'CI_DEPLOY_PASSWORD', value: deploy_token.token, public: false, masked: false } + { key: 'CI_DEPLOY_PASSWORD', value: deploy_token.token, public: false, masked: true } ] end diff --git a/spec/models/clusters/kubernetes_namespace_spec.rb b/spec/models/clusters/kubernetes_namespace_spec.rb index b865909c7fd..579f486f99f 100644 --- a/spec/models/clusters/kubernetes_namespace_spec.rb +++ b/spec/models/clusters/kubernetes_namespace_spec.rb @@ -115,7 +115,7 @@ RSpec.describe Clusters::KubernetesNamespace, type: :model do expect(kubernetes_namespace.predefined_variables).to include( { key: 'KUBE_SERVICE_ACCOUNT', value: kubernetes_namespace.service_account_name, public: true }, { key: 'KUBE_NAMESPACE', value: kubernetes_namespace.namespace, public: true }, - { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false }, + { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false, masked: true }, { key: 'KUBECONFIG', value: kubeconfig, public: false, file: true } ) end diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index 3b32ca8df05..cc93a1b4965 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -269,7 +269,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching it 'sets KUBE_TOKEN' do expect(subject).to include( - { key: 'KUBE_TOKEN', value: kubernetes.token, public: false } + { key: 'KUBE_TOKEN', value: kubernetes.token, public: false, masked: true } ) end end @@ -281,7 +281,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching it 'sets KUBE_TOKEN' do expect(subject).to include( - { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false } + { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false, masked: true } ) end end @@ -297,7 +297,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching it 'sets KUBE_TOKEN' do expect(subject).to include( - { key: 'KUBE_TOKEN', value: kubernetes.token, public: false } + { key: 'KUBE_TOKEN', value: kubernetes.token, public: false, masked: true } ) end end @@ -309,7 +309,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching it 'sets KUBE_TOKEN' do expect(subject).to include( - { key: 'KUBE_TOKEN', value: kubernetes.token, public: false } + { key: 'KUBE_TOKEN', value: kubernetes.token, public: false, masked: true } ) end end @@ -338,7 +338,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching it 'sets KUBE_TOKEN' do expect(subject).to include( - { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false } + { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false, masked: true } ) end end diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb index 9c27357ffaf..47f70e6648a 100644 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -276,7 +276,7 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do it 'sets the variables' do expect(subject.predefined_variables(project: project)).to include( { key: 'KUBE_URL', value: 'https://kube.domain.com', public: true }, - { key: 'KUBE_TOKEN', value: 'token', public: false }, + { key: 'KUBE_TOKEN', value: 'token', public: false, masked: true }, { key: 'KUBE_NAMESPACE', value: namespace, public: true }, { key: 'KUBECONFIG', value: kubeconfig, public: false, file: true }, { key: 'KUBE_CA_PEM', value: 'CA PEM DATA', public: true }, diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b2392f9521f..71bd7972436 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2607,7 +2607,7 @@ describe Project do shared_examples 'same behavior between KubernetesService and Platform::Kubernetes' do it 'returns variables from this service' do expect(project.deployment_variables).to include( - { key: 'KUBE_TOKEN', value: project.deployment_platform.token, public: false } + { key: 'KUBE_TOKEN', value: project.deployment_platform.token, public: false, masked: true } ) end end @@ -2632,7 +2632,7 @@ describe Project do it 'should return token from kubernetes namespace' do expect(project.deployment_variables).to include( - { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false } + { key: 'KUBE_TOKEN', value: kubernetes_namespace.service_account_token, public: false, masked: true } ) end end