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

Enhance Docker Compose handling and improve error management (#7148)

Browse source
This commit is contained in:
Andras Bacsai 2025-11-08 16:14:15 +01:00 committed by GitHub
commit ed51785fb9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 109 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
app/Actions/Service/StartService.php
View file

@ -22,6 +22,10 @@ public function handle(Service $service, bool $pullLatestImages = false, bool $s
$service->isConfigurationChanged(save: true);
$commands[] = 'cd '.$service->workdir();
$commands[] = "echo 'Saved configuration files to {$service->workdir()}.'";
// Ensure .env file exists before docker compose tries to load it
// This is defensive programming - saveComposeConfigs() already creates it,
// but we guarantee it here in case of any edge cases or manual deployments
$commands[] = 'touch .env';
if ($pullLatestImages) {
$commands[] = "echo 'Pulling images.'";
$commands[] = 'docker compose pull';

6
app/Jobs/ApplicationDeploymentJob.php
View file

@ -3029,6 +3029,12 @@ private function stop_running_container(bool $force = false)
private function start_by_compose_file()
{
// Ensure .env file exists before docker compose tries to load it (defensive programming)
$this->execute_remote_command(
["touch {$this->workdir}/.env", 'hidden' => true],
["touch {$this->configuration_dir}/.env", 'hidden' => true],
);
if ($this->application->build_pack === 'dockerimage') {
$this->application_deployment_queue->addLogEntry('Pulling latest images from the registry.');
$this->execute_remote_command(

25
app/Livewire/Project/Service/StackForm.php
View file

@ -5,6 +5,7 @@
use App\Models\Service;
use App\Support\ValidationPatterns;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\DB;
use Livewire\Component;
class StackForm extends Component
@ -22,7 +23,7 @@ class StackForm extends Component
public string $dockerComposeRaw;
public string $dockerCompose;
public ?string $dockerCompose = null;
public ?bool $connectToDockerNetwork = null;
@ -30,7 +31,7 @@ protected function rules(): array
{
$baseRules = [
'dockerComposeRaw' => 'required',
'dockerCompose' => 'required',
'dockerCompose' => 'nullable',
'name' => ValidationPatterns::nameRules(),
'description' => ValidationPatterns::descriptionRules(),
'connectToDockerNetwork' => 'nullable',
@ -140,18 +141,26 @@ public function submit($notify = true)
$this->validate();
$this->syncData(true);
// Validate for command injection BEFORE saving to database
// Validate for command injection BEFORE any database operations
validateDockerComposeForInjection($this->service->docker_compose_raw);
$this->service->save();
$this->service->saveExtraFields($this->fields);
$this->service->parse();
$this->service->refresh();
$this->service->saveComposeConfigs();
// Use transaction to ensure atomicity - if parse fails, save is rolled back
DB::transaction(function () {
$this->service->save();
$this->service->saveExtraFields($this->fields);
$this->service->parse();
$this->service->refresh();
$this->service->saveComposeConfigs();
});
$this->dispatch('refreshEnvs');
$this->dispatch('refreshServices');
$notify && $this->dispatch('success', 'Service saved.');
} catch (\Throwable $e) {
// On error, refresh from database to restore clean state
$this->service->refresh();
$this->syncData(false);
return handleError($e, $this);
} finally {
if (is_null($this->service->config_hash)) {

5
app/Models/Service.php
View file

@ -1287,6 +1287,11 @@ public function workdir()
public function saveComposeConfigs()
{
// Guard against null or empty docker_compose
if (! $this->docker_compose) {
return;
}
$workdir = $this->workdir();
instant_remote_process([

36
bootstrap/helpers/parsers.php
View file

@ -59,11 +59,13 @@ function validateDockerComposeForInjection(string $composeYaml): void
if (isset($volume['source'])) {
$source = $volume['source'];
if (is_string($source)) {
// Allow simple env vars and env vars with defaults (validated in parseDockerVolumeString)
// Allow env vars and env vars with defaults (validated in parseDockerVolumeString)
// Also allow env vars followed by safe path concatenation (e.g., ${VAR}/path)
$isSimpleEnvVar = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}$/', $source);
$isEnvVarWithDefault = preg_match('/^\$\{[^}]+:-[^}]*\}$/', $source);
$isEnvVarWithPath = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}[\/\w\.\-]*$/', $source);
if (! $isSimpleEnvVar && ! $isEnvVarWithDefault) {
if (! $isSimpleEnvVar && ! $isEnvVarWithDefault && ! $isEnvVarWithPath) {
try {
validateShellSafePath($source, 'volume source');
} catch (\Exception $e) {
@ -310,15 +312,17 @@ function parseDockerVolumeString(string $volumeString): array
// Validate source path for command injection attempts
// We validate the final source value after environment variable processing
if ($source !== null) {
// Allow simple environment variables like ${VAR_NAME} or ${VAR}
// but validate everything else for shell metacharacters
// Allow environment variables like ${VAR_NAME} or ${VAR}
// Also allow env vars followed by safe path concatenation (e.g., ${VAR}/path)
$sourceStr = is_string($source) ? $source : $source;
// Skip validation for simple environment variable references
// Pattern: ${WORD_CHARS} with no special characters inside
// Pattern 1: ${WORD_CHARS} with no special characters inside
// Pattern 2: ${WORD_CHARS}/path/to/file (env var with path concatenation)
$isSimpleEnvVar = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}$/', $sourceStr);
$isEnvVarWithPath = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}[\/\w\.\-]*$/', $sourceStr);
if (! $isSimpleEnvVar) {
if (! $isSimpleEnvVar && ! $isEnvVarWithPath) {
try {
validateShellSafePath($sourceStr, 'volume source');
} catch (\Exception $e) {
@ -711,9 +715,12 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
// Validate source and target for command injection (array/long syntax)
if ($source !== null && ! empty($source->value())) {
$sourceValue = $source->value();
// Allow simple environment variable references
// Allow environment variable references and env vars with path concatenation
$isSimpleEnvVar = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}$/', $sourceValue);
if (! $isSimpleEnvVar) {
$isEnvVarWithDefault = preg_match('/^\$\{[^}]+:-[^}]*\}$/', $sourceValue);
$isEnvVarWithPath = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}[\/\w\.\-]*$/', $sourceValue);
if (! $isSimpleEnvVar && ! $isEnvVarWithDefault && ! $isEnvVarWithPath) {
try {
validateShellSafePath($sourceValue, 'volume source');
} catch (\Exception $e) {
@ -1293,6 +1300,9 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
if ($depends_on->count() > 0) {
$payload['depends_on'] = $depends_on;
}
// Auto-inject .env file so Coolify environment variables are available inside containers
// This makes Applications behave consistently with manual .env file usage
$payload['env_file'] = ['.env'];
if ($isPullRequest) {
$serviceName = addPreviewDeploymentSuffix($serviceName, $pullRequestId);
}
@ -1812,9 +1822,12 @@ function serviceParser(Service $resource): Collection
// Validate source and target for command injection (array/long syntax)
if ($source !== null && ! empty($source->value())) {
$sourceValue = $source->value();
// Allow simple environment variable references
// Allow environment variable references and env vars with path concatenation
$isSimpleEnvVar = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}$/', $sourceValue);
if (! $isSimpleEnvVar) {
$isEnvVarWithDefault = preg_match('/^\$\{[^}]+:-[^}]*\}$/', $sourceValue);
$isEnvVarWithPath = preg_match('/^\$\{[a-zA-Z_][a-zA-Z0-9_]*\}[\/\w\.\-]*$/', $sourceValue);
if (! $isSimpleEnvVar && ! $isEnvVarWithDefault && ! $isEnvVarWithPath) {
try {
validateShellSafePath($sourceValue, 'volume source');
} catch (\Exception $e) {
@ -2269,6 +2282,9 @@ function serviceParser(Service $resource): Collection
if ($depends_on->count() > 0) {
$payload['depends_on'] = $depends_on;
}
// Auto-inject .env file so Coolify environment variables are available inside containers
// This makes Services behave consistently with Applications
$payload['env_file'] = ['.env'];
$parsedServices->put($serviceName, $payload);
}

30
tests/Unit/VolumeArrayFormatSecurityTest.php
View file

@ -194,6 +194,36 @@
->not->toThrow(Exception::class);
});
test('array-format with environment variable and path concatenation', function () {
// This is the reported issue #7127 - ${VAR}/path should be allowed
$dockerComposeYaml = <<<'YAML'
services:
web:
image: nginx
volumes:
- type: bind
source: '${VOLUMES_PATH}/mysql'
target: /var/lib/mysql
- type: bind
source: '${DATA_PATH}/config'
target: /etc/config
- type: bind
source: '${VOLUME_PATH}/app_data'
target: /app/data
YAML;
$parsed = Yaml::parse($dockerComposeYaml);
// Verify all three volumes have the correct source format
expect($parsed['services']['web']['volumes'][0]['source'])->toBe('${VOLUMES_PATH}/mysql');
expect($parsed['services']['web']['volumes'][1]['source'])->toBe('${DATA_PATH}/config');
expect($parsed['services']['web']['volumes'][2]['source'])->toBe('${VOLUME_PATH}/app_data');
// The validation should allow this - the reported bug was that it was blocked
expect(fn () => validateDockerComposeForInjection($dockerComposeYaml))
->not->toThrow(Exception::class);
});
test('array-format with malicious environment variable default', function () {
$dockerComposeYaml = <<<'YAML'
services:

21
tests/Unit/VolumeSecurityTest.php
View file

@ -94,6 +94,27 @@
}
});
test('parseDockerVolumeString accepts environment variables with path concatenation', function () {
$volumes = [
'${VOLUMES_PATH}/mysql:/var/lib/mysql',
'${DATA_PATH}/config:/etc/config',
'${VOLUME_PATH}/app_data:/app',
'${MY_VAR_123}/deep/nested/path:/data',
'${VAR}/path:/app',
'${VAR}_suffix:/app',
'${VAR}-suffix:/app',
'${VAR}.ext:/app',
'${VOLUMES_PATH}/mysql:/var/lib/mysql:ro',
'${DATA_PATH}/config:/etc/config:rw',
];
foreach ($volumes as $volume) {
$result = parseDockerVolumeString($volume);
expect($result)->toBeArray();
expect($result['source'])->not->toBeNull();
}
});
test('parseDockerVolumeString rejects environment variables with command injection in default', function () {
$maliciousVolumes = [
'${VAR:-`whoami`}:/app',