Skip to content

Optionally disallow the use of 'root' user for Processes and Tasks of docker lifecycle Apps #4452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions app/models/runtime/process_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -576,8 +576,6 @@ def permitted_users
end

def docker_run_action_user
return AppModel::DEFAULT_CONTAINER_USER unless docker?

desired_droplet&.docker_user.presence || AppModel::DEFAULT_DOCKER_CONTAINER_USER
end

Expand Down
1 change: 1 addition & 0 deletions config/cloud_controller.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ max_retained_deployments_per_app: 100
max_retained_builds_per_app: 100
max_retained_revisions_per_app: 100
additional_allowed_process_users: ['ContainerUser', 'TestUser']
allow_docker_root_user: true

broker_client_default_async_poll_interval_seconds: 60
broker_client_max_async_poll_duration_minutes: 10080
Expand Down
1 change: 1 addition & 0 deletions lib/cloud_controller/config_schemas/api_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class ApiSchema < VCAP::Config
default_health_check_timeout: Integer,
maximum_health_check_timeout: Integer,
additional_allowed_process_users: Array,
allow_docker_root_user: bool,

instance_file_descriptor_limit: Integer,

Expand Down
1 change: 1 addition & 0 deletions lib/cloud_controller/config_schemas/clock_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ class ClockSchema < VCAP::Config
max_retained_builds_per_app: Integer,
max_retained_revisions_per_app: Integer,
additional_allowed_process_users: Array,
allow_docker_root_user: bool,

diego_sync: { frequency_in_seconds: Integer },

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class DeploymentUpdaterSchema < VCAP::Config
maximum_app_disk_in_mb: Integer,
instance_file_descriptor_limit: Integer,
additional_allowed_process_users: Array,
allow_docker_root_user: bool,

deployment_updater: {
update_frequency_in_seconds: Integer,
Expand Down
1 change: 1 addition & 0 deletions lib/cloud_controller/config_schemas/worker_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ class WorkerSchema < VCAP::Config
default_app_log_rate_limit_in_bytes_per_second: Integer,
default_app_ssh_access: bool,
additional_allowed_process_users: Array,
allow_docker_root_user: bool,

jobs: {
global: {
Expand Down
2 changes: 2 additions & 0 deletions lib/cloud_controller/diego/desire_app_handler.rb
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need equivalent logic for surfacing the error when running tasks?

Copy link
Member Author

@acosta11 acosta11 Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, I'm not familiar with task execution error cases and only found this particular layer with some guidance from Tim. Is it a problem for a new execution of a task to fail after the cutover to blocking the root user? I assume that a previously running task isn't an availability concern the same way an already deployed running app would be.

Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ def create_or_update_app(process, client)
if e.name == 'RunnerError' && e.message['the requested resource already exists']
existing_lrp = client.get_app(process)
client.update_app(process, existing_lrp)
elsif e.name == 'UnprocessableEntity'
raise
end
end
end
Expand Down
14 changes: 14 additions & 0 deletions lib/cloud_controller/diego/docker/lifecycle_protocol.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ module CloudController
module Diego
module Docker
class LifecycleProtocol
ROOT_USERS = %w[root 0].freeze

def lifecycle_data(staging_details)
lifecycle_data = Diego::Docker::LifecycleData.new
lifecycle_data.docker_image = staging_details.package.image
Expand All @@ -22,10 +24,18 @@ def staging_action_builder(config, staging_details)
end

def task_action_builder(config, task)
if task.docker? && !docker_run_action_user_permitted?(task.run_action_user)
raise ::CloudController::Errors::ApiError.new_from_details('UnprocessableEntity', 'Attempting to run task as root user, which is not permitted.')
end

TaskActionBuilder.new(config, task, { droplet_path: task.droplet.docker_receipt_image })
end

def desired_lrp_builder(config, process)
if process.docker? && !docker_run_action_user_permitted?(process.run_action_user)
raise ::CloudController::Errors::ApiError.new_from_details('UnprocessableEntity', 'Attempting to run process as root user, which is not permitted.')
end

DesiredLrpBuilder.new(config, builder_opts(process))
end

Expand All @@ -46,6 +56,10 @@ def container_env_vars_for_process(process)
additional_env = []
additional_env + WindowsEnvironmentSage.ponder(process.app)
end

def docker_run_action_user_permitted?(run_action_user)
Config.config.get(:allow_docker_root_user) || ROOT_USERS.exclude?(run_action_user)
end
end
end
end
Expand Down
12 changes: 12 additions & 0 deletions spec/unit/lib/cloud_controller/diego/desire_app_handler_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,18 @@ module Diego
expect(client).to have_received(:get_app).exactly(2).times
end
end

context 'when root user is not permitted' do
it 'raises a CloudController::Errors::ApiError' do
allow(client).to receive(:desire_app).and_raise(CloudController::Errors::ApiError.new_from_details('UnprocessableEntity',
'Attempting to run process as root user, which is not permitted.'))
expect { DesireAppHandler.create_or_update_app(process, client) }.to(raise_error do |error|
expect(error).to be_a(CloudController::Errors::ApiError)
expect(error.name).to eq('UnprocessableEntity')
expect(error.message).to include('Attempting to run process as root user, which is not permitted')
end)
end
end
end
end
end
Expand Down
Loading
Loading