diff --git a/app/Actions/Database/StartDragonfly.php b/app/Actions/Database/StartDragonfly.php index 20f69bb715..8327cf2dfe 100644 --- a/app/Actions/Database/StartDragonfly.php +++ b/app/Actions/Database/StartDragonfly.php @@ -251,14 +251,21 @@ class StartDragonfly $environment_variables = collect(); $this->resolvedRedisPassword = (string) $this->database->dragonfly_password; foreach ($this->database->runtime_environment_variables as $env) { + $usesSecretManager = $this->database->environmentVariableUsesSecretManager($env); + if ($env->key === 'REDIS_PASSWORD' && ! $env->is_shared && ! $usesSecretManager) { + $env->update(['value' => $this->database->dragonfly_password]); + } $rawValue = (string) $this->database->resolveSecretManagerEnvironmentVariableValue($env); $resolvedValue = (string) $this->database->formatEnvironmentVariableValue($env, $rawValue); // Credentials below are placed directly in the compose file (healthcheck, command). $composeFileValue = $this->database->formatComposeFileValue($env, $rawValue); $environment_variables->push($env->key.'='.$resolvedValue); if ($env->key === 'REDIS_PASSWORD') { + if ($env->is_shared && ! $usesSecretManager) { + $this->database->update(['dragonfly_password' => $rawValue]); + } $this->resolvedRedisPassword = $composeFileValue; - $this->redisPasswordFromSecretManager = $this->database->environmentVariableUsesSecretManager($env); + $this->redisPasswordFromSecretManager = $usesSecretManager; } } diff --git a/app/Actions/Database/StartKeydb.php b/app/Actions/Database/StartKeydb.php index f6c2ff0bc7..6bd3c4b236 100644 --- a/app/Actions/Database/StartKeydb.php +++ b/app/Actions/Database/StartKeydb.php @@ -251,14 +251,21 @@ class StartKeydb $environment_variables = collect(); $this->resolvedRedisPassword = (string) $this->database->keydb_password; foreach ($this->database->runtime_environment_variables as $env) { + $usesSecretManager = $this->database->environmentVariableUsesSecretManager($env); + if ($env->key === 'REDIS_PASSWORD' && ! $env->is_shared && ! $usesSecretManager) { + $env->update(['value' => $this->database->keydb_password]); + } $rawValue = (string) $this->database->resolveSecretManagerEnvironmentVariableValue($env); $resolvedValue = (string) $this->database->formatEnvironmentVariableValue($env, $rawValue); // Credentials below are placed directly in the compose file (healthcheck, command). $composeFileValue = $this->database->formatComposeFileValue($env, $rawValue); $environment_variables->push($env->key.'='.$resolvedValue); if ($env->key === 'REDIS_PASSWORD') { + if ($env->is_shared && ! $usesSecretManager) { + $this->database->update(['keydb_password' => $rawValue]); + } $this->resolvedRedisPassword = $composeFileValue; - $this->redisPasswordFromSecretManager = $this->database->environmentVariableUsesSecretManager($env); + $this->redisPasswordFromSecretManager = $usesSecretManager; } } diff --git a/app/Http/Controllers/Api/ApplicationsController.php b/app/Http/Controllers/Api/ApplicationsController.php index b2d84113f2..238a318ce3 100644 --- a/app/Http/Controllers/Api/ApplicationsController.php +++ b/app/Http/Controllers/Api/ApplicationsController.php @@ -40,6 +40,7 @@ use Symfony\Component\Yaml\Yaml; class ApplicationsController extends Controller { use Concerns\HandlesTagsApi; + use Concerns\RequiresDeployForOutsideHostPaths; private const APPLICATION_SETTING_FIELDS = [ 'is_git_submodules_enabled', @@ -5042,7 +5043,7 @@ class ApplicationsController extends Controller 'is_preview_suffix_enabled' => ['type' => 'boolean', 'description' => 'Whether to add -pr-N suffix for preview deployments.'], 'name' => ['type' => 'string', 'description' => 'The volume name (persistent only, not allowed for read-only storages).'], 'mount_path' => ['type' => 'string', 'description' => 'The container mount path (not allowed for read-only storages).'], - 'content' => ['type' => 'string', 'nullable' => true, 'description' => 'The file content (file only, not allowed for read-only storages).'], + 'content' => ['type' => 'string', 'nullable' => true, 'description' => 'The file content (file only, not allowed for read-only storages). Changing the content of a file outside the resource directory needs the deploy permission.'], ], additionalProperties: false, ), @@ -5067,6 +5068,10 @@ class ApplicationsController extends Controller response: 404, ref: '#/components/responses/404', ), + new OA\Response( + response: 403, + description: 'The token needs the deploy permission to change the content of a file outside the resource directory.', + ), new OA\Response( response: 422, ref: '#/components/responses/422', @@ -5173,6 +5178,10 @@ class ApplicationsController extends Controller } } + if (! $isReadOnly && ($forbidden = $this->outsideContentChangeForbiddenResponse($request, $storage))) { + return $forbidden; + } + // Always allowed if ($request->has('is_preview_suffix_enabled')) { $storage->is_preview_suffix_enabled = $request->is_preview_suffix_enabled; @@ -5243,7 +5252,7 @@ class ApplicationsController extends Controller 'mount_path' => ['type' => 'string', 'description' => 'The container mount path.'], 'content' => ['type' => 'string', 'nullable' => true, 'description' => 'File content (file only, optional).'], 'is_directory' => ['type' => 'boolean', 'description' => 'Whether this is a directory mount (file only, default false).'], - 'fs_path' => ['type' => 'string', 'description' => 'Host path. Required for directory mounts and host file mounts. Optional for file mounts with content (default: inside the resource directory). An absolute path can be anywhere on the host; a relative path is inside the resource directory. Coolify never deletes a path outside the resource directory.'], + 'fs_path' => ['type' => 'string', 'description' => 'Host path. Required for directory mounts and host file mounts. Optional for file mounts with content (default: inside the resource directory). An absolute path can be anywhere on the host; a relative path is inside the resource directory. A directory or file mount outside the resource directory needs a token with the deploy permission. Coolify never deletes a path outside the resource directory.'], ], additionalProperties: false, ), @@ -5259,6 +5268,7 @@ class ApplicationsController extends Controller new OA\Response(response: 401, ref: '#/components/responses/401'), new OA\Response(response: 400, ref: '#/components/responses/400'), new OA\Response(response: 404, ref: '#/components/responses/404'), + new OA\Response(response: 403, description: 'The token needs the deploy permission for a mount outside the resource directory.'), new OA\Response(response: 422, ref: '#/components/responses/422'), ] )] @@ -5369,6 +5379,15 @@ class ApplicationsController extends Controller try { $fsPath = LocalFileVolume::resolveHostPath(application_configuration_dir().'/'.$application->uuid, $request->fs_path, 'storage source path'); $mountPath = validateFileMountPath($request->mount_path, 'storage destination path'); + $forbidden = $this->outsideHostPathForbiddenResponse($request, new LocalFileVolume([ + 'fs_path' => $fsPath, + 'is_directory' => true, + 'resource_id' => $application->id, + 'resource_type' => get_class($application), + ])); + if ($forbidden) { + return $forbidden; + } LocalFileVolume::assertHostPathOnServer(application_configuration_dir().'/'.$application->uuid, $fsPath, $application->destination->server, isDirectory: true); } catch (\Throwable $e) { return response()->json([ @@ -5431,6 +5450,14 @@ class ApplicationsController extends Controller try { if ($request->filled('fs_path')) { $fsPath = LocalFileVolume::resolveHostPath(application_configuration_dir().'/'.$application->uuid, $request->fs_path, 'file storage source path'); + $forbidden = $this->outsideHostPathForbiddenResponse($request, new LocalFileVolume([ + 'fs_path' => $fsPath, + 'resource_id' => $application->id, + 'resource_type' => get_class($application), + ])); + if ($forbidden) { + return $forbidden; + } LocalFileVolume::assertHostPathOnServer(application_configuration_dir().'/'.$application->uuid, $fsPath, $application->destination->server, isDirectory: false); } else { $fsPath = confineFileMountPath(application_configuration_dir().'/'.$application->uuid, $mountPath, 'file storage path'); diff --git a/app/Http/Controllers/Api/Concerns/RequiresDeployForOutsideHostPaths.php b/app/Http/Controllers/Api/Concerns/RequiresDeployForOutsideHostPaths.php new file mode 100644 index 0000000000..90d3a136fa --- /dev/null +++ b/app/Http/Controllers/Api/Concerns/RequiresDeployForOutsideHostPaths.php @@ -0,0 +1,41 @@ +is_host_file || ! $storage->isOutsideResourceDirectory()) { + return null; + } + if ($request->user()->tokenCan('deploy') || $request->user()->tokenCan('root')) { + return null; + } + + return response()->json([ + 'message' => 'Missing required permissions: deploy. A mount outside the resource directory needs a token with the deploy permission.', + ], 403); + } + + private function outsideContentChangeForbiddenResponse(Request $request, LocalFileVolume|LocalPersistentVolume $storage): ?JsonResponse + { + if (! $storage instanceof LocalFileVolume || $storage->is_directory || ! $request->has('content')) { + return null; + } + if ((string) $request->input('content') === (string) $storage->content) { + return null; + } + + return $this->outsideHostPathForbiddenResponse($request, $storage); + } +} diff --git a/app/Http/Controllers/Api/DatabasesController.php b/app/Http/Controllers/Api/DatabasesController.php index 38c304f3f1..b32a16fc61 100644 --- a/app/Http/Controllers/Api/DatabasesController.php +++ b/app/Http/Controllers/Api/DatabasesController.php @@ -36,6 +36,7 @@ class DatabasesController extends Controller { use Concerns\HandlesDatabaseImportsApi; use Concerns\HandlesTagsApi; + use Concerns\RequiresDeployForOutsideHostPaths; #[OA\Post( path: '/databases/{uuid}/imports/uploads', @@ -4290,7 +4291,7 @@ class DatabasesController extends Controller 'mount_path' => ['type' => 'string', 'description' => 'The container mount path.'], 'content' => ['type' => 'string', 'nullable' => true, 'description' => 'File content (file only, optional).'], 'is_directory' => ['type' => 'boolean', 'description' => 'Whether this is a directory mount (file only, default false).'], - 'fs_path' => ['type' => 'string', 'description' => 'Host path. Required for directory mounts and host file mounts. Optional for file mounts with content (default: inside the resource directory). An absolute path can be anywhere on the host; a relative path is inside the resource directory. Coolify never deletes a path outside the resource directory.'], + 'fs_path' => ['type' => 'string', 'description' => 'Host path. Required for directory mounts and host file mounts. Optional for file mounts with content (default: inside the resource directory). An absolute path can be anywhere on the host; a relative path is inside the resource directory. A directory or file mount outside the resource directory needs a token with the deploy permission. Coolify never deletes a path outside the resource directory.'], ], additionalProperties: false, ), @@ -4306,6 +4307,7 @@ class DatabasesController extends Controller new OA\Response(response: 401, ref: '#/components/responses/401'), new OA\Response(response: 400, ref: '#/components/responses/400'), new OA\Response(response: 404, ref: '#/components/responses/404'), + new OA\Response(response: 403, description: 'The token needs the deploy permission for a mount outside the resource directory.'), new OA\Response(response: 422, ref: '#/components/responses/422'), ] )] @@ -4412,6 +4414,15 @@ class DatabasesController extends Controller try { $fsPath = LocalFileVolume::resolveHostPath(database_configuration_dir().'/'.$database->uuid, $request->fs_path, 'storage source path'); $mountPath = validateFileMountPath($request->mount_path, 'storage destination path'); + $forbidden = $this->outsideHostPathForbiddenResponse($request, new LocalFileVolume([ + 'fs_path' => $fsPath, + 'is_directory' => true, + 'resource_id' => $database->id, + 'resource_type' => get_class($database), + ])); + if ($forbidden) { + return $forbidden; + } LocalFileVolume::assertHostPathOnServer(database_configuration_dir().'/'.$database->uuid, $fsPath, $database->destination->server, isDirectory: true); } catch (\Throwable $e) { return response()->json([ @@ -4474,6 +4485,14 @@ class DatabasesController extends Controller try { if ($request->filled('fs_path')) { $fsPath = LocalFileVolume::resolveHostPath(database_configuration_dir().'/'.$database->uuid, $request->fs_path, 'file storage source path'); + $forbidden = $this->outsideHostPathForbiddenResponse($request, new LocalFileVolume([ + 'fs_path' => $fsPath, + 'resource_id' => $database->id, + 'resource_type' => get_class($database), + ])); + if ($forbidden) { + return $forbidden; + } LocalFileVolume::assertHostPathOnServer(database_configuration_dir().'/'.$database->uuid, $fsPath, $database->destination->server, isDirectory: false); } else { $fsPath = confineFileMountPath(database_configuration_dir().'/'.$database->uuid, $mountPath, 'file storage path'); @@ -4543,7 +4562,7 @@ class DatabasesController extends Controller 'is_preview_suffix_enabled' => ['type' => 'boolean', 'description' => 'Whether to add -pr-N suffix for preview deployments.'], 'name' => ['type' => 'string', 'description' => 'The volume name (persistent only, not allowed for read-only storages).'], 'mount_path' => ['type' => 'string', 'description' => 'The container mount path (not allowed for read-only storages).'], - 'content' => ['type' => 'string', 'nullable' => true, 'description' => 'The file content (file only, not allowed for read-only storages).'], + 'content' => ['type' => 'string', 'nullable' => true, 'description' => 'The file content (file only, not allowed for read-only storages). Changing the content of a file outside the resource directory needs the deploy permission.'], ], additionalProperties: false, ), @@ -4568,6 +4587,10 @@ class DatabasesController extends Controller response: 404, ref: '#/components/responses/404', ), + new OA\Response( + response: 403, + description: 'The token needs the deploy permission to change the content of a file outside the resource directory.', + ), new OA\Response( response: 422, ref: '#/components/responses/422', @@ -4671,6 +4694,10 @@ class DatabasesController extends Controller } } + if (! $isReadOnly && ($forbidden = $this->outsideContentChangeForbiddenResponse($request, $storage))) { + return $forbidden; + } + // Always allowed if ($request->has('is_preview_suffix_enabled')) { $storage->is_preview_suffix_enabled = $request->is_preview_suffix_enabled; diff --git a/app/Http/Controllers/Api/ServicesController.php b/app/Http/Controllers/Api/ServicesController.php index 7ad3684f43..521e848a88 100644 --- a/app/Http/Controllers/Api/ServicesController.php +++ b/app/Http/Controllers/Api/ServicesController.php @@ -31,6 +31,7 @@ use Symfony\Component\Yaml\Yaml; class ServicesController extends Controller { use Concerns\HandlesTagsApi; + use Concerns\RequiresDeployForOutsideHostPaths; protected function findTaggableResource(string $uuid, int|string $teamId): mixed { @@ -2476,7 +2477,7 @@ class ServicesController extends Controller 'mount_path' => ['type' => 'string', 'description' => 'The container mount path.'], 'content' => ['type' => 'string', 'nullable' => true, 'description' => 'File content (file only, optional).'], 'is_directory' => ['type' => 'boolean', 'description' => 'Whether this is a directory mount (file only, default false).'], - 'fs_path' => ['type' => 'string', 'description' => 'Host path. Required for directory mounts and host file mounts. Optional for file mounts with content (default: inside the resource directory). An absolute path can be anywhere on the host; a relative path is inside the resource directory. Coolify never deletes a path outside the resource directory.'], + 'fs_path' => ['type' => 'string', 'description' => 'Host path. Required for directory mounts and host file mounts. Optional for file mounts with content (default: inside the resource directory). An absolute path can be anywhere on the host; a relative path is inside the resource directory. A directory or file mount outside the resource directory needs a token with the deploy permission. Coolify never deletes a path outside the resource directory.'], ], additionalProperties: false, ), @@ -2492,6 +2493,7 @@ class ServicesController extends Controller new OA\Response(response: 401, ref: '#/components/responses/401'), new OA\Response(response: 400, ref: '#/components/responses/400'), new OA\Response(response: 404, ref: '#/components/responses/404'), + new OA\Response(response: 403, description: 'The token needs the deploy permission for a mount outside the resource directory.'), new OA\Response(response: 422, ref: '#/components/responses/422'), ] )] @@ -2607,6 +2609,15 @@ class ServicesController extends Controller try { $fsPath = LocalFileVolume::resolveHostPath(service_configuration_dir().'/'.$service->uuid, $request->fs_path, 'storage source path'); $mountPath = validateFileMountPath($request->mount_path, 'storage destination path'); + $forbidden = $this->outsideHostPathForbiddenResponse($request, new LocalFileVolume([ + 'fs_path' => $fsPath, + 'is_directory' => true, + 'resource_id' => $subResource->id, + 'resource_type' => get_class($subResource), + ])); + if ($forbidden) { + return $forbidden; + } LocalFileVolume::assertHostPathOnServer(service_configuration_dir().'/'.$service->uuid, $fsPath, $service->server, isDirectory: true); } catch (\Throwable $e) { return response()->json([ @@ -2669,6 +2680,14 @@ class ServicesController extends Controller try { if ($request->filled('fs_path')) { $fsPath = LocalFileVolume::resolveHostPath(service_configuration_dir().'/'.$service->uuid, $request->fs_path, 'file storage source path'); + $forbidden = $this->outsideHostPathForbiddenResponse($request, new LocalFileVolume([ + 'fs_path' => $fsPath, + 'resource_id' => $subResource->id, + 'resource_type' => get_class($subResource), + ])); + if ($forbidden) { + return $forbidden; + } LocalFileVolume::assertHostPathOnServer(service_configuration_dir().'/'.$service->uuid, $fsPath, $service->server, isDirectory: false); } else { $fsPath = confineFileMountPath(service_configuration_dir().'/'.$service->uuid, $mountPath, 'file storage path'); @@ -2738,7 +2757,7 @@ class ServicesController extends Controller 'is_preview_suffix_enabled' => ['type' => 'boolean', 'description' => 'Whether to add -pr-N suffix for preview deployments.'], 'name' => ['type' => 'string', 'description' => 'The volume name (persistent only, not allowed for read-only storages).'], 'mount_path' => ['type' => 'string', 'description' => 'The container mount path (not allowed for read-only storages).'], - 'content' => ['type' => 'string', 'nullable' => true, 'description' => 'The file content (file only, not allowed for read-only storages).'], + 'content' => ['type' => 'string', 'nullable' => true, 'description' => 'The file content (file only, not allowed for read-only storages). Changing the content of a file outside the resource directory needs the deploy permission.'], ], additionalProperties: false, ), @@ -2763,6 +2782,10 @@ class ServicesController extends Controller response: 404, ref: '#/components/responses/404', ), + new OA\Response( + response: 403, + description: 'The token needs the deploy permission to change the content of a file outside the resource directory.', + ), new OA\Response( response: 422, ref: '#/components/responses/422', @@ -2896,6 +2919,10 @@ class ServicesController extends Controller } } + if (! $isReadOnly && ($forbidden = $this->outsideContentChangeForbiddenResponse($request, $storage))) { + return $forbidden; + } + // Always allowed if ($request->has('is_preview_suffix_enabled')) { $storage->is_preview_suffix_enabled = $request->is_preview_suffix_enabled; diff --git a/app/Http/Controllers/Webhook/Github.php b/app/Http/Controllers/Webhook/Github.php index 1dec6b71d9..8f44152306 100644 --- a/app/Http/Controllers/Webhook/Github.php +++ b/app/Http/Controllers/Webhook/Github.php @@ -316,10 +316,15 @@ class Github extends Controller ? GithubRunnerExecution::query()->where('github_app_id', $githubApp->id)->where('runner_name', $runnerName)->first() : null; - if ($action === 'in_progress' && $execution && in_array($execution->status, [GithubRunnerStatus::Provisioning, GithubRunnerStatus::Idle], true)) { - $execution->update([...$jobDetails, 'status' => GithubRunnerStatus::Running, 'started_at' => now()]); - - return response('Runner marked running.'); + // The completed hook can arrive before the in_progress hook. + if (in_array($action, ['in_progress', 'completed'], true) && $execution && in_array($execution->status, [GithubRunnerStatus::Provisioning, GithubRunnerStatus::Idle], true)) { + $replacement = $execution->assignJob($jobId, $jobDetails); + if ($replacement) { + ProvisionGithubRunnerJob::dispatch($replacement->id); + } + if ($action === 'in_progress') { + return response('Runner marked running.'); + } } if ($action === 'completed') { diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 7ffafc33c8..3379b267d4 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -2265,7 +2265,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue */ private function ensureRegistryImageForMultipleServers(): void { - if ($this->pull_request_id !== 0) { + if ($this->pull_request_id !== 0 || $this->application->build_pack === 'dockercompose') { return; } if ($this->application->additional_servers()->doesntExist()) { diff --git a/app/Jobs/ScheduledJobManager.php b/app/Jobs/ScheduledJobManager.php index e1411412fb..4c3beb97cd 100644 --- a/app/Jobs/ScheduledJobManager.php +++ b/app/Jobs/ScheduledJobManager.php @@ -497,15 +497,17 @@ class ScheduledJobManager implements ShouldQueue ->where('stop_recovery_pending', true) ->orWhere('s3_cleanup_pending', true)) ->where('recovery_needs_attention', false) - ->where(fn (Builder $query) => $query - ->whereNull('recovery_next_retry_at') - ->orWhere('recovery_next_retry_at', '<=', now())) + ->with('scheduledVolumeBackup.backupable.resource') ->chunkById(self::CHUNK_SIZE, function ($executions) use (&$dispatched): bool { foreach ($executions as $execution) { if ($dispatched >= self::VOLUME_BACKUP_RECOVERY_MAX_DISPATCHES) { return false; } + if ($execution->stop_recovery_pending && $execution->scheduledVolumeBackup?->server()?->isFunctional() === false) { + continue; + } + if (Cache::add(VolumeBackupRecoveryJob::dispatchCacheKey($execution->id), true, now()->addMinutes(self::VOLUME_BACKUP_RECOVERY_INTERVAL_MINUTES))) { VolumeBackupRecoveryJob::dispatch($execution); $dispatched++; diff --git a/app/Jobs/VolumeBackupRecoveryJob.php b/app/Jobs/VolumeBackupRecoveryJob.php index 7f475d24ce..bd3310712e 100644 --- a/app/Jobs/VolumeBackupRecoveryJob.php +++ b/app/Jobs/VolumeBackupRecoveryJob.php @@ -3,6 +3,7 @@ namespace App\Jobs; use App\Models\ScheduledVolumeBackupExecution; +use App\Notifications\VolumeBackup\RecoveryFailed; use Aws\S3\Exception\S3Exception; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldBeEncrypted; @@ -21,7 +22,7 @@ class VolumeBackupRecoveryJob implements ShouldBeEncrypted, ShouldBeUnique, Shou use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; /** - * ScheduledJobManager dispatches pending recovery again after a backoff, so this job does not retry by itself. + * ScheduledJobManager dispatches recovery again once an unreachable server is functional, so this job does not retry by itself. */ public int $tries = 1; @@ -29,12 +30,6 @@ class VolumeBackupRecoveryJob implements ShouldBeEncrypted, ShouldBeUnique, Shou public int $uniqueFor = 300; - public const MAX_ATTEMPTS = 8; - - public const BACKOFF_BASE_MINUTES = 5; - - public const BACKOFF_CAP_MINUTES = 360; - public const CATEGORY_S3_AUTH = 's3_auth'; public const CATEGORY_S3_BUCKET = 's3_bucket'; @@ -86,8 +81,8 @@ class VolumeBackupRecoveryJob implements ShouldBeEncrypted, ShouldBeUnique, Shou } /** - * Runs one recovery attempt and records its outcome instead of throwing, so a persistent failure backs off - * and eventually needs attention rather than creating a failed job every few minutes. + * Runs one recovery attempt and records its outcome instead of throwing. An unreachable server waits for the + * server to be functional again; any other failure needs attention and notifies the team. */ public static function recoverWithBounds(ScheduledVolumeBackupExecution $execution): void { @@ -157,16 +152,6 @@ class VolumeBackupRecoveryJob implements ShouldBeEncrypted, ShouldBeUnique, Shou return $failure; } - /** - * Returns the minutes to wait before the next automatic attempt after the given number of failed attempts. - */ - public static function backoffMinutes(int $attempts): int - { - $exponent = max(0, min($attempts - 1, 16)); - - return (int) min(self::BACKOFF_BASE_MINUTES * (2 ** $exponent), self::BACKOFF_CAP_MINUTES); - } - public static function dispatchCacheKey(int $executionId): string { return "volume-backup-recovery-dispatched:{$executionId}"; @@ -175,9 +160,7 @@ class VolumeBackupRecoveryJob implements ShouldBeEncrypted, ShouldBeUnique, Shou private static function recordSuccess(ScheduledVolumeBackupExecution $execution): void { $execution->update([ - 'recovery_attempts' => 0, 'recovery_last_attempt_at' => null, - 'recovery_next_retry_at' => null, 'recovery_error' => null, 'recovery_needs_attention' => false, ]); @@ -185,18 +168,18 @@ class VolumeBackupRecoveryJob implements ShouldBeEncrypted, ShouldBeUnique, Shou private static function recordFailure(ScheduledVolumeBackupExecution $execution, string $category): void { - $recorded = $execution->getConnection()->transaction(function () use ($execution, $category): ?ScheduledVolumeBackupExecution { + $needsAttention = $category !== self::CATEGORY_SERVER_UNREACHABLE; + $enteredNeedsAttention = false; + + $recorded = $execution->getConnection()->transaction(function () use ($execution, $category, $needsAttention, &$enteredNeedsAttention): ?ScheduledVolumeBackupExecution { $current = ScheduledVolumeBackupExecution::query()->whereKey($execution->id)->lockForUpdate()->first(); if (! $current) { return null; } - $attempts = $current->recovery_attempts + 1; - $needsAttention = $attempts >= self::MAX_ATTEMPTS; + $enteredNeedsAttention = $needsAttention && ! $current->recovery_needs_attention; $current->update([ - 'recovery_attempts' => $attempts, 'recovery_last_attempt_at' => now(), - 'recovery_next_retry_at' => $needsAttention ? null : now()->addMinutes(self::backoffMinutes($attempts)), 'recovery_error' => $category, 'recovery_needs_attention' => $needsAttention, ]); @@ -214,10 +197,25 @@ class VolumeBackupRecoveryJob implements ShouldBeEncrypted, ShouldBeUnique, Shou 'execution_id' => $recorded->id, 'execution_uuid' => $recorded->uuid, 'category' => $category, - 'attempts' => $recorded->recovery_attempts, 'needs_attention' => $recorded->recovery_needs_attention, - 'next_retry_at' => $recorded->recovery_next_retry_at?->toIso8601String(), ]); + + if ($enteredNeedsAttention) { + self::notifyTeam($recorded); + } + } + + private static function notifyTeam(ScheduledVolumeBackupExecution $execution): void + { + try { + $execution->loadMissing('scheduledVolumeBackup.team'); + $execution->scheduledVolumeBackup?->team?->notify(new RecoveryFailed($execution)); + } catch (Throwable $exception) { + Log::channel('scheduled-errors')->error('Failed to send volume backup recovery notification', [ + 'execution_id' => $execution->id, + 'error' => $exception->getMessage(), + ]); + } } private static function categorizeS3Failure(Throwable $exception): string diff --git a/app/Livewire/Project/Shared/Storages/VolumeBackups.php b/app/Livewire/Project/Shared/Storages/VolumeBackups.php index 9768d67a55..4ceffef502 100644 --- a/app/Livewire/Project/Shared/Storages/VolumeBackups.php +++ b/app/Livewire/Project/Shared/Storages/VolumeBackups.php @@ -316,8 +316,6 @@ class VolumeBackups extends Component } $execution->update([ - 'recovery_attempts' => 0, - 'recovery_next_retry_at' => null, 'recovery_needs_attention' => false, ]); Cache::forget(VolumeBackupRecoveryJob::dispatchCacheKey($execution->id)); diff --git a/app/Models/GithubRunnerExecution.php b/app/Models/GithubRunnerExecution.php index 77a55fd075..e7c36d7d8a 100644 --- a/app/Models/GithubRunnerExecution.php +++ b/app/Models/GithubRunnerExecution.php @@ -4,8 +4,11 @@ namespace App\Models; use App\Enums\GithubRunnerStatus; use App\Jobs\DeregisterGithubRunnerJob; +use App\Jobs\ProvisionGithubRunnerJob; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Relations\BelongsTo; +use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Log; class GithubRunnerExecution extends BaseModel { @@ -81,6 +84,65 @@ class GithubRunnerExecution extends BaseModel (clone $query)->delete(); } + /** + * Marks the runner running. When a job without a pending runner took it, returns a new queued execution for the original job. + * + * @param array $jobDetails + */ + public function assignJob(int $jobId, array $jobDetails): ?self + { + $replacement = DB::transaction(function () use ($jobId, $jobDetails): ?self { + $execution = self::query()->whereKey($this->id)->lockForUpdate()->first(); + if (! $execution || ! in_array($execution->status, [GithubRunnerStatus::Provisioning, GithubRunnerStatus::Idle], true)) { + return null; + } + + $running = [...$jobDetails, 'workflow_job_id' => $jobId, 'status' => GithubRunnerStatus::Running, 'started_at' => now()]; + $jobExecution = $execution->trigger_workflow_job_id === $jobId ? $execution : self::query() + ->where('github_app_id', $execution->github_app_id) + ->where('trigger_workflow_job_id', $jobId) + ->lockForUpdate() + ->first(); + if ($jobExecution?->isActive()) { + $execution->update($running); + + return null; + } + + $replacement = [ + ...$execution->only(['github_app_id', 'trigger_workflow_job_id', 'workflow_job_html_url', 'workflow_name', 'job_name', 'repository_full_name', 'labels', 'is_pull_request', 'provision_attempts']), + 'workflow_job_id' => $execution->trigger_workflow_job_id, + 'status' => GithubRunnerStatus::Queued, + 'queued_at' => now(), + ]; + $jobExecution?->delete(); + $execution->update([...$running, 'trigger_workflow_job_id' => $jobId]); + + if ($replacement['provision_attempts'] >= ProvisionGithubRunnerJob::MAX_ATTEMPTS) { + Log::warning('GitHub runner taken by another job too often; no new runner is started.', [ + 'github_app_id' => $execution->github_app_id, + 'workflow_job_id' => $replacement['trigger_workflow_job_id'], + 'attempts' => $replacement['provision_attempts'], + ]); + + return null; + } + + $hasMatchingConfig = GithubRunnerConfig::query() + ->where('github_app_id', $execution->github_app_id) + ->where('is_enabled', true) + ->get() + ->contains(fn (GithubRunnerConfig $config) => $config->matchesLabels($replacement['labels'] ?? []) + && ($config->allow_pull_requests || ! $replacement['is_pull_request'])); + + return $hasMatchingConfig ? self::create($replacement) : null; + }); + + $this->refresh(); + + return $replacement; + } + public function isActive(): bool { return $this->status->isActive(); diff --git a/app/Models/ScheduledVolumeBackupExecution.php b/app/Models/ScheduledVolumeBackupExecution.php index eec799218e..664efa11c1 100644 --- a/app/Models/ScheduledVolumeBackupExecution.php +++ b/app/Models/ScheduledVolumeBackupExecution.php @@ -21,9 +21,7 @@ class ScheduledVolumeBackupExecution extends BaseModel 'local_storage_deleted', 's3_storage_deleted', 's3_uploaded', - 'recovery_attempts', 'recovery_last_attempt_at', - 'recovery_next_retry_at', 'recovery_error', 'recovery_needs_attention', ]; @@ -39,9 +37,7 @@ class ScheduledVolumeBackupExecution extends BaseModel 'local_storage_deleted' => 'boolean', 's3_storage_deleted' => 'boolean', 's3_uploaded' => 'boolean', - 'recovery_attempts' => 'integer', 'recovery_last_attempt_at' => 'datetime', - 'recovery_next_retry_at' => 'datetime', 'recovery_needs_attention' => 'boolean', ]; } diff --git a/app/Notifications/VolumeBackup/RecoveryFailed.php b/app/Notifications/VolumeBackup/RecoveryFailed.php new file mode 100644 index 0000000000..ecb2220ec5 --- /dev/null +++ b/app/Notifications/VolumeBackup/RecoveryFailed.php @@ -0,0 +1,134 @@ +onQueue('high'); + $backup = $execution->scheduledVolumeBackup; + $resource = $backup?->targetResource(); + $this->resourceName = $resource?->name ?? 'Unknown resource'; + $this->target = $backup ? $backup->targetType().' '.$backup->targetName() : 'Unknown storage'; + $this->reason = match ($execution->recovery_error) { + VolumeBackupRecoveryJob::CATEGORY_S3_AUTH => 'S3 credentials were rejected', + VolumeBackupRecoveryJob::CATEGORY_S3_BUCKET => 'S3 bucket is missing', + VolumeBackupRecoveryJob::CATEGORY_REMOTE_COMMAND => 'A command on the server failed', + default => 'Unknown error', + }; + $this->containersStopped = (bool) $execution->stop_recovery_pending; + + $linkable = $resource instanceof ServiceApplication || $resource instanceof ServiceDatabase ? $resource->service : $resource; + if ($linkable && method_exists($linkable, 'link')) { + $this->url = $linkable->link(); + } + } + + public function via(object $notifiable): array + { + return $notifiable->getEnabledChannels('backup_failure'); + } + + public function summary(): string + { + $summary = "Recovery after the storage backup of {$this->target} ({$this->resourceName}) failed: {$this->reason}."; + + return $this->containersStopped + ? $summary.' The containers stopped for the backup are still stopped, and new backups of this storage are skipped until you retry the recovery.' + : $summary.' The partial S3 upload was not removed. Retry the recovery after fixing the S3 storage.'; + } + + public function toMail(): MailMessage + { + $mail = new MailMessage; + $mail->subject("Coolify: [ACTION REQUIRED] Storage backup recovery FAILED for {$this->resourceName}"); + $mail->view('emails.volume-backup-recovery-failed', [ + 'summary' => $this->summary(), + 'url' => $this->url, + ]); + + return $mail; + } + + public function toDiscord(): DiscordMessage + { + $message = new DiscordMessage( + title: ':cross_mark: Storage backup recovery failed', + description: $this->summary(), + color: DiscordMessage::errorColor(), + isCritical: $this->containersStopped, + ); + + if ($this->url) { + $message->addField('Resource', '[Link]('.$this->url.')'); + } + + return $message; + } + + public function toTelegram(): array + { + $data = ['message' => "Coolify: [ACTION REQUIRED] {$this->summary()}"]; + + if ($this->url) { + $data['buttons'] = [['text' => 'Open resource in Coolify', 'url' => $this->url]]; + } + + return $data; + } + + public function toPushover(): PushoverMessage + { + return new PushoverMessage( + title: 'Storage backup recovery failed', + level: 'error', + message: "[ACTION REQUIRED] {$this->summary()}", + buttons: $this->url ? [['text' => 'Open resource in Coolify', 'url' => $this->url]] : [], + ); + } + + public function toSlack(): SlackMessage + { + return new SlackMessage( + title: 'Coolify: [ACTION REQUIRED] Storage backup recovery failed', + description: $this->summary().($this->url ? "\n\n<{$this->url}|Open resource in Coolify>" : ''), + color: SlackMessage::errorColor(), + ); + } + + public function toWebhook(): array + { + return [ + 'success' => false, + 'message' => 'Storage backup recovery failed', + 'event' => 'volume_backup_recovery_failed', + 'resource_name' => $this->resourceName, + 'storage' => $this->target, + 'execution_uuid' => $this->execution->uuid, + 'reason' => $this->execution->recovery_error, + 'containers_stopped' => $this->containersStopped, + 'url' => $this->url, + ]; + } +} diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index 7f39cfb408..8947b3964b 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -87,9 +87,10 @@ function validateDockerComposeForInjection(string $composeYaml, ?string $resourc if (is_array($serviceConfig) && isset($serviceConfig['networks']) && is_array($serviceConfig['networks'])) { foreach ($serviceConfig['networks'] as $networkKey => $networkDetails) { if (is_int($networkKey) && (is_string($networkDetails) || is_int($networkDetails))) { - validateComposeNetworkName((string) $networkDetails, 'service network'); + validateComposeNetworkNameWithVariables((string) $networkDetails, 'service network', (string) $serviceName); } elseif (is_string($networkKey) || is_int($networkKey)) { - validateComposeNetworkName((string) $networkKey, 'service network'); + // Compose does not interpolate keys, so a key must be a plain network name. + validateComposeNetworkName((string) $networkKey, 'service network', (string) $serviceName); } } } @@ -259,40 +260,44 @@ function ensureComposeNetworkNameVariables(Application|Service $resource, iterab } /** - * A network `name:` may be such a variable: only Docker Compose reads this value and it never runs a - * shell; Coolify's own network commands use the network keys. The default must still be a valid - * network name, and nothing else is allowed around the variable. + * A network `name:` may contain variables: only Docker Compose reads this value and it never runs a + * shell; Coolify's own network commands use the network keys. * - * @throws Exception If the value is not a valid network name or such a variable + * @throws Exception If the value is not a valid network name with safe variables */ function validateComposeNetworkNameField(string $name): void +{ + validateComposeNetworkNameWithVariables($name, 'network name field'); +} + +/** + * Allows $VAR, ${VAR}, ${VAR:-default} and ${VAR-default} with safe defaults in a network value that Compose interpolates. + */ +function validateComposeNetworkNameWithVariables(string $name, string $context, ?string $serviceName = null): void { $variable = composeNetworkNameVariable($name); if ($variable !== null) { - if ($variable['default'] !== null) { - validateComposeNetworkName($variable['default'], 'network name field'); - } + $isValid = $variable['default'] === null || ValidationPatterns::isValidDockerNetwork($variable['default']); + } else { + $hasSafeDefaults = true; + $withoutVariables = preg_replace_callback( + '/\$\{([A-Za-z_][A-Za-z0-9_]*)(?::?-([^}]*))?\}|\$[A-Za-z_][A-Za-z0-9_]*/', + function (array $matches) use (&$hasSafeDefaults): string { + $default = $matches[2] ?? ''; + if ($default !== '' && preg_match('/\A[A-Za-z0-9_.-]+\z/', $default) !== 1) { + $hasSafeDefaults = false; + } - return; + return 'x'; + }, + $name, + ); + $isValid = $hasSafeDefaults && $withoutVariables !== null && ValidationPatterns::isValidDockerNetwork($withoutVariables); } - // Compose also resolves variables inside a longer name, for example ${COMPOSE_PROJECT_NAME}_default. - // Each variable must be $VAR, ${VAR}, ${VAR:-default} or ${VAR-default} with a safe default; the name - // is then checked with each variable replaced, so no shell syntax can remain. - $withoutVariables = preg_replace_callback( - '/\$\{([A-Za-z_][A-Za-z0-9_]*)(?::?-([^}]*))?\}|\$[A-Za-z_][A-Za-z0-9_]*/', - function (array $matches): string { - $default = $matches[2] ?? ''; - if ($default !== '' && preg_match('/\A[A-Za-z0-9_.-]+\z/', $default) !== 1) { - throw new Exception('Invalid Docker Compose network name field. Variable defaults may contain only alphanumeric characters, dots, hyphens, and underscores.'); - } - - return 'x'; - }, - $name, - ); - - validateComposeNetworkName($withoutVariables ?? $name, 'network name field'); + if (! $isValid) { + throw invalidComposeNetworkNameException($name, $context, $serviceName, allowsVariables: true); + } } /** @@ -300,16 +305,23 @@ function validateComposeNetworkNameField(string $name): void * * @throws Exception If the network name is not a valid Docker network identifier */ -function validateComposeNetworkName(string $networkName, string $context = 'network name'): void +function validateComposeNetworkName(string $networkName, string $context = 'network name', ?string $serviceName = null): void { if ($networkName === '' || ! ValidationPatterns::isValidDockerNetwork($networkName)) { - throw new Exception( - 'Invalid Docker Compose '.$context. - '. Network names must start with an alphanumeric character and contain only alphanumeric characters, dots, hyphens, and underscores.' - ); + throw invalidComposeNetworkNameException($networkName, $context, $serviceName); } } +function invalidComposeNetworkNameException(string $name, string $context, ?string $serviceName, bool $allowsVariables = false): Exception +{ + $location = $serviceName === null ? '' : " in service {$serviceName}"; + + return new Exception( + "Invalid Docker Compose {$context} \"{$name}\"{$location}. Network names must start with an alphanumeric character and contain only alphanumeric characters, dots, hyphens, and underscores" + .($allowsVariables ? ', and can use variables such as ${NETWORK:-default}.' : '.') + ); +} + /** * Tells if a top-level Compose volume declaration is external: `external: true` or the old * `external: {name: x}` syntax. Docker Compose does not create or remove an external volume. diff --git a/database/migrations/2026_10_03_172204_add_recovery_bounds_to_scheduled_volume_backup_executions_table.php b/database/migrations/2026_10_03_172204_add_recovery_bounds_to_scheduled_volume_backup_executions_table.php index f10f334912..2a2d518019 100644 --- a/database/migrations/2026_10_03_172204_add_recovery_bounds_to_scheduled_volume_backup_executions_table.php +++ b/database/migrations/2026_10_03_172204_add_recovery_bounds_to_scheduled_volume_backup_executions_table.php @@ -12,14 +12,12 @@ return new class extends Migration public function up(): void { Schema::table('scheduled_volume_backup_executions', function (Blueprint $table) { - $table->unsignedSmallInteger('recovery_attempts')->default(0); $table->timestampTz('recovery_last_attempt_at')->nullable(); - $table->timestampTz('recovery_next_retry_at')->nullable(); $table->string('recovery_error', 32)->nullable(); $table->boolean('recovery_needs_attention')->default(false); $table->index( - ['stop_recovery_pending', 's3_cleanup_pending', 'recovery_needs_attention', 'recovery_next_retry_at'], + ['stop_recovery_pending', 's3_cleanup_pending', 'recovery_needs_attention'], 'scheduled_volume_executions_recovery_index', ); }); @@ -33,9 +31,7 @@ return new class extends Migration Schema::table('scheduled_volume_backup_executions', function (Blueprint $table) { $table->dropIndex('scheduled_volume_executions_recovery_index'); $table->dropColumn([ - 'recovery_attempts', 'recovery_last_attempt_at', - 'recovery_next_retry_at', 'recovery_error', 'recovery_needs_attention', ]); diff --git a/resources/views/emails/volume-backup-recovery-failed.blade.php b/resources/views/emails/volume-backup-recovery-failed.blade.php new file mode 100644 index 0000000000..a1653341d1 --- /dev/null +++ b/resources/views/emails/volume-backup-recovery-failed.blade.php @@ -0,0 +1,7 @@ + +{{ $summary }} + +@if ($url) +Click [here]({{ $url }}) to open the resource. +@endif + diff --git a/tests/Feature/ApplicationComposeInjectionValidationTest.php b/tests/Feature/ApplicationComposeInjectionValidationTest.php index aee515917f..4db2d25dad 100644 --- a/tests/Feature/ApplicationComposeInjectionValidationTest.php +++ b/tests/Feature/ApplicationComposeInjectionValidationTest.php @@ -356,3 +356,24 @@ test('the Compose deployment validates the repository file before any command us ->and($loadPosition)->toBeLessThan(strpos($body, 'base64 -d')) ->and($body)->toContain('"stat -c \'%F\' ".escapeshellarg($realPathInGit)'); }); + +test('loading a Compose file with a variable service network saves it', function () { + $compose = "services:\n web:\n image: nginx\n networks:\n - \${NET:-proxy}\nnetworks:\n proxy:\n external: true\n"; + fakeRepositoryCompose($compose); + + $this->application->loadComposeFile(); + + expect($this->application->fresh()->docker_compose_raw)->toBe(trim($compose)); +}); + +test('the deployment log names the service and network that failed validation', function () { + fakeRepositoryCompose("services:\n web:\n image: nginx\n networks:\n - '\$(id)'\n"); + $logEntries = []; + $job = composeDeploymentJob($this->application, 0, $logEntries); + + expect(fn () => (new ReflectionMethod(ApplicationDeploymentJob::class, 'loadComposeFileForDeployment'))->invoke($job)) + ->toThrow(DeploymentException::class); + + expect(collect($logEntries)->pluck(0)->implode("\n")) + ->toContain('Invalid Docker Compose service network "$(id)" in service web.'); +}); diff --git a/tests/Feature/GithubRunnerFlowTest.php b/tests/Feature/GithubRunnerFlowTest.php index c452f3b7bd..37663a26a9 100644 --- a/tests/Feature/GithubRunnerFlowTest.php +++ b/tests/Feature/GithubRunnerFlowTest.php @@ -270,6 +270,154 @@ describe('workflow_job webhook', function () { ->and($execution->started_at)->not->toBeNull(); }); + it('starts a new runner for the job whose runner another job took', function () { + $stolen = runnerTestExecution($this->githubApp, [ + 'trigger_workflow_job_id' => 5001, + 'workflow_job_id' => 5001, + 'job_name' => 'build', + 'status' => GithubRunnerStatus::Idle, + 'server_id' => $this->server->id, + 'runner_name' => 'coolify-runner-a', + 'provision_attempts' => 1, + ]); + + foreach ([1, 2] as $delivery) { + sendWorkflowJobWebhook($this, $this->githubApp, 'runner-secret', 'in_progress', [ + 'id' => 6001, + 'name' => 'pr-test', + 'runner_name' => 'coolify-runner-a', + ])->assertOk(); + } + + $stolen->refresh(); + $replacement = GithubRunnerExecution::query()->where('trigger_workflow_job_id', 5001)->sole(); + expect($stolen->status)->toBe(GithubRunnerStatus::Running) + ->and($stolen->trigger_workflow_job_id)->toBe(6001) + ->and($stolen->workflow_job_id)->toBe(6001) + ->and($replacement->id)->not->toBe($stolen->id) + ->and($replacement->status)->toBe(GithubRunnerStatus::Queued) + ->and($replacement->job_name)->toBe('build') + ->and($replacement->labels)->toBe(['self-hosted', 'coolify']) + ->and($replacement->provision_attempts)->toBe(1); + Queue::assertPushed(ProvisionGithubRunnerJob::class, 1); + Queue::assertPushed(ProvisionGithubRunnerJob::class, fn ($job) => $job->executionId === $replacement->id); + + sendWorkflowJobWebhook($this, $this->githubApp, 'runner-secret', 'completed', [ + 'id' => 6001, + 'runner_name' => 'coolify-runner-a', + 'conclusion' => 'success', + ])->assertOk(); + + expect($stolen->fresh()->status)->toBe(GithubRunnerStatus::Completed) + ->and($replacement->fresh()->status)->toBe(GithubRunnerStatus::Queued); + }); + + it('starts a new runner when the completed hook of the other job arrives before its in_progress hook', function () { + $stolen = runnerTestExecution($this->githubApp, [ + 'trigger_workflow_job_id' => 5001, + 'status' => GithubRunnerStatus::Idle, + 'server_id' => $this->server->id, + 'runner_name' => 'coolify-runner-a', + 'provision_attempts' => 1, + ]); + + sendWorkflowJobWebhook($this, $this->githubApp, 'runner-secret', 'completed', [ + 'id' => 6001, + 'runner_name' => 'coolify-runner-a', + 'conclusion' => 'success', + ])->assertOk(); + + expect($stolen->fresh()->status)->toBe(GithubRunnerStatus::Completed) + ->and($stolen->fresh()->trigger_workflow_job_id)->toBe(6001) + ->and(GithubRunnerExecution::query()->where('trigger_workflow_job_id', 5001)->sole()->status)->toBe(GithubRunnerStatus::Queued); + Queue::assertPushed(ProvisionGithubRunnerJob::class, 1); + }); + + it('replaces a stale execution of the job that took the runner', function () { + $stale = runnerTestExecution($this->githubApp, ['trigger_workflow_job_id' => 6001, 'status' => GithubRunnerStatus::TimedOut]); + $stolen = runnerTestExecution($this->githubApp, [ + 'trigger_workflow_job_id' => 5001, + 'status' => GithubRunnerStatus::Idle, + 'server_id' => $this->server->id, + 'runner_name' => 'coolify-runner-a', + 'provision_attempts' => 1, + ]); + + sendWorkflowJobWebhook($this, $this->githubApp, 'runner-secret', 'in_progress', [ + 'id' => 6001, + 'runner_name' => 'coolify-runner-a', + ])->assertOk(); + + expect($stale->fresh())->toBeNull() + ->and($stolen->fresh()->trigger_workflow_job_id)->toBe(6001) + ->and(GithubRunnerExecution::query()->where('trigger_workflow_job_id', 5001)->sole()->status)->toBe(GithubRunnerStatus::Queued); + Queue::assertPushed(ProvisionGithubRunnerJob::class, 1); + }); + + it('does not start a new runner when the runner took another job with its own pending runner', function () { + $first = runnerTestExecution($this->githubApp, [ + 'trigger_workflow_job_id' => 5001, + 'status' => GithubRunnerStatus::Idle, + 'server_id' => $this->server->id, + 'runner_name' => 'coolify-runner-a', + ]); + $second = runnerTestExecution($this->githubApp, ['trigger_workflow_job_id' => 6001]); + + sendWorkflowJobWebhook($this, $this->githubApp, 'runner-secret', 'in_progress', [ + 'id' => 6001, + 'runner_name' => 'coolify-runner-a', + ])->assertOk(); + + expect(GithubRunnerExecution::count())->toBe(2) + ->and($first->fresh()->status)->toBe(GithubRunnerStatus::Running) + ->and($first->fresh()->workflow_job_id)->toBe(6001) + ->and($second->fresh()->status)->toBe(GithubRunnerStatus::Queued); + Queue::assertNotPushed(ProvisionGithubRunnerJob::class); + }); + + it('does not start a new runner for a job whose runner was taken', function (Closure $setup, array $attributes) { + $setup(); + $stolen = runnerTestExecution($this->githubApp, [ + 'trigger_workflow_job_id' => 5001, + 'status' => GithubRunnerStatus::Idle, + 'server_id' => $this->server->id, + 'runner_name' => 'coolify-runner-a', + 'provision_attempts' => 1, + ...$attributes, + ]); + + sendWorkflowJobWebhook($this, $this->githubApp, 'runner-secret', 'in_progress', [ + 'id' => 6001, + 'runner_name' => 'coolify-runner-a', + ])->assertOk(); + + expect(GithubRunnerExecution::count())->toBe(1) + ->and($stolen->fresh()->status)->toBe(GithubRunnerStatus::Running); + Queue::assertNotPushed(ProvisionGithubRunnerJob::class); + })->with([ + 'runner limit of the job reached' => [fn () => null, ['provision_attempts' => ProvisionGithubRunnerJob::MAX_ATTEMPTS]], + 'configuration disabled' => [fn () => GithubRunnerConfig::query()->update(['is_enabled' => false]), []], + 'pull request job no longer allowed' => [fn () => null, ['is_pull_request' => true]], + ]); + + it('marks the runner running without a new runner when it took the job it was started for', function () { + $execution = runnerTestExecution($this->githubApp, [ + 'trigger_workflow_job_id' => 5001, + 'status' => GithubRunnerStatus::Idle, + 'server_id' => $this->server->id, + 'runner_name' => 'coolify-runner-a', + ]); + + sendWorkflowJobWebhook($this, $this->githubApp, 'runner-secret', 'in_progress', ['runner_name' => 'coolify-runner-a'])->assertOk(); + + $execution->refresh(); + expect(GithubRunnerExecution::count())->toBe(1) + ->and($execution->status)->toBe(GithubRunnerStatus::Running) + ->and($execution->trigger_workflow_job_id)->toBe(5001) + ->and($execution->workflow_job_id)->toBe(5001); + Queue::assertNotPushed(ProvisionGithubRunnerJob::class); + }); + it('completes the runner and queues its cleanup', function () { $execution = runnerTestExecution($this->githubApp, [ 'status' => GithubRunnerStatus::Running, diff --git a/tests/Feature/KeydbDragonflyLegacyPasswordQuotingTest.php b/tests/Feature/KeydbDragonflyLegacyPasswordQuotingTest.php index 823017c0a8..703a52f306 100644 --- a/tests/Feature/KeydbDragonflyLegacyPasswordQuotingTest.php +++ b/tests/Feature/KeydbDragonflyLegacyPasswordQuotingTest.php @@ -8,6 +8,7 @@ use App\Models\IntegrationToken; use App\Models\PrivateKey; use App\Models\Project; use App\Models\Server; +use App\Models\SharedEnvironmentVariable; use App\Models\StandaloneDocker; use App\Models\Team; use App\Services\DatabaseStartCommandExecutor; @@ -51,7 +52,7 @@ beforeEach(function () { }); /** - * @return array{command: string, healthcheck: list} + * @return array{command: string, healthcheck: list, environment: list} */ function legacyPasswordQuotingStartedService(object $executor, string $uuid): array { @@ -59,7 +60,7 @@ function legacyPasswordQuotingStartedService(object $executor, string $uuid): ar if (preg_match("#^echo '([^']+)' \| base64 -d \| tee \S+/docker-compose\.yml#", $command, $matches)) { $service = Yaml::parse(base64_decode($matches[1]))['services'][$uuid]; - return ['command' => $service['command'], 'healthcheck' => $service['healthcheck']['test']]; + return ['command' => $service['command'], 'healthcheck' => $service['healthcheck']['test'], 'environment' => $service['environment']]; } } @@ -195,3 +196,71 @@ it('quotes shell control characters in the password of a database created after 'space' => 'a b', 'pipe' => 'x|y', ]); + +it('runs with the stored password and syncs a plain REDIS_PASSWORD variable to it', function (string $create, string $start, string $passwordColumn, string $commandFormat) { + $database = $create($this->environment->id, $this->destination, [$passwordColumn => 'storedpass']); + $env = $database->runtime_environment_variables()->create(['key' => 'REDIS_PASSWORD', 'value' => 'envpass']); + + $start::run($database->fresh(), new Activity); + + $service = legacyPasswordQuotingStartedService($this->executor, $database->uuid); + expect($service['command'])->toBe(sprintf($commandFormat, "'storedpass'")) + ->and($service['healthcheck'][3])->toBe('storedpass') + ->and($service['environment'])->toContain('REDIS_PASSWORD=storedpass') + ->and($env->fresh()->value)->toBe('storedpass') + ->and($database->fresh()->{$passwordColumn})->toBe('storedpass'); +})->with('legacy-password-quoting-engines'); + +it('keeps the exact v4.3.23 command of a legacy database with a different plain REDIS_PASSWORD variable', function (string $create, string $start, string $passwordColumn, string $commandFormat) { + $database = $create($this->environment->id, $this->destination, [$passwordColumn => 'stored;pass']); + DB::table($database->getTable())->where('id', $database->id)->update(['legacy_password_quoting' => true]); + $database->runtime_environment_variables()->create(['key' => 'REDIS_PASSWORD', 'value' => 'envpass']); + + $start::run($database->fresh(), new Activity); + + $service = legacyPasswordQuotingStartedService($this->executor, $database->uuid); + expect($service['command'])->toBe(sprintf($commandFormat, 'stored;pass')) + ->and($service['healthcheck'][3])->toBe('stored;pass') + ->and($database->fresh()->legacy_password_quoting)->toBeTrue(); +})->with('legacy-password-quoting-engines'); + +it('runs with a remote secret REDIS_PASSWORD without storing it', function (string $create, string $start, string $passwordColumn, string $commandFormat) { + Http::fake(['https://api.doppler.com/*' => Http::response(['REDIS_PASSWORD' => 'secretpass'])]); + $token = IntegrationToken::query()->create([ + 'team_id' => $this->environment->project->team_id, + 'provider' => 'doppler', + 'name' => 'Doppler', + 'token' => 'the-secret-token', + 'capabilities' => ['secrets'], + ]); + $database = $create($this->environment->id, $this->destination, [$passwordColumn => 'storedpass']); + $database->secretManagerLink()->create(['integration_token_id' => $token->id]); + $env = $database->runtime_environment_variables()->create(['key' => 'REDIS_PASSWORD', 'value' => '{{vault.REDIS_PASSWORD}}']); + + $start::run($database->fresh(), new Activity); + + $service = legacyPasswordQuotingStartedService($this->executor, $database->uuid); + expect($service['command'])->toBe(sprintf($commandFormat, "'secretpass'")) + ->and($service['healthcheck'][3])->toBe('secretpass') + ->and($env->fresh()->value)->toBe('{{vault.REDIS_PASSWORD}}') + ->and($database->fresh()->{$passwordColumn})->toBe('storedpass'); +})->with('legacy-password-quoting-engines'); + +it('copies a shared REDIS_PASSWORD variable into the stored password', function (string $create, string $start, string $passwordColumn, string $commandFormat) { + SharedEnvironmentVariable::query()->create([ + 'key' => 'REDIS_PASSWORD', + 'value' => 'sharedpass', + 'type' => 'team', + 'team_id' => $this->environment->project->team_id, + ]); + $database = $create($this->environment->id, $this->destination, [$passwordColumn => 'storedpass']); + $env = $database->runtime_environment_variables()->create(['key' => 'REDIS_PASSWORD', 'value' => '{{team.REDIS_PASSWORD}}']); + + $start::run($database->fresh(), new Activity); + + $service = legacyPasswordQuotingStartedService($this->executor, $database->uuid); + expect($service['command'])->toBe(sprintf($commandFormat, "'sharedpass'")) + ->and($service['healthcheck'][3])->toBe('sharedpass') + ->and($env->fresh()->value)->toBe('{{team.REDIS_PASSWORD}}') + ->and($database->fresh()->{$passwordColumn})->toBe('sharedpass'); +})->with('legacy-password-quoting-engines'); diff --git a/tests/Feature/MultiServerApplicationTest.php b/tests/Feature/MultiServerApplicationTest.php index 4e7214ffca..2f586e07b8 100644 --- a/tests/Feature/MultiServerApplicationTest.php +++ b/tests/Feature/MultiServerApplicationTest.php @@ -112,6 +112,13 @@ describe('registry image guard', function () { callMultiServerDeploymentJob($job, 'ensureRegistryImageForMultipleServers'); })->throwsNoExceptions(); + test('allows docker compose applications, which build on each server', function () { + $this->application->update(['build_pack' => 'dockercompose']); + $job = makeMultiServerDeploymentJob(['application' => $this->application->fresh()]); + + callMultiServerDeploymentJob($job, 'ensureRegistryImageForMultipleServers'); + })->throwsNoExceptions(); + test('allows preview deployments, which run only on the main server', function () { $job = makeMultiServerDeploymentJob(['application' => $this->application, 'pull_request_id' => 7]); diff --git a/tests/Feature/StorageHostPathTest.php b/tests/Feature/StorageHostPathTest.php index 166cd07563..c22cce8ca1 100644 --- a/tests/Feature/StorageHostPathTest.php +++ b/tests/Feature/StorageHostPathTest.php @@ -1,5 +1,6 @@ $abilities + */ +function storageHostPathToken(User $user, array $abilities = ['*']): string { // A token request must not use the session user of the Livewire tests. auth()->forgetGuards(); @@ -78,7 +82,7 @@ function storageHostPathToken(User $user): string $token = $user->tokens()->create([ 'name' => 'host-path-token', 'token' => hash('sha256', $plainTextToken), - 'abilities' => ['*'], + 'abilities' => $abilities, 'team_id' => test()->team->id, ]); @@ -363,3 +367,85 @@ test('relative host paths stay inside the resource directory', function () { Process::assertRan(fn ($process) => str_contains($process->command, 'readlink -f') && str_contains($process->command, "'{$workdir}' '{$workdir}/data/zz'")); }); + +test('a token without deploy cannot create a mount outside the resource directory', function (string $resourceType, array $mount) { + [$url, $payload] = storageHostPathApiTarget($resourceType); + + $this->withHeaders(['Authorization' => 'Bearer '.storageHostPathToken($this->admin, ['read', 'write'])]) + ->postJson($url, [...$payload, 'type' => 'file', ...$mount]) + ->assertForbidden() + ->assertJsonPath('message', 'Missing required permissions: deploy. A mount outside the resource directory needs a token with the deploy permission.'); + + expect(LocalFileVolume::query()->count())->toBe(0); + Bus::assertNotDispatched(ServerStorageSaveJob::class); + Process::assertNothingRan(); +})->with(['application', 'database', 'service'])->with([ + 'directory' => [['is_directory' => true, 'fs_path' => '/root/.ssh', 'mount_path' => '/data']], + 'file with content' => [['fs_path' => '/root/.ssh/authorized_keys', 'mount_path' => '/etc/keys', 'content' => 'ssh-ed25519 AAAA']], + 'empty file' => [['fs_path' => '/etc/zz/app.conf', 'mount_path' => '/etc/app.conf']], +]); + +test('a deploy or root token creates a mount outside the resource directory', function (string $resourceType, array $abilities) { + [$url, $payload, $resource] = storageHostPathApiTarget($resourceType); + $token = storageHostPathToken($this->admin, $abilities); + + $this->withHeaders(['Authorization' => 'Bearer '.$token]) + ->postJson($url, [...$payload, 'type' => 'file', 'is_directory' => true, 'fs_path' => '/srv/zz', 'mount_path' => '/data']) + ->assertCreated(); + $this->withHeaders(['Authorization' => 'Bearer '.$token]) + ->postJson($url, [...$payload, 'type' => 'file', 'fs_path' => '/etc/zz/app.conf', 'mount_path' => '/etc/app.conf', 'content' => 'listen 8080;']) + ->assertCreated(); + + expect($resource->fileStorages()->orderBy('id')->pluck('fs_path')->all())->toBe(['/srv/zz', '/etc/zz/app.conf']); +})->with(['application', 'database', 'service'])->with([ + 'deploy' => [['read', 'write', 'deploy']], + 'root' => [['root']], +]); + +test('a write token creates mounts inside the resource directory', function (string $resourceType) { + [$url, $payload, $resource] = storageHostPathApiTarget($resourceType); + $token = storageHostPathToken($this->admin, ['read', 'write']); + $workdir = $resource instanceof ServiceApplication ? $resource->service->workdir() : $resource->workdir(); + + $this->withHeaders(['Authorization' => 'Bearer '.$token]) + ->postJson($url, [...$payload, 'type' => 'file', 'is_directory' => true, 'fs_path' => 'data/zz', 'mount_path' => '/data']) + ->assertCreated(); + $this->withHeaders(['Authorization' => 'Bearer '.$token]) + ->postJson($url, [...$payload, 'type' => 'file', 'fs_path' => $workdir.'/config/app.conf', 'mount_path' => '/etc/app.conf', 'content' => 'listen 8080;']) + ->assertCreated(); + $this->withHeaders(['Authorization' => 'Bearer '.$token]) + ->postJson($url, [...$payload, 'type' => 'file', 'mount_path' => '/etc/other.conf', 'content' => 'listen 8081;']) + ->assertCreated(); + $this->withHeaders(['Authorization' => 'Bearer '.$token]) + ->postJson($url, [...$payload, 'type' => 'file', 'is_host_file' => true, 'fs_path' => '/etc/hosts', 'mount_path' => '/etc/hosts']) + ->assertCreated(); + + expect($resource->fileStorages()->count())->toBe(4); +})->with(['application', 'database', 'service']); + +test('changing the content of a mount outside the resource directory needs deploy', function (string $resourceType) { + [$url, , $resource] = storageHostPathApiTarget($resourceType); + $storage = LocalFileVolume::create([ + 'fs_path' => '/etc/zz/app.conf', + 'mount_path' => '/etc/app.conf', + 'content' => 'listen 8080;', + 'resource_id' => $resource->id, + 'resource_type' => $resource->getMorphClass(), + ]); + + $this->withHeaders(['Authorization' => 'Bearer '.storageHostPathToken($this->admin, ['read', 'write'])]) + ->patchJson($url, ['type' => 'file', 'uuid' => $storage->uuid, 'content' => 'ssh-ed25519 AAAA']) + ->assertForbidden() + ->assertJsonPath('message', 'Missing required permissions: deploy. A mount outside the resource directory needs a token with the deploy permission.'); + expect($storage->fresh()->content)->toBe('listen 8080;'); + + $this->withHeaders(['Authorization' => 'Bearer '.storageHostPathToken($this->admin, ['read', 'write'])]) + ->patchJson($url, ['type' => 'file', 'uuid' => $storage->uuid, 'mount_path' => '/etc/app2.conf', 'content' => 'listen 8080;']) + ->assertOk(); + expect($storage->fresh()->mount_path)->toBe('/etc/app2.conf'); + + $this->withHeaders(['Authorization' => 'Bearer '.storageHostPathToken($this->admin, ['read', 'write', 'deploy'])]) + ->patchJson($url, ['type' => 'file', 'uuid' => $storage->uuid, 'content' => 'listen 9090;']) + ->assertOk(); + expect($storage->fresh()->content)->toBe('listen 9090;'); +})->with(['application', 'database', 'service']); diff --git a/tests/Feature/VolumeBackupRecoveryBoundsTest.php b/tests/Feature/VolumeBackupRecoveryBoundsTest.php index f65c17ae63..ff3ed2d031 100644 --- a/tests/Feature/VolumeBackupRecoveryBoundsTest.php +++ b/tests/Feature/VolumeBackupRecoveryBoundsTest.php @@ -17,12 +17,15 @@ use App\Models\Server; use App\Models\StandaloneDocker; use App\Models\Team; use App\Models\User; +use App\Notifications\Channels\DiscordChannel; +use App\Notifications\VolumeBackup\RecoveryFailed; use Aws\Command; use Aws\S3\Exception\S3Exception; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\Notification; use Illuminate\Support\Facades\Process; use Illuminate\Support\Facades\Queue; use Illuminate\Support\Facades\Storage; @@ -33,6 +36,7 @@ uses(RefreshDatabase::class); beforeEach(function () { Server::flushIdentityMap(); + Notification::fake(); config(['broadcasting.default' => 'null']); InstanceSettings::unguarded(fn () => InstanceSettings::firstOrCreate(['id' => 0])); }); @@ -130,9 +134,22 @@ function makeRecoveryBoundsServerFunctional(Server $server): void ]); } -it('doubles the recovery backoff and caps it at six hours', function () { - expect(collect(range(1, 9))->map(fn (int $attempts) => VolumeBackupRecoveryJob::backoffMinutes($attempts))->all()) - ->toBe([5, 10, 20, 40, 80, 160, 320, 360, 360]); +it('notifies the team once when recovery needs attention', function () { + $team = Team::factory()->create(); + $team->discordNotificationSettings()->update(['discord_enabled' => true, 'backup_failure_discord_notifications' => true]); + $team->webhookNotificationSettings()->update(['webhook_enabled' => true, 'backup_failure_webhook_notifications' => false]); + [, , , $backup] = createRecoveryBoundsBackup($team); + $execution = createRecoveryBoundsExecution($backup); + failRecoveryBoundsS3Delete('AccessDenied'); + + VolumeBackupRecoveryJob::recoverWithBounds($execution); + VolumeBackupRecoveryJob::recoverWithBounds($execution); + + Notification::assertSentToTimes($team, RecoveryFailed::class, 1); + Notification::assertSentTo($team, RecoveryFailed::class, fn (RecoveryFailed $notification, array $channels) => $notification->execution->is($execution) + && $channels === [DiscordChannel::class] + && str_contains($notification->toDiscord()->description, 'S3 credentials were rejected') + && str_contains((string) $notification->toMail()->render(), 'S3 credentials were rejected')); }); it('records a categorized S3 cleanup failure without throwing or leaking the S3 message', function () { @@ -148,11 +165,9 @@ it('records a categorized S3 cleanup failure without throwing or leaking the S3 $execution->refresh(); expect($execution->s3_cleanup_pending)->toBeTrue() ->and($execution->s3_storage_deleted)->toBeFalse() - ->and($execution->recovery_attempts)->toBe(1) ->and($execution->recovery_error)->toBe('s3_auth') ->and($execution->recovery_last_attempt_at->equalTo(now()))->toBeTrue() - ->and($execution->recovery_next_retry_at->equalTo(now()->addMinutes(5)))->toBeTrue() - ->and($execution->recovery_needs_attention)->toBeFalse() + ->and($execution->recovery_needs_attention)->toBeTrue() ->and($execution->message)->toBe('Worker timed out'); $logger->shouldHaveReceived('warning')->once()->with( 'Volume backup recovery failed', @@ -162,38 +177,37 @@ it('records a categorized S3 cleanup failure without throwing or leaking the S3 ); }); -it('categorizes a missing S3 bucket and grows the backoff on each failure', function () { +it('categorizes a missing S3 bucket and does not try again after the first failure', function () { Carbon::setTestNow('2026-10-03 12:00:00'); [, , , $backup] = createRecoveryBoundsBackup(Team::factory()->create()); $execution = createRecoveryBoundsExecution($backup); - failRecoveryBoundsS3Delete('NoSuchBucket'); + $disk = Mockery::mock(); + $disk->shouldReceive('delete')->once()->andThrow(UnableToDeleteFile::atLocation('partial.tar.gz', '', new S3Exception( + 'The specified bucket does not exist', + new Command('DeleteObject'), + ['code' => 'NoSuchBucket'], + ))); + Storage::shouldReceive('build')->once()->andReturn($disk); VolumeBackupRecoveryJob::recoverWithBounds($execution); VolumeBackupRecoveryJob::recoverWithBounds($execution); $execution->refresh(); expect($execution->recovery_error)->toBe('s3_bucket') - ->and($execution->recovery_attempts)->toBe(2) - ->and($execution->recovery_next_retry_at->equalTo(now()->addMinutes(10)))->toBeTrue(); + ->and($execution->recovery_needs_attention)->toBeTrue(); }); -it('needs attention after the maximum attempts and stops automatic recovery', function () { +it('stops automatic recovery after the first failure that is not an unreachable server', function () { Carbon::setTestNow('2026-10-03 12:00:00'); config(['constants.coolify.self_hosted' => true]); Queue::fake(); [, , , $backup] = createRecoveryBoundsBackup(Team::factory()->create()); - $execution = createRecoveryBoundsExecution($backup, [ - 'recovery_attempts' => VolumeBackupRecoveryJob::MAX_ATTEMPTS - 1, - ]); + $execution = createRecoveryBoundsExecution($backup); failRecoveryBoundsS3Delete('AccessDenied'); - VolumeBackupRecoveryJob::recoverWithBounds($execution); VolumeBackupRecoveryJob::recoverWithBounds($execution); - $execution->refresh(); - expect($execution->recovery_needs_attention)->toBeTrue() - ->and($execution->recovery_attempts)->toBe(VolumeBackupRecoveryJob::MAX_ATTEMPTS) - ->and($execution->recovery_next_retry_at)->toBeNull(); + expect($execution->fresh()->recovery_needs_attention)->toBeTrue(); Carbon::setTestNow('2026-10-04 12:00:00'); (new ScheduledJobManager)->handle(); @@ -201,20 +215,27 @@ it('needs attention after the maximum attempts and stops automatic recovery', fu Queue::assertNotPushed(VolumeBackupRecoveryJob::class); }); -it('waits for the backoff before dispatching recovery again', function () { +it('dispatches container recovery again only when the server is functional, once per execution', function () { Carbon::setTestNow('2026-10-03 12:00:00'); config(['constants.coolify.self_hosted' => true]); + Storage::fake('ssh-mux'); Queue::fake(); - [, , , $backup] = createRecoveryBoundsBackup(Team::factory()->create()); + [, , $server, $backup] = createRecoveryBoundsBackup(Team::factory()->create()); + $server->settings()->update(['is_reachable' => false]); createRecoveryBoundsExecution($backup, [ - 'recovery_attempts' => 3, - 'recovery_next_retry_at' => now()->addMinutes(20), + 'stop_recovery_pending' => true, + 'recovery_error' => 'server_unreachable', + 'recovery_last_attempt_at' => now(), ]); + (new ScheduledJobManager)->handle(); + Carbon::setTestNow('2026-10-04 12:00:00'); (new ScheduledJobManager)->handle(); Queue::assertNotPushed(VolumeBackupRecoveryJob::class); - Carbon::setTestNow('2026-10-03 12:20:00'); + makeRecoveryBoundsServerFunctional($server); + Server::flushIdentityMap(); + (new ScheduledJobManager)->handle(); (new ScheduledJobManager)->handle(); Queue::assertPushed(VolumeBackupRecoveryJob::class, 1); }); @@ -242,16 +263,15 @@ it('cleans the S3 upload even when container recovery fails', function () { ->and($execution->s3_cleanup_pending)->toBeFalse() ->and($execution->s3_storage_deleted)->toBeTrue() ->and($execution->recovery_error)->toBe('server_unreachable') - ->and($execution->recovery_attempts)->toBe(1); + ->and($execution->recovery_needs_attention)->toBeFalse(); + Notification::assertNothingSent(); }); it('resets the recovery state after a successful attempt', function () { [, , , $backup] = createRecoveryBoundsBackup(Team::factory()->create()); $execution = createRecoveryBoundsExecution($backup, [ - 'recovery_attempts' => 4, - 'recovery_error' => 's3_auth', + 'recovery_error' => 'server_unreachable', 'recovery_last_attempt_at' => now()->subHour(), - 'recovery_next_retry_at' => now()->subMinute(), ]); $disk = Mockery::mock(); $disk->shouldReceive('delete')->once()->andReturnTrue(); @@ -262,9 +282,9 @@ it('resets the recovery state after a successful attempt', function () { $execution->refresh(); expect($execution->s3_cleanup_pending)->toBeFalse() ->and($execution->s3_storage_deleted)->toBeTrue() - ->and($execution->recovery_attempts)->toBe(0) ->and($execution->recovery_error)->toBeNull() - ->and($execution->recovery_next_retry_at)->toBeNull(); + ->and($execution->recovery_last_attempt_at)->toBeNull() + ->and($execution->recovery_needs_attention)->toBeFalse(); }); it('only blocks new scheduled backups while stopped containers await recovery', function (array $pending, bool $dispatched) { @@ -340,7 +360,6 @@ it('shows the recovery state and retries recovery manually', function () { signInForRecoveryBounds($this, $team); [$application, $volume, , $backup] = createRecoveryBoundsBackup($team); $execution = createRecoveryBoundsExecution($backup, [ - 'recovery_attempts' => VolumeBackupRecoveryJob::MAX_ATTEMPTS, 'recovery_error' => 's3_auth', 'recovery_needs_attention' => true, ]); @@ -354,10 +373,9 @@ it('shows the recovery state and retries recovery manually', function () { ->assertSee('Recovery pending'); $execution->refresh(); - expect($execution->recovery_attempts)->toBe(0) - ->and($execution->recovery_needs_attention)->toBeFalse() - ->and($execution->recovery_next_retry_at)->toBeNull() + expect($execution->recovery_needs_attention)->toBeFalse() ->and(Cache::has(VolumeBackupRecoveryJob::dispatchCacheKey($execution->id)))->toBeFalse(); + Queue::assertPushed(VolumeBackupRecoveryJob::class, 1); Queue::assertPushed(VolumeBackupRecoveryJob::class, fn (VolumeBackupRecoveryJob $job) => $job->execution->is($execution)); }); @@ -365,7 +383,6 @@ it('prevents another team from retrying recovery', function () { Queue::fake(); [$application, $volume, , $backup] = createRecoveryBoundsBackup(Team::factory()->create()); $execution = createRecoveryBoundsExecution($backup, [ - 'recovery_attempts' => VolumeBackupRecoveryJob::MAX_ATTEMPTS, 'recovery_needs_attention' => true, ]); $otherTeam = Team::factory()->create(); @@ -378,7 +395,6 @@ it('prevents another team from retrying recovery', function () { ->call('retryRecovery', $execution->id) ->assertDispatched('error'); - expect($execution->fresh()->recovery_needs_attention)->toBeTrue() - ->and($execution->fresh()->recovery_attempts)->toBe(VolumeBackupRecoveryJob::MAX_ATTEMPTS); + expect($execution->fresh()->recovery_needs_attention)->toBeTrue(); Queue::assertNotPushed(VolumeBackupRecoveryJob::class); }); diff --git a/tests/Feature/VolumeBackupTest.php b/tests/Feature/VolumeBackupTest.php index 55aec44db1..a4d2c2baac 100644 --- a/tests/Feature/VolumeBackupTest.php +++ b/tests/Feature/VolumeBackupTest.php @@ -2676,6 +2676,7 @@ it('dispatches pending recovery for an execution at most once every five minutes Queue::fake(); $team = Team::factory()->create(); [$application, $volume] = createVolumeBackupApplication($team); + $application->destination->server->settings()->update(['is_reachable' => true, 'is_usable' => true, 'force_disabled' => false]); $backup = $volume->scheduledBackups()->create([ 'team_id' => $team->id, 'frequency' => 'daily', diff --git a/tests/Unit/DockerComposeInjectionCompatibilityTest.php b/tests/Unit/DockerComposeInjectionCompatibilityTest.php index 21db47628e..8192a41ddd 100644 --- a/tests/Unit/DockerComposeInjectionCompatibilityTest.php +++ b/tests/Unit/DockerComposeInjectionCompatibilityTest.php @@ -229,3 +229,44 @@ test('Compose injection payloads are rejected', function (string $compose, strin 'mixed network name with an unsafe default' => ["services:\n web:\n image: nginx\nnetworks:\n edge:\n name: 'app-\${ENV:-x y}'\n", 'Invalid Docker Compose network name field'], 'network name variable with an unsafe default' => ["services:\n web:\n image: nginx\nnetworks:\n edge:\n name: '\${NET:-x\$(touch /tmp/pwned)}'\n", 'Invalid Docker Compose network name field'], ]); + +test('service network list entries accept the same variables as network names', function (string $network) { + $compose = "services:\n web:\n image: nginx\n networks:\n - '{$network}'\nnetworks:\n proxy:\n external: true\n"; + + expect(fn () => validateDockerComposeForInjection($compose))->not->toThrow(Exception::class); +})->with([ + 'variable with a default' => ['${NET:-proxy}'], + 'variable with an unset-only default' => ['${NET-proxy}'], + 'braced variable' => ['${NET}'], + 'plain variable' => ['$NET'], + 'variable mixed with text' => ['${COMPOSE_PROJECT_NAME}_default'], + 'text with a variable default' => ['app-${ENV:-prod}'], +]); + +test('unsafe service network list entries with variables are rejected', function (string $network) { + $compose = "services:\n web:\n image: nginx\n networks:\n - '{$network}'\n"; + + expect(fn () => validateDockerComposeForInjection($compose))->toThrow(Exception::class, 'Invalid Docker Compose service network'); +})->with([ + 'command substitution' => ['$(id)'], + 'backticks' => ['`id`'], + 'variable with command substitution' => ['${NET}$(id)'], + 'default with command substitution' => ['${NET:-$(id)}'], + 'default with a space' => ['${NET:-x y}'], + 'default with backticks' => ['${NET:-`id`}'], +]); + +test('a service network key with a variable is rejected because Compose does not interpolate keys', function () { + $compose = "services:\n web:\n image: nginx\n networks:\n '\${NET:-proxy}': {}\n"; + + expect(fn () => validateDockerComposeForInjection($compose))->toThrow(Exception::class, 'Invalid Docker Compose service network'); +}); + +test('network errors name the service and the invalid value', function () { + $compose = "services:\n web:\n image: nginx\n networks:\n - 'net;id'\n"; + + expect(fn () => validateDockerComposeForInjection($compose)) + ->toThrow(Exception::class, 'Invalid Docker Compose service network "net;id" in service web. Network names must start with an alphanumeric character and contain only alphanumeric characters, dots, hyphens, and underscores, and can use variables such as ${NETWORK:-default}.'); + expect(fn () => validateDockerComposeForInjection("services:\n web:\n image: nginx\nnetworks:\n 'net;id': {}\n")) + ->toThrow(Exception::class, 'Invalid Docker Compose network name "net;id".'); +});