diff --git a/app/Actions/Application/CleanupPreviewDeployment.php b/app/Actions/Application/CleanupPreviewDeployment.php index 6c408bdf8b..661abc8d43 100644 --- a/app/Actions/Application/CleanupPreviewDeployment.php +++ b/app/Actions/Application/CleanupPreviewDeployment.php @@ -56,7 +56,7 @@ class CleanupPreviewDeployment if ($result['cancelled_deployments'] > 0) { try { - next_after_cancel($server); + next_after_cancel($server, $application); } catch (\Throwable $e) { \Log::warning("Failed to advance deployment queue after cleaning up preview for application {$application->id}: {$e->getMessage()}"); } diff --git a/app/Actions/Database/StartRedis.php b/app/Actions/Database/StartRedis.php index c5d2d66f8e..7fc89b25ce 100644 --- a/app/Actions/Database/StartRedis.php +++ b/app/Actions/Database/StartRedis.php @@ -4,7 +4,6 @@ namespace App\Actions\Database; use App\Exceptions\DatabaseStartException; use App\Helpers\SslHelper; -use App\Models\EnvironmentVariable; use App\Models\SslCertificate; use App\Models\StandaloneRedis; use App\Traits\ExecutesDatabaseStartCommands; @@ -24,7 +23,9 @@ class StartRedis private ?SslCertificate $ssl_certificate = null; - private ?string $resolvedRedisPassword = null; + private string $resolvedRedisPassword = ''; + + private bool $redisPasswordFromSecretManager = false; private ?string $resolvedRedisUsername = null; @@ -245,21 +246,26 @@ class StartRedis private function generate_environment_variables() { $environment_variables = collect(); + // The stored REDIS_PASSWORD value, unresolved: the v4.3.23 server password of legacy databases. + $this->resolvedRedisPassword = (string) $this->database->redis_password; + $passwordVariable = $this->database->serverPasswordEnvironmentVariable(); foreach ($this->database->runtime_environment_variables as $env) { $usesSecretManager = $this->database->environmentVariableUsesSecretManager($env); + if ($passwordVariable?->is($env)) { + $rawValue = (string) $this->database->resolveSecretManagerEnvironmentVariableValue($env); + + if ($rawValue !== '') { + // Credentials below are placed directly in the compose file (command). + $this->resolvedRedisPassword = $this->database->formatComposeFileValue($env, $rawValue); + $this->redisPasswordFromSecretManager = $usesSecretManager; + } + } + if ($env->is_shared) { $environment_variables->push($env->key.'='.$this->database->resolveSecretManagerEnvironmentVariable($env)); - if ($env->key === 'REDIS_PASSWORD') { - $this->resolvedRedisPassword = $this->redisPasswordForComposeFile($env); - - if (! $usesSecretManager) { - $this->database->update(['redis_password' => $this->resolvedRedisPassword]); - } - } - if ($env->key === 'REDIS_USERNAME') { $this->resolvedRedisUsername = $this->database->resolveSecretManagerEnvironmentVariableValue($env); @@ -268,15 +274,11 @@ class StartRedis } } } else { - if ($env->key === 'REDIS_PASSWORD' && ! $usesSecretManager) { - $env->update(['value' => $this->database->redis_password]); - } elseif ($env->key === 'REDIS_USERNAME' && ! $usesSecretManager) { + if ($env->key === 'REDIS_USERNAME' && ! $usesSecretManager) { $env->update(['value' => $this->database->redis_username]); } - if ($env->key === 'REDIS_PASSWORD') { - $this->resolvedRedisPassword = $this->redisPasswordForComposeFile($env); - } elseif ($env->key === 'REDIS_USERNAME') { + if ($env->key === 'REDIS_USERNAME') { $this->resolvedRedisUsername = $this->database->resolveSecretManagerEnvironmentVariableValue($env); } @@ -289,20 +291,9 @@ class StartRedis return $environment_variables->all(); } - /** - * The password is placed directly in the compose command, so a remote secret value is escaped - * the same way as its environment entry. Other values stay unchanged. - */ - private function redisPasswordForComposeFile(EnvironmentVariable $env): ?string - { - $value = $this->database->resolveSecretManagerEnvironmentVariableValue($env); - - return $value === null ? null : $this->database->formatComposeFileValue($env, $value); - } - private function buildStartCommand(): string { - $redisPassword = $this->resolvedRedisPassword ?? $this->database->redis_password; + $redisPassword = $this->requirePassArgument(); $hasRedisConf = ! is_null($this->database->redis_conf) && ! empty($this->database->redis_conf); $redisConfPath = '/usr/local/etc/redis/redis.conf'; @@ -336,6 +327,21 @@ class StartRedis return $command; } + /** + * The password argument of the start command. Docker Compose splits the command like a shell, so the + * password is quoted. Databases created before this release keep their unquoted v4.3.23 argument when + * quoting would change it (quotes, backslashes, whitespace, ; & | < >): Compose splits or cuts the old + * command there, so quoting now would change the password or options and can lose data. + */ + private function requirePassArgument(): string + { + $keepsUnquotedPassword = $this->database->legacy_password_quoting + && ! $this->redisPasswordFromSecretManager + && strpbrk($this->resolvedRedisPassword, "\\'\";&|<> \t\r\n") !== false; + + return $keepsUnquotedPassword ? $this->resolvedRedisPassword : escapeshellarg($this->resolvedRedisPassword); + } + private function add_custom_redis() { if (is_null($this->database->redis_conf) || empty($this->database->redis_conf)) { diff --git a/app/Http/Controllers/Api/ApplicationsController.php b/app/Http/Controllers/Api/ApplicationsController.php index d33cb1e76b..0a94e00a8d 100644 --- a/app/Http/Controllers/Api/ApplicationsController.php +++ b/app/Http/Controllers/Api/ApplicationsController.php @@ -162,6 +162,11 @@ class ApplicationsController extends Controller $settings['custom_container_name_prefix'] = str($settings['custom_container_name_prefix'])->slug()->value() ?: null; } + // Container names use custom_internal_name only with consistent container naming, so a name sent alone turns it on. + if (filled($settings['custom_internal_name'] ?? null) && ! array_key_exists('is_consistent_container_name_enabled', $settings)) { + $settings['is_consistent_container_name_enabled'] = true; + } + return $settings; } @@ -185,16 +190,11 @@ class ApplicationsController extends Controller } /** - * Container names honor custom_internal_name only with consistent container naming, so reject a name that would be ignored. + * Container names honor custom_internal_name only with consistent container naming, so reject a name sent while the request turns it off. */ - private function customInternalNameValidationResponse(array $settings, ?Application $application = null): ?JsonResponse + private function customInternalNameValidationResponse(array $settings): ?JsonResponse { - if (! filled($settings['custom_internal_name'] ?? null)) { - return null; - } - $consistentNaming = $settings['is_consistent_container_name_enabled'] - ?? (bool) $application?->settings?->is_consistent_container_name_enabled; - if ($consistentNaming) { + if (! filled($settings['custom_internal_name'] ?? null) || ($settings['is_consistent_container_name_enabled'] ?? true)) { return null; } @@ -440,7 +440,7 @@ class ApplicationsController extends Controller 'gpu_device_ids' => ['type' => 'string', 'nullable' => true, 'description' => 'Comma-separated GPU device IDs.'], 'gpu_options' => ['type' => 'string', 'nullable' => true, 'description' => 'Additional GPU options.'], 'is_consistent_container_name_enabled' => ['type' => 'boolean', 'description' => 'Use a consistent container name across deployments.'], - 'custom_internal_name' => ['type' => 'string', 'nullable' => true, 'description' => 'Custom internal container name. Requires is_consistent_container_name_enabled to be true.'], + 'custom_internal_name' => ['type' => 'string', 'nullable' => true, 'description' => 'Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422.'], 'custom_container_name_prefix' => ['type' => 'string', 'nullable' => true, 'description' => 'Prefix for generated container names (prefix-20260908T141530). Slugified and unique across the instance.'], 'preview_url_template' => ['type' => 'string', 'description' => 'Preview URL template.'], 'max_restart_count' => ['type' => 'integer', 'minimum' => 0, 'description' => 'Maximum container restart count before stopping.'], @@ -639,7 +639,7 @@ class ApplicationsController extends Controller 'gpu_device_ids' => ['type' => 'string', 'nullable' => true, 'description' => 'Comma-separated GPU device IDs.'], 'gpu_options' => ['type' => 'string', 'nullable' => true, 'description' => 'Additional GPU options.'], 'is_consistent_container_name_enabled' => ['type' => 'boolean', 'description' => 'Use a consistent container name across deployments.'], - 'custom_internal_name' => ['type' => 'string', 'nullable' => true, 'description' => 'Custom internal container name. Requires is_consistent_container_name_enabled to be true.'], + 'custom_internal_name' => ['type' => 'string', 'nullable' => true, 'description' => 'Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422.'], 'custom_container_name_prefix' => ['type' => 'string', 'nullable' => true, 'description' => 'Prefix for generated container names (prefix-20260908T141530). Slugified and unique across the instance.'], 'preview_url_template' => ['type' => 'string', 'description' => 'Preview URL template.'], 'max_restart_count' => ['type' => 'integer', 'minimum' => 0, 'description' => 'Maximum container restart count before stopping.'], @@ -838,7 +838,7 @@ class ApplicationsController extends Controller 'gpu_device_ids' => ['type' => 'string', 'nullable' => true, 'description' => 'Comma-separated GPU device IDs.'], 'gpu_options' => ['type' => 'string', 'nullable' => true, 'description' => 'Additional GPU options.'], 'is_consistent_container_name_enabled' => ['type' => 'boolean', 'description' => 'Use a consistent container name across deployments.'], - 'custom_internal_name' => ['type' => 'string', 'nullable' => true, 'description' => 'Custom internal container name. Requires is_consistent_container_name_enabled to be true.'], + 'custom_internal_name' => ['type' => 'string', 'nullable' => true, 'description' => 'Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422.'], 'custom_container_name_prefix' => ['type' => 'string', 'nullable' => true, 'description' => 'Prefix for generated container names (prefix-20260908T141530). Slugified and unique across the instance.'], 'preview_url_template' => ['type' => 'string', 'description' => 'Preview URL template.'], 'max_restart_count' => ['type' => 'integer', 'minimum' => 0, 'description' => 'Maximum container restart count before stopping.'], @@ -1008,7 +1008,7 @@ class ApplicationsController extends Controller 'gpu_device_ids' => ['type' => 'string', 'nullable' => true, 'description' => 'Comma-separated GPU device IDs.'], 'gpu_options' => ['type' => 'string', 'nullable' => true, 'description' => 'Additional GPU options.'], 'is_consistent_container_name_enabled' => ['type' => 'boolean', 'description' => 'Use a consistent container name across deployments.'], - 'custom_internal_name' => ['type' => 'string', 'nullable' => true, 'description' => 'Custom internal container name. Requires is_consistent_container_name_enabled to be true.'], + 'custom_internal_name' => ['type' => 'string', 'nullable' => true, 'description' => 'Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422.'], 'custom_container_name_prefix' => ['type' => 'string', 'nullable' => true, 'description' => 'Prefix for generated container names (prefix-20260908T141530). Slugified and unique across the instance.'], 'preview_url_template' => ['type' => 'string', 'description' => 'Preview URL template.'], 'max_restart_count' => ['type' => 'integer', 'minimum' => 0, 'description' => 'Maximum container restart count before stopping.'], @@ -1174,7 +1174,7 @@ class ApplicationsController extends Controller 'gpu_device_ids' => ['type' => 'string', 'nullable' => true, 'description' => 'Comma-separated GPU device IDs.'], 'gpu_options' => ['type' => 'string', 'nullable' => true, 'description' => 'Additional GPU options.'], 'is_consistent_container_name_enabled' => ['type' => 'boolean', 'description' => 'Use a consistent container name across deployments.'], - 'custom_internal_name' => ['type' => 'string', 'nullable' => true, 'description' => 'Custom internal container name. Requires is_consistent_container_name_enabled to be true.'], + 'custom_internal_name' => ['type' => 'string', 'nullable' => true, 'description' => 'Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422.'], 'custom_container_name_prefix' => ['type' => 'string', 'nullable' => true, 'description' => 'Prefix for generated container names (prefix-20260908T141530). Slugified and unique across the instance.'], 'preview_url_template' => ['type' => 'string', 'description' => 'Preview URL template.'], 'max_restart_count' => ['type' => 'integer', 'minimum' => 0, 'description' => 'Maximum container restart count before stopping.'], @@ -3178,7 +3178,7 @@ class ApplicationsController extends Controller 'gpu_device_ids' => ['type' => 'string', 'nullable' => true, 'description' => 'Comma-separated GPU device IDs.'], 'gpu_options' => ['type' => 'string', 'nullable' => true, 'description' => 'Additional GPU options.'], 'is_consistent_container_name_enabled' => ['type' => 'boolean', 'description' => 'Use a consistent container name across deployments.'], - 'custom_internal_name' => ['type' => 'string', 'nullable' => true, 'description' => 'Custom internal container name. Requires is_consistent_container_name_enabled to be true.'], + 'custom_internal_name' => ['type' => 'string', 'nullable' => true, 'description' => 'Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422.'], 'custom_container_name_prefix' => ['type' => 'string', 'nullable' => true, 'description' => 'Prefix for generated container names (prefix-20260908T141530). Slugified and unique across the instance.'], 'preview_url_template' => ['type' => 'string', 'description' => 'Preview URL template.'], 'max_restart_count' => ['type' => 'integer', 'minimum' => 0, 'description' => 'Maximum container restart count before stopping.'], @@ -3364,7 +3364,7 @@ class ApplicationsController extends Controller if ($prefixValidation = $this->containerNamePrefixValidationResponse($applicationSettings, $application->destination->server, $application)) { return $prefixValidation; } - if ($internalNameValidation = $this->customInternalNameValidationResponse($applicationSettings, $application)) { + if ($internalNameValidation = $this->customInternalNameValidationResponse($applicationSettings)) { return $internalNameValidation; } $requestedBuildPack = $request->input('build_pack', $application->build_pack); diff --git a/app/Http/Controllers/Api/DeployController.php b/app/Http/Controllers/Api/DeployController.php index fc7a9d5460..5c405f3e17 100644 --- a/app/Http/Controllers/Api/DeployController.php +++ b/app/Http/Controllers/Api/DeployController.php @@ -294,7 +294,7 @@ class DeployController extends Controller } finally { if ($cancelled) { try { - next_after_cancel($deploymentServer); + next_after_cancel($deploymentServer, $deployment->application); } catch (\Throwable $e) { \Log::warning("Failed to advance deployment queue after cancelling deployment {$deployment->id}: {$e->getMessage()}"); } diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 3a63d36e34..36fb321fa6 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -646,6 +646,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue return; } + $this->validateDeploymentEnvironmentVariableKeys(buildsImage: true); // Save build-time .env file BEFORE the build $this->save_buildtime_environment_variables(); @@ -792,6 +793,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue } $this->generate_image_names(); $this->cleanup_git(); + $this->validateDeploymentEnvironmentVariableKeys(buildsImage: true); $this->generate_build_env_variables(); @@ -1108,6 +1110,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue return; } } + $this->validateDeploymentEnvironmentVariableKeys(buildsImage: true); $this->cleanup_git(); $this->generate_compose_file(); @@ -1141,6 +1144,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue return; } } + $this->validateDeploymentEnvironmentVariableKeys(buildsImage: true); $this->clone_repository(); $this->cleanup_git(); $this->generate_nixpacks_confs(); @@ -1174,6 +1178,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue return; } } + $this->validateDeploymentEnvironmentVariableKeys(buildsImage: true); $this->clone_repository(); $this->cleanup_git(); $this->generate_compose_file(); @@ -1205,6 +1210,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue return; } } + $this->validateDeploymentEnvironmentVariableKeys(buildsImage: true); $this->clone_repository(); $this->cleanup_git(); $this->generate_compose_file(); @@ -2280,26 +2286,31 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue /** * Build-time names go into shell and Docker build commands, so they must be valid when the - * deployment builds an image. Runtime-only variables only go into the .env file: existing ones - * with names that new variables can no longer use (such as my-var) keep working, unless the - * name would break a .env line. Deployments without a build step (Docker image) treat - * build-time variables the same way, because no build receives them. + * deployment builds an image ($buildsImage, checked right before the build steps). Restarts and + * deployments that reuse an existing image do not pass them to a build, so handle() only warns. + * Runtime-only variables only go into the .env file: existing ones with names that new variables + * can no longer use (such as my-var) keep working, unless the name would break a .env line. + * Deployments without a build step (Docker image) never validate build-time names. */ - private function validateDeploymentEnvironmentVariableKeys(): void + private function validateDeploymentEnvironmentVariableKeys(bool $buildsImage = false): void { $environmentVariables = $this->pull_request_id === 0 ? $this->application->environment_variables()->get(['key', 'is_buildtime', 'is_runtime']) : $this->application->environment_variables_preview()->get(['key', 'is_buildtime', 'is_runtime']); - $passesBuildtimeVariables = $this->deploymentPassesBuildtimeVariables(); + $validatesBuildtimeKeys = $buildsImage && $this->deploymentPassesBuildtimeVariables(); foreach ($environmentVariables as $environmentVariable) { $key = (string) $environmentVariable->key; $isEnvFileSafe = $key !== '' && strpbrk($key, "=\n\r\0") === false; - if (($environmentVariable->is_buildtime && $passesBuildtimeVariables) || ! $isEnvFileSafe) { + if (($environmentVariable->is_buildtime && $validatesBuildtimeKeys) || ! $isEnvFileSafe) { $this->validatedBuildtimeEnvironmentVariableKey($key, 'the deployment environment'); continue; } + // The deployment start already logged the warnings. + if ($buildsImage) { + continue; + } if (! ValidationPatterns::isValidEnvironmentVariableKey($key)) { $this->logLegacyEnvironmentVariableKey($key, (bool) $environmentVariable->is_buildtime, (bool) $environmentVariable->is_runtime); } @@ -2325,7 +2336,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $displayKey = ValidationPatterns::displayShellEnvironmentVariableKey($key); if ($isBuildtime) { - $this->application_deployment_queue->addLogEntry("⚠️ Build-time variable {$displayKey} uses a name that new variables cannot use. This deployment does not build an image, so it is not used as a build-time variable. Rename it before you use it in a build.", 'stderr'); + $this->application_deployment_queue->addLogEntry("⚠️ Build-time variable {$displayKey} uses a name that new variables cannot use. It is not used as a build-time variable unless the deployment builds an image, and that build fails until you rename it.", 'stderr'); } if ($isRuntime) { $this->application_deployment_queue->addLogEntry("⚠️ Runtime variable {$displayKey} uses a name that new variables cannot use. It is still passed to the container, but shell scripts cannot read it.", 'stderr'); @@ -2665,6 +2676,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $this->check_git_if_build_needed(); $this->clone_repository(); $this->cleanup_git(); + $this->validateDeploymentEnvironmentVariableKeys(buildsImage: true); if ($this->application->build_pack === 'nixpacks') { $this->generate_nixpacks_confs(); } @@ -4875,7 +4887,9 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); $this->generate_env_variables(); } - $variables = $this->env_args; + // Invalid names stay out of the helper container: a build fails on them before it starts, + // and restarts or deployments that reuse an image do not need them. + $variables = $this->env_args->filter(fn ($value, $key): bool => ValidationPatterns::isValidEnvironmentVariableKey((string) $key)); if ($this->build_pack === 'railpack') { $variables = $this->without_reserved_docker_client_variables($variables); diff --git a/app/Jobs/DeleteResourceJob.php b/app/Jobs/DeleteResourceJob.php index a5df1ebfa4..52fa592766 100644 --- a/app/Jobs/DeleteResourceJob.php +++ b/app/Jobs/DeleteResourceJob.php @@ -260,7 +260,7 @@ class DeleteResourceJob implements ShouldBeEncrypted, ShouldQueue if ($cancelledDeployments > 0) { try { - next_after_cancel($server); + next_after_cancel($server, $application); } catch (\Throwable $e) { \Log::warning("Failed to advance deployment queue after deleting preview {$this->resource->id}: {$e->getMessage()}"); } diff --git a/app/Jobs/PushServerUpdateJob.php b/app/Jobs/PushServerUpdateJob.php index b6b358512a..768ebddb22 100644 --- a/app/Jobs/PushServerUpdateJob.php +++ b/app/Jobs/PushServerUpdateJob.php @@ -14,6 +14,7 @@ use App\Actions\Shared\ComplexStatusCheck; use App\Events\ServiceChecked; use App\Models\Application; use App\Models\ApplicationPreview; +use App\Models\NotificationThrottle; use App\Models\Server; use App\Models\ServiceApplication; use App\Models\ServiceDatabase; @@ -30,6 +31,7 @@ use App\Models\StandaloneSqlite; use App\Models\SwarmDocker; use App\Notifications\Application\RestartLimitReached as ApplicationRestartLimitReached; use App\Notifications\Container\ContainerRestarted; +use App\Notifications\Server\HighDiskUsage; use App\Services\ContainerStatusAggregator; use App\Services\RestartCountTracker; use App\Traits\CalculatesExcludedStatus; @@ -197,14 +199,22 @@ class PushServerUpdateJob implements ShouldBeEncrypted, ShouldQueue, Silenced // it is wasted work — and most servers sit well below the threshold. $diskThreshold = data_get($this->server, 'settings.server_disk_usage_notification_threshold', 80); $storageCacheKey = 'storage-check:'.$this->server->id; + // Set while usage was high, so the throttle is released only once when usage recovers. + $highUsageCacheKey = 'storage-high:'.$this->server->id; $lastPercentage = Cache::get($storageCacheKey); if ($filesystemUsageRoot !== null && $filesystemUsageRoot >= $diskThreshold && (string) $lastPercentage !== (string) $filesystemUsageRoot) { Cache::put($storageCacheKey, $filesystemUsageRoot, 600); + Cache::forever($highUsageCacheKey, true); ServerStorageCheckJob::dispatch($this->server, $filesystemUsageRoot); } elseif ($filesystemUsageRoot !== null && $filesystemUsageRoot < $diskThreshold) { Cache::forget($storageCacheKey); + // The storage check does not run below the threshold, so the next spike alerts again only + // when the throttle is released here. + if (HighDiskUsage::hasRecovered($filesystemUsageRoot, $diskThreshold) && Cache::pull($highUsageCacheKey)) { + NotificationThrottle::release($this->server, HighDiskUsage::class); + } } if ($this->containers->isEmpty() && ! $this->isCompleteSnapshot()) { diff --git a/app/Jobs/ServerStorageCheckJob.php b/app/Jobs/ServerStorageCheckJob.php index 1e1b2c3c2a..f8bac23516 100644 --- a/app/Jobs/ServerStorageCheckJob.php +++ b/app/Jobs/ServerStorageCheckJob.php @@ -60,7 +60,7 @@ class ServerStorageCheckJob implements ShouldBeEncrypted, ShouldQueue, Silenced } if ($this->percentage > $serverDiskUsageNotificationThreshold) { $team->notify(new HighDiskUsage($this->server, $this->percentage, $serverDiskUsageNotificationThreshold)); - } else { + } elseif (HighDiskUsage::hasRecovered($this->percentage, $serverDiskUsageNotificationThreshold)) { // Usage recovered: the next spike should alert again instead of waiting for the interval. NotificationThrottle::release($this->server, HighDiskUsage::class); } diff --git a/app/Livewire/Project/Application/DeploymentNavbar.php b/app/Livewire/Project/Application/DeploymentNavbar.php index 22f2b8043d..654b571db3 100644 --- a/app/Livewire/Project/Application/DeploymentNavbar.php +++ b/app/Livewire/Project/Application/DeploymentNavbar.php @@ -119,7 +119,7 @@ class DeploymentNavbar extends Component // Still mark as cancelled even if cleanup fails return handleError($e, $this); } finally { - next_after_cancel($server); + next_after_cancel($server, $this->application); } } } diff --git a/app/Mcp/Tools/CancelDeployment.php b/app/Mcp/Tools/CancelDeployment.php index 5dcee3ed38..c9913c4e5f 100644 --- a/app/Mcp/Tools/CancelDeployment.php +++ b/app/Mcp/Tools/CancelDeployment.php @@ -84,7 +84,7 @@ class CancelDeployment extends Tool try { $deploymentServer = Server::whereTeamId($teamId)->find($deployment->server_id); - next_after_cancel($deploymentServer); + next_after_cancel($deploymentServer, $application); } catch (\Throwable $e) { \Log::warning("Failed to advance deployment queue after cancelling deployment {$deployment->id}: {$e->getMessage()}"); } diff --git a/app/Models/Server.php b/app/Models/Server.php index 149cbc9ee5..b7e4e9ba53 100644 --- a/app/Models/Server.php +++ b/app/Models/Server.php @@ -7,6 +7,7 @@ use App\Actions\Server\InstallDocker; use App\Actions\Server\InstallPrerequisites; use App\Actions\Server\StartSentinel; use App\Actions\Server\ValidatePrerequisites; +use App\Enums\ApplicationDeploymentStatus; use App\Enums\ProxyTypes; use App\Enums\ServerRole; use App\Events\ServerReachabilityChanged; @@ -244,11 +245,35 @@ class Server extends BaseModel $server->notificationThrottles()->delete(); }); + static::deleted(function (Server $server) { + $server->failQueuedDeployments(); + }); + static::updated(function () { static::flushIdentityMap(); }); } + /** + * Fail the queued deployments of a deleted server, because they can never start. + */ + public function failQueuedDeployments(): void + { + ApplicationDeploymentQueue::query() + ->where('server_id', $this->id) + ->where('status', ApplicationDeploymentStatus::QUEUED->value) + ->eachById(function (ApplicationDeploymentQueue $deployment) { + $updated = ApplicationDeploymentQueue::query() + ->whereKey($deployment->id) + ->where('status', ApplicationDeploymentStatus::QUEUED->value) + ->update(['status' => ApplicationDeploymentStatus::FAILED->value]); + + if ($updated > 0) { + $deployment->addLogEntry('The server was deleted.', 'stderr'); + } + }); + } + /** * Find a Server by ID using the identity map cache. * This prevents N+1 queries when the same Server is accessed multiple times. diff --git a/app/Models/StandaloneRedis.php b/app/Models/StandaloneRedis.php index 3bfcc5434e..2e6f158cf1 100644 --- a/app/Models/StandaloneRedis.php +++ b/app/Models/StandaloneRedis.php @@ -66,6 +66,8 @@ class StandaloneRedis extends BaseModel 'redis_password', 'internal_db_url', 'external_db_url', + // Internal start-command state, not part of the API. + 'legacy_password_quoting', ]; protected $casts = [ @@ -74,6 +76,7 @@ class StandaloneRedis extends BaseModel 'health_check_timeout' => 'integer', 'health_check_retries' => 'integer', 'health_check_start_period' => 'integer', + 'legacy_password_quoting' => 'boolean', 'public_port_timeout' => 'integer', 'restart_count' => 'integer', 'last_restart_at' => 'datetime', @@ -313,7 +316,7 @@ class StandaloneRedis extends BaseModel get: function () { $redis_version = $this->getRedisVersion(); $username_part = version_compare($redis_version, '6.0', '>=') ? rawurlencode($this->redis_username).':' : ''; - $encodedPass = rawurlencode($this->redis_password); + $encodedPass = rawurlencode($this->connectionPassword()); $scheme = $this->enable_ssl ? 'rediss' : 'redis'; $port = $this->enable_ssl ? 6380 : 6379; $url = "{$scheme}://{$username_part}{$encodedPass}@{$this->uuid}:{$port}/0"; @@ -338,7 +341,7 @@ class StandaloneRedis extends BaseModel } $redis_version = $this->getRedisVersion(); $username_part = version_compare($redis_version, '6.0', '>=') ? rawurlencode($this->redis_username).':' : ''; - $encodedPass = rawurlencode($this->redis_password); + $encodedPass = rawurlencode($this->connectionPassword()); $scheme = $this->enable_ssl ? 'rediss' : 'redis'; $url = "{$scheme}://{$username_part}{$encodedPass}@{$serverIp}:{$this->public_port}/0"; @@ -381,6 +384,47 @@ class StandaloneRedis extends BaseModel return $this->morphMany(EnvironmentVariable::class, 'resourceable'); } + /** + * The REDIS_PASSWORD variable whose resolved value sets the server password. Databases created + * before this release (legacy_password_quoting) keep their v4.3.23 server password, the stored + * value as it is, unless the variable reads a remote secret. + */ + public function serverPasswordEnvironmentVariable(): ?EnvironmentVariable + { + $environmentVariable = $this->runtime_environment_variables()->where('key', 'REDIS_PASSWORD')->first(); + + if (! $environmentVariable) { + return null; + } + + if ($this->legacy_password_quoting && ! $this->environmentVariableUsesSecretManager($environmentVariable)) { + return null; + } + + return $environmentVariable; + } + + /** + * The server password for connection URLs. A remote secret is not fetched here, so the URL + * shows its reference instead of a stale or wrong password. + */ + public function connectionPassword(): string + { + $environmentVariable = $this->serverPasswordEnvironmentVariable(); + + if ($environmentVariable) { + $password = $this->environmentVariableUsesSecretManager($environmentVariable) + ? (string) $environmentVariable->value + : (string) $this->resolveSecretManagerEnvironmentVariableValue($environmentVariable); + + if ($password !== '') { + return $password; + } + } + + return (string) $this->redis_password; + } + public function persistentStorages() { return $this->morphMany(LocalPersistentVolume::class, 'resource'); diff --git a/app/Notifications/Server/HighDiskUsage.php b/app/Notifications/Server/HighDiskUsage.php index be0517b759..acf938427b 100644 --- a/app/Notifications/Server/HighDiskUsage.php +++ b/app/Notifications/Server/HighDiskUsage.php @@ -13,6 +13,17 @@ use Illuminate\Notifications\Messages\MailMessage; class HighDiskUsage extends CustomEmailNotification implements ThrottledNotification { + /** + * Percentage points below the threshold that disk usage must reach before a new spike alerts + * again, so usage that moves around the threshold does not send an alert on every change. + */ + public const RECOVERY_MARGIN = 5; + + public static function hasRecovered(int|float $diskUsage, int|float $threshold): bool + { + return $diskUsage <= $threshold - self::RECOVERY_MARGIN; + } + public function __construct(public Server $server, public int $disk_usage, public int $server_disk_usage_notification_threshold) { $this->onQueue('high'); diff --git a/bootstrap/helpers/applications.php b/bootstrap/helpers/applications.php index 8b90de0f40..c2a012692f 100644 --- a/bootstrap/helpers/applications.php +++ b/bootstrap/helpers/applications.php @@ -153,7 +153,7 @@ function queue_next_deployment(Application $application, ?int $finished_deployme ->where('status', ApplicationDeploymentStatus::QUEUED) ->distinct() ->pluck('server_id'); - $server_ids = collect([$application->destination->server_id, $finished_deployment_server_id]) + $server_ids = collect([$application->destination?->server_id, $finished_deployment_server_id]) ->merge($application_queued_server_ids) ->filter(fn ($server_id) => $server_id !== null) ->unique() @@ -164,7 +164,11 @@ function queue_next_deployment(Application $application, ?int $finished_deployme ->sortBy('created_at'); foreach ($queued_deployments as $next_deployment) { - start_queued_deployment($next_deployment); + try { + start_queued_deployment($next_deployment); + } catch (Throwable $e) { + Log::warning("Failed to start queued deployment {$next_deployment->deployment_uuid}: {$e->getMessage()}"); + } } } @@ -264,7 +268,11 @@ function next_queuable(string $server_id, string $application_id, string $commit } // Check server's concurrent build limit + // A deleted server keeps its queued deployments; they cannot run anymore. $server = Server::find($server_id); + if (! $server) { + return false; + } $concurrent_builds = $server->settings->concurrent_builds; $active_deployments = ApplicationDeploymentQueue::where('server_id', $server_id) ->where('status', ApplicationDeploymentStatus::IN_PROGRESS->value) @@ -276,8 +284,17 @@ function next_queuable(string $server_id, string $application_id, string $commit return true; } -function next_after_cancel(?Server $server = null) +/** + * Start the queued deployments that can run after a cancellation. With the application of the + * cancelled deployment, its queued deployments on all servers start too, as when a deployment ends. + */ +function next_after_cancel(?Server $server = null, ?Application $application = null) { + if ($application) { + queue_next_deployment($application, $server?->id); + + return; + } if ($server) { $next_found = ApplicationDeploymentQueue::where('server_id', data_get($server, 'id')) ->where('status', ApplicationDeploymentStatus::QUEUED) diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index 85cb6e2bf1..44e1ed5c85 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -1338,7 +1338,7 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int } // Parse the rest of the services - $previewOwnExternalVolumes = collect([]); + $previewOwnVolumes = collect([]); foreach ($services as $serviceName => $service) { $image = data_get_str($service, 'image'); $restart = data_get_str($service, 'restart', RESTART_MODE); @@ -1489,28 +1489,29 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int } } } elseif ($type->value() === 'volume') { - // A preview never mounts an external volume: two deployments that write into the same - // data directory (for example two databases) can corrupt it. The preview gets its own - // volume with the name that older Coolify versions gave it, so it keeps its data, and - // the preview cleanup removes it. - $isPreviewOwnExternalVolume = $isPullRequest - && composeExternalVolumeDeclaration($topLevel->get('volumes'), $source->value()) !== null; + // A preview never mounts an external or network (NFS, CIFS) volume: two deployments + // that write into the same data directory (for example two databases) can corrupt it. + // The preview gets its own volume with the name that older Coolify versions gave + // it, so it keeps its data, and the preview cleanup removes it. + $declaration = $topLevel->get('volumes')->get($source->value()); + $isNetworkVolume = in_array(data_get($declaration, 'driver_opts.type'), ['cifs', 'nfs'], true); + $isPreviewOwnVolume = $isPullRequest + && ($isNetworkVolume || composeExternalVolumeDeclaration($topLevel->get('volumes'), $source->value()) !== null); if (! $isPullRequest && useComposeExternalVolumeAsWritten($resource, $originalResource, $topLevel->get('volumes'), $source->value(), "{$uuid}_".Str::slug($source, '-'))) { // The external volume gets no row, so Coolify never removes it. $volumesParsed->put($index, $volume); continue; } - if ($isPreviewOwnExternalVolume) { - $previewOwnExternalVolumes->put($source->value(), true); - } - $declaration = $topLevel->get('volumes')->get($source->value()); - if (in_array(data_get($declaration, 'driver_opts.type'), ['cifs', 'nfs'], true)) { + if (! $isPullRequest && $isNetworkVolume) { // Network volumes are used as written. $volumesParsed->put($index, $volume); continue; } + if ($isPreviewOwnVolume) { + $previewOwnVolumes->put($source->value(), true); + } $slugWithoutUuid = Str::slug($source, '-'); $name = "{$uuid}_{$slugWithoutUuid}"; @@ -1542,7 +1543,7 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int 'resource_type' => get_class($originalResource), ] ); - $topLevel->get('volumes')->put($name, $isPreviewOwnExternalVolume + $topLevel->get('volumes')->put($name, $isPreviewOwnVolume ? ['name' => $name] : composeRenamedVolumeDeclarationFor($declaration, $name, $persistentVolume, $isPullRequest)); } @@ -2149,8 +2150,8 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int $parsedServices->put($serviceName, $payload); } - // The preview mounts its own volumes instead of these external volumes (see above), so it does not declare them. - $topLevel->put('volumes', $topLevel->get('volumes')->except($previewOwnExternalVolumes->keys()->all())); + // The preview mounts its own volumes instead of these external and network volumes (see above), so it does not declare them. + $topLevel->put('volumes', $topLevel->get('volumes')->except($previewOwnVolumes->keys()->all())); $topLevel->put('services', $parsedServices); $customOrder = ['services', 'volumes', 'networks', 'configs', 'secrets']; diff --git a/bootstrap/helpers/sudo.php b/bootstrap/helpers/sudo.php index ec79300e30..acbe45c1c9 100644 --- a/bootstrap/helpers/sudo.php +++ b/bootstrap/helpers/sudo.php @@ -30,6 +30,25 @@ function ownershipCommand(string $path, Server $server): string return "find $path -user root -exec chown $server->user:$server->user {} + && chmod o-rwx $path"; } +/** + * Adds the ownership step right after `sudo mkdir -p `. When more commands follow on the line + * (`&& ...` or `; ...`), they stay unchanged after the ownership step. Other forms (redirects, `||`, + * pipes, several paths) are left as they are. + */ +function addOwnershipAfterMkdir(string $line, Server $server): string +{ + if (! preg_match('/^(sudo mkdir -p ([^\s;&|<>]+))(\s*(?:(?:&&|;).*)?)$/s', $line, $matches)) { + return $line; + } + $path = $matches[2]; + if (! shouldChangeOwnership($path)) { + return $line; + } + + // No sudo here: the && rule adds it. `sudo sudo` fails where root is not in sudoers (Alpine). + return "{$matches[1]} && ".ownershipCommand($path, $server).($matches[3] ?? ''); +} + /** * A line that is one `sudo sh -c '...'` call already runs completely as root. Adding sudo inside the script * fails where root is not in sudoers (Alpine), and wrapping it again in `bash -c` needs bash. @@ -135,17 +154,7 @@ function parseCommandsByLineForSudo(Collection $commands, Server $server): array }); $commands = $commands->map(function ($line) use ($server) { - if (Str::startsWith($line, 'sudo mkdir -p')) { - $path = trim(Str::after($line, 'sudo mkdir -p')); - if (shouldChangeOwnership($path)) { - // No sudo here: the && rule below adds it. `sudo sudo` fails where root is not in sudoers (Alpine). - return "$line && ".ownershipCommand($path, $server); - } - - return $line; - } - - return $line; + return addOwnershipAfterMkdir($line, $server); }); $commands = $commands->map(function ($line) { @@ -197,13 +206,7 @@ function parseLineForSudo(string $command, Server $server): string if (! str($command)->startSwith('cd') && ! str($command)->startSwith('command')) { $command = "sudo $command"; } - if (Str::startsWith($command, 'sudo mkdir -p')) { - $path = trim(Str::after($command, 'sudo mkdir -p')); - if (shouldChangeOwnership($path)) { - // No sudo here: the && rule below adds it. - $command = "$command && ".ownershipCommand($path, $server); - } - } + $command = addOwnershipAfterMkdir($command, $server); if (isSingleSudoShellScript($command)) { return $command; } diff --git a/database/migrations/2026_10_05_204113_add_legacy_password_quoting_to_standalone_redis.php b/database/migrations/2026_10_05_204113_add_legacy_password_quoting_to_standalone_redis.php new file mode 100644 index 0000000000..7ecb900946 --- /dev/null +++ b/database/migrations/2026_10_05_204113_add_legacy_password_quoting_to_standalone_redis.php @@ -0,0 +1,30 @@ +boolean('legacy_password_quoting')->default(false); + }); + + DB::table('standalone_redis')->update(['legacy_password_quoting' => true]); + } + + public function down(): void + { + Schema::table('standalone_redis', function (Blueprint $table) { + $table->dropColumn('legacy_password_quoting'); + }); + } +}; diff --git a/openapi.json b/openapi.json index 597de7ccce..9b681adc06 100644 --- a/openapi.json +++ b/openapi.json @@ -950,7 +950,7 @@ "custom_internal_name": { "type": "string", "nullable": true, - "description": "Custom internal container name. Requires is_consistent_container_name_enabled to be true." + "description": "Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422." }, "custom_container_name_prefix": { "type": "string", @@ -1571,7 +1571,7 @@ "custom_internal_name": { "type": "string", "nullable": true, - "description": "Custom internal container name. Requires is_consistent_container_name_enabled to be true." + "description": "Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422." }, "custom_container_name_prefix": { "type": "string", @@ -2192,7 +2192,7 @@ "custom_internal_name": { "type": "string", "nullable": true, - "description": "Custom internal container name. Requires is_consistent_container_name_enabled to be true." + "description": "Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422." }, "custom_container_name_prefix": { "type": "string", @@ -2708,7 +2708,7 @@ "custom_internal_name": { "type": "string", "nullable": true, - "description": "Custom internal container name. Requires is_consistent_container_name_enabled to be true." + "description": "Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422." }, "custom_container_name_prefix": { "type": "string", @@ -3204,7 +3204,7 @@ "custom_internal_name": { "type": "string", "nullable": true, - "description": "Custom internal container name. Requires is_consistent_container_name_enabled to be true." + "description": "Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422." }, "custom_container_name_prefix": { "type": "string", @@ -3971,7 +3971,7 @@ "custom_internal_name": { "type": "string", "nullable": true, - "description": "Custom internal container name. Requires is_consistent_container_name_enabled to be true." + "description": "Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422." }, "custom_container_name_prefix": { "type": "string", diff --git a/openapi.yaml b/openapi.yaml index 1d50d90093..a31fc5b1df 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -623,7 +623,7 @@ paths: custom_internal_name: type: string nullable: true - description: 'Custom internal container name. Requires is_consistent_container_name_enabled to be true.' + description: 'Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422.' custom_container_name_prefix: type: string nullable: true @@ -1032,7 +1032,7 @@ paths: custom_internal_name: type: string nullable: true - description: 'Custom internal container name. Requires is_consistent_container_name_enabled to be true.' + description: 'Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422.' custom_container_name_prefix: type: string nullable: true @@ -1441,7 +1441,7 @@ paths: custom_internal_name: type: string nullable: true - description: 'Custom internal container name. Requires is_consistent_container_name_enabled to be true.' + description: 'Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422.' custom_container_name_prefix: type: string nullable: true @@ -1791,7 +1791,7 @@ paths: custom_internal_name: type: string nullable: true - description: 'Custom internal container name. Requires is_consistent_container_name_enabled to be true.' + description: 'Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422.' custom_container_name_prefix: type: string nullable: true @@ -2127,7 +2127,7 @@ paths: custom_internal_name: type: string nullable: true - description: 'Custom internal container name. Requires is_consistent_container_name_enabled to be true.' + description: 'Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422.' custom_container_name_prefix: type: string nullable: true @@ -2636,7 +2636,7 @@ paths: custom_internal_name: type: string nullable: true - description: 'Custom internal container name. Requires is_consistent_container_name_enabled to be true.' + description: 'Custom internal container name. Turns is_consistent_container_name_enabled on when that field is not sent; sending it as false together with a name returns 422.' custom_container_name_prefix: type: string nullable: true diff --git a/tests/Feature/Api/ApplicationCustomInternalNameConsistentNamingApiTest.php b/tests/Feature/Api/ApplicationCustomInternalNameConsistentNamingApiTest.php index f745425781..e50274f3d4 100644 --- a/tests/Feature/Api/ApplicationCustomInternalNameConsistentNamingApiTest.php +++ b/tests/Feature/Api/ApplicationCustomInternalNameConsistentNamingApiTest.php @@ -51,13 +51,28 @@ function customInternalNameCreatePayload(array $overrides): array const CUSTOM_INTERNAL_NAME_NEEDS_CONSISTENT_NAMING = 'Set is_consistent_container_name_enabled to true to use custom_internal_name. Coolify ignores the custom internal name while consistent container naming is turned off.'; -test('updating only custom_internal_name is rejected while consistent container naming is off', function () { +test('updating only custom_internal_name turns consistent container naming on', function () { $this->withToken($this->bearerToken) ->patchJson("/api/v1/applications/{$this->application->uuid}", ['custom_internal_name' => 'my-app']) + ->assertOk(); + + $settings = $this->application->fresh()->settings; + expect($settings->custom_internal_name)->toBe('my-app') + ->and($settings->is_consistent_container_name_enabled)->toBeTrue(); +}); + +test('updating custom_internal_name with consistent container naming explicitly off is rejected', function () { + $this->withToken($this->bearerToken) + ->patchJson("/api/v1/applications/{$this->application->uuid}", [ + 'custom_internal_name' => 'my-app', + 'is_consistent_container_name_enabled' => false, + ]) ->assertUnprocessable() ->assertJsonPath('errors.custom_internal_name.0', CUSTOM_INTERNAL_NAME_NEEDS_CONSISTENT_NAMING); - expect($this->application->fresh()->settings->custom_internal_name)->toBeNull(); + $settings = $this->application->fresh()->settings; + expect($settings->custom_internal_name)->toBeNull() + ->and($settings->is_consistent_container_name_enabled)->toBeFalse(); }); test('updating custom_internal_name while turning consistent container naming off is rejected', function () { @@ -99,13 +114,30 @@ test('clearing custom_internal_name does not need consistent container naming', $this->withToken($this->bearerToken) ->patchJson("/api/v1/applications/{$this->application->uuid}", ['custom_internal_name' => $value]) ->assertOk(); + + expect($this->application->fresh()->settings->is_consistent_container_name_enabled)->toBeFalse(); })->with(['null' => [null], 'empty string' => ['']]); -test('creating an application with custom_internal_name requires consistent container naming', function () { +test('creating an application with only custom_internal_name turns consistent container naming on', function () { + Queue::fake(); + + $response = $this->withToken($this->bearerToken) + ->postJson('/api/v1/applications/public', customInternalNameCreatePayload(['custom_internal_name' => 'my-app'])) + ->assertCreated(); + + $settings = Application::query()->where('uuid', $response->json('uuid'))->sole()->settings; + expect($settings->custom_internal_name)->toBe('my-app') + ->and($settings->is_consistent_container_name_enabled)->toBeTrue(); +}); + +test('creating an application with custom_internal_name and consistent container naming explicitly off is rejected', function () { Queue::fake(); $this->withToken($this->bearerToken) - ->postJson('/api/v1/applications/public', customInternalNameCreatePayload(['custom_internal_name' => 'my-app'])) + ->postJson('/api/v1/applications/public', customInternalNameCreatePayload([ + 'custom_internal_name' => 'my-app', + 'is_consistent_container_name_enabled' => false, + ])) ->assertUnprocessable() ->assertJsonPath('errors.custom_internal_name.0', CUSTOM_INTERNAL_NAME_NEEDS_CONSISTENT_NAMING); diff --git a/tests/Feature/Api/LegacyPasswordQuotingApiTest.php b/tests/Feature/Api/LegacyPasswordQuotingApiTest.php index 549f955497..78d3dd548b 100644 --- a/tests/Feature/Api/LegacyPasswordQuotingApiTest.php +++ b/tests/Feature/Api/LegacyPasswordQuotingApiTest.php @@ -38,4 +38,4 @@ it('does not return the internal legacy password quoting flag in the database AP expect($single->json())->not->toHaveKey('legacy_password_quoting') ->and(collect($list->json())->firstWhere('uuid', $database->uuid))->not->toHaveKey('legacy_password_quoting'); -})->with(['keydb' => 'create_standalone_keydb', 'dragonfly' => 'create_standalone_dragonfly']); +})->with(['keydb' => 'create_standalone_keydb', 'dragonfly' => 'create_standalone_dragonfly', 'redis' => 'create_standalone_redis']); diff --git a/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php b/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php index d9ad7f9f5c..076bbae527 100644 --- a/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php +++ b/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php @@ -1026,7 +1026,7 @@ it('rejects unsafe legacy keys before generating BuildKit secret flags', functio expect($job->recordedCommands)->toBeEmpty(); }); -it('rejects an unsafe stored key before running a deployment command', function () { +it('rejects an unsafe stored build-time key before running a build command', function () { [$application, $server] = makeDeploymentControlVarFixture(); $environmentVariable = createApplicationEnvironmentVariable($application, [ 'key' => 'SAFE_KEY', @@ -1036,7 +1036,7 @@ it('rejects an unsafe stored key before running a deployment command', function [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server); - expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys')) + expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys', true)) ->toThrow(DeploymentException::class, 'Invalid environment variable name from the deployment environment'); expect($job->recordedCommands)->toBeEmpty(); @@ -1061,7 +1061,7 @@ it('keeps deploying existing runtime-only variables whose names new variables ca expect($job->recordedCommands)->toBeEmpty(); }); -it('rejects existing variable names that would break the .env file or build commands', function (string $key, bool $isBuildtime) { +it('rejects existing variable names that would break the .env file or build commands', function (string $key, bool $isBuildtime, bool $buildsImage) { [$application, $server] = makeDeploymentControlVarFixture(); $environmentVariable = createApplicationEnvironmentVariable($application, [ 'key' => 'SAFE_KEY', @@ -1072,14 +1072,60 @@ it('rejects existing variable names that would break the .env file or build comm [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server); - expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys')) + expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys', $buildsImage)) ->toThrow(DeploymentException::class, 'Invalid environment variable name from the deployment environment'); })->with([ - 'runtime-only name with =' => ['A=B', false], - 'runtime-only name with a newline' => ["A\nB", false], - 'build-time name with a hyphen' => ['my-var', true], + 'runtime-only name with = before the build is decided' => ['A=B', false, false], + 'runtime-only name with a newline before the build is decided' => ["A\nB", false, false], + 'build-time name with a hyphen when the deployment builds' => ['my-var', true, true], ]); +it('does not fail restarts or deployments that reuse an image on invalid build-time names', function (string $buildPack) { + [$application, $server] = makeDeploymentControlVarFixture(['build_pack' => $buildPack]); + $environmentVariable = createApplicationEnvironmentVariable($application, [ + 'key' => 'SAFE_KEY', + 'value' => 'secret', + 'is_buildtime' => true, + ]); + DB::table('environment_variables')->where('id', $environmentVariable->id)->update(['key' => 'MY-VAR']); + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, ['build_pack' => $buildPack]); + // handle() runs this check before it is known whether the deployment builds an image. + invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys'); + + expect(collect($job->recordedLogEntries)->implode("\n")) + ->toContain('MY-VAR') + ->toContain('Suggested name: MY_VAR') + ->not->toContain('Invalid environment variable name'); + expect($job->recordedCommands)->toBeEmpty(); +})->with(['dockerfile', 'nixpacks', 'static', 'railpack', 'dockercompose']); + +it('keeps invalid build-time names out of the helper container so restarts with build secrets work', function () { + [$application, $server] = makeDeploymentControlVarFixture(['build_pack' => 'nixpacks']); + $application->settings()->update(['use_build_secrets' => true]); + createApplicationEnvironmentVariable($application, [ + 'key' => 'SAFE_KEY', + 'value' => 'safe-build-value', + 'is_buildtime' => true, + ]); + $invalidVariable = createApplicationEnvironmentVariable($application, [ + 'key' => 'OTHER_KEY', + 'value' => 'invalid-build-value', + 'is_buildtime' => true, + ]); + DB::table('environment_variables')->where('id', $invalidVariable->id)->update(['key' => 'MY-VAR']); + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, ['build_pack' => 'nixpacks']); + + $envFlags = invokeDeploymentJobMethod($job, $reflection, 'generate_docker_env_flags_for_secrets'); + + expect($envFlags) + ->toContain("-e 'SAFE_KEY=safe-build-value'") + ->toContain('COOLIFY_BUILD_SECRETS_HASH=') + ->not->toContain('MY-VAR') + ->not->toContain('invalid-build-value'); +}); + it('warns instead of failing for invalid build-time names when the deployment does not build an image', function (bool $isRuntime) { [$application, $server] = makeDeploymentControlVarFixture(['build_pack' => 'dockerimage']); $environmentVariable = createApplicationEnvironmentVariable($application, [ @@ -1147,7 +1193,7 @@ it('fails with a clear message for invalid build-time names when the deployment [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, ['build_pack' => $buildPack]); - expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys')) + expect(fn () => invokeDeploymentJobMethod($job, $reflection, 'validateDeploymentEnvironmentVariableKeys', true)) ->toThrow(DeploymentException::class, 'Invalid environment variable name from the deployment environment: my-var'); expect(collect($job->recordedLogEntries)->implode("\n")) ->toContain('Build-time variable names must start with a letter or underscore'); diff --git a/tests/Feature/ComposeVolumeDriverOptionsTest.php b/tests/Feature/ComposeVolumeDriverOptionsTest.php index 13456556a4..ebd79a1dc3 100644 --- a/tests/Feature/ComposeVolumeDriverOptionsTest.php +++ b/tests/Feature/ComposeVolumeDriverOptionsTest.php @@ -288,6 +288,26 @@ describe('applicationParser', function () { expect($parsed['services']['web']['volumes'])->toBe(['nfs-data:/nfs', "{$uuid}_app-data:/app"]) ->and($parsed['volumes']['nfs-data']['driver_opts']['type'])->toBe('nfs'); }); + + it('gives the preview its own volume instead of the network volume of production', function (string $type, string $device) { + // The preview must not mount the network share of production: two deployments that write into the + // same data directory (for example two databases) can corrupt it. + $compose = str_replace(['type: nfs', "':/exports/data'"], ["type: {$type}", "'{$device}'"], DRIVER_OPTIONS_NFS_MIXED_COMPOSE); + $application = driverOptionsApplication($compose); + $uuid = $application->uuid; + $preview = driverOptionsPreview($application); + $name = addPreviewDeploymentSuffix("{$uuid}_nfs-data", 42); + + $parsed = applicationParser($application, 42, $preview->id)->toArray(); + + expect($parsed['services']['web-pr-42']['volumes'])->toBe(["{$name}:/nfs", addPreviewDeploymentSuffix("{$uuid}_app-data", 42).':/app']) + ->and($parsed['volumes'])->not->toHaveKey('nfs-data') + ->and($parsed['volumes'][$name])->toBe(['name' => $name]) + ->and(LocalPersistentVolume::where('resource_id', $application->id)->where('name', $name)->exists())->toBeTrue(); + })->with([ + 'nfs' => ['nfs', ':/exports/data'], + 'cifs' => ['cifs', '//10.0.0.1/share'], + ]); }); describe('serviceParser', function () { diff --git a/tests/Feature/DeploymentQueueAdvanceAcrossServersTest.php b/tests/Feature/DeploymentQueueAdvanceAcrossServersTest.php new file mode 100644 index 0000000000..aac9f6c8d9 --- /dev/null +++ b/tests/Feature/DeploymentQueueAdvanceAcrossServersTest.php @@ -0,0 +1,136 @@ + Process::result(output: '')]); + config([ + 'constants.ssh.mux_enabled' => false, + 'cache.default' => 'array', + 'app.maintenance.store' => 'array', + ]); + + $settings = new InstanceSettings(['is_api_enabled' => true, 'is_mcp_server_enabled' => true]); + $settings->id = 0; + $settings->save(); + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'owner']); + session(['currentTeam' => $this->team]); + + $privateKey = PrivateKey::factory()->create(['team_id' => $this->team->id]); + $makeServer = function (string $ip) use ($privateKey): Server { + $server = Server::factory()->create(['team_id' => $this->team->id, 'private_key_id' => $privateKey->id, 'ip' => $ip]); + $server->settings->update(['concurrent_builds' => 1]); + + return $server; + }; + $this->primaryServer = $makeServer('203.0.113.31'); + $this->additionalServer = $makeServer('203.0.113.32'); + $this->deletedServer = $makeServer('203.0.113.33'); + + $project = Project::factory()->create(['team_id' => $this->team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + $primaryDestination = StandaloneDocker::where('server_id', $this->primaryServer->id)->firstOrFail(); + $this->application = Application::factory()->create([ + 'environment_id' => $environment->id, + 'destination_id' => $primaryDestination->id, + 'destination_type' => $primaryDestination->getMorphClass(), + ]); + foreach ([$this->additionalServer, $this->deletedServer] as $server) { + $this->application->additional_servers()->attach($server->id, [ + 'standalone_docker_id' => StandaloneDocker::where('server_id', $server->id)->firstOrFail()->id, + ]); + } + + $this->makeQueueDeployment = fn (Server $server, string $status, string $uuid): ApplicationDeploymentQueue => ApplicationDeploymentQueue::create([ + 'application_id' => $this->application->id, + 'deployment_uuid' => $uuid, + 'status' => $status, + 'server_id' => $server->id, + 'destination_id' => StandaloneDocker::where('server_id', $server->id)->firstOrFail()->id, + 'commit' => 'HEAD', + 'pull_request_id' => 0, + 'only_this_server' => true, + ]); +}); + +afterEach(function () { + Server::flushIdentityMap(); +}); + +test('a finished deployment starts the queued deployment on another server when a deleted server also has one queued', function () { + $running = ($this->makeQueueDeployment)($this->primaryServer, 'in_progress', 'queue-across-running'); + $orphaned = ($this->makeQueueDeployment)($this->deletedServer, 'queued', 'queue-across-orphaned'); + $this->travel(1)->seconds(); + $waiting = ($this->makeQueueDeployment)($this->additionalServer, 'queued', 'queue-across-waiting'); + // A server deleted before deletion failed its queued deployments left them queued. + Server::withoutEvents(fn () => $this->deletedServer->delete()); + Server::flushIdentityMap(); + Log::spy(); + + (new ApplicationDeploymentJob($running->id))->failed(new RuntimeException('build failed')); + + Log::shouldNotHaveReceived('warning', [Mockery::pattern('/Starting the next queued deployment failed/')]); + expect($waiting->fresh()->status)->toBe('in_progress') + ->and($orphaned->fresh()->status)->toBe('queued'); + Queue::assertPushed(ApplicationDeploymentJob::class, 1); + Queue::assertPushed(ApplicationDeploymentJob::class, fn (ApplicationDeploymentJob $job) => $job->application_deployment_queue_id === $waiting->id); +}); + +test('cancelling a deployment starts the queued deployment of the same application on another server', function (string $path) { + $running = ($this->makeQueueDeployment)($this->primaryServer, 'in_progress', 'queue-across-running'); + $waiting = ($this->makeQueueDeployment)($this->additionalServer, 'queued', 'queue-across-waiting'); + + if ($path === 'ui') { + $this->actingAs($this->user); + Livewire::test(DeploymentNavbar::class, ['application_deployment_queue' => $running])->call('cancel'); + } elseif ($path === 'api') { + $token = $this->user->createToken('queue-across', ['*'])->plainTextToken; + $this->withHeaders(['Authorization' => 'Bearer '.$token]) + ->postJson("/api/v1/deployments/{$running->deployment_uuid}/cancel") + ->assertSuccessful(); + } else { + $token = $this->user->createToken('queue-across', ['read', 'deploy'])->plainTextToken; + $this->withHeaders([ + 'Accept' => 'application/json, text/event-stream', + 'Authorization' => 'Bearer '.$token, + ])->postJson('/mcp', [ + 'jsonrpc' => '2.0', + 'id' => 1, + 'method' => 'tools/call', + 'params' => ['name' => 'cancel_deployment', 'arguments' => ['uuid' => $running->deployment_uuid]], + ])->assertOk(); + } + + expect($running->fresh()->status)->toBe(ApplicationDeploymentStatus::CANCELLED_BY_USER->value) + ->and($waiting->fresh()->status)->toBe(ApplicationDeploymentStatus::IN_PROGRESS->value); + Queue::assertPushed(ApplicationDeploymentJob::class, fn (ApplicationDeploymentJob $job) => $job->application_deployment_queue_id === $waiting->id); +})->with(['ui', 'api', 'mcp']); diff --git a/tests/Feature/RedisLegacyPasswordQuotingTest.php b/tests/Feature/RedisLegacyPasswordQuotingTest.php new file mode 100644 index 0000000000..dec7077109 --- /dev/null +++ b/tests/Feature/RedisLegacyPasswordQuotingTest.php @@ -0,0 +1,187 @@ + 0]); + config(['app.maintenance.store' => 'array', 'cache.default' => 'array']); + Process::fake(); + Queue::fake(); + Server::flushIdentityMap(); + + $team = Team::factory()->create(); + $privateKey = PrivateKey::factory()->create(['team_id' => $team->id]); + $server = Server::factory()->create(['team_id' => $team->id, 'private_key_id' => $privateKey->id]); + $this->destination = StandaloneDocker::query()->where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $this->environment = Environment::factory()->create(['project_id' => $project->id]); + + SharedEnvironmentVariable::query()->create([ + 'key' => 'RPW', + 'value' => 's3cret', + 'type' => 'environment', + 'environment_id' => $this->environment->id, + 'project_id' => $project->id, + 'team_id' => $team->id, + ]); + + $this->executor = new class + { + public array $commands = []; + + public function execute(array $commands, $database, Activity $activity): Activity + { + $this->commands = $commands; + + return $activity; + } + }; + app()->instance(DatabaseStartCommandExecutor::class, $this->executor); +}); + +function redisLegacyQuotingDatabase(object $test, string $password, bool $legacy, array $otherData = []): StandaloneRedis +{ + $database = create_standalone_redis($test->environment->id, $test->destination, ['redis_password' => $password, 'image' => 'redis:7.2', ...$otherData]); + if ($legacy) { + DB::table($database->getTable())->where('id', $database->id)->update(['legacy_password_quoting' => true]); + } + + return $database->fresh(); +} + +function redisLegacyQuotingStartCommand(object $executor, string $uuid): string +{ + foreach ($executor->commands as $command) { + if (preg_match("#^echo '([^']+)' \| base64 -d \| tee \S+/docker-compose\.yml#", $command, $matches)) { + return Yaml::parse(base64_decode($matches[1]))['services'][$uuid]['command']; + } + } + + throw new RuntimeException('No docker-compose.yml was written.'); +} + +it('keeps the v4.3.23 server password and URL of a legacy database with a shared REDIS_PASSWORD reference', function () { + $database = redisLegacyQuotingDatabase($this, '{{environment.RPW}}', legacy: true, otherData: ['is_public' => true, 'public_port' => 16379]); + + StartRedis::run($database, new Activity); + + $database = $database->fresh(); + $encodedReference = rawurlencode('{{environment.RPW}}'); + expect(redisLegacyQuotingStartCommand($this->executor, $database->uuid)) + ->toBe("redis-server --requirepass '{{environment.RPW}}' --appendonly yes") + ->and($database->internal_db_url)->toContain(":{$encodedReference}@") + ->and($database->external_db_url)->toContain(":{$encodedReference}@") + ->and($database->runtime_environment_variables()->where('key', 'REDIS_PASSWORD')->first()->value)->toBe('{{environment.RPW}}') + ->and($database->legacy_password_quoting)->toBeTrue(); +}); + +it('keeps the unquoted v4.3.23 argument of a legacy password that quoting would change', function (string $password) { + $database = redisLegacyQuotingDatabase($this, $password, legacy: true); + + StartRedis::run($database, new Activity); + + expect(redisLegacyQuotingStartCommand($this->executor, $database->uuid)) + ->toBe("redis-server --requirepass {$password} --appendonly yes"); +})->with([ + 'backslash' => 'ab\c', + 'quote' => "a'b", + 'space' => 'a b', + 'semicolon' => 'ab;cd', +]); + +it('starts a new database with the resolved shared REDIS_PASSWORD and shows it in the URLs', function () { + $database = redisLegacyQuotingDatabase($this, '{{environment.RPW}}', legacy: false, otherData: ['is_public' => true, 'public_port' => 16379]); + + StartRedis::run($database, new Activity); + + $database = $database->fresh(); + expect(redisLegacyQuotingStartCommand($this->executor, $database->uuid)) + ->toBe("redis-server --requirepass 's3cret' --appendonly yes") + ->and($database->internal_db_url)->toContain(':s3cret@') + ->and($database->external_db_url)->toContain(':s3cret@') + ->and($database->legacy_password_quoting)->toBeFalse(); +}); + +it('quotes special characters of the password of a new database', function (string $password) { + SharedEnvironmentVariable::query()->where('key', 'RPW')->firstOrFail()->update(['value' => $password]); + $local = redisLegacyQuotingDatabase($this, $password, legacy: false); + $shared = redisLegacyQuotingDatabase($this, '{{environment.RPW}}', legacy: false); + + StartRedis::run($local, new Activity); + $localCommand = redisLegacyQuotingStartCommand($this->executor, $local->uuid); + StartRedis::run($shared, new Activity); + $sharedCommand = redisLegacyQuotingStartCommand($this->executor, $shared->uuid); + + $expected = 'redis-server --requirepass '.escapeshellarg($password).' --appendonly yes'; + expect($localCommand)->toBe($expected) + ->and($sharedCommand)->toBe($expected) + ->and($shared->fresh()->internal_db_url)->toContain(':'.rawurlencode($password).'@'); +})->with([ + 'quote and space' => "pa ss'w", + 'backslash' => 'ab\c', + 'double quote' => 'a"b', +]); + +it('quotes the password next to a custom redis.conf without requirepass', function () { + $database = redisLegacyQuotingDatabase($this, 'a b', legacy: false, otherData: ['redis_conf' => 'maxmemory 100mb']); + + StartRedis::run($database, new Activity); + + expect(redisLegacyQuotingStartCommand($this->executor, $database->uuid)) + ->toBe("redis-server /usr/local/etc/redis/redis.conf --requirepass 'a b'"); +}); + +it('quotes a remote secret password of a legacy database exactly and shows the reference in the URL', function () { + Http::fake(['https://api.doppler.com/*' => Http::response(['REDIS_PASSWORD' => 'ab\\c'])]); + $token = IntegrationToken::query()->create([ + 'team_id' => $this->environment->project->team_id, + 'provider' => 'doppler', + 'name' => 'Doppler', + 'token' => 'the-secret-token', + 'capabilities' => ['secrets'], + ]); + $database = redisLegacyQuotingDatabase($this, '{{vault.REDIS_PASSWORD}}', legacy: true); + $database->secretManagerLink()->create(['integration_token_id' => $token->id]); + + expect($database->fresh()->internal_db_url)->toContain(':'.rawurlencode('{{vault.REDIS_PASSWORD}}').'@'); + + StartRedis::run($database->fresh(), new Activity); + + expect(redisLegacyQuotingStartCommand($this->executor, $database->uuid)) + ->toBe("redis-server --requirepass 'ab\\c' --appendonly yes"); +}); + +it('marks only Redis databases that exist before the migration as legacy', function () { + $existing = create_standalone_redis($this->environment->id, $this->destination); + + $migration = require database_path('migrations/2026_10_05_204113_add_legacy_password_quoting_to_standalone_redis.php'); + $migration->down(); + expect(Schema::hasColumn('standalone_redis', 'legacy_password_quoting'))->toBeFalse(); + $migration->up(); + + $new = create_standalone_redis($this->environment->id, $this->destination); + + expect($existing->fresh()->legacy_password_quoting)->toBeTrue() + ->and($new->fresh()->legacy_password_quoting)->toBeFalse(); +}); diff --git a/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php b/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php index 1a8d128001..d3846d9ae3 100644 --- a/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php +++ b/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php @@ -199,7 +199,7 @@ test('redis remote passwords are escaped the same way in the environment and the $startCommand = (new ReflectionMethod($action, 'buildStartCommand'))->invoke($action); expect($environmentVariables)->toContain('REDIS_PASSWORD=p4$$word') - ->and($startCommand)->toContain('--requirepass p4$$word'); + ->and($startCommand)->toContain("--requirepass 'p4\$\$word'"); }); test('a service restart keeps the service running when remote secrets cannot be fetched', function () { diff --git a/tests/Feature/SecretManagers/SecretManagerLinkTest.php b/tests/Feature/SecretManagers/SecretManagerLinkTest.php index 3cad814ea6..acaa33f7a7 100644 --- a/tests/Feature/SecretManagers/SecretManagerLinkTest.php +++ b/tests/Feature/SecretManagers/SecretManagerLinkTest.php @@ -264,7 +264,7 @@ test('redis remote credentials stay deployment-local and use raw values in the s ->and($username->fresh()->value)->toBe('{{vault.REDIS_USERNAME}}') ->and($environmentVariables)->toContain('REDIS_PASSWORD=p4$$$$word') ->and($environmentVariables)->toContain('REDIS_USERNAME=remote-user') - ->and($startCommand)->toContain('--requirepass p4$$$$word'); + ->and($startCommand)->toContain("--requirepass 'p4\$\$\$\$word'"); }); test('all deployable environment-variable resources support secret managers', function (string $resourceClass) { diff --git a/tests/Feature/Server/ServerStorageCheckNotificationIntervalTest.php b/tests/Feature/Server/ServerStorageCheckNotificationIntervalTest.php index d4e9563396..58e704c32f 100644 --- a/tests/Feature/Server/ServerStorageCheckNotificationIntervalTest.php +++ b/tests/Feature/Server/ServerStorageCheckNotificationIntervalTest.php @@ -1,5 +1,6 @@ settings->fresh()->server_disk_usage_notification_interval_hours)->toBe(24); }); + +function pushDiskUsage(Server $server, int $percentage): void +{ + (new PushServerUpdateJob($server->fresh(), [ + 'containers' => [], + 'filesystem_usage_root' => ['used_percentage' => $percentage], + ]))->handle(); +} + +it('notifies again on a new spike after disk usage recovered on a Sentinel server', function () { + $server = diskUsageIntervalServer(intervalHours: 24); + + pushDiskUsage($server, 93); + Carbon::setTestNow(now()->addHour()); + pushDiskUsage($server, 40); + Carbon::setTestNow(now()->addHour()); + pushDiskUsage($server, 92); + + Notification::assertSentToTimes($server->team, HighDiskUsage::class, 2); +}); + +it('does not notify again while Sentinel disk usage moves around the threshold', function () { + $server = diskUsageIntervalServer(intervalHours: 24); + + foreach ([81, 80, 81, 80, 79, 81, 78, 82] as $percentage) { + pushDiskUsage($server, $percentage); + Carbon::setTestNow(now()->addMinutes(2)); + } + + Notification::assertSentToTimes($server->team, HighDiskUsage::class, 1); +}); + +it('does not notify again while checked disk usage moves around the threshold', function () { + $server = diskUsageIntervalServer(intervalHours: 24); + + foreach ([81, 80, 81, 76, 82] as $percentage) { + runDiskUsageCheck($server, $percentage); + Carbon::setTestNow(now()->addMinutes(2)); + } + + Notification::assertSentToTimes($server->team, HighDiskUsage::class, 1); +}); diff --git a/tests/Feature/ServerDeleteFailsQueuedDeploymentsTest.php b/tests/Feature/ServerDeleteFailsQueuedDeploymentsTest.php new file mode 100644 index 0000000000..16c5b49786 --- /dev/null +++ b/tests/Feature/ServerDeleteFailsQueuedDeploymentsTest.php @@ -0,0 +1,78 @@ + 'array']); + InstanceSettings::forceCreate(['id' => 0, 'is_api_enabled' => true]); + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'owner']); + session(['currentTeam' => $this->team]); + + $privateKey = PrivateKey::factory()->create(['team_id' => $this->team->id]); + $this->server = Server::factory()->create(['team_id' => $this->team->id, 'private_key_id' => $privateKey->id, 'ip' => '203.0.113.41']); + $destination = StandaloneDocker::where('server_id', $this->server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $this->team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + $application = Application::factory()->create([ + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]); + + $makeDeployment = fn (string $status, string $uuid): ApplicationDeploymentQueue => ApplicationDeploymentQueue::create([ + 'application_id' => $application->id, + 'deployment_uuid' => $uuid, + 'status' => $status, + 'server_id' => $this->server->id, + 'destination_id' => $destination->id, + 'commit' => 'HEAD', + 'pull_request_id' => 0, + ]); + $this->queued = $makeDeployment(ApplicationDeploymentStatus::QUEUED->value, 'server-delete-queued'); + $this->finished = $makeDeployment(ApplicationDeploymentStatus::FINISHED->value, 'server-delete-finished'); +}); + +afterEach(function () { + Server::flushIdentityMap(); +}); + +test('deleting a server fails its queued deployments', function (string $path) { + if ($path === 'api') { + $token = $this->user->createToken('server-delete', ['write'])->plainTextToken; + $this->withHeaders(['Authorization' => 'Bearer '.$token]) + ->deleteJson("/api/v1/servers/{$this->server->uuid}?force=true") + ->assertOk(); + } else { + $this->actingAs($this->user); + Livewire::test(Delete::class, ['server_uuid' => $this->server->uuid]) + ->call('delete', 'password', ['force_delete_resources']); + } + + expect(Server::withTrashed()->find($this->server->id)->trashed())->toBeTrue(); + + $queued = $this->queued->fresh(); + expect($queued->status)->toBe(ApplicationDeploymentStatus::FAILED->value) + ->and(collect(json_decode($queued->logs, true))->pluck('output')->implode("\n"))->toContain('The server was deleted.') + ->and($this->finished->fresh()->status)->toBe(ApplicationDeploymentStatus::FINISHED->value); +})->with(['api', 'ui']); diff --git a/tests/Unit/ParseCommandsByLineForSudoTest.php b/tests/Unit/ParseCommandsByLineForSudoTest.php index f9309ac5b9..5a98398da2 100644 --- a/tests/Unit/ParseCommandsByLineForSudoTest.php +++ b/tests/Unit/ParseCommandsByLineForSudoTest.php @@ -355,6 +355,42 @@ test('ownership changes keep container-owned files and file modes', function (st (new Process(['rm', '-rf', $directory, $path]))->run(); })->with(['command list', 'deployment line']); +test('ownership changes apply only to the mkdir path when more commands follow', function (string $parser, string $separator) { + $directory = sys_get_temp_dir().'/coolify-sudo-'.bin2hex(random_bytes(4)); + mkdir($directory); + file_put_contents("{$directory}/sudo", "#!/bin/sh\nexec \"\$@\"\n"); + foreach (['chown', 'chmod'] as $tool) { + file_put_contents("{$directory}/{$tool}", "#!/bin/sh\necho \"{$tool} \$*\" >> \"\$LOG\"\n"); + } + array_map(fn (string $tool) => chmod("{$directory}/{$tool}", 0755), ['sudo', 'chown', 'chmod']); + $path = '/tmp/coolify/sudo-test-'.bin2hex(random_bytes(4)); + $line = "mkdir -p {$path}/dynamic{$separator} ls -1 {$path}/dynamic | head -n 1000"; + + $command = $parser === 'command list' + ? parseCommandsByLineForSudo(collect([$line]), $this->server)[0] + : parseLineForSudo($line, $this->server); + $process = new Process(['bash', '-c', $command], env: ['PATH' => "{$directory}:".getenv('PATH'), 'LOG' => "{$directory}/log"]); + $process->run(); + + expect($command)->toContain("find {$path}/dynamic -user root -exec chown ubuntu:ubuntu {} + && ") + ->and($command)->toContain("chmod o-rwx {$path}/dynamic{$separator} ") + ->and($command)->toEndWith('head -n 1000'.($parser === 'command list' ? "'" : '')) + ->and($process->getErrorOutput())->toBe('') + ->and($process->isSuccessful())->toBeTrue() + ->and(is_dir("{$path}/dynamic"))->toBeTrue() + ->and(file_get_contents("{$directory}/log"))->toEndWith("chmod o-rwx {$path}/dynamic\n"); + + (new Process(['rm', '-rf', $directory, $path]))->run(); +})->with(['command list', 'deployment line'])->with(['and' => ' &&', 'semicolon' => ';']); + +test('does not add ownership changes when mkdir is followed by other operators', function () { + $result = parseCommandsByLineForSudo(collect([ + 'mkdir -p /data/coolify/proxy > /dev/null 2>&1 || true', + ]), $this->server); + + expect($result[0])->toBe('sudo mkdir -p /data/coolify/proxy > /dev/null 2>&1 || sudo true'); +}); + test('does not add ownership changes for system paths', function () { $commands = collect([ 'mkdir -p /var/log',