From 3fdce06b654fa3b7b4be59c0faaab6b4546c78de Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 25 Mar 2026 23:44:37 +0100 Subject: [PATCH] fix(storage): consistent path validation and escaping for file volumes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../Api/ApplicationsController.php | 27 +++++----- .../Controllers/Api/DatabasesController.php | 13 +++-- .../Controllers/Api/ServicesController.php | 7 ++- app/Livewire/Project/Service/Storage.php | 13 +++-- app/Models/LocalFileVolume.php | 20 +++++--- tests/Unit/FileStorageSecurityTest.php | 50 +++++++++++++++++++ 6 files changed, 101 insertions(+), 29 deletions(-) diff --git a/app/Http/Controllers/Api/ApplicationsController.php b/app/Http/Controllers/Api/ApplicationsController.php index 66f6a1ef8..b081069b7 100644 --- a/app/Http/Controllers/Api/ApplicationsController.php +++ b/app/Http/Controllers/Api/ApplicationsController.php @@ -1002,7 +1002,7 @@ private function create_application(Request $request, $type) $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 @@ private function create_application(Request $request, $type) $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 @@ private function create_application(Request $request, $type) } $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 @@ private function create_application(Request $request, $type) } $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 @@ private function create_application(Request $request, $type) } $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 @@ private function create_application(Request $request, $type) $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 @@ private function create_application(Request $request, $type) ], 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 @@ public function update_by_uuid(Request $request) return invalidTokenResponse(); } $return = validateIncomingRequest($request); - if ($return instanceof \Illuminate\Http\JsonResponse) { + if ($return instanceof JsonResponse) { return $return; } @@ -2530,7 +2530,7 @@ public function update_by_uuid(Request $request) } } $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 @@ public function update_env_by_uuid(Request $request) } $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 @@ public function create_bulk_envs(Request $request) } $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 @@ public function update_storage(Request $request): JsonResponse } $return = validateIncomingRequest($request); - if ($return instanceof \Illuminate\Http\JsonResponse) { + if ($return instanceof JsonResponse) { return $return; } @@ -4361,6 +4361,9 @@ public function create_storage(Request $request): JsonResponse ]); } else { $mountPath = str($request->mount_path)->trim()->start('/')->value(); + + validateShellSafePath($mountPath, 'file storage path'); + $fsPath = application_configuration_dir().'/'.$application->uuid.$mountPath; $storage = LocalFileVolume::create([ diff --git a/app/Http/Controllers/Api/DatabasesController.php b/app/Http/Controllers/Api/DatabasesController.php index 44b66e57e..f9e171eee 100644 --- a/app/Http/Controllers/Api/DatabasesController.php +++ b/app/Http/Controllers/Api/DatabasesController.php @@ -334,7 +334,7 @@ public function update_by_uuid(Request $request) // 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 @@ public function create_backup(Request $request) // 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 @@ public function update_backup(Request $request) } // 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 @@ public function create_database(Request $request, NewDatabaseTypes $type) $this->authorize('create', StandalonePostgresql::class); $return = validateIncomingRequest($request); - if ($return instanceof \Illuminate\Http\JsonResponse) { + if ($return instanceof JsonResponse) { return $return; } @@ -3554,6 +3554,9 @@ public function create_storage(Request $request): JsonResponse ]); } 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 @@ public function update_storage(Request $request): JsonResponse } $return = validateIncomingRequest($request); - if ($return instanceof \Illuminate\Http\JsonResponse) { + if ($return instanceof JsonResponse) { return $return; } diff --git a/app/Http/Controllers/Api/ServicesController.php b/app/Http/Controllers/Api/ServicesController.php index ca565ece0..89635875c 100644 --- a/app/Http/Controllers/Api/ServicesController.php +++ b/app/Http/Controllers/Api/ServicesController.php @@ -302,7 +302,7 @@ public function create_service(Request $request) $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 @@ public function update_by_uuid(Request $request) } $return = validateIncomingRequest($request); - if ($return instanceof \Illuminate\Http\JsonResponse) { + if ($return instanceof JsonResponse) { return $return; } @@ -2110,6 +2110,9 @@ public function create_storage(Request $request): JsonResponse ]); } else { $mountPath = str($request->mount_path)->trim()->start('/')->value(); + + validateShellSafePath($mountPath, 'file storage path'); + $fsPath = service_configuration_dir().'/'.$service->uuid.$mountPath; $storage = LocalFileVolume::create([ diff --git a/app/Livewire/Project/Service/Storage.php b/app/Livewire/Project/Service/Storage.php index 12d8bcbc3..e896f060a 100644 --- a/app/Livewire/Project/Service/Storage.php +++ b/app/Livewire/Project/Service/Storage.php @@ -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 @@ public function mount() $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 @@ public function submitFileStorage() $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 @@ public function submitFileStorage() 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 @@ public function submitFileStorageDirectory() 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, diff --git a/app/Models/LocalFileVolume.php b/app/Models/LocalFileVolume.php index da58ed2f9..b954a1dd5 100644 --- a/app/Models/LocalFileVolume.php +++ b/app/Models/LocalFileVolume.php @@ -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 @@ protected static function booted() { static::created(function (LocalFileVolume $fileVolume) { $fileVolume->load(['service']); - dispatch(new \App\Jobs\ServerStorageSaveJob($fileVolume)); + dispatch(new ServerStorageSaveJob($fileVolume)); }); } @@ -129,15 +130,22 @@ public function saveStorageOnServer() $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 @@ public function saveStorageOnServer() $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); diff --git a/tests/Unit/FileStorageSecurityTest.php b/tests/Unit/FileStorageSecurityTest.php index a89a209b1..192ea8c8f 100644 --- a/tests/Unit/FileStorageSecurityTest.php +++ b/tests/Unit/FileStorageSecurityTest.php @@ -91,3 +91,53 @@ 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 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); +});