tiennm99/coolify
1
0
Fork 0
mirror of https://github.com/tiennm99/coolify.git synced 2026-08-05 10:23:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(storage): consistent path validation and escaping for file volumes

Browse Source 
Ensure all file volume paths are validated and properly escaped before
use. Previously, only directory mount paths were validated at the input
layer — file mount paths now receive the same treatment across Livewire
components, API controllers, and the model layer.

- Validate and escape fs_path at the top of saveStorageOnServer() before
  any commands are built
- Add path validation to submitFileStorage() in Storage Livewire component
- Add path validation to file mount creation in Applications, Services,
  and Databases API controllers
- Add regression tests for path validation coverage

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Andras Bacsai
2026-03-25 23:44:37 +01:00
co-authored by Claude Opus 4.6
parent 47668121a4
commit 3fdce06b65
6 changed files with 101 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/Http/Controllers/Api/ApplicationsController.php
+15 -12
View File
@@ -1002,7 +1002,7 @@ class ApplicationsController extends Controller
$this->authorize('create', Application::class);
$return = validateIncomingRequest($request);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
$allowedFields = ['project_uuid', 'environment_name', 'environment_uuid', 'server_uuid', 'destination_uuid', 'type', 'name', 'description', 'is_static', 'is_spa', 'is_auto_deploy_enabled', 'is_force_https_enabled', 'domains', 'git_repository', 'git_branch', 'git_commit_sha', 'private_key_uuid', 'docker_registry_image_name', 'docker_registry_image_tag', 'build_pack', 'install_command', 'build_command', 'start_command', 'ports_exposes', 'ports_mappings', 'custom_network_aliases', 'base_directory', 'publish_directory', 'health_check_enabled', 'health_check_type', 'health_check_command', 'health_check_path', 'health_check_port', 'health_check_host', 'health_check_method', 'health_check_return_code', 'health_check_scheme', 'health_check_response_text', 'health_check_interval', 'health_check_timeout', 'health_check_retries', 'health_check_start_period', 'limits_memory', 'limits_memory_swap', 'limits_memory_swappiness', 'limits_memory_reservation', 'limits_cpus', 'limits_cpuset', 'limits_cpu_shares', 'custom_labels', 'custom_docker_run_options', 'post_deployment_command', 'post_deployment_command_container', 'pre_deployment_command', 'pre_deployment_command_container', 'manual_webhook_secret_github', 'manual_webhook_secret_gitlab', 'manual_webhook_secret_bitbucket', 'manual_webhook_secret_gitea', 'redirect', 'github_app_uuid', 'instant_deploy', 'dockerfile', 'dockerfile_location', 'docker_compose_location', 'docker_compose_raw', 'docker_compose_custom_start_command', 'docker_compose_custom_build_command', 'docker_compose_domains', 'watch_paths', 'use_build_server', 'static_image', 'custom_nginx_configuration', 'is_http_basic_auth_enabled', 'http_basic_auth_username', 'http_basic_auth_password', 'connect_to_docker_network', 'force_domain_override', 'autogenerate_domain', 'is_container_label_escape_enabled'];
@@ -1150,7 +1150,7 @@ class ApplicationsController extends Controller
$request->offsetSet('name', generate_application_name($request->git_repository, $request->git_branch));
}
$return = $this->validateDataApplications($request, $server);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
@@ -1345,7 +1345,7 @@ class ApplicationsController extends Controller
}
$return = $this->validateDataApplications($request, $server);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
$githubApp = GithubApp::whereTeamId($teamId)->where('uuid', $githubAppUuid)->first();
@@ -1573,7 +1573,7 @@ class ApplicationsController extends Controller
}
$return = $this->validateDataApplications($request, $server);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
$privateKey = PrivateKey::whereTeamId($teamId)->where('uuid', $request->private_key_uuid)->first();
@@ -1742,7 +1742,7 @@ class ApplicationsController extends Controller
}
$return = $this->validateDataApplications($request, $server);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
if (! isBase64Encoded($request->dockerfile)) {
@@ -1850,7 +1850,7 @@ class ApplicationsController extends Controller
$request->offsetSet('name', 'docker-image-'.new Cuid2);
}
$return = $this->validateDataApplications($request, $server);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
// Process docker image name and tag using DockerImageParser
@@ -1974,7 +1974,7 @@ class ApplicationsController extends Controller
], 422);
}
$return = $this->validateDataApplications($request, $server);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
if (! isBase64Encoded($request->docker_compose_raw)) {
@@ -2460,7 +2460,7 @@ class ApplicationsController extends Controller
return invalidTokenResponse();
}
$return = validateIncomingRequest($request);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
@@ -2530,7 +2530,7 @@ class ApplicationsController extends Controller
}
}
$return = $this->validateDataApplications($request, $server);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
$extraFields = array_diff(array_keys($request->all()), $allowedFields);
@@ -2956,7 +2956,7 @@ class ApplicationsController extends Controller
}
$return = validateIncomingRequest($request);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
$application = Application::ownedByCurrentTeamAPI($teamId)->where('uuid', $request->route('uuid'))->first();
@@ -3157,7 +3157,7 @@ class ApplicationsController extends Controller
}
$return = validateIncomingRequest($request);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
$application = Application::ownedByCurrentTeamAPI($teamId)->where('uuid', $request->route('uuid'))->first();
@@ -4077,7 +4077,7 @@ class ApplicationsController extends Controller
}
$return = validateIncomingRequest($request);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
@@ -4361,6 +4361,9 @@ class ApplicationsController extends Controller
]);
} else {
$mountPath = str($request->mount_path)->trim()->start('/')->value();
validateShellSafePath($mountPath, 'file storage path');
$fsPath = application_configuration_dir().'/'.$application->uuid.$mountPath;
$storage = LocalFileVolume::create([
app/Http/Controllers/Api/DatabasesController.php
+8 -5
View File
@@ -334,7 +334,7 @@ class DatabasesController extends Controller
// this check if the request is a valid json
$return = validateIncomingRequest($request);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
$validator = customApiValidator($request->all(), [
@@ -685,7 +685,7 @@ class DatabasesController extends Controller
// Validate incoming request is valid JSON
$return = validateIncomingRequest($request);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
@@ -914,7 +914,7 @@ class DatabasesController extends Controller
}
// this check if the request is a valid json
$return = validateIncomingRequest($request);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
$validator = customApiValidator($request->all(), [
@@ -1590,7 +1590,7 @@ class DatabasesController extends Controller
$this->authorize('create', StandalonePostgresql::class);
$return = validateIncomingRequest($request);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
@@ -3554,6 +3554,9 @@ class DatabasesController extends Controller
]);
} else {
$mountPath = str($request->mount_path)->trim()->start('/')->value();
validateShellSafePath($mountPath, 'file storage path');
$fsPath = database_configuration_dir().'/'.$database->uuid.$mountPath;
$storage = LocalFileVolume::create([
@@ -3646,7 +3649,7 @@ class DatabasesController extends Controller
}
$return = validateIncomingRequest($request);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
app/Http/Controllers/Api/ServicesController.php
+5 -2
View File
@@ -302,7 +302,7 @@ class ServicesController extends Controller
$this->authorize('create', Service::class);
$return = validateIncomingRequest($request);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
$validationRules = [
@@ -925,7 +925,7 @@ class ServicesController extends Controller
}
$return = validateIncomingRequest($request);
if ($return instanceof \Illuminate\Http\JsonResponse) {
if ($return instanceof JsonResponse) {
return $return;
}
@@ -2110,6 +2110,9 @@ class ServicesController extends Controller
]);
} else {
$mountPath = str($request->mount_path)->trim()->start('/')->value();
validateShellSafePath($mountPath, 'file storage path');
$fsPath = service_configuration_dir().'/'.$service->uuid.$mountPath;
$storage = LocalFileVolume::create([
app/Livewire/Project/Service/Storage.php
+9 -4
View File
@@ -2,6 +2,8 @@
namespace App\Livewire\Project\Service;
use App\Models\Application;
use App\Models\LocalFileVolume;
use App\Models\LocalPersistentVolume;
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
use Livewire\Component;
@@ -49,7 +51,7 @@ class Storage extends Component
$this->file_storage_directory_source = application_configuration_dir()."/{$this->resource->uuid}";
}
if ($this->resource->getMorphClass() === \App\Models\Application::class) {
if ($this->resource->getMorphClass() === Application::class) {
if ($this->resource->destination->server->isSwarm()) {
$this->isSwarm = true;
}
@@ -138,7 +140,10 @@ class Storage extends Component
$this->file_storage_path = trim($this->file_storage_path);
$this->file_storage_path = str($this->file_storage_path)->start('/')->value();
if ($this->resource->getMorphClass() === \App\Models\Application::class) {
// Validate path to prevent command injection
validateShellSafePath($this->file_storage_path, 'file storage path');
if ($this->resource->getMorphClass() === Application::class) {
$fs_path = application_configuration_dir().'/'.$this->resource->uuid.$this->file_storage_path;
} elseif (str($this->resource->getMorphClass())->contains('Standalone')) {
$fs_path = database_configuration_dir().'/'.$this->resource->uuid.$this->file_storage_path;
@@ -146,7 +151,7 @@ class Storage extends Component
throw new \Exception('No valid resource type for file mount storage type!');
}
\App\Models\LocalFileVolume::create([
LocalFileVolume::create([
'fs_path' => $fs_path,
'mount_path' => $this->file_storage_path,
'content' => $this->file_storage_content,
@@ -183,7 +188,7 @@ class Storage extends Component
validateShellSafePath($this->file_storage_directory_source, 'storage source path');
validateShellSafePath($this->file_storage_directory_destination, 'storage destination path');
\App\Models\LocalFileVolume::create([
LocalFileVolume::create([
'fs_path' => $this->file_storage_directory_source,
'mount_path' => $this->file_storage_directory_destination,
'is_directory' => true,
app/Models/LocalFileVolume.php
+14 -6
View File
@@ -3,6 +3,7 @@
namespace App\Models;
use App\Events\FileStorageChanged;
use App\Jobs\ServerStorageSaveJob;
use Illuminate\Database\Eloquent\Casts\Attribute;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Symfony\Component\Yaml\Yaml;
@@ -27,7 +28,7 @@ class LocalFileVolume extends BaseModel
{
static::created(function (LocalFileVolume $fileVolume) {
$fileVolume->load(['service']);
dispatch(new \App\Jobs\ServerStorageSaveJob($fileVolume));
dispatch(new ServerStorageSaveJob($fileVolume));
});
}
@@ -129,15 +130,22 @@ class LocalFileVolume extends BaseModel
$server = $this->resource->destination->server;
}
$commands = collect([]);
// Validate fs_path early before any shell interpolation
validateShellSafePath($this->fs_path, 'storage path');
$escapedFsPath = escapeshellarg($this->fs_path);
$escapedWorkdir = escapeshellarg($workdir);
if ($this->is_directory) {
$commands->push("mkdir -p $this->fs_path > /dev/null 2>&1 || true");
$commands->push("mkdir -p $workdir > /dev/null 2>&1 || true");
$commands->push("cd $workdir");
$commands->push("mkdir -p {$escapedFsPath} > /dev/null 2>&1 || true");
$commands->push("mkdir -p {$escapedWorkdir} > /dev/null 2>&1 || true");
$commands->push("cd {$escapedWorkdir}");
}
if (str($this->fs_path)->startsWith('.') || str($this->fs_path)->startsWith('/') || str($this->fs_path)->startsWith('~')) {
$parent_dir = str($this->fs_path)->beforeLast('/');
if ($parent_dir != '') {
$commands->push("mkdir -p $parent_dir > /dev/null 2>&1 || true");
$escapedParentDir = escapeshellarg($parent_dir);
$commands->push("mkdir -p {$escapedParentDir} > /dev/null 2>&1 || true");
}
}
$path = data_get_str($this, 'fs_path');
@@ -147,7 +155,7 @@ class LocalFileVolume extends BaseModel
$path = $workdir.$path;
}
// Validate and escape path to prevent command injection
// Validate and escape resolved path (may differ from fs_path if relative)
validateShellSafePath($path, 'storage path');
$escapedPath = escapeshellarg($path);
tests/Unit/FileStorageSecurityTest.php
+50
View File
@@ -91,3 +91,53 @@ test('file storage accepts paths with underscores and hyphens', function () {
expect(fn () => validateShellSafePath('/tmp/upload_dir-2024', 'storage path'))
->not->toThrow(Exception::class);
});
// --- Regression tests for GHSA-46hp-7m8g-7622 ---
// These verify that file mount paths (not just directory mounts) are validated,
// and that saveStorageOnServer() validates fs_path before any shell interpolation.
test('file storage rejects command injection in file mount path context', function () {
$maliciousPaths = [
'/app/config$(id)',
'/app/config;whoami',
'/app/config|cat /etc/passwd',
'/app/config`id`',
'/app/config&whoami',
'/app/config>/tmp/pwned',
'/app/config</etc/shadow',
"/app/config\nrm -rf /",
];
foreach ($maliciousPaths as $path) {
expect(fn () => validateShellSafePath($path, 'file storage path'))
->toThrow(Exception::class);
}
});
test('file storage rejects variable substitution in paths', function () {
expect(fn () => validateShellSafePath('/data/${IFS}cat${IFS}/etc/passwd', 'file storage path'))
->toThrow(Exception::class);
});
test('file storage accepts safe file mount paths', function () {
$safePaths = [
'/etc/nginx/nginx.conf',
'/app/.env',
'/data/coolify/services/abc123/config.yml',
'/var/www/html/index.php',
'/opt/app/config/database.json',
];
foreach ($safePaths as $path) {
expect(fn () => validateShellSafePath($path, 'file storage path'))
->not->toThrow(Exception::class);
}
});
test('file storage accepts relative dot-prefixed paths', function () {
expect(fn () => validateShellSafePath('./config/app.yaml', 'storage path'))
->not->toThrow(Exception::class);
expect(fn () => validateShellSafePath('./data', 'storage path'))
->not->toThrow(Exception::class);
});