From f573d59413323a9e7398b313da20d4a72330d919 Mon Sep 17 00:00:00 2001 From: Wei Quan Date: Tue, 15 Sep 2026 11:45:39 +0200 Subject: [PATCH 1/3] Redact credentials from buildpack URLs in telemetry logs --- app/actions/v2/app_stage.rb | 2 +- .../runtime/restages_controller.rb | 2 +- app/controllers/v3/builds_controller.rb | 2 +- .../helpers/lifecycle_data_model_mixin.rb | 7 ++++ .../runtime/buildpack_lifecycle_data_model.rb | 5 ++- .../runtime/cnb_lifecycle_data_model.rb | 5 ++- .../runtime/docker_lifecycle_data_model.rb | 3 ++ spec/unit/actions/v2/app_stage_spec.rb | 23 +++++++++++++ .../runtime/restages_controller_spec.rb | 22 ++++++++++++ .../controllers/v3/builds_controller_spec.rb | 34 +++++++++++++++++++ 10 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 app/models/helpers/lifecycle_data_model_mixin.rb diff --git a/app/actions/v2/app_stage.rb b/app/actions/v2/app_stage.rb index 1990fb9c698..1946d71c992 100644 --- a/app/actions/v2/app_stage.rb +++ b/app/actions/v2/app_stage.rb @@ -39,7 +39,7 @@ def stage(process) 'user-id' => build.created_by_user_guid }, { 'lifecycle' => build.lifecycle_type, - 'buildpacks' => build.lifecycle_data&.buildpacks, + 'buildpacks' => build.lifecycle_data&.obfuscated_buildpacks, 'stack' => build.lifecycle_data&.stack } ) diff --git a/app/controllers/runtime/restages_controller.rb b/app/controllers/runtime/restages_controller.rb index 746fd67c37c..7b4842165f9 100644 --- a/app/controllers/runtime/restages_controller.rb +++ b/app/controllers/runtime/restages_controller.rb @@ -49,7 +49,7 @@ def restage(guid) 'user-id' => current_user.guid }, { 'lifecycle' => process.app.lifecycle_type, - 'buildpacks' => process.app.lifecycle_data.buildpacks, + 'buildpacks' => process.app.lifecycle_data.obfuscated_buildpacks, 'stack' => process.app.lifecycle_data.stack } ) diff --git a/app/controllers/v3/builds_controller.rb b/app/controllers/v3/builds_controller.rb index cfb5df2ab75..6e84fcf6028 100644 --- a/app/controllers/v3/builds_controller.rb +++ b/app/controllers/v3/builds_controller.rb @@ -61,7 +61,7 @@ def create }, { 'lifecycle' => build.lifecycle_type, - 'buildpacks' => build.lifecycle_data&.buildpacks, + 'buildpacks' => build.lifecycle_data&.obfuscated_buildpacks, 'stack' => build.lifecycle_data.try(:stack) } ) diff --git a/app/models/helpers/lifecycle_data_model_mixin.rb b/app/models/helpers/lifecycle_data_model_mixin.rb new file mode 100644 index 00000000000..0b591bd21c0 --- /dev/null +++ b/app/models/helpers/lifecycle_data_model_mixin.rb @@ -0,0 +1,7 @@ +require 'cloud_controller/url_secret_obfuscator' + +module LifecycleDataModelMixin + def obfuscated_buildpacks + buildpacks.map { |bp| CloudController::UrlSecretObfuscator.obfuscate(bp) } + end +end diff --git a/app/models/runtime/buildpack_lifecycle_data_model.rb b/app/models/runtime/buildpack_lifecycle_data_model.rb index aae0e098c9f..99b11e297ed 100644 --- a/app/models/runtime/buildpack_lifecycle_data_model.rb +++ b/app/models/runtime/buildpack_lifecycle_data_model.rb @@ -1,8 +1,11 @@ require 'cloud_controller/diego/lifecycles/lifecycles' require 'utils/uri_utils' +require_relative '../helpers/lifecycle_data_model_mixin' module VCAP::CloudController class BuildpackLifecycleDataModel < Sequel::Model(:buildpack_lifecycle_data) + include LifecycleDataModelMixin + LIFECYCLE_TYPE = Lifecycles::BUILDPACK set_field_as_encrypted :buildpack_url, salt: :encrypted_buildpack_url_salt, column: :encrypted_buildpack_url @@ -84,7 +87,7 @@ def first_custom_buildpack_url def to_hash { - buildpacks: buildpacks.map { |buildpack| CloudController::UrlSecretObfuscator.obfuscate(buildpack) }, + buildpacks: obfuscated_buildpacks, stack: stack } end diff --git a/app/models/runtime/cnb_lifecycle_data_model.rb b/app/models/runtime/cnb_lifecycle_data_model.rb index 850fbb02ec3..1cd67929a9e 100644 --- a/app/models/runtime/cnb_lifecycle_data_model.rb +++ b/app/models/runtime/cnb_lifecycle_data_model.rb @@ -1,8 +1,11 @@ require 'cloud_controller/diego/lifecycles/lifecycles' require 'presenters/helpers/censorship' +require_relative '../helpers/lifecycle_data_model_mixin' module VCAP::CloudController class CNBLifecycleDataModel < Sequel::Model(:cnb_lifecycle_data) + include LifecycleDataModelMixin + LIFECYCLE_TYPE = Lifecycles::CNB set_field_as_encrypted :registry_credentials_json, salt: :encrypted_registry_credentials_json_salt, column: :encrypted_registry_credentials_json @@ -68,7 +71,7 @@ def using_custom_buildpack? def to_hash hash = { - buildpacks: buildpacks.map { |buildpack| CloudController::UrlSecretObfuscator.obfuscate(buildpack) }, + buildpacks: obfuscated_buildpacks, stack: stack } hash[:credentials] = Presenters::Censorship::REDACTED_CREDENTIAL unless credentials.nil? diff --git a/app/models/runtime/docker_lifecycle_data_model.rb b/app/models/runtime/docker_lifecycle_data_model.rb index 9d8363aff5e..5523b2d4c8b 100644 --- a/app/models/runtime/docker_lifecycle_data_model.rb +++ b/app/models/runtime/docker_lifecycle_data_model.rb @@ -1,7 +1,10 @@ require 'cloud_controller/diego/lifecycles/lifecycles' +require_relative '../helpers/lifecycle_data_model_mixin' module VCAP::CloudController class DockerLifecycleDataModel + include LifecycleDataModelMixin + LIFECYCLE_TYPE = Lifecycles::DOCKER def buildpacks diff --git a/spec/unit/actions/v2/app_stage_spec.rb b/spec/unit/actions/v2/app_stage_spec.rb index 49c1903fee5..bdf50e21f60 100644 --- a/spec/unit/actions/v2/app_stage_spec.rb +++ b/spec/unit/actions/v2/app_stage_spec.rb @@ -215,6 +215,29 @@ module V2 expect(logger_spy).to have_received(:info).with(Oj.dump(expected_json)) end end + + it 'redacts credentials from custom buildpack URLs in telemetry' do + Timecop.freeze do + buildpack = create(:buildpack_lifecycle_buildpack_model, :custom_buildpack, buildpack_url: 'https://user:secret@github.com/myorg/private-buildpack') + buildpack_lifecycle_data = create(:buildpack_lifecycle_data_model, stack: 'my_stack', buildpack_lifecycle_buildpack_guids: [buildpack.guid]) + + app = create(:app_model) + app.buildpack_lifecycle_data = buildpack_lifecycle_data + app.save + process = create(:process_model, memory: 765, disk_quota: 1234, app: app) + create(:package_model, app: process.app, state: PackageModel::READY_STATE) + process.reload + + action.stage(process) + + expect(logger_spy).to have_received(:info) do |json_str| + logged = Oj.load(json_str) + buildpacks = logged['create-build']['buildpacks'] + expect(buildpacks).to eq(['https://***:***@github.com/myorg/private-buildpack']) + expect(buildpacks.first).not_to include('secret') + end + end + end end context 'stack state warnings' do diff --git a/spec/unit/controllers/runtime/restages_controller_spec.rb b/spec/unit/controllers/runtime/restages_controller_spec.rb index 5452ef52bb1..fd1df26c613 100644 --- a/spec/unit/controllers/runtime/restages_controller_spec.rb +++ b/spec/unit/controllers/runtime/restages_controller_spec.rb @@ -208,6 +208,28 @@ module VCAP::CloudController expect(process.app.revisions.length).to eq(0) end end + + context 'telemetry' do + let(:logger_spy) { spy('logger') } + + before do + allow(VCAP::CloudController::TelemetryLogger).to receive(:logger).and_return(logger_spy) + end + + it 'redacts credentials from custom buildpack URLs in telemetry' do + process.app.lifecycle_data.update(buildpack_url: 'https://user:secret@github.com/myorg/private-buildpack') + + restage_request + + expect(last_response.status).to eq(201) + expect(logger_spy).to have_received(:info) do |json_str| + logged = Oj.load(json_str) + buildpacks = logged['restage-app']['buildpacks'] + expect(buildpacks).to eq(['https://***:***@github.com/myorg/private-buildpack']) + expect(buildpacks.first).not_to include('secret') + end + end + end end end end diff --git a/spec/unit/controllers/v3/builds_controller_spec.rb b/spec/unit/controllers/v3/builds_controller_spec.rb index e9c81b5ee3a..1b84cfb0667 100644 --- a/spec/unit/controllers/v3/builds_controller_spec.rb +++ b/spec/unit/controllers/v3/builds_controller_spec.rb @@ -742,6 +742,40 @@ expect(response.body).to include('Unable to use package. Ensure that the package exists and you have access to it.') end end + + context 'telemetry' do + let(:logger_spy) { spy('logger') } + let(:developer) { make_developer_for_space(space) } + + before do + allow(VCAP::CloudController::TelemetryLogger).to receive(:logger).and_return(logger_spy) + set_current_user(developer) + allow_any_instance_of(VCAP::CloudController::Diego::Stager).to receive(:stage) + end + + it 'redacts credentials from custom buildpack URLs in telemetry' do + req_body_with_creds = { + package: { guid: package.guid }, + lifecycle: { + type: 'buildpack', + data: { + buildpacks: ['https://user:secret@github.com/myorg/private-buildpack'], + stack: VCAP::CloudController::Stack.default.name + } + } + } + + post :create, params: req_body_with_creds, as: :json + + expect(response).to have_http_status(:created) + expect(logger_spy).to have_received(:info) do |json_str| + logged = Oj.load(json_str) + buildpacks = logged['create-build']['buildpacks'] + expect(buildpacks).to eq(['https://***:***@github.com/myorg/private-buildpack']) + expect(buildpacks.first).not_to include('secret') + end + end + end end end From a9e0bb4a13ac563da43f4991a6cefacf4b03e607 Mon Sep 17 00:00:00 2001 From: Wei Quan Date: Tue, 15 Sep 2026 16:41:14 +0200 Subject: [PATCH 2/3] Address PR review: obfuscate buildpacks in staging completion and register mixin --- app/controllers/internal/staging_completion_controller.rb | 2 +- app/models.rb | 1 + app/models/helpers/lifecycle_data_model_mixin.rb | 8 +++++--- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/controllers/internal/staging_completion_controller.rb b/app/controllers/internal/staging_completion_controller.rb index 06e9546f088..8788fad2f5a 100644 --- a/app/controllers/internal/staging_completion_controller.rb +++ b/app/controllers/internal/staging_completion_controller.rb @@ -78,7 +78,7 @@ def build_completed(staging_guid) }, { 'lifecycle' => build.lifecycle_type, - 'buildpacks' => build.lifecycle_data&.buildpacks, + 'buildpacks' => build.lifecycle_data&.obfuscated_buildpacks, 'stack' => build.lifecycle_data.try(:stack) } ) diff --git a/app/models.rb b/app/models.rb index d7db6b83970..7944a73f04b 100644 --- a/app/models.rb +++ b/app/models.rb @@ -1,3 +1,4 @@ +require 'models/helpers/lifecycle_data_model_mixin' require 'models/helpers/metadata_model_mixin' require 'models/runtime/space' diff --git a/app/models/helpers/lifecycle_data_model_mixin.rb b/app/models/helpers/lifecycle_data_model_mixin.rb index 0b591bd21c0..4d8e5f7bdba 100644 --- a/app/models/helpers/lifecycle_data_model_mixin.rb +++ b/app/models/helpers/lifecycle_data_model_mixin.rb @@ -1,7 +1,9 @@ require 'cloud_controller/url_secret_obfuscator' -module LifecycleDataModelMixin - def obfuscated_buildpacks - buildpacks.map { |bp| CloudController::UrlSecretObfuscator.obfuscate(bp) } +module VCAP::CloudController + module LifecycleDataModelMixin + def obfuscated_buildpacks + buildpacks.map { |bp| CloudController::UrlSecretObfuscator.obfuscate(bp) } + end end end From 9da73af456adfa8d9c42a4c77313689653c4be76 Mon Sep 17 00:00:00 2001 From: Wei Quan Date: Fri, 18 Sep 2026 11:43:39 +0200 Subject: [PATCH 3/3] Add telemetry redaction test for staging completion controller --- .../staging_completion_controller_spec.rb | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/spec/unit/controllers/internal/staging_completion_controller_spec.rb b/spec/unit/controllers/internal/staging_completion_controller_spec.rb index 92e20725d79..f35f35efa88 100644 --- a/spec/unit/controllers/internal/staging_completion_controller_spec.rb +++ b/spec/unit/controllers/internal/staging_completion_controller_spec.rb @@ -315,6 +315,31 @@ module VCAP::CloudController end end + it 'redacts credentials from custom buildpack URLs in telemetry' do + build.buildpack_lifecycle_data.update(buildpacks: ['https://user:secret@github.com/myorg/private-buildpack']) + + Timecop.freeze do + expected_json = { + 'telemetry-source' => 'cloud_controller_ng', + 'telemetry-time' => Time.now.to_datetime.rfc3339, + 'build-completed' => { + 'api-version' => 'internal', + 'lifecycle' => 'buildpack', + 'buildpacks' => ['https://***:***@github.com/myorg/private-buildpack'], + 'stack' => 'cflinuxfs4', + 'app-id' => OpenSSL::Digest::SHA256.hexdigest(staged_app.guid), + 'build-id' => OpenSSL::Digest::SHA256.hexdigest(build.guid) + } + } + expect_any_instance_of(ActiveSupport::Logger).to receive(:info).with(Oj.dump(expected_json)) + + allow_any_instance_of(BuildModel).to receive(:in_final_state?).and_return(false) + post url, Oj.dump(staging_response) + + expect(last_response.status).to eq(200), last_response.body + end + end + it 'emits metrics for staging success' do one_hour_in_nanoseconds = (1.hour.to_i * 1e9).to_i expect(statsd_updater).to receive(:report_staging_success_metrics).with(one_hour_in_nanoseconds)