rosslh/coolify
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(security): harden deployment paths and deploy abilities (#8549)

Browse source
This commit is contained in:
Andras Bacsai 2026-02-23 12:13:47 +01:00 committed by GitHub
commit ba3994bc5c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 315 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
app/Http/Controllers/Api/ApplicationsController.php
View file

@ -1101,7 +1101,6 @@ private function create_application(Request $request, $type)
'git_branch' => ['string', 'required', new ValidGitBranch],
'build_pack' => ['required', Rule::enum(BuildPackTypes::class)],
'ports_exposes' => 'string|regex:/^(\d+)(,\d+)*$/|required',
'docker_compose_location' => 'string',
'docker_compose_domains' => 'array|nullable',
'docker_compose_domains.*' => 'array:name,domain',
'docker_compose_domains.*.name' => 'string|required',
@ -1297,7 +1296,6 @@ private function create_application(Request $request, $type)
'ports_exposes' => 'string|regex:/^(\d+)(,\d+)*$/|required',
'github_app_uuid' => 'string|required',
'watch_paths' => 'string|nullable',
'docker_compose_location' => 'string',
'docker_compose_domains' => 'array|nullable',
'docker_compose_domains.*' => 'array:name,domain',
'docker_compose_domains.*.name' => 'string|required',
@ -1525,7 +1523,6 @@ private function create_application(Request $request, $type)
'ports_exposes' => 'string|regex:/^(\d+)(,\d+)*$/|required',
'private_key_uuid' => 'string|required',
'watch_paths' => 'string|nullable',
'docker_compose_location' => 'string',
'docker_compose_domains' => 'array|nullable',
'docker_compose_domains.*' => 'array:name,domain',
'docker_compose_domains.*.name' => 'string|required',
@ -2470,7 +2467,6 @@ public function update_by_uuid(Request $request)
'description' => 'string|nullable',
'static_image' => 'string',
'watch_paths' => 'string|nullable',
'docker_compose_location' => 'string',
'docker_compose_domains' => 'array|nullable',
'docker_compose_domains.*' => 'array:name,domain',
'docker_compose_domains.*.name' => 'string|required',

4
app/Http/Controllers/Api/DeployController.php
View file

@ -127,6 +127,10 @@ public function deployment_by_uuid(Request $request)
if (! $deployment) {
return response()->json(['message' => 'Deployment not found.'], 404);
}
$application = $deployment->application;
if (! $application || data_get($application->team(), 'id') !== $teamId) {
return response()->json(['message' => 'Deployment not found.'], 404);
}
return response()->json($this->removeSensitiveData($deployment));
}

18
app/Jobs/ApplicationDeploymentJob.php
View file

