diff --git a/app/Actions/Database/StartDatabaseImport.php b/app/Actions/Database/StartDatabaseImport.php index eb4d4c33c6..4b59fe7911 100644 --- a/app/Actions/Database/StartDatabaseImport.php +++ b/app/Actions/Database/StartDatabaseImport.php @@ -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.'); + } + } } diff --git a/app/Listeners/CleanupDatabaseImport.php b/app/Listeners/CleanupDatabaseImport.php index 958b5793ee..a3a36b8342 100644 --- a/app/Listeners/CleanupDatabaseImport.php +++ b/app/Listeners/CleanupDatabaseImport.php @@ -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)) { diff --git a/tests/Feature/StartDatabaseImportS3CredentialsTest.php b/tests/Feature/StartDatabaseImportS3CredentialsTest.php new file mode 100644 index 0000000000..356a16cc07 --- /dev/null +++ b/tests/Feature/StartDatabaseImportS3CredentialsTest.php @@ -0,0 +1,104 @@ +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); + }); +}); diff --git a/tests/Unit/DatabaseImport/CleanupDatabaseImportTest.php b/tests/Unit/DatabaseImport/CleanupDatabaseImportTest.php index f48177d53e..895440e7a0 100644 --- a/tests/Unit/DatabaseImport/CleanupDatabaseImportTest.php +++ b/tests/Unit/DatabaseImport/CleanupDatabaseImportTest.php @@ -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([