diff --git a/app/actions/space_diff_manifest.rb b/app/actions/space_diff_manifest.rb index 6247445d8cd..c5b68006228 100644 --- a/app/actions/space_diff_manifest.rb +++ b/app/actions/space_diff_manifest.rb @@ -1,4 +1,5 @@ require 'presenters/v3/app_manifest_presenter' +require 'presenters/helpers/censorship' require 'messages/app_manifest_message' require 'json-diff' @@ -10,6 +11,8 @@ class SpaceDiffManifest 'sidecars' => 'name' }.freeze + REDACTED_KEYS = %w[credentials cnb-credentials].freeze + class << self # rubocop:todo Metrics/CyclomaticComplexity def generate_diff(app_manifests, space) @@ -56,6 +59,8 @@ def generate_diff(app_manifests, space) # To preserve backwards compability, we've decided to skip diffs that satisfy this conditon next if !nested_attribute_exists && %w[disk_quota disk-quota memory].include?(key) + value = obfuscate_buildpack_urls(value) if %w[buildpack buildpacks].include?(key) + key_diffs = JsonDiff.diff( existing_value, value, @@ -68,6 +73,8 @@ def generate_diff(app_manifests, space) diff['op'] = 'add' if diff['op'] == 'replace' && diff['was'].nil? + redact_credentials(diff) if REDACTED_KEYS.include?(key) + json_diff << diff end end @@ -79,6 +86,18 @@ def generate_diff(app_manifests, space) private + def redact_credentials(diff) + diff['value'] = Presenters::Censorship::PRIVATE_DATA_HIDDEN if diff.key?('value') + diff['was'] = Presenters::Censorship::PRIVATE_DATA_HIDDEN if diff.key?('was') + end + + def obfuscate_buildpack_urls(buildpacks) + return CloudController::UrlSecretObfuscator.obfuscate(buildpacks) if buildpacks.is_a?(String) + return buildpacks unless buildpacks.is_a?(Array) + + buildpacks.map { |bp| bp.is_a?(String) ? CloudController::UrlSecretObfuscator.obfuscate(bp) : bp } + end + # rubocop:todo Metrics/CyclomaticComplexity def filter_manifest_app_hash(manifest_app_hash) if manifest_app_hash.key? 'sidecars' diff --git a/app/messages/app_manifest_message.rb b/app/messages/app_manifest_message.rb index cfd5e71d0a0..0f44fc387a8 100644 --- a/app/messages/app_manifest_message.rb +++ b/app/messages/app_manifest_message.rb @@ -43,6 +43,7 @@ class AppManifestMessage < BaseMessage sidecars stack timeout + credentials cnb_credentials features ] @@ -75,6 +76,7 @@ def self.underscore_keys(hash) validate :validate_docker_enabled! validate :validate_cnb_enabled! validate :validate_docker_buildpacks_combination! + validate :validate_credentials_combination! validate :validate_service_bindings_message!, if: ->(record) { record.requested?(:services) } validate :validate_env_update_message!, if: ->(record) { record.requested?(:env) } validate :validate_manifest_singular_buildpack_message!, if: ->(record) { record.requested?(:buildpack) } @@ -126,6 +128,7 @@ def manifest_features_update_message def audit_hash result = original_yaml.dup result['env'] = Presenters::Censorship::PRIVATE_DATA_HIDDEN if result.key?('env') + result['credentials'] = Presenters::Censorship::PRIVATE_DATA_HIDDEN if result.key?('credentials') result['cnb-credentials'] = Presenters::Censorship::PRIVATE_DATA_HIDDEN if result.key?('cnb-credentials') obfuscate_buildpack_urls(result) redact_service_parameters(result) @@ -133,20 +136,15 @@ def audit_hash end def app_lifecycle_hash - lifecycle_type = if requested?(:lifecycle) && @lifecycle == 'cnb' - Lifecycles::CNB - elsif requested?(:lifecycle) && @lifecycle == 'buildpack' - Lifecycles::BUILDPACK - elsif requested?(:docker) - Lifecycles::DOCKER - end + lifecycle_type = requested_lifecycle_type data = { buildpacks: requested_buildpacks, - stack: @stack, - credentials: @cnb_credentials + stack: @stack }.compact + data[:credentials] = @credentials || @cnb_credentials if requested?(:credentials) || requested?(:cnb_credentials) + if lifecycle_type == Lifecycles::DOCKER lifecycle = { type: Lifecycles::DOCKER @@ -496,6 +494,19 @@ def validate_docker_buildpacks_combination! errors.add(:base, 'Cannot specify both buildpack(s) and docker keys') end + def validate_credentials_combination! + return unless requested?(:credentials) && requested?(:cnb_credentials) + + errors.add(:base, 'Cannot specify both credentials and cnb-credentials keys') + end + + def requested_lifecycle_type + return Lifecycles::CNB if requested?(:lifecycle) && @lifecycle == 'cnb' + return Lifecycles::BUILDPACK if requested?(:lifecycle) && @lifecycle == 'buildpack' + + Lifecycles::DOCKER if requested?(:docker) + end + def validate_features! manifest_features_update_message.valid? manifest_features_update_message.errors.full_messages.each do |error_message| diff --git a/app/models/runtime/buildpack_lifecycle_data_model.rb b/app/models/runtime/buildpack_lifecycle_data_model.rb index 99b11e297ed..7334bfc331a 100644 --- a/app/models/runtime/buildpack_lifecycle_data_model.rb +++ b/app/models/runtime/buildpack_lifecycle_data_model.rb @@ -9,6 +9,9 @@ class BuildpackLifecycleDataModel < Sequel::Model(:buildpack_lifecycle_data) LIFECYCLE_TYPE = Lifecycles::BUILDPACK set_field_as_encrypted :buildpack_url, salt: :encrypted_buildpack_url_salt, column: :encrypted_buildpack_url + set_field_as_encrypted :credentials_json, + salt: :encrypted_credentials_json_salt, + column: :encrypted_credentials_json many_to_one :droplet, class: '::VCAP::CloudController::DropletModel', @@ -85,11 +88,23 @@ def first_custom_buildpack_url buildpack_lifecycle_buildpacks.find(&:custom?)&.buildpack_url || legacy_buildpack_url end + def credentials + return unless credentials_json + + Oj.load(credentials_json) + end + + def credentials=(creds) + self.credentials_json = creds.nil? ? nil : Oj.dump(creds) + end + def to_hash - { + hash = { buildpacks: obfuscated_buildpacks, stack: stack } + hash[:credentials] = Presenters::Censorship::REDACTED_CREDENTIAL unless credentials.nil? + hash end def validate diff --git a/app/presenters/v3/app_manifest_presenters/buildpack_presenter.rb b/app/presenters/v3/app_manifest_presenters/buildpack_presenter.rb index 81d0ea882f9..9425dfa3647 100644 --- a/app/presenters/v3/app_manifest_presenters/buildpack_presenter.rb +++ b/app/presenters/v3/app_manifest_presenters/buildpack_presenter.rb @@ -1,3 +1,5 @@ +require 'presenters/helpers/censorship' + module VCAP::CloudController module Presenters module V3 @@ -7,8 +9,9 @@ def to_hash(app:, **_) return {} if app.docker? { - buildpacks: app.lifecycle_data.buildpacks.presence, - stack: app.lifecycle_data.stack + buildpacks: app.lifecycle_data.obfuscated_buildpacks.presence, + stack: app.lifecycle_data.stack, + credentials: app.lifecycle_data.credentials.nil? ? nil : Presenters::Censorship::REDACTED_CREDENTIAL } end end diff --git a/app/repositories/app_event_repository.rb b/app/repositories/app_event_repository.rb index 4bf531f95cc..c55af1ee42c 100644 --- a/app/repositories/app_event_repository.rb +++ b/app/repositories/app_event_repository.rb @@ -207,6 +207,7 @@ def obfuscate_buildpack_urls(changes) lifecycle_data['buildpack'] = CloudController::UrlSecretObfuscator.obfuscate(lifecycle_data['buildpack']) if lifecycle_data['buildpack'] lifecycle_data['buildpacks'] = lifecycle_data['buildpacks'].map { |b| CloudController::UrlSecretObfuscator.obfuscate(b) } if lifecycle_data['buildpacks'] + lifecycle_data['credentials'] = Presenters::Censorship::PRIVATE_DATA_HIDDEN if lifecycle_data.key?('credentials') end end end diff --git a/db/migrations/20261001120000_add_buildpack_lifecycle_credentials.rb b/db/migrations/20261001120000_add_buildpack_lifecycle_credentials.rb new file mode 100644 index 00000000000..4292830cda1 --- /dev/null +++ b/db/migrations/20261001120000_add_buildpack_lifecycle_credentials.rb @@ -0,0 +1,18 @@ +Sequel.migration do + up do + add_column :buildpack_lifecycle_data, :encrypted_credentials_json, String, size: 16_000, if_not_exists: true + add_column :buildpack_lifecycle_data, :encrypted_credentials_json_salt, String, size: 255, if_not_exists: true + # encryption_key_label and encryption_iterations already exist (added by earlier migrations for buildpack_url encryption) + + # Existing CNB credentials column was created unsized (varchar(255) on MySQL), + # too small for an encrypted, base64-encoded multi-host credentials map. Grow it to match. + set_column_type :cnb_lifecycle_data, :encrypted_registry_credentials_json, String, size: 16_000 + end + + down do + drop_column :buildpack_lifecycle_data, :encrypted_credentials_json, if_exists: true + drop_column :buildpack_lifecycle_data, :encrypted_credentials_json_salt, if_exists: true + + set_column_type :cnb_lifecycle_data, :encrypted_registry_credentials_json, String, size: 255 + end +end diff --git a/docs/v3/source/includes/concepts/_lifecycles.md.erb b/docs/v3/source/includes/concepts/_lifecycles.md.erb index 9b25ec75430..707a859957a 100644 --- a/docs/v3/source/includes/concepts/_lifecycles.md.erb +++ b/docs/v3/source/includes/concepts/_lifecycles.md.erb @@ -41,6 +41,7 @@ Name | Type | Description **type** | _string_ | `buildpack` **data.buildpacks** | _list of strings_ | A list of the names of buildpacks, URLs from which they may be downloaded, or `null` to auto-detect a suitable buildpack during staging **data.stack** | _string_ | The root filesystem to use with the buildpack, for example `cflinuxfs4` +**data.credentials** | _object_ | Credentials used to download custom buildpacks from private git repositories, keyed by host. Each entry is a username/password pair. If any buildpack URL embeds credentials, **data.credentials** must be empty; provide credentials in one place only. For records whose historical lifecycle data is no longer available, **data.buildpacks** may be an empty list and **data.stack** may be `null`. @@ -83,7 +84,7 @@ Name | Type | Description ---- | ---- | ----------- **type** | _string_ | `cnb` **data.buildpacks** | _list of strings_ | A list of URLs with either `docker://` or `http(s)://` scheme, pointing to the Cloud Native Buildpack OCI image.
When the scheme is `http(s)://`, an OCI tarball is expected to be present at the specified location. -**data.credentials** | _object_ | Credentials used to download the configured buildpacks. This can either contain username/password or a token. +**data.credentials** | _object_ | Credentials used to download the configured buildpacks, keyed by registry host. Each entry contains either a username/password pair or a token. CNB buildpack URLs must not embed credentials; use **data.credentials** instead. **data.stack** | _string_ | The root filesystem to use with the buildpack, for example `cflinuxfs4` For records whose historical lifecycle data is no longer available, diff --git a/docs/v3/source/includes/resources/manifests/_object.md.erb b/docs/v3/source/includes/resources/manifests/_object.md.erb index 9542902f705..8727c331be8 100644 --- a/docs/v3/source/includes/resources/manifests/_object.md.erb +++ b/docs/v3/source/includes/resources/manifests/_object.md.erb @@ -105,6 +105,8 @@ Name | Type | Description **name** | _string_ | Name of the app **lifecycle** | _string_ | Type of the lifecycle; valid values are `buildpack`, `cnb`, `docker`. Defaults to `buildpack`, unless the **docker** property is present. **buildpacks** | _list of strings_ | a) An empty array, which will automatically select the appropriate default buildpack according to the coding language (incompatible with **lifecycle: cnb**)
b) An array of one or more URLs pointing to buildpacks
c) An array of one or more installed buildpack names
Replaces the legacy `buildpack` field +**credentials** | _object_ | Credentials used to download custom buildpacks (buildpack lifecycle) or buildpack OCI images (cnb lifecycle) from private git repositories or registries, keyed by host. For the buildpack lifecycle each entry is a username/password pair; for the cnb lifecycle each entry is a username/password pair or a token. Set to `null` to clear previously stored credentials. If any buildpack URL embeds credentials, **credentials** must be empty; provide credentials in one place only. +**cnb-credentials** | _object_ | Backward-compatible alias for **credentials**. Specifying both **credentials** and **cnb-credentials** is an error. **docker** | _object_ | If present, the created app will have Docker lifecycle type; the value of this key is ignored by the API but may be used by clients to source the registry address of the image and credentials, if needed; the [generate manifest endpoint](#generate-a-manifest-for-an-app) will return the registry address of the image and username provided with this key **env** | _object_ | A key-value mapping of environment variables to be used for the app when running **features** | _object_ | A key-value mapping of feature names to booleans (true = enabled, false = disabled) diff --git a/lib/cloud_controller/diego/buildpack_entry_generator.rb b/lib/cloud_controller/diego/buildpack_entry_generator.rb index ebd66c3923b..6e2164f2081 100644 --- a/lib/cloud_controller/diego/buildpack_entry_generator.rb +++ b/lib/cloud_controller/diego/buildpack_entry_generator.rb @@ -6,14 +6,15 @@ def initialize(blobstore_url_generator, type) @type = type end - def buildpack_entries(buildpack_infos, stack_name) + def buildpack_entries(buildpack_infos, stack_name, credentials: nil) return default_admin_buildpacks(stack_name) if buildpack_infos.empty? buildpack_infos.map do |buildpack_info| if buildpack_info.buildpack_exists_in_db? && buildpack_info.buildpack_enabled? admin_buildpack_entry(buildpack_info.buildpack_record).merge(skip_detect: true) elsif buildpack_info.buildpack_url - { name: 'custom', key: buildpack_info.buildpack_url, url: buildpack_info.buildpack_url, skip_detect: true } + url = inject_credentials(buildpack_info.buildpack_url, credentials) + { name: 'custom', key: url, url: url, skip_detect: true } else raise "Unsupported buildpack type: '#{buildpack_info.buildpack}'" end @@ -22,6 +23,25 @@ def buildpack_entries(buildpack_infos, stack_name) private + def inject_credentials(url, credentials) + return url if credentials.blank? + + credentials = credentials.deep_stringify_keys + uri = URI.parse(url) + cred = credentials[uri.host] + return url if cred.blank? + + username = cred['username'] + password = cred['password'] + return url if username.blank? + + uri.user = CGI.escape(username) + uri.password = CGI.escape(password) if password.present? + uri.to_s + rescue URI::InvalidURIError + url + end + def custom_buildpack_entry(buildpack) { name: 'custom', key: buildpack.url, url: buildpack.url } end diff --git a/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb b/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb index 9bce82bf361..dbe75fa2714 100644 --- a/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb +++ b/lib/cloud_controller/diego/cnb/lifecycle_protocol.rb @@ -30,6 +30,11 @@ def new_lifecycle_data(staging_details) lifecycle_data end + # CNB credentials travel via CNB_REGISTRY_CREDS, never in the buildpack URL. + def url_credentials_for_staging(_staging_details) + nil + end + def type VCAP::CloudController::Lifecycles::CNB end diff --git a/lib/cloud_controller/diego/lifecycle_protocol.rb b/lib/cloud_controller/diego/lifecycle_protocol.rb index 967a8290e12..de76fbe3db8 100644 --- a/lib/cloud_controller/diego/lifecycle_protocol.rb +++ b/lib/cloud_controller/diego/lifecycle_protocol.rb @@ -34,8 +34,12 @@ def lifecycle_data(staging_details) lifecycle_data.build_artifacts_cache_download_uri = @blobstore_url_generator.buildpack_cache_download_url(staging_details.package.app_guid, stack) lifecycle_data.build_artifacts_cache_upload_uri = @blobstore_url_generator.buildpack_cache_upload_url(staging_details.package.app_guid, stack) lifecycle_data.droplet_upload_uri = @blobstore_url_generator.droplet_upload_url(staging_details.staging_guid) - lifecycle_data.buildpacks = @buildpack_entry_generator.buildpack_entries(staging_details.lifecycle.buildpack_infos, stack) - lifecycle_data.stack = stack + lifecycle_data.buildpacks = @buildpack_entry_generator.buildpack_entries( + staging_details.lifecycle.buildpack_infos, + stack, + credentials: url_credentials_for_staging(staging_details) + ) + lifecycle_data.stack = stack lifecycle_data.message rescue Membrane::SchemaValidationError => e @@ -45,6 +49,11 @@ def lifecycle_data(staging_details) raise InvalidDownloadUri.new("Failed to get blobstore download url for package #{staging_details.package.guid}") end + # Buildpack lifecycle injects these into custom buildpack URLs; CNB returns nil (uses CNB_REGISTRY_CREDS). + def url_credentials_for_staging(staging_details) + staging_details.lifecycle.credentials_for_staging + end + private def droplet_checksum_info(droplet) diff --git a/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb b/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb index 91bdd2b5f44..1c2cffdf67d 100644 --- a/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb +++ b/lib/cloud_controller/diego/lifecycles/app_base_lifecycle.rb @@ -1,27 +1,50 @@ +require 'cloud_controller/diego/lifecycles/buildpack_info' +require 'cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator' +require 'fetchers/buildpack_lifecycle_fetcher' + module VCAP::CloudController class AppBaseLifecycle - def update_lifecycle_data_model(app) - if [update_lifecycle_data_buildpacks(app), - update_lifecycle_data_stack(app)].any? - app.lifecycle_data.save - end - end + def initialize(message, app=nil) + @message = message - def update_lifecycle_data_buildpacks(app) - return unless message.buildpack_data.requested?(:buildpacks) + db_result = BuildpackLifecycleFetcher.fetch(effective_buildpacks(app), stack, type) - app.lifecycle_data.buildpacks = buildpacks + @validator = validator_class.new( + buildpack_infos: db_result[:buildpack_infos], + stack: db_result[:stack], + credentials: effective_credentials(app), + credentials_from_message: message.buildpack_data.requested?(:credentials) + ) end - def update_lifecycle_data_stack(app) - return unless message.buildpack_data.requested?(:stack) + delegate :valid?, :errors, to: :validator + + def update_lifecycle_data_model(app) + requested = %i[buildpacks stack credentials].select { |field| message.buildpack_data.requested?(field) } + return if requested.empty? - app.lifecycle_data.stack = message.buildpack_data.stack + app.lifecycle_data.buildpacks = buildpacks if requested.include?(:buildpacks) + app.lifecycle_data.stack = message.buildpack_data.stack if requested.include?(:stack) + app.lifecycle_data.credentials = message.buildpack_data.credentials if requested.include?(:credentials) + app.lifecycle_data.save end private - attr_reader :message + attr_reader :message, :validator + + def validator_class + BuildpackLifecycleDataValidator + end + + def effective_buildpacks(app) + message.buildpack_data.requested?(:buildpacks) ? buildpacks : (app&.lifecycle_data&.buildpacks || []) + end + + def effective_credentials(app) + stored = app&.lifecycle_data&.then { |ld| ld.respond_to?(:credentials) ? ld.credentials : nil } + message.buildpack_data.requested?(:credentials) ? message.buildpack_data.credentials : stored + end def buildpacks message.buildpack_data.requested?(:buildpacks) ? (message.buildpack_data.buildpacks || []) : [] diff --git a/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle.rb b/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle.rb index 994b46d9981..4cd343dca96 100644 --- a/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle.rb +++ b/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle.rb @@ -1,36 +1,18 @@ -require 'cloud_controller/diego/lifecycles/buildpack_info' -require 'cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator' require 'cloud_controller/diego/lifecycles/app_base_lifecycle' -require 'fetchers/buildpack_lifecycle_fetcher' module VCAP::CloudController class AppBuildpackLifecycle < AppBaseLifecycle - def initialize(message) - @message = message - - db_result = BuildpackLifecycleFetcher.fetch(buildpacks, stack, type) - @validator = BuildpackLifecycleDataValidator.new({ - buildpack_infos: db_result[:buildpack_infos], - stack: db_result[:stack] - }) - end - - delegate :valid?, :errors, to: :validator - def create_lifecycle_data_model(app) app.buildpack_lifecycle_data = BuildpackLifecycleDataModel.create( - buildpacks:, - stack:, - app: + buildpacks: buildpacks, + stack: stack, + credentials: message.buildpack_data.credentials, + app: app ) end def type Lifecycles::BUILDPACK end - - private - - attr_reader :validator end end diff --git a/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle.rb b/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle.rb index b056a704e21..9618f91979b 100644 --- a/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle.rb +++ b/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle.rb @@ -1,47 +1,25 @@ -require 'cloud_controller/diego/lifecycles/buildpack_info' -require 'cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator' +require 'cloud_controller/diego/lifecycles/cnb_lifecycle_data_validator' require 'cloud_controller/diego/lifecycles/app_base_lifecycle' -require 'fetchers/buildpack_lifecycle_fetcher' module VCAP::CloudController class AppCNBLifecycle < AppBaseLifecycle - def initialize(message) - @message = message - - db_result = BuildpackLifecycleFetcher.fetch(buildpacks, stack, type) - @validator = BuildpackLifecycleDataValidator.new({ - buildpack_infos: db_result[:buildpack_infos], - stack: db_result[:stack] - }) - end - - delegate :valid?, :errors, to: :validator - def create_lifecycle_data_model(app) app.cnb_lifecycle_data = CNBLifecycleDataModel.create( - buildpacks:, - stack:, - credentials:, - app: + buildpacks: buildpacks, + stack: stack, + credentials: message.buildpack_data.credentials, + app: app ) end - def update_lifecycle_data_credentials(app) - return unless message.buildpack_data.requested?(:credentials) - - app.lifecycle_data.credentials = message.buildpack_data.credentials - end - def type Lifecycles::CNB end - def credentials - message.buildpack_data.credentials - end - private - attr_reader :validator + def validator_class + CNBLifecycleDataValidator + end end end diff --git a/lib/cloud_controller/diego/lifecycles/app_lifecycle_provider.rb b/lib/cloud_controller/diego/lifecycles/app_lifecycle_provider.rb index 39c9e9d5085..73edad7587e 100644 --- a/lib/cloud_controller/diego/lifecycles/app_lifecycle_provider.rb +++ b/lib/cloud_controller/diego/lifecycles/app_lifecycle_provider.rb @@ -28,7 +28,7 @@ def self.provide(message, app) Config.config.get(:default_app_lifecycle) end - TYPE_TO_LIFECYCLE_CLASS_MAP[type].new(message) + TYPE_TO_LIFECYCLE_CLASS_MAP[type].new(message, app) end private_class_method :provide end diff --git a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb index 08bed855085..7196895458c 100644 --- a/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb +++ b/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator.rb @@ -4,10 +4,11 @@ module VCAP::CloudController class BuildpackLifecycleDataValidator include ActiveModel::Model - attr_accessor :buildpack_infos, :stack + attr_accessor :buildpack_infos, :stack, :credentials, :credentials_from_message validate :buildpacks_are_uri_or_nil validate :stack_exists_in_db + validate :no_dual_credential_sources def buildpacks_are_uri_or_nil buildpack_infos.each do |buildpack_info| @@ -26,5 +27,22 @@ def buildpacks_are_uri_or_nil def stack_exists_in_db errors.add(:stack, 'must be an existing stack') if stack.nil? end + + def no_dual_credential_sources + return unless url_has_userinfo? && credentials.present? + + if credentials_from_message + errors.add(:base, 'Provide buildpack credentials either in the buildpack URI or in data.credentials, not both') + else + errors.add(:base, 'This app already has stored buildpack credentials (data.credentials); a buildpack URI with embedded credentials conflicts with them. ' \ + 'Remove the credentials from the URI, or clear the stored credentials with "credentials: null".') + end + end + + private + + def url_has_userinfo? + buildpack_infos.any? { |bi| bi.buildpack_url && UriUtils.url_has_userinfo?(bi.buildpack_url) } + end end end diff --git a/lib/cloud_controller/diego/lifecycles/cnb_lifecycle.rb b/lib/cloud_controller/diego/lifecycles/cnb_lifecycle.rb index da797f81c93..a82c202ec39 100644 --- a/lib/cloud_controller/diego/lifecycles/cnb_lifecycle.rb +++ b/lib/cloud_controller/diego/lifecycles/cnb_lifecycle.rb @@ -1,6 +1,6 @@ require 'cloud_controller/diego/lifecycles/buildpack_info' require 'cloud_controller/diego/lifecycles/lifecycle_base' -require 'cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator' +require 'cloud_controller/diego/lifecycles/cnb_lifecycle_data_validator' require 'fetchers/buildpack_lifecycle_fetcher' module VCAP::CloudController @@ -14,7 +14,7 @@ def create_lifecycle_data_model(build) buildpacks: Array(buildpacks_to_use), stack: staging_stack, build: build, - credentials: credentials_to_use + credentials: credentials_for_staging ) end @@ -23,17 +23,17 @@ def staging_environment_variables end def credentials - Oj.dump(credentials_to_use) + Oj.dump(credentials_for_staging) end private - def app_stack - @package.app.cnb_lifecycle_data.try(:stack) + def validator_class + CNBLifecycleDataValidator end - def credentials_to_use - @staging_message.buildpack_data.credentials || @package.app.lifecycle_data.credentials + def app_stack + @package.app.cnb_lifecycle_data.try(:stack) end end end diff --git a/lib/cloud_controller/diego/lifecycles/cnb_lifecycle_data_validator.rb b/lib/cloud_controller/diego/lifecycles/cnb_lifecycle_data_validator.rb new file mode 100644 index 00000000000..a09e6f6e31c --- /dev/null +++ b/lib/cloud_controller/diego/lifecycles/cnb_lifecycle_data_validator.rb @@ -0,0 +1,16 @@ +require 'cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator' + +module VCAP::CloudController + class CNBLifecycleDataValidator < BuildpackLifecycleDataValidator + validate :no_cnb_userinfo_in_url + + def no_cnb_userinfo_in_url + buildpack_infos.each do |buildpack_info| + next unless buildpack_info.buildpack_url + next unless UriUtils.url_has_userinfo?(buildpack_info.buildpack_url) + + errors.add(:buildpack, 'CNB buildpack URIs must not contain credentials; use data.credentials instead') + end + end + end +end diff --git a/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb b/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb index 671d0bd3d92..1e78212858a 100644 --- a/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb +++ b/lib/cloud_controller/diego/lifecycles/lifecycle_base.rb @@ -3,7 +3,7 @@ module VCAP::CloudController class LifecycleBase - attr_reader :staging_message, :buildpack_infos + attr_reader :staging_message, :buildpack_infos, :credentials_for_staging def initialize(package, staging_message) @staging_message = staging_message @@ -11,7 +11,16 @@ def initialize(package, staging_message) db_result = BuildpackLifecycleFetcher.fetch(buildpacks_to_use, staging_stack, type) @buildpack_infos = db_result[:buildpack_infos] - @validator = BuildpackLifecycleDataValidator.new({ buildpack_infos: buildpack_infos, stack: db_result[:stack] }) + + stored = @package.app.lifecycle_data&.then { |ld| ld.respond_to?(:credentials) ? ld.credentials : nil } + @credentials_for_staging = staging_message.buildpack_data.requested?(:credentials) ? staging_message.buildpack_data.credentials : stored + + @validator = validator_class.new( + buildpack_infos: @buildpack_infos, + stack: db_result[:stack], + credentials: @credentials_for_staging, + credentials_from_message: staging_message.buildpack_data.requested?(:credentials) + ) end delegate :valid?, :errors, to: :validator @@ -22,6 +31,10 @@ def staging_stack private + def validator_class + BuildpackLifecycleDataValidator + end + def buildpacks_to_use staging_message.buildpack_data.buildpacks || @package.app.lifecycle_data.buildpacks end diff --git a/lib/utils/uri_utils.rb b/lib/utils/uri_utils.rb index 828aef15fda..eb43d3a80e7 100644 --- a/lib/utils/uri_utils.rb +++ b/lib/utils/uri_utils.rb @@ -35,6 +35,15 @@ def is_cnb_buildpack_uri?(candidate) false end + def url_has_userinfo?(url) + return false unless url.is_a?(String) + + uri = URI.parse(url) + uri.user.present? || uri.password.present? + rescue URI::InvalidURIError + false + end + def is_uri_path?(candidate) !!(candidate.is_a?(String) && candidate =~ %r{^(?:/|/([^\s/]\S*)?)$}) end diff --git a/spec/migrations/20261001120000_add_buildpack_lifecycle_credentials_spec.rb b/spec/migrations/20261001120000_add_buildpack_lifecycle_credentials_spec.rb new file mode 100644 index 00000000000..69ae5b27284 --- /dev/null +++ b/spec/migrations/20261001120000_add_buildpack_lifecycle_credentials_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' +require 'migrations/helpers/migration_shared_context' + +RSpec.describe 'migration to add encrypted_credentials_json columns to buildpack_lifecycle_data', isolation: :truncation, type: :migration do + include_context 'migration' do + let(:migration_filename) { '20261001120000_add_buildpack_lifecycle_credentials.rb' } + end + + describe 'buildpack_lifecycle_data table' do + it 'adds and removes credential columns, handling idempotency gracefully' do + # Before migration: columns must not exist + expect(db[:buildpack_lifecycle_data].columns).not_to include(:encrypted_credentials_json) + expect(db[:buildpack_lifecycle_data].columns).not_to include(:encrypted_credentials_json_salt) + + # Run up migration + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error + expect(db[:buildpack_lifecycle_data].columns).to include(:encrypted_credentials_json) + expect(db[:buildpack_lifecycle_data].columns).to include(:encrypted_credentials_json_salt) + + # Run up again - idempotent due to if_not_exists + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error + expect(db[:buildpack_lifecycle_data].columns).to include(:encrypted_credentials_json) + expect(db[:buildpack_lifecycle_data].columns).to include(:encrypted_credentials_json_salt) + + # Run down migration + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error + expect(db[:buildpack_lifecycle_data].columns).not_to include(:encrypted_credentials_json) + expect(db[:buildpack_lifecycle_data].columns).not_to include(:encrypted_credentials_json_salt) + + # Run down again - idempotent due to if_exists + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error + expect(db[:buildpack_lifecycle_data].columns).not_to include(:encrypted_credentials_json) + expect(db[:buildpack_lifecycle_data].columns).not_to include(:encrypted_credentials_json_salt) + end + end +end diff --git a/spec/unit/actions/space_diff_manifest_spec.rb b/spec/unit/actions/space_diff_manifest_spec.rb index d5d5afd2d39..b16441c9e1f 100644 --- a/spec/unit/actions/space_diff_manifest_spec.rb +++ b/spec/unit/actions/space_diff_manifest_spec.rb @@ -83,6 +83,82 @@ module VCAP::CloudController end end + %w[credentials cnb-credentials].each do |key| + context "when the manifest specifies #{key}" do + before do + default_manifest['applications'][0][key] = { 'registry.example.com' => { 'username' => 'user', 'password' => 'sekret' } } + end + + it "redacts the #{key} value in the diff" do + credentials_diff = subject.find { |d| d['path'] == "/applications/0/#{key}" } + + expect(credentials_diff).not_to be_nil + expect(credentials_diff['value']).to eq(Presenters::Censorship::PRIVATE_DATA_HIDDEN) + expect(subject.to_s).not_to include('sekret') + end + end + end + + context 'when the existing app already has stored credentials and the manifest submits new ones' do + before do + app1_model.lifecycle_data.credentials = { 'registry.example.com' => { 'username' => 'stored', 'password' => 'stored-secret' } } + app1_model.lifecycle_data.save + default_manifest['applications'][0]['credentials'] = { 'registry.example.com' => { 'username' => 'user', 'password' => 'new-secret' } } + end + + it 'does not leak either the stored or submitted secret' do + expect(subject.to_s).not_to include('stored-secret') + expect(subject.to_s).not_to include('new-secret') + end + + it 'redacts both sides of any credentials diff entry' do + subject.select { |d| d['path'] == '/applications/0/credentials' }.each do |d| + expect(d['value']).to eq(Presenters::Censorship::PRIVATE_DATA_HIDDEN) if d.key?('value') + expect(d['was']).to eq(Presenters::Censorship::PRIVATE_DATA_HIDDEN) if d.key?('was') + end + end + end + + context 'when the existing app has credentials embedded in a buildpack URL' do + before do + app1_model.lifecycle_data.update(buildpacks: ['https://user:sekret@git.example.com/greendale/community-buildpack'], stack: process1.stack.name) + default_manifest['applications'][0]['buildpacks'] = ['https://git.example.com/greendale/spanish-101-buildpack'] + end + + it 'does not leak the embedded credentials in the diff' do + expect(subject.to_s).not_to include('sekret') + end + end + + context 'when the submitted manifest has credentials embedded in a buildpack URL' do + before do + app1_model.lifecycle_data.update(buildpacks: ['https://git.example.com/greendale/community-buildpack'], stack: process1.stack.name) + default_manifest['applications'][0]['buildpacks'] = ['https://user:sekret@git.example.com/greendale/community-buildpack'] + end + + it 'obfuscates the embedded credentials in the submitted buildpack URL' do + expect(subject.to_s).not_to include('sekret') + add_diff = subject.find { |d| d['path'].start_with?('/applications/0/buildpacks') && d.key?('value') } + expect(add_diff).not_to be_nil + expect(add_diff['value']).to eq('https://***:***@git.example.com/greendale/community-buildpack') + end + end + + context 'when the submitted manifest has credentials in the singular buildpack URL' do + before do + app1_model.lifecycle_data.update(buildpacks: ['https://git.example.com/greendale/community-buildpack'], stack: process1.stack.name) + default_manifest['applications'][0].delete('buildpacks') + default_manifest['applications'][0]['buildpack'] = 'https://user:sekret@git.example.com/greendale/community-buildpack' + end + + it 'obfuscates the embedded credentials in the submitted singular buildpack URL' do + expect(subject.to_s).not_to include('sekret') + bp_diff = subject.find { |d| d['path'] == '/applications/0/buildpack' && d.key?('value') } + expect(bp_diff).not_to be_nil + expect(bp_diff['value']).to eq('https://***:***@git.example.com/greendale/community-buildpack') + end + end + context 'processes' do context 'when processes are updated' do before do diff --git a/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb index 43b0d46d4f5..8d951296c97 100644 --- a/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/buildpack/lifecycle_protocol_spec.rb @@ -29,7 +29,8 @@ module Buildpack Diego::StagingDetails.new.tap do |details| details.staging_guid = droplet.guid details.package = package - details.lifecycle = instance_double(BuildpackLifecycle, staging_stack: 'potato-stack', buildpack_infos: buildpack_infos) + details.lifecycle = instance_double(BuildpackLifecycle, staging_stack: 'potato-stack', buildpack_infos: buildpack_infos, + credentials_for_staging: nil) end end let(:buildpack_infos) { [BuildpackInfo.new('http://some-buildpack.url', nil)] } @@ -59,7 +60,8 @@ module Buildpack details.environment_variables = { 'nightshade_fruit' => 'potato' } details.staging_memory_in_mb = 42 details.staging_disk_in_mb = 51 - details.lifecycle = instance_double(BuildpackLifecycle, staging_stack: 'potato-stack', buildpack_infos: buildpack_infos) + details.lifecycle = instance_double(BuildpackLifecycle, staging_stack: 'potato-stack', buildpack_infos: buildpack_infos, + credentials_for_staging: nil) end end @@ -100,6 +102,29 @@ module Buildpack end end + context 'when a custom buildpack is requested and credentials are provided' do + let(:buildpack) { 'https://tanagra.example.com/kamin/resht-buildpack' } + let(:staging_details) do + Diego::StagingDetails.new.tap do |details| + details.staging_guid = droplet.guid + details.package = package + details.environment_variables = { 'nightshade_fruit' => 'potato' } + details.staging_memory_in_mb = 42 + details.staging_disk_in_mb = 51 + details.lifecycle = instance_double(BuildpackLifecycle, staging_stack: 'potato-stack', buildpack_infos: buildpack_infos, + credentials_for_staging: { 'tanagra.example.com' => { 'username' => 'kamin', + 'password' => 'kaylah' } }) + end + end + + it 'injects the credentials into the custom buildpack URL' do + lifecycle_data = lifecycle_protocol.lifecycle_data(staging_details) + + bp = lifecycle_data[:buildpacks][0] + expect(bp[:url]).to eq('https://kamin:kaylah@tanagra.example.com/kamin/resht-buildpack') + end + end + context 'when the generated message has invalid data' do let(:buildpack_infos) { [] } @@ -135,7 +160,8 @@ module Buildpack let(:droplet) { create(:droplet_model) } let(:staging_details) do StagingDetails.new.tap do |details| - details.lifecycle = instance_double(BuildpackLifecycle, staging_stack: 'potato-stack', buildpack_infos: 'some buildpack info') + details.lifecycle = instance_double(BuildpackLifecycle, staging_stack: 'potato-stack', buildpack_infos: 'some buildpack info', + credentials_for_staging: nil) details.package = package details.staging_guid = droplet.guid end diff --git a/spec/unit/lib/cloud_controller/diego/buildpack_entry_generator_spec.rb b/spec/unit/lib/cloud_controller/diego/buildpack_entry_generator_spec.rb index 7b270405f1a..cdeabd65cb5 100644 --- a/spec/unit/lib/cloud_controller/diego/buildpack_entry_generator_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/buildpack_entry_generator_spec.rb @@ -116,6 +116,77 @@ module Diego expect { buildpack_entry_generator.buildpack_entries(buildpack_infos, stack.name) }.to raise_error(/Unsupported buildpack type/) end end + + context 'when credentials are provided and the custom buildpack host matches' do + let(:creds) { { 'git.example.com' => { 'username' => 'octocat', 'password' => 'hunter2' } } } + let(:custom_buildpack_info) { BuildpackInfo.new('https://git.example.com/acme/ruby-buildpack.git', nil) } + let(:buildpack_infos) { [custom_buildpack_info] } + + it 'injects credentials into the custom buildpack URL as userinfo' do + result = buildpack_entry_generator.buildpack_entries(buildpack_infos, stack.name, credentials: creds) + expect(result.first[:url]).to eq('https://octocat:hunter2@git.example.com/acme/ruby-buildpack.git') + expect(result.first[:key]).to eq('https://octocat:hunter2@git.example.com/acme/ruby-buildpack.git') + end + end + + context 'when the credentials come from a request message (symbol-keyed)' do + let(:message) do + BuildpackLifecycleDataMessage.new(credentials: { 'git.example.com' => { 'username' => 'octocat', 'password' => 'hunter2' } }) + end + let(:custom_buildpack_info) { BuildpackInfo.new('https://git.example.com/acme/ruby-buildpack.git', nil) } + let(:buildpack_infos) { [custom_buildpack_info] } + + it 'injects credentials despite the symbolized keys' do + result = buildpack_entry_generator.buildpack_entries(buildpack_infos, stack.name, credentials: message.credentials) + expect(result.first[:url]).to eq('https://octocat:hunter2@git.example.com/acme/ruby-buildpack.git') + expect(result.first[:key]).to eq('https://octocat:hunter2@git.example.com/acme/ruby-buildpack.git') + end + end + + context 'when credentials are provided but no host matches' do + let(:creds) { { 'other.example.com' => { 'username' => 'u', 'password' => 'p' } } } + let(:custom_buildpack_info) { BuildpackInfo.new('https://git.example.com/acme/ruby-buildpack.git', nil) } + let(:buildpack_infos) { [custom_buildpack_info] } + + it 'emits the URL without modification' do + result = buildpack_entry_generator.buildpack_entries(buildpack_infos, stack.name, credentials: creds) + expect(result.first[:url]).to eq('https://git.example.com/acme/ruby-buildpack.git') + expect(result.first[:key]).to eq('https://git.example.com/acme/ruby-buildpack.git') + end + end + + context 'when a matching credential entry has no username' do + let(:creds) { { 'git.example.com' => { 'password' => 'p' } } } + let(:custom_buildpack_info) { BuildpackInfo.new('https://git.example.com/acme/ruby-buildpack.git', nil) } + let(:buildpack_infos) { [custom_buildpack_info] } + + it 'emits the URL without modification' do + result = buildpack_entry_generator.buildpack_entries(buildpack_infos, stack.name, credentials: creds) + expect(result.first[:url]).to eq('https://git.example.com/acme/ruby-buildpack.git') + expect(result.first[:key]).to eq('https://git.example.com/acme/ruby-buildpack.git') + end + end + + context 'when credentials are provided and buildpack is an admin buildpack' do + let(:creds) { { 'git.example.com' => { 'username' => 'u', 'password' => 'p' } } } + let(:buildpack_infos) { [BuildpackInfo.new('java', java_buildpack)] } + + it 'does not inject credentials into admin buildpack entries' do + result = buildpack_entry_generator.buildpack_entries(buildpack_infos, stack.name, credentials: creds) + expect(result.first[:url]).to eq(admin_buildpack_download_url) + expect(result.first[:url]).not_to include('@') + end + end + + context 'when credentials are nil' do + let(:custom_buildpack_info) { BuildpackInfo.new('https://git.example.com/acme/ruby-buildpack.git', nil) } + let(:buildpack_infos) { [custom_buildpack_info] } + + it 'emits the URL without modification' do + result = buildpack_entry_generator.buildpack_entries(buildpack_infos, stack.name, credentials: nil) + expect(result.first[:url]).to eq('https://git.example.com/acme/ruby-buildpack.git') + end + end end end end diff --git a/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb b/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb index 473b2111b17..49f1e9267db 100644 --- a/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/cnb/lifecycle_protocol_spec.rb @@ -77,6 +77,32 @@ module CNB end end + context 'when a custom buildpack is requested and credentials are present' do + let(:buildpack) { 'docker://registry.example.com/greendale/community-buildpack' } + let(:staging_details) do + Diego::StagingDetails.new.tap do |details| + details.staging_guid = droplet.guid + details.package = package + details.environment_variables = { 'nightshade_fruit' => 'potato' } + details.staging_memory_in_mb = 42 + details.staging_disk_in_mb = 51 + details.lifecycle = instance_double(CNBLifecycle, staging_stack: 'potato-stack', buildpack_infos: buildpack_infos, + credentials: '{"registry.example.com":{"username":"user","password":"sekret"}}', + credentials_for_staging: { 'registry.example.com' => { 'username' => 'user', + 'password' => 'sekret' } }) + end + end + + it 'does NOT inject credentials into the buildpack URL (CNB uses CNB_REGISTRY_CREDS)' do + lifecycle_data = lifecycle_protocol.lifecycle_data(staging_details) + + bp = lifecycle_data[:buildpacks][0] + expect(bp[:url]).to eq('docker://registry.example.com/greendale/community-buildpack') + expect(bp[:url]).not_to include('sekret') + expect(bp[:url]).not_to include('user:') + end + end + context 'when buildpack_infos is empty' do let(:buildpack_infos) { [] } diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle_spec.rb index bceee37ff27..5bcabd74861 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/app_buildpack_lifecycle_spec.rb @@ -102,7 +102,7 @@ module VCAP::CloudController describe 'validation' do let(:validator) { instance_double(BuildpackLifecycleDataValidator) } - let(:stubbed_fetcher_data) { { stack: 'foo', buildpack_infos: 'bar' } } + let(:stubbed_fetcher_data) { { stack: 'foo', buildpack_infos: [] } } before do allow(validator).to receive(:valid?) @@ -115,7 +115,12 @@ module VCAP::CloudController it 'constructs the validator correctly' do lifecycle.valid? - expect(BuildpackLifecycleDataValidator).to have_received(:new).with(buildpack_infos: 'bar', stack: 'foo') + expect(BuildpackLifecycleDataValidator).to have_received(:new).with( + buildpack_infos: [], + stack: 'foo', + credentials: nil, + credentials_from_message: false + ) end it 'delegates #valid? to a BuildpackLifecycleDataValidator' do @@ -128,5 +133,101 @@ module VCAP::CloudController expect(validator).to have_received(:errors) end end + + describe '#create_lifecycle_data_model with credentials' do + context 'when credentials are provided' do + let(:creds) { { 'git.example.com' => { 'username' => 'u', 'password' => 'p' } } } + let(:lifecycle_request_data) { { buildpacks: ['http://git.example.com/acme/bp.git'], credentials: creds } } + let!(:app) { create(:app_model) } + + it 'stores credentials on the lifecycle data model' do + lifecycle.create_lifecycle_data_model(app) + expect(app.buildpack_lifecycle_data.reload.credentials).to eq(creds) + end + end + end + + describe '#update_lifecycle_data_model credentials handling' do + let(:app) { create(:app_model, :buildpack) } + + context 'when credentials are requested' do + let(:creds) { { 'git.example.com' => { 'username' => 'u', 'password' => 'p' } } } + let(:message) { AppUpdateMessage.new({ lifecycle: { type: 'buildpack', data: { credentials: creds } } }) } + + it 'updates the credentials on the lifecycle data' do + lifecycle.update_lifecycle_data_model(app) + expect(app.buildpack_lifecycle_data.reload.credentials).to eq(creds) + end + end + + context 'when credentials are set to null (clear)' do + let(:message) { AppUpdateMessage.new({ lifecycle: { type: 'buildpack', data: { credentials: nil } } }) } + + before do + app.buildpack_lifecycle_data.update(credentials_json: Oj.dump({ 'git.example.com' => { 'username' => 'u' } })) + end + + it 'clears the credentials' do + lifecycle.update_lifecycle_data_model(app) + expect(app.buildpack_lifecycle_data.reload.credentials).to be_nil + end + end + + context 'when credentials are absent from the request (not requested)' do + let(:message) { AppUpdateMessage.new({ lifecycle: { type: 'buildpack', data: { stack: app.buildpack_lifecycle_data.stack } } }) } + + before do + stored_creds = { 'git.example.com' => { 'username' => 'stored' } } + app.buildpack_lifecycle_data.update(credentials_json: Oj.dump(stored_creds)) + end + + it 'leaves stored credentials untouched' do + lifecycle.update_lifecycle_data_model(app) + expect(app.buildpack_lifecycle_data.reload.credentials).to eq({ 'git.example.com' => { 'username' => 'stored' } }) + end + end + + context 'when PATCH sends only credentials and stored buildpack URL has userinfo' do + let(:app) { create(:app_model, :buildpack) } + let(:creds) { { 'git.example.com' => { 'username' => 'u', 'password' => 'p' } } } + let(:message) { AppUpdateMessage.new({ lifecycle: { type: 'buildpack', data: { credentials: creds } } }) } + + before do + app.buildpack_lifecycle_data.update(buildpacks: ['https://user:pass@git.example.com/bp.git']) + end + + it 'is invalid because stored buildpack URL already has userinfo' do + lc = AppBuildpackLifecycle.new(message, app) + expect(lc.valid?).to be false + expect(lc.errors.full_messages.join).to match(/credentials/) + end + end + + context 'when the request submits a buildpack URL with userinfo and the app has stored credentials' do + let(:app) { create(:app_model, :buildpack) } + let(:message) { AppUpdateMessage.new({ lifecycle: { type: 'buildpack', data: { buildpacks: ['https://user:pass@git.example.com/bp.git'] } } }) } + + before do + app.buildpack_lifecycle_data.credentials = { 'git.example.com' => { 'username' => 'stored', 'password' => 'stored' } } + app.buildpack_lifecycle_data.save + end + + it 'is invalid and explains how to clear the stored credentials' do + lc = AppBuildpackLifecycle.new(message, app) + expect(lc.valid?).to be false + expect(lc.errors.full_messages.join).to include('already has stored buildpack credentials') + expect(lc.errors.full_messages.join).to include('credentials: null') + end + end + + context 'when the app is currently a docker app' do + let(:app) { create(:app_model, :docker) } + let(:message) { AppUpdateMessage.new({ lifecycle: { type: 'buildpack', data: {} } }) } + + it 'does not raise when resolving credentials from docker lifecycle data' do + expect { AppBuildpackLifecycle.new(message, app) }.not_to raise_error + end + end + end end end diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle_spec.rb index 08e3d4701fe..34642d8fa44 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/app_cnb_lifecycle_spec.rb @@ -125,5 +125,47 @@ module VCAP::CloudController end end end + + describe '#update_lifecycle_data_model credentials handling' do + let(:app) { create(:app_model, :cnb) } + + context 'when credentials are requested' do + let(:creds) { { 'registry.io' => { 'username' => 'u', 'password' => 'p' } } } + let(:message) { AppUpdateMessage.new({ lifecycle: { type: 'cnb', data: { credentials: creds } } }) } + + it 'updates the credentials on the lifecycle data' do + lifecycle.update_lifecycle_data_model(app) + expect(app.cnb_lifecycle_data.reload.credentials).to eq(creds) + end + end + + context 'when credentials are set to null (clear)' do + let(:message) { AppUpdateMessage.new({ lifecycle: { type: 'cnb', data: { credentials: nil } } }) } + + before do + app.cnb_lifecycle_data.credentials = { 'registry.io' => { 'username' => 'u' } } + app.cnb_lifecycle_data.save + end + + it 'clears the credentials' do + lifecycle.update_lifecycle_data_model(app) + expect(app.cnb_lifecycle_data.reload.credentials).to be_nil + end + end + + context 'when credentials are absent from the request (not requested)' do + let(:message) { AppUpdateMessage.new({ lifecycle: { type: 'cnb', data: { buildpacks: ['docker://nodejs:latest'] } } }) } + + before do + app.cnb_lifecycle_data.credentials = { 'registry.io' => { 'username' => 'stored' } } + app.cnb_lifecycle_data.save + end + + it 'leaves stored credentials untouched' do + lifecycle.update_lifecycle_data_model(app) + expect(app.cnb_lifecycle_data.reload.credentials).to eq({ 'registry.io' => { 'username' => 'stored' } }) + end + end + end end end diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator_spec.rb index f40f5d90072..e6c33ccadc3 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_data_validator_spec.rb @@ -83,5 +83,70 @@ module VCAP::CloudController end end end + + describe 'rejects buildpack URL userinfo combined with data.credentials' do + let(:url_with_creds_info) { BuildpackInfo.new('https://user:pass@git.example.com/acme/bp.git', nil) } + let(:url_without_creds_info) { BuildpackInfo.new('https://git.example.com/acme/bp.git', nil) } + + context 'when the conflicting credentials were supplied in this request' do + it 'reports the both-in-one-request message' do + validator = BuildpackLifecycleDataValidator.new( + buildpack_infos: [url_with_creds_info], + stack: create(:stack), + credentials: { 'git.example.com' => { 'username' => 'u', 'password' => 'p' } }, + credentials_from_message: true + ) + expect(validator).not_to be_valid + expect(validator.errors.full_messages.join).to include('either in the buildpack URI or in data.credentials, not both') + end + end + + context 'when the conflicting credentials are stored (not in this request)' do + it 'tells the user how to clear the stored credentials' do + validator = BuildpackLifecycleDataValidator.new( + buildpack_infos: [url_with_creds_info], + stack: create(:stack), + credentials: { 'git.example.com' => { 'username' => 'u', 'password' => 'p' } }, + credentials_from_message: false + ) + expect(validator).not_to be_valid + expect(validator.errors.full_messages.join).to include('already has stored buildpack credentials') + expect(validator.errors.full_messages.join).to include('credentials: null') + end + end + + context 'when buildpack URL has userinfo and no data.credentials' do + it 'is valid' do + validator = BuildpackLifecycleDataValidator.new( + buildpack_infos: [url_with_creds_info], + stack: create(:stack), + credentials: nil + ) + expect(validator).to be_valid + end + end + + context 'when buildpack URL is clean and data.credentials are set' do + it 'is valid' do + validator = BuildpackLifecycleDataValidator.new( + buildpack_infos: [url_without_creds_info], + stack: create(:stack), + credentials: { 'git.example.com' => { 'username' => 'u', 'password' => 'p' } } + ) + expect(validator).to be_valid + end + end + + context 'when buildpack URL is clean and no data.credentials' do + it 'is valid' do + validator = BuildpackLifecycleDataValidator.new( + buildpack_infos: [url_without_creds_info], + stack: create(:stack), + credentials: nil + ) + expect(validator).to be_valid + end + end + end end end diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb index 40fd4dff7c4..a69a89cfd2c 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/buildpack_lifecycle_spec.rb @@ -156,8 +156,33 @@ module VCAP::CloudController end end + describe '#credentials_for_staging' do + let(:app) { AppModel.create(name: 'some-app', space: create(:space), lifecycle_type: 'buildpack') } + let!(:package) { create(:package_model, type: PackageModel::BITS_TYPE, app: app) } + + context 'when the app has stored credentials and the staging message has none' do + let(:request_data) { {} } + + before do + app.buildpack_lifecycle_data = BuildpackLifecycleDataModel.create( + app: app, + buildpacks: ['https://git.example.com/greendale/community-buildpack'], + stack: Stack.default.name, + credentials: { 'git.example.com' => { 'username' => 'user', 'password' => 'sekret' } } + ) + app.save + app.reload + end + + it 'reads the stored credentials from the app lifecycle data' do + expect(buildpack_lifecycle.credentials_for_staging).to eq({ 'git.example.com' => { 'username' => 'user', 'password' => 'sekret' } }) + end + end + end + describe '#buildpack_infos' do - let(:stubbed_data) { { stack: Stack.default.name, buildpack_infos: [instance_double(BuildpackInfo)] } } + let(:buildpack_info_double) { instance_double(BuildpackInfo, buildpack_url: nil) } + let(:stubbed_data) { { stack: Stack.default.name, buildpack_infos: [buildpack_info_double] } } let(:request_data) do { buildpacks: %w[cool-buildpack rad-buildpack] @@ -177,7 +202,7 @@ module VCAP::CloudController describe 'validation' do let(:validator) { instance_double(BuildpackLifecycleDataValidator) } - let(:stubbed_fetcher_data) { { stack: 'foo', buildpack_infos: 'bar' } } + let(:stubbed_fetcher_data) { { stack: 'foo', buildpack_infos: [] } } before do allow(validator).to receive(:valid?) @@ -190,7 +215,12 @@ module VCAP::CloudController it 'constructs the validator correctly' do buildpack_lifecycle.valid? - expect(BuildpackLifecycleDataValidator).to have_received(:new).with(buildpack_infos: 'bar', stack: 'foo') + expect(BuildpackLifecycleDataValidator).to have_received(:new).with( + buildpack_infos: [], + stack: 'foo', + credentials: nil, + credentials_from_message: false + ) end it 'delegates #valid? to a BuildpackLifecycleDataValidator' do diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/cnb_lifecycle_data_validator_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/cnb_lifecycle_data_validator_spec.rb new file mode 100644 index 00000000000..f1cd4daf471 --- /dev/null +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/cnb_lifecycle_data_validator_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' +require 'cloud_controller/diego/lifecycles/cnb_lifecycle_data_validator' + +module VCAP::CloudController + RSpec.describe CNBLifecycleDataValidator do + describe 'rejects userinfo embedded in a CNB buildpack URL' do + let(:cnb_url_with_creds) { BuildpackInfo.new('https://user:pass@example.org/buildpack.git', nil) } + let(:cnb_url_clean) { BuildpackInfo.new('https://example.org/buildpack.git', nil) } + + context 'when a CNB buildpack URL has userinfo' do + it 'is not valid regardless of credentials' do + validator = CNBLifecycleDataValidator.new( + buildpack_infos: [cnb_url_with_creds], + stack: create(:stack), + credentials: nil + ) + expect(validator).not_to be_valid + expect(validator.errors.full_messages.join).to match(/CNB.*credentials|credentials.*CNB/i) + end + end + + context 'when a CNB buildpack URL is clean' do + it 'is valid' do + validator = CNBLifecycleDataValidator.new( + buildpack_infos: [cnb_url_clean], + stack: create(:stack), + credentials: { 'example.org' => { 'username' => 'u', 'password' => 'p' } } + ) + expect(validator).to be_valid + end + end + end + end +end diff --git a/spec/unit/lib/cloud_controller/diego/lifecycles/cnb_lifecycle_spec.rb b/spec/unit/lib/cloud_controller/diego/lifecycles/cnb_lifecycle_spec.rb index 0b1c843f75d..e80bb625479 100644 --- a/spec/unit/lib/cloud_controller/diego/lifecycles/cnb_lifecycle_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/lifecycles/cnb_lifecycle_spec.rb @@ -103,6 +103,20 @@ module VCAP::CloudController end end + context 'when the user explicitly clears credentials with null' do + let(:app) { create(:app_model, :cnb, name: 'some-app', space: create(:space)) } + let(:request_data) { { credentials: nil } } + + before do + app.lifecycle_data.update(credentials: '{"auth": {}}') + end + + it 'does not fall back to the stored credentials' do + data_model = cnb_lifecycle.create_lifecycle_data_model(create(:build_model, :cnb)) + expect(data_model.credentials).to be_nil + end + end + context 'when the user specifies a stack' do let(:request_data) do { stack: 'cool-stack' } @@ -168,7 +182,8 @@ module VCAP::CloudController end describe '#buildpack_infos' do - let(:stubbed_data) { { stack: app.lifecycle_data.stack, buildpack_infos: [instance_double(BuildpackInfo)] } } + let(:buildpack_info_double) { instance_double(BuildpackInfo, buildpack_url: nil) } + let(:stubbed_data) { { stack: app.lifecycle_data.stack, buildpack_infos: [buildpack_info_double] } } let(:request_data) do { buildpacks: %w[cool-buildpack docker://rad-buildpack] diff --git a/spec/unit/lib/utils/uri_utils_spec.rb b/spec/unit/lib/utils/uri_utils_spec.rb index a47fa3d05da..2cd837ef793 100644 --- a/spec/unit/lib/utils/uri_utils_spec.rb +++ b/spec/unit/lib/utils/uri_utils_spec.rb @@ -174,4 +174,46 @@ expect { UriUtils.parse_docker_uri('publish/buildpack@sha256:e118d0') }.to raise_error(UriUtils::InvalidDockerURI) end end + + describe '.url_has_userinfo?' do + it 'returns true for an https URL with user:pass@' do + expect(UriUtils.url_has_userinfo?('https://user:pass@git.example.com/acme/buildpack.git')).to be true + end + + it 'returns true for an http URL with user only' do + expect(UriUtils.url_has_userinfo?('http://user@example.com/buildpack.git')).to be true + end + + it 'returns false for a clean https URL' do + expect(UriUtils.url_has_userinfo?('https://git.example.com/acme/buildpack.git')).to be false + end + + it 'returns false for an admin buildpack name' do + expect(UriUtils.url_has_userinfo?('ruby_buildpack')).to be false + end + + it 'returns false for nil' do + expect(UriUtils.url_has_userinfo?(nil)).to be false + end + + it 'returns false for a git+ssh URL (no userinfo field)' do + expect(UriUtils.url_has_userinfo?('git@git.example.com:acme/buildpack.git')).to be false + end + + it 'returns true for an https URL with an empty username but a password (token-style auth)' do + expect(UriUtils.url_has_userinfo?('https://:token@registry.example.com/buildpack')).to be true + end + + it 'returns true for an https URL with an IPv6 host and credentials' do + expect(UriUtils.url_has_userinfo?('https://user:pass@[::1]:8080/buildpack')).to be true + end + + it 'returns false for an https URL with an IPv6 host and no credentials' do + expect(UriUtils.url_has_userinfo?('https://[::1]:8080/buildpack')).to be false + end + + it 'returns false for a malformed URL that cannot be parsed' do + expect(UriUtils.url_has_userinfo?('https://exa mple.com/buildpack')).to be false + end + end end diff --git a/spec/unit/messages/app_manifest_message_spec.rb b/spec/unit/messages/app_manifest_message_spec.rb index 900a59a58c7..e134e6ae7b4 100644 --- a/spec/unit/messages/app_manifest_message_spec.rb +++ b/spec/unit/messages/app_manifest_message_spec.rb @@ -1395,6 +1395,20 @@ module VCAP::CloudController end end + context 'when "credentials" key is present' do + let(:parsed_yaml) do + { + 'credentials' => { 'example.org' => { 'username' => 'foo', 'password' => 'bar' } } + } + end + + it 'redacts the credentials' do + message = AppManifestMessage.create_from_yml(parsed_yaml) + + expect(message.audit_hash).to eq('credentials' => '[PRIVATE DATA HIDDEN]') + end + end + context 'when "buildpack" contains credentials in the URL' do let(:parsed_yaml) do { @@ -2179,6 +2193,71 @@ module VCAP::CloudController expect(message.app_update_message.buildpack_data.credentials).to eq({ registry: { username: 'password' } }) end end + + context 'when the generic credentials key is specified' do + let(:parsed_yaml) { { name: 'cnb', lifecycle: 'cnb', buildpacks: %w[nodejs java], stack: stack.name, credentials: { registry: { username: 'password' } } } } + + it 'adds credentials to the lifecycle_data' do + message = AppManifestMessage.create_from_yml(parsed_yaml) + + expect(message).to be_valid + expect(message.app_update_message.buildpack_data.credentials).to eq({ registry: { username: 'password' } }) + end + end + + context 'when the credentials key is explicitly null' do + let(:parsed_yaml) { { name: 'cnb', lifecycle: 'cnb', buildpacks: %w[nodejs java], stack: stack.name, credentials: nil } } + + it 'requests credentials with a nil value so stored credentials are cleared' do + message = AppManifestMessage.create_from_yml(parsed_yaml) + + expect(message).to be_valid + expect(message.app_update_message.buildpack_data.requested?(:credentials)).to be(true) + expect(message.app_update_message.buildpack_data.credentials).to be_nil + end + end + + context 'when both credentials and cnb-credentials are specified' do + let(:parsed_yaml) do + { name: 'cnb', lifecycle: 'cnb', buildpacks: %w[nodejs java], stack: stack.name, + credentials: { registry: { username: 'a' } }, cnb_credentials: { registry: { username: 'b' } } } + end + + it 'is invalid' do + message = AppManifestMessage.create_from_yml(parsed_yaml) + + expect(message).not_to be_valid + expect(message.errors.full_messages).to include('Cannot specify both credentials and cnb-credentials keys') + end + end + end + + context 'when buildpack lifecycle and credentials key is specified' do + let(:parsed_yaml) do + { name: 'app', lifecycle: 'buildpack', buildpacks: ['ruby_buildpack'], stack: stack.name, + credentials: { 'git.example.com' => { username: 'user', password: 'secret' } } } + end + + it 'adds credentials to the lifecycle_data' do + message = AppManifestMessage.create_from_yml(parsed_yaml) + + expect(message).to be_valid + expect(message.app_update_message.buildpack_data.credentials).to eq({ 'git.example.com': { username: 'user', password: 'secret' } }) + end + end + + context 'when buildpack lifecycle and credentials key is explicitly null' do + let(:parsed_yaml) do + { name: 'app', lifecycle: 'buildpack', buildpacks: ['ruby_buildpack'], stack: stack.name, credentials: nil } + end + + it 'requests credentials with a nil value so stored credentials are cleared' do + message = AppManifestMessage.create_from_yml(parsed_yaml) + + expect(message).to be_valid + expect(message.app_update_message.buildpack_data.requested?(:credentials)).to be(true) + expect(message.app_update_message.buildpack_data.credentials).to be_nil + end end context 'when buildpack is the default_app_lifecycle but lifecycle is not set' do diff --git a/spec/unit/models/runtime/buildpack_lifecycle_data_model_spec.rb b/spec/unit/models/runtime/buildpack_lifecycle_data_model_spec.rb index 854a6acc37c..71467051b35 100644 --- a/spec/unit/models/runtime/buildpack_lifecycle_data_model_spec.rb +++ b/spec/unit/models/runtime/buildpack_lifecycle_data_model_spec.rb @@ -11,6 +11,50 @@ module VCAP::CloudController let(:attr_salt) { :encrypted_buildpack_url_salt } end + it_behaves_like 'a model with an encrypted attribute' do + let(:value_to_encrypt) { '{"git.example.com":{"username":"u","password":"p"}}' } + let(:encrypted_attr) { :credentials_json } + let(:storage_column) { :encrypted_credentials_json } + let(:attr_salt) { :encrypted_credentials_json_salt } + end + + describe '#credentials' do + it 'round-trips a credentials hash via Oj' do + creds = { 'git.example.com' => { 'username' => 'u', 'password' => 'p' } } + lifecycle_data.credentials = creds + lifecycle_data.save + expect(lifecycle_data.reload.credentials).to eq(creds) + end + + it 'returns nil when not set' do + lifecycle_data.save + expect(lifecycle_data.reload.credentials).to be_nil + end + + it 'stores ciphertext, not plaintext' do + lifecycle_data.credentials = { 'git.example.com' => { 'username' => 'u', 'password' => 'secret' } } + lifecycle_data.save + raw = BuildpackLifecycleDataModel.db[:buildpack_lifecycle_data].where(guid: lifecycle_data.guid).first + expect(raw[:encrypted_credentials_json]).not_to include('secret') + expect(raw[:encrypted_credentials_json_salt]).not_to be_nil + end + end + + describe '#to_hash credentials handling' do + context 'when credentials are set' do + it 'redacts credentials to ***' do + lifecycle_data.credentials = { 'git.example.com' => { 'username' => 'u', 'password' => 'p' } } + expect(lifecycle_data.to_hash[:credentials]).to eq('***') + end + end + + context 'when credentials are nil' do + it 'omits credentials from to_hash' do + expect(lifecycle_data.to_hash.keys).not_to include(:credentials) + end + end + end + describe 'buildpack_lifecycle_buildpacks association' do it 'orders by id via the default_order_by_id extension' do lifecycle_data.save diff --git a/spec/unit/presenters/v3/app_manifest_presenter_spec.rb b/spec/unit/presenters/v3/app_manifest_presenter_spec.rb index 31e0d73303e..d88b44e9713 100644 --- a/spec/unit/presenters/v3/app_manifest_presenter_spec.rb +++ b/spec/unit/presenters/v3/app_manifest_presenter_spec.rb @@ -201,11 +201,57 @@ module VCAP::CloudController::Presenters::V3 ) end - it 'presents the buildpacks in the order originally specified (not alphabetical)' do + it 'presents the buildpacks in the order originally specified (not alphabetical), obfuscating credentials in URLs' do result = AppManifestPresenter.new(app, service_bindings, app.route_mappings).to_hash application = result[:applications].first expect(application[:stack]).to eq(app.lifecycle_data.stack) - expect(application[:buildpacks]).to eq(['limabean', 'git://user:pass@github.com/repo']) + expect(application[:buildpacks]).to eq(['limabean', 'git://***:***@github.com/repo']) + end + end + + context 'when the app is a buildpack app with data.credentials' do + before do + create(:buildpack, name: 'limabean') + app.lifecycle_data.update(buildpacks: ['limabean'], stack: 'the-happiest-stack') + app.lifecycle_data.credentials = { 'git.example.com' => { 'username' => 'user', 'password' => 'sekret' } } + app.lifecycle_data.save + end + + it 'presents the credentials key with a redacted value' do + result = AppManifestPresenter.new(app, service_bindings, app.route_mappings).to_hash + application = result[:applications].first + expect(application[:credentials]).to eq('***') + expect(result.to_s).not_to include('sekret') + end + end + + context 'when the app is a buildpack app without data.credentials' do + before do + create(:buildpack, name: 'limabean') + app.lifecycle_data.update(buildpacks: ['limabean'], stack: 'the-happiest-stack') + end + + it 'omits the credentials key' do + result = AppManifestPresenter.new(app, service_bindings, app.route_mappings).to_hash + application = result[:applications].first + expect(application).not_to have_key(:credentials) + end + end + + context 'when the app is a cnb app with data.credentials' do + let(:app) { create(:app_model, :cnb, environment_variables:) } + + before do + app.lifecycle_data.update(buildpacks: ['docker://example.org/bp:latest']) + app.lifecycle_data.credentials = { 'example.org' => { 'token' => 'sekret' } } + app.lifecycle_data.save + end + + it 'presents the credentials key with a redacted value' do + result = AppManifestPresenter.new(app, service_bindings, app.route_mappings).to_hash + application = result[:applications].first + expect(application[:credentials]).to eq('***') + expect(result.to_s).not_to include('sekret') end end diff --git a/spec/unit/repositories/app_event_repository_spec.rb b/spec/unit/repositories/app_event_repository_spec.rb index 78bae9d0949..e73ac02b304 100644 --- a/spec/unit/repositories/app_event_repository_spec.rb +++ b/spec/unit/repositories/app_event_repository_spec.rb @@ -527,6 +527,86 @@ module Repositories expect(event.metadata.fetch('request')).to eq expected_request end end + + context 'when lifecycle.data.credentials is present' do + let(:attrs) do + { + 'lifecycle' => { + 'type' => 'buildpack', + 'data' => { 'credentials' => { 'git.example.com' => { 'username' => 'u', 'password' => 's' } } } + } + } + end + + it 'redacts credentials with PRIVATE_DATA_HIDDEN' do + event = app_event_repository.record_app_update(app, space, user_audit_info, attrs).reload + + expect(event.metadata.fetch('request')).to eq( + 'lifecycle' => { + 'type' => 'buildpack', + 'data' => { 'credentials' => Presenters::Censorship::PRIVATE_DATA_HIDDEN } + } + ) + end + end + + context 'when lifecycle.data.credentials is present for CNB' do + let(:attrs) do + { + 'lifecycle' => { + 'type' => 'cnb', + 'data' => { 'credentials' => { 'git.example.com' => { 'username' => 'u', 'password' => 's' } } } + } + } + end + + it 'redacts credentials with PRIVATE_DATA_HIDDEN' do + event = app_event_repository.record_app_update(app, space, user_audit_info, attrs).reload + + expect(event.metadata.fetch('request')).to eq( + 'lifecycle' => { + 'type' => 'cnb', + 'data' => { 'credentials' => Presenters::Censorship::PRIVATE_DATA_HIDDEN } + } + ) + end + end + + context 'when lifecycle.data.credentials uses the token form' do + let(:attrs) do + { + 'lifecycle' => { + 'type' => 'cnb', + 'data' => { 'credentials' => { 'registry.example.com' => { 'token' => 'sekret-token' } } } + } + } + end + + it 'redacts the credentials and does not leak the token' do + event = app_event_repository.record_app_update(app, space, user_audit_info, attrs).reload + + expect(event.metadata.fetch('request').dig('lifecycle', 'data', 'credentials')).to eq(Presenters::Censorship::PRIVATE_DATA_HIDDEN) + expect(event.metadata.to_s).not_to include('sekret-token') + end + end + + context 'when credentials are present on app create' do + let(:attrs) do + { + 'lifecycle' => { + 'type' => 'buildpack', + 'data' => { 'credentials' => { 'git.example.com' => { 'username' => 'u', 'password' => 'sekret' } } } + } + } + end + + it 'redacts credentials in the create event' do + event = app_event_repository.record_app_create(app, space, user_audit_info, attrs).reload + + expect(event.metadata.dig('request', 'lifecycle', 'data', 'credentials')).to eq(Presenters::Censorship::PRIVATE_DATA_HIDDEN) + expect(event.metadata.to_s).not_to include('sekret') + end + end end context 'v3 with a buildpacks array' do