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

fix(validation): allow ampersands and quotes in shell-safe command pattern

Browse source 
Previously, the SHELL_SAFE_COMMAND_PATTERN was overly restrictive and blocked
legitimate characters needed for common Docker operations:

- Allow & for command chaining with && in multi-step build commands
- Allow " for build arguments with spaces (e.g., --build-arg KEY="value")

Update validation messages to reflect the new allowed operators and refactor
code to use imports instead of full class paths for better readability.
This commit is contained in:
Andras Bacsai 2026-03-25 20:27:21 +01:00
parent c8a96b6f12
commit e2ba44d0c3
3 changed files with 31 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
app/Livewire/Project/Application/General.php
View file

@ -3,11 +3,14 @@
namespace App\Livewire\Project\Application;
use App\Actions\Application\GenerateConfig;
use App\Jobs\ApplicationDeploymentJob;
use App\Models\Application;
use App\Support\ValidationPatterns;
use Illuminate\Auth\Access\AuthorizationException;
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
use Illuminate\Support\Collection;
use Livewire\Component;
use Livewire\Features\SupportEvents\Event;
use Spatie\Url\Url;
use Visus\Cuid2\Cuid2;
@ -194,9 +197,9 @@ protected function messages(): array
'baseDirectory.regex' => 'The base directory must be a valid path starting with / and containing only safe characters.',
'publishDirectory.regex' => 'The publish directory must be a valid path starting with / and containing only safe characters.',
'dockerfileTargetBuild.regex' => 'The Dockerfile target build must contain only alphanumeric characters, dots, hyphens, and underscores.',
'dockerComposeCustomStartCommand.regex' => 'The Docker Compose start command contains invalid characters. Shell operators like ;, &, |, $, and backticks are not allowed.',
'dockerComposeCustomBuildCommand.regex' => 'The Docker Compose build command contains invalid characters. Shell operators like ;, &, |, $, and backticks are not allowed.',
'customDockerRunOptions.regex' => 'The custom Docker run options contain invalid characters. Shell operators like ;, &, |, $, and backticks are not allowed.',
'dockerComposeCustomStartCommand.regex' => 'The Docker Compose start command contains invalid characters. Shell operators like ;, |, $, and backticks are not allowed.',
'dockerComposeCustomBuildCommand.regex' => 'The Docker Compose build command contains invalid characters. Shell operators like ;, |, $, and backticks are not allowed.',
'customDockerRunOptions.regex' => 'The custom Docker run options contain invalid characters. Shell operators like ;, |, $, and backticks are not allowed.',
'preDeploymentCommandContainer.regex' => 'The pre-deployment command container name must contain only alphanumeric characters, dots, hyphens, and underscores.',
'postDeploymentCommandContainer.regex' => 'The post-deployment command container name must contain only alphanumeric characters, dots, hyphens, and underscores.',
'name.required' => 'The Name field is required.',
@ -288,7 +291,7 @@ public function mount()
$this->authorize('update', $this->application);
$this->application->fqdn = null;
$this->application->settings->save();
} catch (\Illuminate\Auth\Access\AuthorizationException $e) {
} catch (AuthorizationException $e) {
// User doesn't have update permission, just continue without saving
}
}
@ -309,7 +312,7 @@ public function mount()
$this->customLabels = str(implode('|coolify|', generateLabelsApplication($this->application)))->replace('|coolify|', "\n");
$this->application->custom_labels = base64_encode($this->customLabels);
$this->application->save();
} catch (\Illuminate\Auth\Access\AuthorizationException $e) {
} catch (AuthorizationException $e) {
// User doesn't have update permission, just use existing labels
// $this->customLabels = str(implode('|coolify|', generateLabelsApplication($this->application)))->replace('|coolify|', "\n");
}
@ -321,7 +324,7 @@ public function mount()
$this->authorize('update', $this->application);
$this->initLoadingCompose = true;
$this->dispatch('info', 'Loading docker compose file.');
} catch (\Illuminate\Auth\Access\AuthorizationException $e) {
} catch (AuthorizationException $e) {
// User doesn't have update permission, skip loading compose file
}
}
@ -587,7 +590,7 @@ public function updatedBuildPack()
// Check if user has permission to update
try {
$this->authorize('update', $this->application);
} catch (\Illuminate\Auth\Access\AuthorizationException $e) {
} catch (AuthorizationException $e) {
// User doesn't have permission, revert the change and return
$this->application->refresh();
$this->syncData();
@ -612,7 +615,7 @@ public function updatedBuildPack()
$this->fqdn = null;
$this->application->fqdn = null;
$this->application->settings->save();
} catch (\Illuminate\Auth\Access\AuthorizationException $e) {
} catch (AuthorizationException $e) {
// User doesn't have update permission, just continue without saving
}
}
@ -809,7 +812,7 @@ public function submit($showToaster = true)
restoreBaseDirectory: $oldBaseDirectory,
restoreDockerComposeLocation: $oldDockerComposeLocation
);
if ($compose_return instanceof \Livewire\Features\SupportEvents\Event) {
if ($compose_return instanceof Event) {
// Validation failed - restore original values to component properties
$this->baseDirectory = $oldBaseDirectory;
$this->dockerComposeLocation = $oldDockerComposeLocation;
@ -939,7 +942,7 @@ public function getDockerComposeBuildCommandPreviewProperty(): string
$command = injectDockerComposeFlags(
$this->dockerComposeCustomBuildCommand,
".{$normalizedBase}{$this->dockerComposeLocation}",
\App\Jobs\ApplicationDeploymentJob::BUILD_TIME_ENV_PATH
ApplicationDeploymentJob::BUILD_TIME_ENV_PATH
);
// Inject build args if not using build secrets

8
app/Support/ValidationPatterns.php
View file

@ -37,11 +37,13 @@ class ValidationPatterns
/**
* Pattern for shell-safe command strings (docker compose commands, docker run options)
* Blocks dangerous shell metacharacters: ; & | ` $ ( ) > < newlines and carriage returns
* Also blocks backslashes, single quotes, and double quotes to prevent escape-sequence attacks
* Blocks dangerous shell metacharacters: ; | ` $ ( ) > < newlines and carriage returns
* Allows & for command chaining (&&) which is common in multi-step build commands
* Allows double quotes for build args with spaces (e.g. --build-arg KEY="value")
* Blocks backslashes and single quotes to prevent escape-sequence attacks
* Uses [ \t] instead of \s to explicitly exclude \n and \r (which act as command separators)
*/
public const SHELL_SAFE_COMMAND_PATTERN = '/^[a-zA-Z0-9 \t._\-\/=:@,+\[\]{}#%^~]+$/';
public const SHELL_SAFE_COMMAND_PATTERN = '/^[a-zA-Z0-9 \t._\-\/=:@,+\[\]{}#%^~&"]+$/';
/**
* Pattern for Docker container names

24
tests/Feature/CommandInjectionSecurityTest.php
View file

@ -1,6 +1,7 @@
<?php
use App\Jobs\ApplicationDeploymentJob;
use App\Support\ValidationPatterns;
describe('deployment job path field validation', function () {
test('rejects shell metacharacters in dockerfile_location', function () {
@ -197,7 +198,7 @@
// The merged rules for docker_compose_location should be the safe regex, not just 'string'
expect($merged['docker_compose_location'])->toBeArray();
expect($merged['docker_compose_location'])->toContain('regex:'.\App\Support\ValidationPatterns::FILE_PATH_PATTERN);
expect($merged['docker_compose_location'])->toContain('regex:'.ValidationPatterns::FILE_PATH_PATTERN);
});
});
@ -285,7 +286,7 @@
$job = new ReflectionClass(ApplicationDeploymentJob::class);
// Test that validateShellSafeCommand is also available as a pattern
$pattern = \App\Support\ValidationPatterns::DOCKER_TARGET_PATTERN;
$pattern = ValidationPatterns::DOCKER_TARGET_PATTERN;
expect(preg_match($pattern, 'production'))->toBe(1);
expect(preg_match($pattern, 'build; env'))->toBe(0);
expect(preg_match($pattern, 'target`whoami`'))->toBe(0);
@ -364,15 +365,15 @@
expect($validator->fails())->toBeTrue();
});
test('rejects ampersand chaining in docker_compose_custom_start_command', function () {
test('allows ampersand chaining in docker_compose_custom_start_command', function () {
$rules = sharedDataApplications();
$validator = validator(
['docker_compose_custom_start_command' => 'docker compose up && rm -rf /'],
['docker_compose_custom_start_command' => 'docker compose up && docker compose logs'],
['docker_compose_custom_start_command' => $rules['docker_compose_custom_start_command']]
);
expect($validator->fails())->toBeTrue();
expect($validator->fails())->toBeFalse();
});
test('rejects command substitution in docker_compose_custom_build_command', function () {
@ -399,6 +400,7 @@
'docker compose build',
'docker compose up -d --build',
'docker compose -f custom.yml build --no-cache',
'docker compose build && docker tag registry.example.com/app:beta localhost:5000/app:beta && docker push localhost:5000/app:beta',
]);
test('rejects backslash in docker_compose_custom_start_command', function () {
@ -423,15 +425,15 @@
expect($validator->fails())->toBeTrue();
});
test('rejects double quotes in docker_compose_custom_start_command', function () {
test('allows double quotes in docker_compose_custom_start_command', function () {
$rules = sharedDataApplications();
$validator = validator(
['docker_compose_custom_start_command' => 'docker compose up -d --build "malicious"'],
['docker_compose_custom_start_command' => 'docker compose up -d --build --build-arg VERSION="1.0.0"'],
['docker_compose_custom_start_command' => $rules['docker_compose_custom_start_command']]
);
expect($validator->fails())->toBeTrue();
expect($validator->fails())->toBeFalse();
});
test('rejects newline injection in docker_compose_custom_start_command', function () {
@ -564,7 +566,7 @@
expect($merged)->toHaveKey('dockerfile_target_build');
expect($merged['dockerfile_target_build'])->toBeArray();
expect($merged['dockerfile_target_build'])->toContain('regex:'.\App\Support\ValidationPatterns::DOCKER_TARGET_PATTERN);
expect($merged['dockerfile_target_build'])->toContain('regex:'.ValidationPatterns::DOCKER_TARGET_PATTERN);
});
});
@ -582,7 +584,7 @@
$merged = array_merge($sharedRules, $localRules);
expect($merged['docker_compose_custom_start_command'])->toBeArray();
expect($merged['docker_compose_custom_start_command'])->toContain('regex:'.\App\Support\ValidationPatterns::SHELL_SAFE_COMMAND_PATTERN);
expect($merged['docker_compose_custom_start_command'])->toContain('regex:'.ValidationPatterns::SHELL_SAFE_COMMAND_PATTERN);
});
test('docker_compose_custom_build_command safe regex is not overridden by local rules', function () {
@ -595,7 +597,7 @@
$merged = array_merge($sharedRules, $localRules);
expect($merged['docker_compose_custom_build_command'])->toBeArray();
expect($merged['docker_compose_custom_build_command'])->toContain('regex:'.\App\Support\ValidationPatterns::SHELL_SAFE_COMMAND_PATTERN);
expect($merged['docker_compose_custom_build_command'])->toContain('regex:'.ValidationPatterns::SHELL_SAFE_COMMAND_PATTERN);
});
});