@ -251,7 +251,7 @@ public function __construct(public int $application_deployment_queue_id)
}
if ($this->application->build_pack === 'dockerfile') {
if (data_get($this->application, 'dockerfile_location')) {
$this->dockerfile_location = $this->application->dockerfile_location;
$this->dockerfile_location = $this->validatePathField($this->application->dockerfile_location, 'dockerfile_location');
}
}
}
@ -571,7 +571,7 @@ private function deploy_dockerimage_buildpack()
private function deploy_docker_compose_buildpack()
{
if (data_get($this->application, 'docker_compose_location')) {
$this->docker_compose_location = $this->application->docker_compose_location;
$this->docker_compose_location = $this->validatePathField($this->application->docker_compose_location, 'docker_compose_location');
}
if (data_get($this->application, 'docker_compose_custom_start_command')) {
$this->docker_compose_custom_start_command = $this->application->docker_compose_custom_start_command;
@ -831,7 +831,7 @@ private function deploy_dockerfile_buildpack()
$this->server = $this->build_server;
}
if (data_get($this->application, 'dockerfile_location')) {
$this->dockerfile_location = $this->application->dockerfile_location;
$this->dockerfile_location = $this->validatePathField($this->application->dockerfile_location, 'dockerfile_location');
}
$this->prepare_builder_image();
$this->check_git_if_build_needed();
@ -3879,6 +3879,18 @@ private function add_build_secrets_to_compose($composeFile)
return $composeFile;
}
private function validatePathField(string $value, string $fieldName): string
{
if (! preg_match('/^\/[a-zA-Z0-9._\-\/]+$/', $value)) {
throw new \RuntimeException("Invalid {$fieldName}: contains forbidden characters.");
}
if (str_contains($value, '..')) {
throw new \RuntimeException("Invalid {$fieldName}: path traversal detected.");
}
return $value;
}
private function run_pre_deployment_command()
{
if (empty($this->application->pre_deployment_command)) {

4
app/Livewire/Project/Application/General.php
View file

@ -73,7 +73,7 @@ class General extends Component
#[Validate(['string', 'nullable'])]
public ?string $dockerfile = null;
#[Validate(['string', 'nullable'])]
#[Validate(['string', 'nullable', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'])]
public ?string $dockerfileLocation = null;
#[Validate(['string', 'nullable'])]
@ -85,7 +85,7 @@ class General extends Component
#[Validate(['string', 'nullable'])]
public ?string $dockerRegistryImageTag = null;
#[Validate(['string', 'nullable'])]
#[Validate(['string', 'nullable', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'])]
public ?string $dockerComposeLocation = null;
#[Validate(['string', 'nullable'])]

2
app/Livewire/Project/New/GithubPrivateRepository.php
View file

@ -163,10 +163,12 @@ public function submit()
'selected_repository_owner' => $this->selected_repository_owner,
'selected_repository_repo' => $this->selected_repository_repo,
'selected_branch_name' => $this->selected_branch_name,
'docker_compose_location' => $this->docker_compose_location,
], [
'selected_repository_owner' => 'required|string|regex:/^[a-zA-Z0-9\-_]+$/',
'selected_repository_repo' => 'required|string|regex:/^[a-zA-Z0-9\-_\.]+$/',
'selected_branch_name' => ['required', 'string', new ValidGitBranch],
'docker_compose_location' => ['nullable', 'string', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'],
]);
if ($validator->fails()) {

2
app/Livewire/Project/New/GithubPrivateRepositoryDeployKey.php
View file

@ -64,6 +64,7 @@ class GithubPrivateRepositoryDeployKey extends Component
'is_static' => 'required|boolean',
'publish_directory' => 'nullable|string',
'build_pack' => 'required|string',
'docker_compose_location' => ['nullable', 'string', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'],
];
protected function rules()
@ -75,6 +76,7 @@ protected function rules()
'is_static' => 'required|boolean',
'publish_directory' => 'nullable|string',
'build_pack' => 'required|string',
'docker_compose_location' => ['nullable', 'string', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'],
];
}

4
app/Livewire/Project/New/PublicGitRepository.php
View file

@ -70,7 +70,7 @@ class PublicGitRepository extends Component
'publish_directory' => 'nullable|string',
'build_pack' => 'required|string',
'base_directory' => 'nullable|string',
'docker_compose_location' => 'nullable|string',
'docker_compose_location' => ['nullable', 'string', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'],
];
protected function rules()
@ -82,7 +82,7 @@ protected function rules()
'publish_directory' => 'nullable|string',
'build_pack' => 'required|string',
'base_directory' => 'nullable|string',
'docker_compose_location' => 'nullable|string',
'docker_compose_location' => ['nullable', 'string', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'],
'git_branch' => ['required', 'string', new ValidGitBranch],
];
}

4
bootstrap/helpers/api.php
View file

