mirror of
https://github.com/coollabsio/coolify.git
synced 2026-09-25 17:01:49 -04:00
fix(database): keep S3 import credentials out of activity commands
Pass S3 endpoint and keys as helper container env vars and run mc alias via those variables so the stored import command no longer contains access keys or secrets. Start the helper in a separate remote process, use the storage filesystem adapter for object checks, and remove a credential temp file during import cleanup when the path is safe.
This commit is contained in:
@@ -7,7 +7,6 @@ use App\Models\S3Storage;
|
||||
use App\Models\Server;
|
||||
use App\Models\ServiceDatabase;
|
||||
use App\Models\SwarmDocker;
|
||||
use App\Rules\SafeWebhookUrl;
|
||||
use App\Support\DatabaseBackupFileValidator;
|
||||
use App\Support\DatabaseImport\DatabaseImportCommandBuilder;
|
||||
use App\Support\DatabaseImport\DatabaseImportException;
|
||||
@@ -19,6 +18,7 @@ use Illuminate\Support\Facades\Storage;
|
||||
use Illuminate\Support\Str;
|
||||
use Lorisleiva\Actions\Concerns\AsAction;
|
||||
use Spatie\Activitylog\Models\Activity;
|
||||
use Throwable;
|
||||
|
||||
class StartDatabaseImport
|
||||
{
|
||||
@@ -117,17 +117,16 @@ class StartDatabaseImport
|
||||
}
|
||||
$key = ltrim((string) $source->path, '/');
|
||||
$this->assertS3Path($key);
|
||||
$disk = Storage::build(['driver' => 's3', 'region' => $storage->region, 'key' => $storage->key, 'secret' => $storage->secret, 'bucket' => $storage->bucket, 'endpoint' => $storage->endpoint, 'use_path_style_endpoint' => true, 'http' => SafeWebhookUrl::httpClientOptions($storage->endpoint)]);
|
||||
$disk = $storage->filesystem();
|
||||
if (! $disk->exists($key) || $disk->size($key) > self::MAX_BYTES) {
|
||||
throw new DatabaseImportException('The S3 backup was not found or exceeds the 10 GiB limit.');
|
||||
}
|
||||
$helper = "s3-restore-{$operation}";
|
||||
$serverPath = "/tmp/s3-restore-{$operation}";
|
||||
$this->startS3HelperWithEnv($storage, $server, $helper, $network);
|
||||
$sourceArg = escapeshellarg("s3temp/{$storage->bucket}/{$key}");
|
||||
$commandList = [
|
||||
'docker rm -f '.escapeshellarg($helper).' 2>/dev/null || true',
|
||||
'docker run -d --network '.escapeshellarg($network).' --name '.escapeshellarg($helper).' '.escapeshellarg(coolifyHelperImage().':'.getHelperVersion()).' sleep 3600',
|
||||
'docker exec '.escapeshellarg($helper).' mc alias set s3temp '.escapeshellarg($storage->endpoint).' '.escapeshellarg($storage->key).' '.escapeshellarg($storage->secret),
|
||||
'docker exec '.escapeshellarg($helper).' sh -c '.escapeshellarg('mc alias set s3temp "$S3_ENDPOINT" "$S3_ACCESS_KEY" "$S3_SECRET_KEY"'),
|
||||
'docker exec '.escapeshellarg($helper).' mc cp '.$sourceArg.' /tmp/restore',
|
||||
'docker cp '.escapeshellarg("{$helper}:/tmp/restore").' '.escapeshellarg($serverPath),
|
||||
'docker cp '.escapeshellarg($serverPath).' '.escapeshellarg("{$container}:{$containerPath}"),
|
||||
@@ -176,4 +175,25 @@ class StartDatabaseImport
|
||||
throw new DatabaseImportException('The S3 path is invalid.');
|
||||
}
|
||||
}
|
||||
|
||||
private function startS3HelperWithEnv(S3Storage $storage, Server $server, string $helper, string $network): void
|
||||
{
|
||||
$image = escapeshellarg(coolifyHelperImage().':'.getHelperVersion());
|
||||
|
||||
try {
|
||||
instant_remote_process([
|
||||
'docker rm -f '.escapeshellarg($helper).' 2>/dev/null || true',
|
||||
'docker run -d --network '.escapeshellarg($network)
|
||||
.' --name '.escapeshellarg($helper)
|
||||
.' -e S3_ENDPOINT='.escapeshellarg((string) $storage->endpoint)
|
||||
.' -e S3_ACCESS_KEY='.escapeshellarg((string) $storage->key)
|
||||
.' -e S3_SECRET_KEY='.escapeshellarg((string) $storage->secret)
|
||||
.' '.$image.' sleep 3600',
|
||||
], $server);
|
||||
} catch (Throwable) {
|
||||
instant_remote_process(['docker rm -f '.escapeshellarg($helper).' 2>/dev/null || true'], $server, throwError: false);
|
||||
|
||||
throw new DatabaseImportException('Unable to start the S3 restore helper.');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -41,6 +41,10 @@ class CleanupDatabaseImport implements ShouldQueue
|
||||
$commands[] = 'rm -f '.escapeshellarg($data['serverTmpPath']).' 2>/dev/null || true';
|
||||
}
|
||||
|
||||
if (isSafeTmpPath($data['credentialTmpPath'] ?? null)) {
|
||||
$commands[] = 'rm -f '.escapeshellarg($data['credentialTmpPath']).' 2>/dev/null || true';
|
||||
}
|
||||
|
||||
if (filled($data['container'] ?? null)) {
|
||||
foreach (['containerTmpPath', 'scriptPath'] as $key) {
|
||||
if (isSafeTmpPath($data[$key] ?? null)) {
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
<?php
|
||||
|
||||
use App\Actions\Database\StartDatabaseImport;
|
||||
use App\Jobs\CoolifyTask;
|
||||
use App\Models\Environment;
|
||||
use App\Models\InstanceSettings;
|
||||
use App\Models\PrivateKey;
|
||||
use App\Models\Project;
|
||||
use App\Models\S3Storage;
|
||||
use App\Models\Server;
|
||||
use App\Models\StandaloneDocker;
|
||||
use App\Models\StandalonePostgresql;
|
||||
use App\Models\Team;
|
||||
use App\Support\DatabaseImport\DatabaseImportSource;
|
||||
use Illuminate\Filesystem\FilesystemAdapter;
|
||||
use Illuminate\Filesystem\FilesystemManager;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Process;
|
||||
use Illuminate\Support\Facades\Queue;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
use Illuminate\Support\Str;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
beforeEach(function () {
|
||||
config()->set('cache.default', 'array');
|
||||
config()->set('constants.ssh.mux_enabled', false);
|
||||
InstanceSettings::forceCreate(['id' => 0]);
|
||||
|
||||
$this->team = Team::factory()->create();
|
||||
$this->privateKey = PrivateKey::factory()->create(['team_id' => $this->team->id]);
|
||||
$this->server = Server::factory()->create([
|
||||
'team_id' => $this->team->id,
|
||||
'private_key_id' => $this->privateKey->id,
|
||||
'user' => 'root',
|
||||
]);
|
||||
$this->destination = StandaloneDocker::firstOrCreate(
|
||||
['server_id' => $this->server->id, 'network' => 'coolify'],
|
||||
['uuid' => (string) Str::uuid(), 'name' => 'docker']
|
||||
);
|
||||
$this->project = Project::factory()->create(['team_id' => $this->team->id]);
|
||||
$this->environment = Environment::factory()->create(['project_id' => $this->project->id]);
|
||||
$this->database = StandalonePostgresql::create([
|
||||
'uuid' => (string) Str::uuid(),
|
||||
'name' => 'db',
|
||||
'postgres_user' => 'postgres',
|
||||
'postgres_password' => 'password',
|
||||
'postgres_db' => 'db',
|
||||
'image' => 'postgres:17',
|
||||
'status' => 'running',
|
||||
'environment_id' => $this->environment->id,
|
||||
'destination_id' => $this->destination->id,
|
||||
'destination_type' => $this->destination->getMorphClass(),
|
||||
]);
|
||||
});
|
||||
|
||||
test('s3 import activity command does not contain storage key or secret', function () {
|
||||
$accessKey = 'AKIA_TEST_ACCESS_KEY_LEAK';
|
||||
$secret = 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYTESTSECRET';
|
||||
$storage = S3Storage::create([
|
||||
'name' => 'Import S3',
|
||||
'region' => 'us-east-1',
|
||||
'key' => $accessKey,
|
||||
'secret' => $secret,
|
||||
'bucket' => 'test-bucket',
|
||||
'endpoint' => 'https://8.8.8.8',
|
||||
'is_usable' => true,
|
||||
'team_id' => $this->team->id,
|
||||
]);
|
||||
|
||||
$disk = Mockery::mock(FilesystemAdapter::class);
|
||||
$disk->shouldReceive('exists')->once()->with('backups/restore.sql')->andReturn(true);
|
||||
$disk->shouldReceive('size')->once()->with('backups/restore.sql')->andReturn(1024);
|
||||
$filesystem = Mockery::mock(FilesystemManager::class, [app()])->makePartial();
|
||||
$filesystem->shouldReceive('build')->once()->andReturn($disk);
|
||||
Storage::swap($filesystem);
|
||||
|
||||
Process::fake();
|
||||
Queue::fake();
|
||||
|
||||
$activity = app(StartDatabaseImport::class)->handle(
|
||||
$this->database,
|
||||
new DatabaseImportSource('s3', path: 'backups/restore.sql', s3StorageUuid: $storage->uuid),
|
||||
$this->team->id,
|
||||
);
|
||||
|
||||
$command = (string) $activity->getExtraProperty('command');
|
||||
|
||||
expect($command)
|
||||
->not->toContain($accessKey)
|
||||
->not->toContain($secret)
|
||||
->not->toContain('.env')
|
||||
->not->toContain('S3_ACCESS_KEY=')
|
||||
->not->toContain('S3_SECRET_KEY=')
|
||||
->toContain('mc alias set s3temp "$S3_ENDPOINT" "$S3_ACCESS_KEY" "$S3_SECRET_KEY"');
|
||||
|
||||
Queue::assertPushed(CoolifyTask::class, function (CoolifyTask $job) {
|
||||
$cleanup = $job->call_event_data;
|
||||
|
||||
return is_array($cleanup)
|
||||
&& ! array_key_exists('credentialTmpPath', $cleanup)
|
||||
&& filled($cleanup['containerName'] ?? null);
|
||||
});
|
||||
});
|
||||
@@ -46,9 +46,11 @@ test('builds S3, upload, and server-path cleanup commands', function () {
|
||||
expect($listener->commands(importCleanupPayload([
|
||||
'containerName' => 's3-restore-op',
|
||||
'serverTmpPath' => '/tmp/s3-restore-op',
|
||||
'credentialTmpPath' => '/tmp/s3-restore-op.env',
|
||||
])))->toBe([
|
||||
'docker rm -f '.escapeshellarg('s3-restore-op').' 2>/dev/null || true',
|
||||
'rm -f '.escapeshellarg('/tmp/s3-restore-op').' 2>/dev/null || true',
|
||||
'rm -f '.escapeshellarg('/tmp/s3-restore-op.env').' 2>/dev/null || true',
|
||||
'docker exec '.escapeshellarg('postgres-abc').' rm -f '.escapeshellarg('/tmp/restore_op').' 2>/dev/null || true',
|
||||
'docker exec '.escapeshellarg('postgres-abc').' rm -f '.escapeshellarg('/tmp/restore_op.sh').' 2>/dev/null || true',
|
||||
]);
|
||||
@@ -73,6 +75,7 @@ test('omits unsafe paths, missing container execs, and empty payloads', function
|
||||
expect($listener->commands(importCleanupPayload([
|
||||
'containerName' => 's3-restore-op',
|
||||
'serverTmpPath' => '/tmp/../etc/passwd',
|
||||
'credentialTmpPath' => '/tmp/../etc/shadow',
|
||||
'containerTmpPath' => '/etc/shadow',
|
||||
'scriptPath' => '/tmp/../../etc/shadow',
|
||||
])))->toBe([
|
||||
|
||||
Reference in New Issue
Block a user