From 14d8dc8cea168d092fa4a73b14dc507f4d767ff7 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Wed, 23 Sep 2026 11:23:59 +0200 Subject: [PATCH] Add role check to process and build update --- app/controllers/v3/builds_controller.rb | 17 +++++++++ app/controllers/v3/processes_controller.rb | 8 ++++ spec/request/builds_spec.rb | 29 ++++++++++++++- spec/request/processes_spec.rb | 43 +++++++++++++++++++++- 4 files changed, 95 insertions(+), 2 deletions(-) diff --git a/app/controllers/v3/builds_controller.rb b/app/controllers/v3/builds_controller.rb index cfb5df2ab75..849ee7d2548 100644 --- a/app/controllers/v3/builds_controller.rb +++ b/app/controllers/v3/builds_controller.rb @@ -43,6 +43,8 @@ def create unprocessable_package! unless package && permission_queryer.can_manage_apps_in_active_space?(package.space.id) require_writable_space!(package.space) + ensure_can_set_custom_staging_input!(message, package.space) + FeatureFlag.raise_unless_enabled!(:diego_docker) if package.type == PackageModel::DOCKER_TYPE lifecycle = LifecycleProvider.provide(package, message) @@ -112,6 +114,21 @@ def can_read_build?(space) permission_queryer.can_update_build_state? || permission_queryer.can_read_from_space?(space.id, space.organization_id) end + def ensure_can_set_custom_staging_input!(message, space) + return unless custom_staging_input?(message) + + unauthorized! unless permission_queryer.can_write_to_active_space?(space.id) + end + + def custom_staging_input?(message) + return true if message.requested?(:environment_variables) + + lifecycle_data = message.lifecycle_data + return false unless lifecycle_data.is_a?(Hash) + + %i[buildpacks stack credentials].any? { |key| lifecycle_data.key?(key) } + end + def create_valid_update_message message = VCAP::CloudController::BuildUpdateMessage.new(hashed_params[:body]) unprocessable!(message.errors.full_messages) unless message.valid? diff --git a/app/controllers/v3/processes_controller.rb b/app/controllers/v3/processes_controller.rb index 6d653370fd4..f1120a076ed 100644 --- a/app/controllers/v3/processes_controller.rb +++ b/app/controllers/v3/processes_controller.rb @@ -73,6 +73,8 @@ def update message = ProcessUpdateMessage.new(hashed_params[:body]) unprocessable!(message.errors.full_messages) unless message.valid? + ensure_can_set_execution_fields!(message) + ProcessUpdate.new(user_audit_info).update(@process, message, NonManifestStrategy) render status: :ok, json: Presenters::V3::ProcessPresenter.new(@process) @@ -147,6 +149,12 @@ def ensure_can_write require_writable_space!(@space) end + def ensure_can_set_execution_fields!(message) + return unless message.requested?(:command) || message.requested?(:user) + + unauthorized! unless permission_queryer.can_write_to_active_space?(@space.id) + end + def process_not_found! resource_not_found!(:process) end diff --git a/spec/request/builds_spec.rb b/spec/request/builds_spec.rb index fae138935f0..5e4106c7f67 100644 --- a/spec/request/builds_spec.rb +++ b/spec/request/builds_spec.rb @@ -135,7 +135,8 @@ code: 201 } h['space_supporter'] = { - code: 201 + code: 403, + errors: CF_NOT_AUTHORIZED } h end @@ -169,6 +170,32 @@ end end + context 'permissions for a plain restage without custom staging input' do + let(:create_request) do + { + package: { + guid: package.guid + } + } + end + + let(:api_call) { ->(user_headers) { post '/v3/builds', create_request.to_json, user_headers } } + let(:org) { space.organization } + let(:user) { create(:user) } + + let(:expected_codes_and_responses) do + h = Hash.new( + { code: 422 }.freeze + ) + h['admin'] = { code: 201 } + h['space_developer'] = { code: 201 } + h['space_supporter'] = { code: 201 } + h + end + + it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + end + context 'telemetry' do let(:logger_spy) { spy('logger') } diff --git a/spec/request/processes_spec.rb b/spec/request/processes_spec.rb index 729c97613af..fcceb53eb90 100644 --- a/spec/request/processes_spec.rb +++ b/spec/request/processes_spec.rb @@ -952,7 +952,7 @@ h = Hash.new({ code: 403, errors: CF_NOT_AUTHORIZED }.freeze) h['admin'] = { code: 200, response_object: expected_response } h['space_developer'] = { code: 200, response_object: expected_response } - h['space_supporter'] = { code: 200, response_object: expected_response } + h['space_supporter'] = { code: 403, errors: CF_NOT_AUTHORIZED } h['org_auditor'] = { code: 404 } h['org_billing_manager'] = { code: 404 } h['no_role'] = { code: 404 } @@ -976,6 +976,47 @@ end end + context 'permissions when the update does not set command or user' do + let(:update_request) do + { + health_check: { + type: 'process', + data: { + timeout: 20, + interval: 5 + } + }, + readiness_health_check: { + type: 'port', + data: { + invocation_timeout: 10, + interval: 6 + } + }, + metadata: metadata + }.to_json + end + + let(:expected_response_without_command) do + expected_response.merge('command' => 'rackup') + end + + let(:api_call) { ->(user_headers) { patch "/v3/processes/#{process.guid}", update_request, user_headers } } + + let(:expected_codes_and_responses) do + h = Hash.new({ code: 403, errors: CF_NOT_AUTHORIZED }.freeze) + h['admin'] = { code: 200, response_object: expected_response_without_command } + h['space_developer'] = { code: 200, response_object: expected_response_without_command } + h['space_supporter'] = { code: 200, response_object: expected_response_without_command } + h['org_auditor'] = { code: 404 } + h['org_billing_manager'] = { code: 404 } + h['no_role'] = { code: 404 } + h + end + + it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + end + it 'updates the process' do patch "/v3/processes/#{process.guid}", update_request, developer_headers.merge('CONTENT_TYPE' => 'application/json')