fix(security): encrypt stored remote commands and remove them after the final attempt

remote_process() stored the full shell command as plain text in
activity_log and kept it for 60 days. Database starts store the compose file
with the database passwords, imports store S3 keys, and the Cloudflare
tunnel setup stores its token.

- Store the command encrypted. The queue worker decrypts it right before the
  SSH call and still reads plain commands of tasks queued before the upgrade.
- Remove the command when the task is finished: after a successful run, in
  failed() when no attempt is left, and for a stopped import. A failed attempt
  that the queue retries keeps it.
- Do not write a command preview into the error log.
- Migration: remove stored commands from activities that no longer run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Andras Bacsai
2026-09-28 11:41:56 +02:00
co-authored by Claude Opus 5.5
parent aa61214233
commit f540c49f68
12 changed files with 263 additions and 15 deletions
+2 -1
View File
@@ -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);
+10 -1
View File
@@ -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 {
+64
View File
@@ -0,0 +1,64 @@
<?php
namespace App\Support;
use Illuminate\Support\Facades\Crypt;
use Spatie\Activitylog\Models\Activity;
/**
* The command of a remote_process() activity.
*
* The queue worker reads the command from the activity, so it must be stored until the
* task is finished. Commands can contain secrets (database passwords, S3 keys, tunnel
* tokens), so the command is stored encrypted and removed after the final attempt.
*/
class RemoteProcessCommand
{
public const PROPERTY = 'command';
public const ENCRYPTED_PROPERTY = 'command_encrypted';
/**
* @return array{command: string, command_encrypted: true}
*/
public static function properties(string $command): array
{
return [
self::PROPERTY => 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;
}
}
+2 -1
View File
@@ -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,
@@ -0,0 +1,41 @@
<?php
use App\Enums\ProcessStatus;
use App\Support\RemoteProcessCommand;
use Illuminate\Database\Migrations\Migration;
use Spatie\Activitylog\Models\Activity;
return new class extends Migration
{
/**
* Remove the stored command from remote process activities that no longer run.
*
* Before this release the command was stored as plain text and kept for 60 days. It can
* contain secrets such as database passwords, S3 keys and tunnel tokens. Queued or running
* tasks keep their command, because the queue worker still reads it; the task removes it
* itself when it ends. Queued or running rows older than one day are stuck and are cleaned too.
*/
public function up(): void
{
Activity::query()
->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.
}
};
+4 -3
View File
@@ -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');
});
@@ -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 () {
@@ -0,0 +1,87 @@
<?php
use App\Actions\CoolifyTask\RunRemoteProcess;
use App\Enums\ProcessStatus;
use App\Jobs\CoolifyTask;
use App\Models\InstanceSettings;
use App\Models\PrivateKey;
use App\Models\Server;
use App\Models\Team;
use App\Support\RemoteProcessCommand;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\Process;
use Illuminate\Support\Facades\Queue;
use Spatie\Activitylog\Models\Activity;
uses(RefreshDatabase::class);
beforeEach(function () {
config()->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);
});
@@ -0,0 +1,40 @@
<?php
use App\Enums\ProcessStatus;
use Carbon\CarbonInterface;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Spatie\Activitylog\Models\Activity;
uses(RefreshDatabase::class);
function storedCommandActivity(string $status, ?CarbonInterface $updatedAt = null): Activity
{
$activity = activity()
->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);
});
@@ -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')
@@ -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)
+3 -2
View File
@@ -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'"
);