@ -132,8 +132,8 @@ function sharedDataApplications()
'manual_webhook_secret_gitlab' => 'string|nullable',
'manual_webhook_secret_bitbucket' => 'string|nullable',
'manual_webhook_secret_gitea' => 'string|nullable',
'dockerfile_location' => 'string|nullable',
'docker_compose_location' => 'string',
'dockerfile_location' => ['string', 'nullable', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'],
'docker_compose_location' => ['string', 'nullable', 'max:255', 'regex:/^\/[a-zA-Z0-9._\-\/]+$/'],
'docker_compose' => 'string|nullable',
'docker_compose_domains' => 'array|nullable',
'docker_compose_custom_start_command' => 'string|nullable',

2
database/seeders/ApplicationSeeder.php
View file

@ -21,7 +21,7 @@ public function run(): void
'git_repository' => 'coollabsio/coolify-examples',
'git_branch' => 'v4.x',
'base_directory' => '/docker-compose',
'docker_compose_location' => 'docker-compose-test.yaml',
'docker_compose_location' => '/docker-compose-test.yaml',
'build_pack' => 'dockercompose',
'ports_exposes' => '80',
'environment_id' => 1,

18
routes/api.php
View file

@ -121,9 +121,9 @@
Route::delete('/applications/{uuid}/envs/{env_uuid}', [ApplicationsController::class, 'delete_env_by_uuid'])->middleware(['api.ability:write']);
Route::get('/applications/{uuid}/logs', [ApplicationsController::class, 'logs_by_uuid'])->middleware(['api.ability:read']);
Route::match(['get', 'post'], '/applications/{uuid}/start', [ApplicationsController::class, 'action_deploy'])->middleware(['api.ability:write']);
Route::match(['get', 'post'], '/applications/{uuid}/restart', [ApplicationsController::class, 'action_restart'])->middleware(['api.ability:write']);
Route::match(['get', 'post'], '/applications/{uuid}/stop', [ApplicationsController::class, 'action_stop'])->middleware(['api.ability:write']);
Route::match(['get', 'post'], '/applications/{uuid}/start', [ApplicationsController::class, 'action_deploy'])->middleware(['api.ability:deploy']);
Route::match(['get', 'post'], '/applications/{uuid}/restart', [ApplicationsController::class, 'action_restart'])->middleware(['api.ability:deploy']);
Route::match(['get', 'post'], '/applications/{uuid}/stop', [ApplicationsController::class, 'action_stop'])->middleware(['api.ability:deploy']);
Route::get('/github-apps', [GithubController::class, 'list_github_apps'])->middleware(['api.ability:read']);
Route::post('/github-apps', [GithubController::class, 'create_github_app'])->middleware(['api.ability:write']);
@ -152,9 +152,9 @@
Route::delete('/databases/{uuid}/backups/{scheduled_backup_uuid}', [DatabasesController::class, 'delete_backup_by_uuid'])->middleware(['api.ability:write']);
Route::delete('/databases/{uuid}/backups/{scheduled_backup_uuid}/executions/{execution_uuid}', [DatabasesController::class, 'delete_execution_by_uuid'])->middleware(['api.ability:write']);
Route::match(['get', 'post'], '/databases/{uuid}/start', [DatabasesController::class, 'action_deploy'])->middleware(['api.ability:write']);
Route::match(['get', 'post'], '/databases/{uuid}/restart', [DatabasesController::class, 'action_restart'])->middleware(['api.ability:write']);
Route::match(['get', 'post'], '/databases/{uuid}/stop', [DatabasesController::class, 'action_stop'])->middleware(['api.ability:write']);
Route::match(['get', 'post'], '/databases/{uuid}/start', [DatabasesController::class, 'action_deploy'])->middleware(['api.ability:deploy']);
Route::match(['get', 'post'], '/databases/{uuid}/restart', [DatabasesController::class, 'action_restart'])->middleware(['api.ability:deploy']);
Route::match(['get', 'post'], '/databases/{uuid}/stop', [DatabasesController::class, 'action_stop'])->middleware(['api.ability:deploy']);
Route::get('/services', [ServicesController::class, 'services'])->middleware(['api.ability:read']);
Route::post('/services', [ServicesController::class, 'create_service'])->middleware(['api.ability:write']);
@ -169,9 +169,9 @@
Route::patch('/services/{uuid}/envs', [ServicesController::class, 'update_env_by_uuid'])->middleware(['api.ability:write']);
Route::delete('/services/{uuid}/envs/{env_uuid}', [ServicesController::class, 'delete_env_by_uuid'])->middleware(['api.ability:write']);
Route::match(['get', 'post'], '/services/{uuid}/start', [ServicesController::class, 'action_deploy'])->middleware(['api.ability:write']);
Route::match(['get', 'post'], '/services/{uuid}/restart', [ServicesController::class, 'action_restart'])->middleware(['api.ability:write']);
Route::match(['get', 'post'], '/services/{uuid}/stop', [ServicesController::class, 'action_stop'])->middleware(['api.ability:write']);
Route::match(['get', 'post'], '/services/{uuid}/start', [ServicesController::class, 'action_deploy'])->middleware(['api.ability:deploy']);
Route::match(['get', 'post'], '/services/{uuid}/restart', [ServicesController::class, 'action_restart'])->middleware(['api.ability:deploy']);
Route::match(['get', 'post'], '/services/{uuid}/stop', [ServicesController::class, 'action_stop'])->middleware(['api.ability:deploy']);
Route::get('/applications/{uuid}/scheduled-tasks', [ScheduledTasksController::class, 'scheduled_tasks_by_application_uuid'])->middleware(['api.ability:read']);
Route::post('/applications/{uuid}/scheduled-tasks', [ScheduledTasksController::class, 'create_scheduled_task_by_application_uuid'])->middleware(['api.ability:write']);

276
tests/Feature/CommandInjectionSecurityTest.php Normal file
View file

@ -0,0 +1,276 @@
<?php
use App\Jobs\ApplicationDeploymentJob;
describe('deployment job path field validation', function () {
test('rejects shell metacharacters in dockerfile_location', function () {
$job = new ReflectionClass(ApplicationDeploymentJob::class);
$method = $job->getMethod('validatePathField');
$method->setAccessible(true);
$instance = $job->newInstanceWithoutConstructor();
expect(fn () => $method->invoke($instance, '/Dockerfile; echo pwned', 'dockerfile_location'))
->toThrow(RuntimeException::class, 'contains forbidden characters');
});
test('rejects backtick injection', function () {
$job = new ReflectionClass(ApplicationDeploymentJob::class);
$method = $job->getMethod('validatePathField');
$method->setAccessible(true);
$instance = $job->newInstanceWithoutConstructor();
expect(fn () => $method->invoke($instance, '/Dockerfile`whoami`', 'dockerfile_location'))
->toThrow(RuntimeException::class, 'contains forbidden characters');
});
test('rejects dollar sign variable expansion', function () {
$job = new ReflectionClass(ApplicationDeploymentJob::class);
$method = $job->getMethod('validatePathField');
$method->setAccessible(true);
$instance = $job->newInstanceWithoutConstructor();
expect(fn () => $method->invoke($instance, '/Dockerfile$(whoami)', 'dockerfile_location'))
->toThrow(RuntimeException::class, 'contains forbidden characters');
});
test('rejects pipe injection', function () {
$job = new ReflectionClass(ApplicationDeploymentJob::class);
$method = $job->getMethod('validatePathField');
$method->setAccessible(true);
$instance = $job->newInstanceWithoutConstructor();
expect(fn () => $method->invoke($instance, '/Dockerfile | cat /etc/passwd', 'dockerfile_location'))
->toThrow(RuntimeException::class, 'contains forbidden characters');
});
test('rejects ampersand injection', function () {
$job = new ReflectionClass(ApplicationDeploymentJob::class);
$method = $job->getMethod('validatePathField');
$method->setAccessible(true);
$instance = $job->newInstanceWithoutConstructor();
expect(fn () => $method->invoke($instance, '/Dockerfile && env', 'dockerfile_location'))
->toThrow(RuntimeException::class, 'contains forbidden characters');
});
test('rejects path traversal', function () {
$job = new ReflectionClass(ApplicationDeploymentJob::class);
$method = $job->getMethod('validatePathField');
$method->setAccessible(true);
$instance = $job->newInstanceWithoutConstructor();
expect(fn () => $method->invoke($instance, '/../../../etc/passwd', 'dockerfile_location'))
->toThrow(RuntimeException::class, 'path traversal detected');
});
test('allows valid simple path', function () {
$job = new ReflectionClass(ApplicationDeploymentJob::class);
$method = $job->getMethod('validatePathField');
$method->setAccessible(true);
$instance = $job->newInstanceWithoutConstructor();
expect($method->invoke($instance, '/Dockerfile', 'dockerfile_location'))
->toBe('/Dockerfile');
});
test('allows valid nested path with dots and hyphens', function () {
$job = new ReflectionClass(ApplicationDeploymentJob::class);
$method = $job->getMethod('validatePathField');
$method->setAccessible(true);
$instance = $job->newInstanceWithoutConstructor();
expect($method->invoke($instance, '/docker/Dockerfile.prod', 'dockerfile_location'))
->toBe('/docker/Dockerfile.prod');
});
test('allows valid compose file path', function () {
$job = new ReflectionClass(ApplicationDeploymentJob::class);
$method = $job->getMethod('validatePathField');
$method->setAccessible(true);
$instance = $job->newInstanceWithoutConstructor();
expect($method->invoke($instance, '/docker-compose.prod.yml', 'docker_compose_location'))
->toBe('/docker-compose.prod.yml');
});
});
describe('API validation rules for path fields', function () {
test('dockerfile_location validation rejects shell metacharacters', function () {
$rules = sharedDataApplications();
$validator = validator(
['dockerfile_location' => '/Dockerfile; echo pwned; #'],
['dockerfile_location' => $rules['dockerfile_location']]
);
expect($validator->fails())->toBeTrue();
});
test('dockerfile_location validation allows valid paths', function () {
$rules = sharedDataApplications();
$validator = validator(
['dockerfile_location' => '/docker/Dockerfile.prod'],
['dockerfile_location' => $rules['dockerfile_location']]
);
expect($validator->fails())->toBeFalse();
});
test('docker_compose_location validation rejects shell metacharacters', function () {
$rules = sharedDataApplications();
$validator = validator(
['docker_compose_location' => '/docker-compose.yml; env; #'],
['docker_compose_location' => $rules['docker_compose_location']]
);
expect($validator->fails())->toBeTrue();
});
test('docker_compose_location validation allows valid paths', function () {
$rules = sharedDataApplications();
$validator = validator(
['docker_compose_location' => '/docker/docker-compose.prod.yml'],
['docker_compose_location' => $rules['docker_compose_location']]
);
expect($validator->fails())->toBeFalse();
});
});
describe('sharedDataApplications rules survive array_merge in controller', function () {
test('docker_compose_location safe regex is not overridden by local rules', function () {
$sharedRules = sharedDataApplications();
// Simulate what ApplicationsController does: array_merge(shared, local)
// After our fix, local no longer contains docker_compose_location,
// so the shared regex rule must survive
$localRules = [
'name' => 'string|max:255',
'docker_compose_domains' => 'array|nullable',
];
$merged = array_merge($sharedRules, $localRules);
// The merged rules for docker_compose_location should be the safe regex, not just 'string'
expect($merged['docker_compose_location'])->toBeArray();
expect($merged['docker_compose_location'])->toContain('regex:/^\/[a-zA-Z0-9._\-\/]+$/');
});
});
describe('path fields require leading slash', function () {
test('dockerfile_location without leading slash is rejected by API rules', function () {
$rules = sharedDataApplications();
$validator = validator(
['dockerfile_location' => 'Dockerfile'],
['dockerfile_location' => $rules['dockerfile_location']]
);
expect($validator->fails())->toBeTrue();
});
test('docker_compose_location without leading slash is rejected by API rules', function () {
$rules = sharedDataApplications();
$validator = validator(
['docker_compose_location' => 'docker-compose.yaml'],
['docker_compose_location' => $rules['docker_compose_location']]
);
expect($validator->fails())->toBeTrue();
});
test('deployment job rejects path without leading slash', function () {
$job = new ReflectionClass(ApplicationDeploymentJob::class);
$method = $job->getMethod('validatePathField');
$method->setAccessible(true);
$instance = $job->newInstanceWithoutConstructor();
expect(fn () => $method->invoke($instance, 'docker-compose.yaml', 'docker_compose_location'))
->toThrow(RuntimeException::class, 'contains forbidden characters');
});
});
describe('API route middleware for deploy actions', function () {
test('application start route requires deploy ability', function () {
$routes = app('router')->getRoutes();
$route = $routes->getByAction('App\Http\Controllers\Api\ApplicationsController@action_deploy');
expect($route)->not->toBeNull();
$middleware = $route->gatherMiddleware();
expect($middleware)->toContain('api.ability:deploy');
expect($middleware)->not->toContain('api.ability:write');
});
test('application restart route requires deploy ability', function () {
$routes = app('router')->getRoutes();
$matchedRoute = null;
foreach ($routes as $route) {
if (str_contains($route->uri(), 'applications') && str_contains($route->uri(), 'restart')) {
$matchedRoute = $route;
break;
}
}
expect($matchedRoute)->not->toBeNull();
$middleware = $matchedRoute->gatherMiddleware();
expect($middleware)->toContain('api.ability:deploy');
});
test('application stop route requires deploy ability', function () {
$routes = app('router')->getRoutes();
$matchedRoute = null;
foreach ($routes as $route) {
if (str_contains($route->uri(), 'applications') && str_contains($route->uri(), 'stop')) {
$matchedRoute = $route;
break;
}
}
expect($matchedRoute)->not->toBeNull();
$middleware = $matchedRoute->gatherMiddleware();
expect($middleware)->toContain('api.ability:deploy');
});
test('database start route requires deploy ability', function () {
$routes = app('router')->getRoutes();
$matchedRoute = null;
foreach ($routes as $route) {
if (str_contains($route->uri(), 'databases') && str_contains($route->uri(), 'start')) {
$matchedRoute = $route;
break;
}
}
expect($matchedRoute)->not->toBeNull();
$middleware = $matchedRoute->gatherMiddleware();
expect($middleware)->toContain('api.ability:deploy');
});
test('service start route requires deploy ability', function () {
$routes = app('router')->getRoutes();
$matchedRoute = null;
foreach ($routes as $route) {
if (str_contains($route->uri(), 'services') && str_contains($route->uri(), 'start')) {
$matchedRoute = $route;
break;
}
}
expect($matchedRoute)->not->toBeNull();
$middleware = $matchedRoute->gatherMiddleware();
expect($middleware)->toContain('api.ability:deploy');
});
});