diff --git a/app/Actions/CoolifyTask/RunRemoteProcess.php b/app/Actions/CoolifyTask/RunRemoteProcess.php index 93a46d75a2..12639d9317 100644 --- a/app/Actions/CoolifyTask/RunRemoteProcess.php +++ b/app/Actions/CoolifyTask/RunRemoteProcess.php @@ -8,6 +8,7 @@ use App\Helpers\SshMultiplexingHelper; use App\Jobs\ApplicationDeploymentJob; use App\Models\Server; use App\Support\DatabaseImport\DatabaseImportCleanup; +use App\Support\RemoteProcessCommand; use Illuminate\Contracts\Process\ProcessResult; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; @@ -142,7 +143,7 @@ class RunRemoteProcess protected function getCommand(): string { $server_uuid = $this->activity->getExtraProperty('server_uuid'); - $command = $this->activity->getExtraProperty('command'); + $command = RemoteProcessCommand::read($this->activity) ?? throw new \RuntimeException('The command of this task was already removed.'); $server = Server::whereUuid($server_uuid)->firstOrFail(); return SshMultiplexingHelper::generateSshCommand($server, $command); diff --git a/app/Jobs/CoolifyTask.php b/app/Jobs/CoolifyTask.php index f25258b075..59bec472cb 100755 --- a/app/Jobs/CoolifyTask.php +++ b/app/Jobs/CoolifyTask.php @@ -5,6 +5,7 @@ namespace App\Jobs; use App\Actions\CoolifyTask\RunRemoteProcess; use App\Enums\ProcessStatus; use App\Support\DatabaseImport\DatabaseImportCleanup; +use App\Support\RemoteProcessCommand; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldBeEncrypted; use Illuminate\Contracts\Queue\ShouldQueue; @@ -54,6 +55,8 @@ class CoolifyTask implements ShouldBeEncrypted, ShouldQueue // A database import that Coolify stopped after a restart must not run again when // the queue retries this job. Its cleanup is already queued. if (DatabaseImportCleanup::stopRequested($this->activity)) { + RemoteProcessCommand::forget($this->activity); + return; } @@ -65,6 +68,10 @@ class CoolifyTask implements ShouldBeEncrypted, ShouldQueue ]); $remote_process(); + + // The task is finished. A failed run throws before this line and is removed in failed(), + // because a retry of the job must still be able to read the command. + RemoteProcessCommand::forget($this->activity); } /** @@ -84,7 +91,6 @@ class CoolifyTask implements ShouldBeEncrypted, ShouldQueue 'job' => 'CoolifyTask', 'activity_id' => $this->activity->id, 'server_uuid' => $this->activity->getExtraProperty('server_uuid'), - 'command_preview' => substr($this->activity->getExtraProperty('command') ?? '', 0, 200), 'error' => $exception?->getMessage(), 'total_attempts' => $this->attempts(), 'trace' => $exception?->getTraceAsString(), @@ -101,6 +107,9 @@ class CoolifyTask implements ShouldBeEncrypted, ShouldQueue ], fn ($value) => $value !== null)); $this->activity->save(); + // No attempt is left, so the command (which can contain secrets) is no longer needed. + RemoteProcessCommand::forget($this->activity); + // Dispatch cleanup event on failure (same as on success) if ($this->call_event_on_finish) { try { diff --git a/app/Support/RemoteProcessCommand.php b/app/Support/RemoteProcessCommand.php new file mode 100644 index 0000000000..339201e980 --- /dev/null +++ b/app/Support/RemoteProcessCommand.php @@ -0,0 +1,64 @@ + Crypt::encryptString($command), + self::ENCRYPTED_PROPERTY => true, + ]; + } + + /** + * The plain command, or null when it was already removed. + * Activities queued before encryption was added store the command as plain text. + */ + public static function read(Activity $activity): ?string + { + $command = $activity->getExtraProperty(self::PROPERTY); + if (! is_string($command)) { + return null; + } + + return $activity->getExtraProperty(self::ENCRYPTED_PROPERTY) === true + ? Crypt::decryptString($command) + : $command; + } + + /** + * Remove the command after the final attempt of the task. + * Reads the stored row first, so a status that Coolify set meanwhile is kept. + */ + public static function forget(Activity $activity): void + { + $stored = Activity::query()->whereKey($activity->getKey())->first(); + if ($stored === null) { + return; + } + + $stored->properties = $stored->properties->except([self::PROPERTY, self::ENCRYPTED_PROPERTY]); + $stored->save(); + + $activity->properties = $stored->properties; + } +} diff --git a/bootstrap/helpers/remoteProcess.php b/bootstrap/helpers/remoteProcess.php index 8e767ed484..62c9ae7d8d 100644 --- a/bootstrap/helpers/remoteProcess.php +++ b/bootstrap/helpers/remoteProcess.php @@ -9,6 +9,7 @@ use App\Models\Application; use App\Models\ApplicationDeploymentQueue; use App\Models\PrivateKey; use App\Models\Server; +use App\Support\RemoteProcessCommand; use Carbon\Carbon; use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Collection; @@ -49,7 +50,7 @@ function remote_process( $properties = [ ...$properties, 'server_uuid' => $server->uuid, - 'command' => $command_string, + ...RemoteProcessCommand::properties($command_string), 'type' => $type, 'type_uuid' => $type_uuid, 'status' => ProcessStatus::QUEUED->value, diff --git a/database/migrations/2026_09_28_093936_remove_stored_commands_from_finished_activities.php b/database/migrations/2026_09_28_093936_remove_stored_commands_from_finished_activities.php new file mode 100644 index 0000000000..cfd1008f9c --- /dev/null +++ b/database/migrations/2026_09_28_093936_remove_stored_commands_from_finished_activities.php @@ -0,0 +1,41 @@ +whereNotNull('properties->'.RemoteProcessCommand::PROPERTY) + ->where(fn ($query) => $query + ->whereNotIn('properties->status', [ProcessStatus::QUEUED->value, ProcessStatus::IN_PROGRESS->value]) + ->orWhereNull('properties->status') + ->orWhere('updated_at', '<', now()->subDay())) + ->chunkById(500, function ($activities) { + foreach ($activities as $activity) { + $activity->properties = $activity->properties->except([ + RemoteProcessCommand::PROPERTY, + RemoteProcessCommand::ENCRYPTED_PROPERTY, + ]); + $activity->saveQuietly(); + } + }); + } + + public function down(): void + { + // The removed commands cannot be restored. + } +}; diff --git a/tests/Feature/ComposeExternalVolumeTest.php b/tests/Feature/ComposeExternalVolumeTest.php index 54807046d0..9c2597bd19 100644 --- a/tests/Feature/ComposeExternalVolumeTest.php +++ b/tests/Feature/ComposeExternalVolumeTest.php @@ -21,6 +21,7 @@ use App\Models\ServiceDatabase; use App\Models\StandaloneDocker; use App\Models\Team; use App\Models\User; +use App\Support\RemoteProcessCommand; use Illuminate\Database\Eloquent\Model; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Collection; @@ -811,7 +812,7 @@ describe('warnings', function () { $uuid = $service->uuid; legacyExternalVolumeRow(legacyExternalVolumeServiceResource($service, 'web'), "{$uuid}_shared-data"); - $command = StartService::run($service->fresh())->getExtraProperty('command'); + $command = RemoteProcessCommand::read(StartService::run($service->fresh())); $echo = 'echo '.escapeshellarg('Warning: '.legacyExternalVolumeWarning("{$uuid}_shared-data")); expect($command)->toContain($echo) @@ -824,7 +825,7 @@ describe('warnings', function () { $serviceApplication = legacyExternalVolumeServiceResource($service, 'web'); legacyExternalVolumeRow($serviceApplication, "{$uuid}_shared-data"); - $command = DeployServiceApplication::run($serviceApplication->fresh())->getExtraProperty('command'); + $command = RemoteProcessCommand::read(DeployServiceApplication::run($serviceApplication->fresh())); $echo = 'echo '.escapeshellarg('Warning: '.legacyExternalVolumeWarning("{$uuid}_shared-data")); expect($command)->toContain($echo) @@ -847,7 +848,7 @@ describe('warnings', function () { it('shows no warning when a service starts with the external volume as written', function () { $service = externalVolumeService(EXTERNAL_VOLUME_SHORT_COMPOSE); - $command = StartService::run($service->fresh())->getExtraProperty('command'); + $command = RemoteProcessCommand::read(StartService::run($service->fresh())); expect($command)->not->toContain('declared as external'); }); diff --git a/tests/Feature/ContentFilesBeforeComposeUpTest.php b/tests/Feature/ContentFilesBeforeComposeUpTest.php index bd76d94531..6a76566554 100644 --- a/tests/Feature/ContentFilesBeforeComposeUpTest.php +++ b/tests/Feature/ContentFilesBeforeComposeUpTest.php @@ -16,6 +16,7 @@ use App\Models\Server; use App\Models\Service; use App\Models\StandaloneDocker; use App\Models\Team; +use App\Support\RemoteProcessCommand; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Bus; use Illuminate\Support\Facades\Process; @@ -386,7 +387,7 @@ test('starting a service writes missing content files before docker compose up', $activity = StartService::run($service->fresh()); - $command = $activity->getExtraProperty('command'); + $command = RemoteProcessCommand::read($activity); expect(contentFilesStateChecks())->toHaveCount(1) ->and($command)->toContain("echo 'Writing 1 missing configuration file.'") ->and($command)->toContain("echo 'Warning: A directory that is not empty is at {$workerConf->fs_path} on the server.") @@ -414,7 +415,7 @@ test('deploying one service application writes only its missing content files', $activity = DeployServiceApplication::run($service->applications()->where('name', 'app')->firstOrFail()); - $command = $activity->getExtraProperty('command'); + $command = RemoteProcessCommand::read($activity); expect(contentFilesStateChecks())->toHaveCount(1) ->and(contentFilesStateChecks()[0])->not->toContain(escapeshellarg($workerConf->fs_path)) ->and(strpos($command, 'Writing 1 missing configuration file.'))->toBeLessThan(strpos($command, ' up -d')); @@ -436,7 +437,7 @@ test('starting a service without content files does not run the file check', fun expect(contentFilesStateChecks())->toBe([]) ->and($activity)->toBeInstanceOf(Activity::class) - ->and($activity->getExtraProperty('command'))->not->toContain('missing configuration file'); + ->and(RemoteProcessCommand::read($activity))->not->toContain('missing configuration file'); }); test('the content file step continues when the server does not report a state', function () { diff --git a/tests/Feature/Jobs/RemoteProcessCommandStorageTest.php b/tests/Feature/Jobs/RemoteProcessCommandStorageTest.php new file mode 100644 index 0000000000..a9fde57d5e --- /dev/null +++ b/tests/Feature/Jobs/RemoteProcessCommandStorageTest.php @@ -0,0 +1,87 @@ +set('cache.default', 'array'); + config()->set('constants.ssh.mux_enabled', false); + InstanceSettings::forceCreate(['id' => 0]); + + $team = Team::factory()->create(); + $privateKey = PrivateKey::factory()->create(['team_id' => $team->id]); + $this->server = Server::factory()->create([ + 'team_id' => $team->id, + 'private_key_id' => $privateKey->id, + 'user' => 'root', + ]); + $this->secretCommand = "echo 'POSTGRES_PASSWORD=super-secret-value' > /tmp/compose.env"; +}); + +it('stores the command of a remote process encrypted', function () { + Queue::fake(); + + $activity = remote_process([$this->secretCommand], $this->server); + + $stored = Activity::query()->findOrFail($activity->id); + expect(json_encode($stored->properties))->not->toContain('super-secret-value') + ->and(RemoteProcessCommand::read($stored))->toBe($this->secretCommand); + Queue::assertPushed(CoolifyTask::class); +}); + +it('still reads a plain command that was queued before the upgrade', function () { + $activity = activity() + ->withProperties(['server_uuid' => $this->server->uuid, 'command' => 'echo legacy', 'type' => 'inline', 'status' => ProcessStatus::QUEUED->value]) + ->event('inline') + ->log('[]'); + + expect(RemoteProcessCommand::read($activity))->toBe('echo legacy'); +}); + +it('removes the command after a successful final run', function () { + Queue::fake(); + Process::fake(fn () => Process::result(output: 'ok')); + $activity = remote_process([$this->secretCommand], $this->server); + + (new CoolifyTask(Activity::query()->findOrFail($activity->id), ignore_errors: false, call_event_on_finish: null, call_event_data: null))->handle(); + + $stored = Activity::query()->findOrFail($activity->id); + expect($stored->getExtraProperty('command'))->toBeNull() + ->and($stored->getExtraProperty('command_encrypted'))->toBeNull() + ->and($stored->getExtraProperty('status'))->toBe(ProcessStatus::FINISHED->value); +}); + +it('keeps the command after one attempt so the queue can retry the task', function () { + Queue::fake(); + Process::fake(fn () => Process::result(errorOutput: 'Connection reset', exitCode: 255)); + $activity = remote_process([$this->secretCommand], $this->server); + + $run = new RunRemoteProcess(activity: Activity::query()->findOrFail($activity->id)); + expect(fn () => $run())->toThrow(RuntimeException::class); + + expect(RemoteProcessCommand::read(Activity::query()->findOrFail($activity->id)))->toBe($this->secretCommand); +}); + +it('removes the command when the task fails permanently', function () { + Queue::fake(); + $activity = remote_process([$this->secretCommand], $this->server); + + (new CoolifyTask(Activity::query()->findOrFail($activity->id), ignore_errors: false, call_event_on_finish: null, call_event_data: null)) + ->failed(new RuntimeException('SSH connection failed')); + + $stored = Activity::query()->findOrFail($activity->id); + expect($stored->getExtraProperty('command'))->toBeNull() + ->and($stored->getExtraProperty('status'))->toBe(ProcessStatus::ERROR->value); +}); diff --git a/tests/Feature/RemoveStoredActivityCommandsMigrationTest.php b/tests/Feature/RemoveStoredActivityCommandsMigrationTest.php new file mode 100644 index 0000000000..118899f711 --- /dev/null +++ b/tests/Feature/RemoveStoredActivityCommandsMigrationTest.php @@ -0,0 +1,40 @@ +withProperties(['command' => 'echo POSTGRES_PASSWORD=secret', 'type' => 'inline', 'status' => $status, 'server_uuid' => 'server']) + ->event('inline') + ->log('[]'); + + if ($updatedAt !== null) { + Activity::query()->whereKey($activity->id)->toBase()->update(['updated_at' => $updatedAt]); + } + + return $activity; +} + +it('removes stored commands from activities that no longer run', function () { + $finished = storedCommandActivity(ProcessStatus::FINISHED->value); + $failed = storedCommandActivity(ProcessStatus::ERROR->value); + $running = storedCommandActivity(ProcessStatus::IN_PROGRESS->value); + $queued = storedCommandActivity(ProcessStatus::QUEUED->value); + $stuck = storedCommandActivity(ProcessStatus::IN_PROGRESS->value, now()->subDays(2)); + + (require database_path('migrations/2026_09_28_093936_remove_stored_commands_from_finished_activities.php'))->up(); + + $command = fn (Activity $activity) => Activity::query()->findOrFail($activity->id)->getExtraProperty('command'); + expect($command($finished))->toBeNull() + ->and($command($failed))->toBeNull() + ->and($command($stuck))->toBeNull() + ->and($command($running))->toBe('echo POSTGRES_PASSWORD=secret') + ->and($command($queued))->toBe('echo POSTGRES_PASSWORD=secret') + ->and(Activity::query()->findOrFail($finished->id)->getExtraProperty('status'))->toBe(ProcessStatus::FINISHED->value); +}); diff --git a/tests/Feature/StartDatabaseImportBackupPreparationTest.php b/tests/Feature/StartDatabaseImportBackupPreparationTest.php index 4d301d0a79..4016fb6dac 100644 --- a/tests/Feature/StartDatabaseImportBackupPreparationTest.php +++ b/tests/Feature/StartDatabaseImportBackupPreparationTest.php @@ -12,6 +12,7 @@ use App\Models\StandaloneDocker; use App\Models\StandalonePostgresql; use App\Models\Team; use App\Support\DatabaseImport\DatabaseImportSource; +use App\Support\RemoteProcessCommand; use App\Support\ResourceStartActivity; use Illuminate\Filesystem\FilesystemAdapter; use Illuminate\Filesystem\FilesystemManager; @@ -72,7 +73,7 @@ function importServerBackup(object $test, string $path, string $magicHex): strin $test->team->id, ); - return (string) $activity->getExtraProperty('command'); + return (string) RemoteProcessCommand::read($activity); } test('plain and gzip server backups are copied into the database container unchanged', function (string $path, string $magicHex) { @@ -115,7 +116,7 @@ test('uploaded backups use the same preparation as server backups', function (st new DatabaseImportSource('upload'), $this->team->id, ); - $command = (string) $activity->getExtraProperty('command'); + $command = (string) RemoteProcessCommand::read($activity); expect(str_contains($command, "'backup-decompress-"))->toBe($usesHelper) ->and(str_contains($command, "docker cp '/tmp/database-import-"))->toBeTrue(); @@ -149,7 +150,7 @@ test('s3 backups are prepared in the s3 helper and streamed into the database co new DatabaseImportSource('s3', path: 'backups/restore.sql.xz', s3StorageUuid: $storage->uuid), $this->team->id, ); - $command = (string) $activity->getExtraProperty('command'); + $command = (string) RemoteProcessCommand::read($activity); expect($command) ->toContain('unxz -c') diff --git a/tests/Feature/StartDatabaseImportS3CredentialsTest.php b/tests/Feature/StartDatabaseImportS3CredentialsTest.php index 066b1476a5..8a45132d69 100644 --- a/tests/Feature/StartDatabaseImportS3CredentialsTest.php +++ b/tests/Feature/StartDatabaseImportS3CredentialsTest.php @@ -13,6 +13,7 @@ use App\Models\StandalonePostgresql; use App\Models\Team; use App\Support\DatabaseImport\DatabaseImportCleanup; use App\Support\DatabaseImport\DatabaseImportSource; +use App\Support\RemoteProcessCommand; use Illuminate\Filesystem\FilesystemAdapter; use Illuminate\Filesystem\FilesystemManager; use Illuminate\Foundation\Testing\RefreshDatabase; @@ -85,7 +86,7 @@ test('s3 import activity command does not contain storage key or secret', functi $this->team->id, ); - $command = (string) $activity->getExtraProperty('command'); + $command = (string) RemoteProcessCommand::read($activity); expect($command) ->not->toContain($accessKey) diff --git a/tests/Unit/UpdateCoolifyTest.php b/tests/Unit/UpdateCoolifyTest.php index 833fe36033..a6744268d7 100644 --- a/tests/Unit/UpdateCoolifyTest.php +++ b/tests/Unit/UpdateCoolifyTest.php @@ -6,6 +6,7 @@ use App\Models\InstanceSettings; use App\Models\Server; use App\Models\Team; use App\Models\User; +use App\Support\RemoteProcessCommand; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Http; @@ -121,7 +122,7 @@ it('passes the saved registry URL to the upgrade script command', function () { (new UpdateCoolify)->handle(); - expect(Activity::query()->latest('id')->first()?->getExtraProperty('command'))->toBe( + expect(RemoteProcessCommand::read(Activity::query()->latest('id')->first()))->toBe( "curl -fsSL https://cdn.example.com/upgrade.sh -o /data/coolify/source/upgrade.sh\n". "bash /data/coolify/source/upgrade.sh '4.0.10' '1.0.14' 'ghcr.io'" ); @@ -150,7 +151,7 @@ it('falls back to docker io for the upgrade script command when no registry is s (new UpdateCoolify)->handle(); - expect(Activity::query()->latest('id')->first()?->getExtraProperty('command'))->toBe( + expect(RemoteProcessCommand::read(Activity::query()->latest('id')->first()))->toBe( "curl -fsSL https://cdn.example.com/upgrade.sh -o /data/coolify/source/upgrade.sh\n". "bash /data/coolify/source/upgrade.sh '4.0.10' '1.0.14' 'docker.io'" );