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

This commit is contained in:
Andras Bacsai 2026-03-26 07:44:46 +01:00 committed by GitHub
commit fecb80b596
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 101 additions and 29 deletions

View file

@ -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([

View file

@ -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;
}

View file

@ -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([

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 @@ 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,

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 @@ 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);

View file

@ -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</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);
});