From 0da202b39df52880ff39204478ff9d3f32accd8b Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:02:56 +0200 Subject: [PATCH 1/5] fix: protect restores, preview imports, and analytics access --- app/Livewire/Analytics.php | 6 + app/Livewire/Dashboard/TrafficAnalytics.php | 6 + .../ServerTransfer/ServerTransferImporter.php | 44 +++-- .../DatabaseImportCommandBuilder.php | 3 + .../ServerTransferPreviewOwnershipTest.php | 154 ++++++++++++++++++ .../TrafficAnalyticsAuthorizationTest.php | 98 +++++++++++ .../DatabaseImportRestoreScriptTest.php | 34 +++- 7 files changed, 331 insertions(+), 14 deletions(-) create mode 100644 tests/Feature/ServerTransferPreviewOwnershipTest.php create mode 100644 tests/Feature/TrafficAnalyticsAuthorizationTest.php diff --git a/app/Livewire/Analytics.php b/app/Livewire/Analytics.php index 327d9d220b..08014942fc 100644 --- a/app/Livewire/Analytics.php +++ b/app/Livewire/Analytics.php @@ -329,6 +329,12 @@ class Analytics extends Component public function loadData(): void { + $this->resetData(); + + foreach ($this->servers as $server) { + $this->authorize('view', $server); + } + if ($this->servers->isEmpty()) { return; } diff --git a/app/Livewire/Dashboard/TrafficAnalytics.php b/app/Livewire/Dashboard/TrafficAnalytics.php index 047f461d32..2d3228e139 100644 --- a/app/Livewire/Dashboard/TrafficAnalytics.php +++ b/app/Livewire/Dashboard/TrafficAnalytics.php @@ -56,6 +56,12 @@ class TrafficAnalytics extends Component public function loadData(): void { + $this->reset('overview', 'latencyApproximate', 'uniquesApproximate', 'series'); + + foreach ($this->servers as $server) { + $this->authorize('view', $server); + } + if ($this->servers->isEmpty()) { return; } diff --git a/app/Services/ServerTransfer/ServerTransferImporter.php b/app/Services/ServerTransfer/ServerTransferImporter.php index 92945b8dc4..90fe5954d0 100644 --- a/app/Services/ServerTransfer/ServerTransferImporter.php +++ b/app/Services/ServerTransfer/ServerTransferImporter.php @@ -814,7 +814,7 @@ class ServerTransferImporter $this->importFileStorages(data_get($payload, 'file_storages', []), $application); $this->importScheduledTasks(data_get($payload, 'scheduled_tasks', []), $application, $teamId); $this->importTags(data_get($payload, 'tags', []), $application, $teamId); - $this->importPreviews(data_get($payload, 'previews', []), $application, $preserveUuids, $adoptMode); + $this->importPreviews(data_get($payload, 'previews', []), $application, $preserveUuids, $adoptMode, $teamId); return $application; } @@ -1451,7 +1451,7 @@ class ServerTransferImporter /** * @param list> $previews */ - private function importPreviews(array $previews, Application $application, bool $preserveUuids, bool $adoptMode): void + private function importPreviews(array $previews, Application $application, bool $preserveUuids, bool $adoptMode, int $teamId): void { foreach ($previews as $previewPayload) { $uuid = $preserveUuids && filled(data_get($previewPayload, 'uuid')) @@ -1459,6 +1459,11 @@ class ServerTransferImporter : new_public_id(); $preview = ApplicationPreview::withTrashed()->where('uuid', $uuid)->first(); + if ($preview && ! $this->previewBelongsToTeam($preview, $teamId)) { + throw ValidationException::withMessages([ + 'previews' => ['An imported preview UUID conflicts with a preview outside the target team.'], + ]); + } if (! $preview) { $pullRequestId = data_get($previewPayload, 'pull_request_id'); if ($pullRequestId !== null) { @@ -1475,17 +1480,24 @@ class ServerTransferImporter } $fqdn = data_get($previewPayload, 'fqdn'); - // FQDN is globally unique; free it from other rows (including soft-deleted leftovers). + // FQDN is globally unique; only release conflicts owned by the target team. if (filled($fqdn)) { - ApplicationPreview::withoutEvents(function () use ($fqdn, $preview) { - ApplicationPreview::withTrashed() - ->where('fqdn', $fqdn) - ->when($preview, fn ($q) => $q->where('id', '!=', $preview->id)) - ->get() - ->each(function (ApplicationPreview $conflict) { - $conflict->fqdn = null; - $conflict->saveQuietly(); - }); + $conflicts = ApplicationPreview::withTrashed() + ->where('fqdn', $fqdn) + ->when($preview, fn ($q) => $q->where('id', '!=', $preview->id)) + ->get(); + foreach ($conflicts as $conflict) { + if (! $this->previewBelongsToTeam($conflict, $teamId)) { + throw ValidationException::withMessages([ + 'previews' => ['An imported preview domain conflicts with a preview outside the target team.'], + ]); + } + } + ApplicationPreview::withoutEvents(function () use ($conflicts) { + $conflicts->each(function (ApplicationPreview $conflict) { + $conflict->fqdn = null; + $conflict->saveQuietly(); + }); }); } @@ -1530,6 +1542,14 @@ class ServerTransferImporter } } + private function previewBelongsToTeam(ApplicationPreview $preview, int $teamId): bool + { + return Application::withTrashed() + ->whereKey($preview->application_id) + ->whereRelation('environment.project', 'team_id', $teamId) + ->exists(); + } + /** * @param list> $certificates */ diff --git a/app/Support/DatabaseImport/DatabaseImportCommandBuilder.php b/app/Support/DatabaseImport/DatabaseImportCommandBuilder.php index 7e7c05ae6f..bdc37974d0 100644 --- a/app/Support/DatabaseImport/DatabaseImportCommandBuilder.php +++ b/app/Support/DatabaseImport/DatabaseImportCommandBuilder.php @@ -22,6 +22,9 @@ is_tar() { [ "$(stream | head -c 262 | tail -c 5)" = ustar ]; } is_text() { [ "$(stream | head -c 65536 | tr -d '\000' | wc -c | tr -d ' ')" = "$(stream | head -c 65536 | wc -c | tr -d ' ')" ]; } extract_tar() { work=$(mktemp -d) && trap 'rm -rf "$work"' EXIT && stream | tar -xf - -C "$work" || fail 'The tar backup cannot be extracted.'; } use_single_tar_member() { [ "$(find "$work" -type f | wc -l | tr -d ' ')" = 1 ] && backup=$(find "$work" -type f); } +if is_gzip; then + gunzip -t "$backup" || fail 'The gzip backup is corrupt or incomplete. Nothing was changed.' +fi SH; diff --git a/tests/Feature/ServerTransferPreviewOwnershipTest.php b/tests/Feature/ServerTransferPreviewOwnershipTest.php new file mode 100644 index 0000000000..03cabfa874 --- /dev/null +++ b/tests/Feature/ServerTransferPreviewOwnershipTest.php @@ -0,0 +1,154 @@ +set('app.env', 'local'); + config()->set('app.maintenance.driver', 'file'); + Server::flushIdentityMap(); + Storage::fake('ssh-keys'); + Storage::fake('ssh-mux'); + Process::fake(); + InstanceSettings::forceCreate(['id' => 0, 'is_api_enabled' => true]); + $this->mock(ServerTransferClaimer::class)->shouldReceive('claim')->andReturn([]); +}); + +/** @return array{application: Application, preview: ApplicationPreview, bundle: array} */ +function previewOwnershipFixture(Team $team, bool $trashed = false): array +{ + $key = PrivateKey::factory()->create(['team_id' => $team->id]); + $server = Server::factory()->create(['team_id' => $team->id, 'private_key_id' => $key->id]); + $destination = StandaloneDocker::where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $application = Application::factory()->create([ + 'environment_id' => $project->environments()->firstOrFail()->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]); + $preview = ApplicationPreview::create([ + 'application_id' => $application->id, + 'pull_request_id' => 42, + 'pull_request_html_url' => 'https://github.com/example/app/pull/42', + 'fqdn' => 'https://preview.example.com', + ]); + $bundle = app(ServerTransferExporter::class)->export($server); + $bundle['server']['uuid'] = new_public_id(); + $bundle['server']['ip'] = '192.0.2.99'; + if ($trashed) { + $application->delete(); + $preview->delete(); + } + + return compact('application', 'preview', 'bundle'); +} + +test('preview imports reject another team UUID and roll back all imported records', function (bool $trashed, string $role) { + $fixture = previewOwnershipFixture(Team::factory()->create(), $trashed); + $preview = $fixture['preview']; + $volume = LocalPersistentVolume::create([ + 'resource_type' => $preview->getMorphClass(), + 'resource_id' => $preview->id, + 'name' => 'victim-preview-data', + 'mount_path' => '/data', + ]); + $bundle = $fixture['bundle']; + $bundle['projects'][0]['environments'][0]['applications'][0]['previews'][0]['pull_request_id'] = 99; + $bundle['projects'][0]['environments'][0]['applications'][0]['previews'][0]['fqdn'] = 'https://imported.example.com'; + $team = Team::factory()->create(); + $user = User::factory()->create(); + $team->members()->attach($user->id, ['role' => $role]); + session(['currentTeam' => $team]); + $token = $user->createToken('preview-import', ['write']); + $token->accessToken->forceFill(['team_id' => $team->id])->save(); + $counts = [Server::count(), Application::withTrashed()->count(), PrivateKey::count(), Project::count()]; + + $this->withToken($token->plainTextToken)->postJson('/api/v1/servers/import', ['bundle' => $bundle]) + ->assertUnprocessable() + ->assertJsonValidationErrors('previews'); + + expect($preview->fresh()->application_id)->toBe($fixture['application']->id) + ->and($preview->fresh()->pull_request_id)->toBe(42) + ->and($preview->fresh()->fqdn)->toBe('https://preview.example.com') + ->and($preview->fresh()->trashed())->toBe($trashed); + expect($volume->fresh()->resource_id)->toBe($preview->id); + expect([Server::count(), Application::withTrashed()->count(), PrivateKey::count(), Project::count()])->toBe($counts); +})->with([ + 'active preview, owner' => [false, 'owner'], + 'deleted preview and application, admin' => [true, 'admin'], +]); + +test('preview imports reject another team domain without clearing it', function (bool $trashed, bool $preserveUuids) { + $fixture = previewOwnershipFixture(Team::factory()->create(), $trashed); + $bundle = $fixture['bundle']; + $bundle['projects'][0]['environments'][0]['applications'][0]['previews'][0]['uuid'] = new_public_id(); + $team = Team::factory()->create(); + $serverCount = Server::count(); + + try { + app(ServerTransferImporter::class)->import($bundle, $team->id, preserveUuids: $preserveUuids); + $this->fail('A foreign preview domain must be rejected.'); + } catch (ValidationException $exception) { + expect($exception->errors())->toHaveKey('previews'); + } + + expect($fixture['preview']->fresh()->fqdn)->toBe('https://preview.example.com') + ->and($fixture['preview']->fresh()->application_id)->toBe($fixture['application']->id) + ->and($fixture['preview']->fresh()->trashed())->toBe($trashed); + expect(Server::count())->toBe($serverCount); + $this->assertDatabaseCount('application_previews', 1); +})->with([ + 'active, preserve UUIDs' => [false, true], + 'deleted, replace UUIDs' => [true, false], +]); + +test('preview imports preserve same-team UUID handoffs and preview volumes', function (bool $trashed) { + $team = Team::factory()->create(); + $fixture = previewOwnershipFixture($team, $trashed); + $preview = $fixture['preview']; + $volume = LocalPersistentVolume::create([ + 'resource_type' => $preview->getMorphClass(), + 'resource_id' => $preview->id, + 'name' => 'same-team-preview-data', + 'mount_path' => '/data', + ]); + + $result = app(ServerTransferImporter::class)->import($fixture['bundle'], $team->id); + + $imported = Server::whereUuid($result['server_uuid'])->firstOrFail()->applications()->firstOrFail(); + expect($preview->fresh()->application_id)->toBe($imported->id) + ->and($preview->fresh()->fqdn)->toBe('https://preview.example.com') + ->and($preview->fresh()->trashed())->toBeFalse(); + expect($volume->fresh()->resource_id)->toBe($preview->id); + $this->assertDatabaseCount('application_previews', 1); +})->with(['active' => false, 'deleted preview and application' => true]); + +test('preview imports can release a same-team domain collision', function () { + $team = Team::factory()->create(); + $fixture = previewOwnershipFixture($team); + $bundle = $fixture['bundle']; + $bundle['projects'][0]['environments'][0]['applications'][0]['previews'][0]['uuid'] = new_public_id(); + + $result = app(ServerTransferImporter::class)->import($bundle, $team->id); + + $imported = Server::whereUuid($result['server_uuid'])->firstOrFail()->applications()->firstOrFail(); + expect($fixture['preview']->fresh()->fqdn)->toBeNull(); + expect($imported->previews()->firstOrFail()->fqdn)->toBe('https://preview.example.com'); +}); diff --git a/tests/Feature/TrafficAnalyticsAuthorizationTest.php b/tests/Feature/TrafficAnalyticsAuthorizationTest.php new file mode 100644 index 0000000000..55d47648bd --- /dev/null +++ b/tests/Feature/TrafficAnalyticsAuthorizationTest.php @@ -0,0 +1,98 @@ +reads++; + + return str_contains($url, '/traffic/overview') + ? json_encode(['requests' => 123, 'unique_visitors' => 12]) + : '[]'; + } +} + +beforeEach(function () { + Server::flushIdentityMap(); + InstanceSettings::forceCreate(['id' => 0]); + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'member']); + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + + $key = PrivateKey::factory()->create(['team_id' => $this->team->id]); + $this->server = Server::factory()->create(['team_id' => $this->team->id, 'private_key_id' => $key->id]); + $this->server->settings()->update(['is_traffic_analytics_enabled' => true]); + $this->client = new AuthorizationTrafficClient($this->server); + app()->bind(SentinelTrafficClient::class, fn () => $this->client); +}); + +test('removed members cannot refresh mounted traffic analytics', function (string $componentClass) { + $component = loadLazy(Livewire::test($componentClass))->assertSet('overview.requests', 123); + $reads = $this->client->reads; + + $this->team->members()->detach($this->user->id); + $this->actingAs($this->user->fresh()); + + $component->call('loadData')->assertForbidden(); + expect($this->client->reads)->toBe($reads); +})->with([[Analytics::class], [TrafficAnalytics::class]]); + +test('members can refresh mounted traffic after switching to their own team', function (string $componentClass) { + $component = loadLazy(Livewire::test($componentClass))->assertSet('overview.requests', 123); + $personalTeam = Team::factory()->create(); + $personalTeam->members()->attach($this->user->id, ['role' => 'owner']); + session(['currentTeam' => $personalTeam]); + + $component->call('loadData')->assertOk()->assertSet('overview.requests', 123); +})->with([[Analytics::class], [TrafficAnalytics::class]]); + +test('traffic analytics denies another teams mounted servers and clears previous traffic', function (string $componentClass) { + $component = loadLazy(Livewire::test($componentClass))->assertSet('overview.requests', 123)->instance(); + $otherTeam = Team::factory()->create(); + $foreignServer = Server::factory()->create(['team_id' => $otherTeam->id]); + $component->servers = collect([$this->server, $foreignServer]); + $reads = $this->client->reads; + + expect(fn () => $component->loadData())->toThrow(AuthorizationException::class); + expect($component->overview)->toBeNull() + ->and($component->series)->toBe([]) + ->and($this->client->reads)->toBe($reads); + + if ($component instanceof Analytics) { + expect($component->topApps)->toBe([]) + ->and($component->topHosts)->toBe([]) + ->and($component->topPaths)->toBe([]) + ->and($component->breakdowns)->toBe([]); + } +})->with([[Analytics::class], [TrafficAnalytics::class]]); + +test('another teams owner cannot refresh the mounted traffic analytics', function (string $componentClass) { + $component = loadLazy(Livewire::test($componentClass))->assertSet('overview.requests', 123); + $otherTeam = Team::factory()->create(); + $otherUser = User::factory()->create(); + $otherTeam->members()->attach($otherUser->id, ['role' => 'owner']); + $this->actingAs($otherUser); + session(['currentTeam' => $otherTeam]); + $reads = $this->client->reads; + + $component->call('loadData')->assertForbidden(); + expect($this->client->reads)->toBe($reads); +})->with([[Analytics::class], [TrafficAnalytics::class]]); diff --git a/tests/Unit/DatabaseImport/DatabaseImportRestoreScriptTest.php b/tests/Unit/DatabaseImport/DatabaseImportRestoreScriptTest.php index b0ceb807df..d5cb871d95 100644 --- a/tests/Unit/DatabaseImport/DatabaseImportRestoreScriptTest.php +++ b/tests/Unit/DatabaseImport/DatabaseImportRestoreScriptTest.php @@ -352,13 +352,13 @@ function restoreScriptNormalize(string $contents): array * * @return array{exit: int, stdout: string, stderr: string, databases: array} */ -function postgresReplacementScriptRun(string $target = 'app', string $failure = '', bool $gzip = false): array +function postgresReplacementScriptRun(string $target = 'app', string $failure = '', bool $gzip = false, ?string $contents = null): array { $dir = restoreScriptTempDir(); try { $backup = $dir.'/backup.sql'; - file_put_contents($backup, $gzip ? gzencode('SELECT 1;') : 'SELECT 1;'); + file_put_contents($backup, $contents ?? ($gzip ? gzencode('SELECT 1;') : 'SELECT 1;')); $script = (new DatabaseImportCommandBuilder)->buildRestoreCommand(restoreScriptResource('postgresql'), $backup, false, true); preg_match('/new=(\w+)/', $script, $new); preg_match('/old=(\w+)/', $script, $old); @@ -449,6 +449,36 @@ test('failed SQL replacement preserves every existing PostgreSQL database and re ->and($run['databases'])->toEqual(['coolify_restore_new' => 'unrelated-new', 'coolify_restore_old' => 'unrelated-old', 'app' => 'original']); })->with(['restore', 'swap']); +dataset('corrupt restore gzip', [ + 'missing footer' => fn (string $contents): string => substr(gzencode($contents), 0, -8), + 'truncated later member' => fn (string $contents): string => gzencode($contents).substr(gzencode('SELECT 2;'), 0, 10), +]); + +test('corrupt gzip SQL replacement preserves every existing PostgreSQL database', function (Closure $corrupt) { + $run = postgresReplacementScriptRun(contents: $corrupt('SELECT 1;')); + + expect($run['exit'])->toBe(1) + ->and($run['stderr'])->toContain('The gzip backup is corrupt or incomplete. Nothing was changed.') + ->and($run['databases'])->toEqual(['coolify_restore_new' => 'unrelated-new', 'coolify_restore_old' => 'unrelated-old', 'app' => 'original']); +})->with('corrupt restore gzip'); + +test('rejects corrupt gzip backups before any database client runs', function (string $engine, string $fixture, bool $dumpAll, bool $replaceExisting, Closure $corrupt) { + $run = restoreScriptRun($engine, $corrupt(restoreScriptFixture($fixture)), $dumpAll, $replaceExisting); + + expect($run['exit'])->toBe(1) + ->and($run['stderr'])->toContain('The gzip backup is corrupt or incomplete. Nothing was changed.') + ->and($run['calls'])->toBe([]) + ->and($run['leftovers'])->toBe([]); +})->with([ + 'single PostgreSQL SQL' => ['postgresql', 'pg-sql', false, false], + 'PostgreSQL archive replacement' => ['postgresql', 'pg-custom', false, true], + 'all PostgreSQL databases' => ['postgresql', 'pg-sql', true, false], + 'single MySQL database' => ['mysql', 'mysql-sql', false, false], + 'all MySQL databases' => ['mysql', 'mysql-sql', true, false], + 'all MariaDB databases' => ['mariadb', 'mysql-sql', true, false], + 'MongoDB replacement' => ['mongodb', 'mongo-archive', false, true], +])->with('corrupt restore gzip'); + test('SQL replacement refuses scratch database name collisions without changing existing data', function (string $failure) { $run = postgresReplacementScriptRun(failure: $failure); From 3987f97a8e75fe6534e8448e0430d349aef3fffd Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:04:05 +0200 Subject: [PATCH 2/5] fix(subscription): require team admin for billing actions Subscription actions ran on the session team without a role check. A user who switched the session to a team where they are only a member could cancel, refund, or change that team's subscription. - Lock the team id at mount and require admin or owner of that team - Check the locked team and the role again in every billing action - Reject actions after the session switched to another team --- app/Livewire/Subscription/Actions.php | 67 ++++++-- .../SubscriptionActionsAuthorizationTest.php | 157 ++++++++++++++++++ 2 files changed, 209 insertions(+), 15 deletions(-) create mode 100644 tests/Feature/Subscription/SubscriptionActionsAuthorizationTest.php diff --git a/app/Livewire/Subscription/Actions.php b/app/Livewire/Subscription/Actions.php index d2dc0f4fd8..dcb99a1115 100644 --- a/app/Livewire/Subscription/Actions.php +++ b/app/Livewire/Subscription/Actions.php @@ -11,11 +11,15 @@ use App\Models\Team; use Carbon\Carbon; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Hash; +use Livewire\Attributes\Locked; use Livewire\Component; use Stripe\StripeClient; class Actions extends Component { + #[Locked] + public ?int $teamId = null; + public $server_limits = 0; public int $quantity = UpdateSubscriptionQuantity::MIN_SERVER_LIMIT; @@ -46,16 +50,21 @@ class Actions extends Component public function mount(): void { + $team = currentTeam(); + abort_unless($team && auth()->user()->isAdminOfTeam($team->id), 403); + $this->teamId = $team->id; + $this->server_limits = Team::serverLimit(); $this->quantity = (int) $this->server_limits; - $this->billingInterval = currentTeam()->subscription?->billingInterval() ?? 'monthly'; + $this->billingInterval = $team->subscription?->billingInterval() ?? 'monthly'; $this->canDeleteAccount = app(DeleteUserAccount::class)->blockers(Auth::user(), removeTeamResources: true) === []; } public function loadPricePreview(int $quantity): void { + $team = $this->authorizedTeam(); $this->quantity = $quantity; - $result = (new UpdateSubscriptionQuantity)->fetchPricePreview(currentTeam(), $quantity); + $result = (new UpdateSubscriptionQuantity)->fetchPricePreview($team, $quantity); $this->pricePreview = $result['success'] ? $result['preview'] : null; } @@ -64,6 +73,8 @@ class Actions extends Component // non-destructive, reversible billing adjustment (prorated by Stripe). public function updateQuantity(string $password = ''): bool { + $team = $this->authorizedTeam(); + if ($this->quantity < UpdateSubscriptionQuantity::MIN_SERVER_LIMIT) { $this->dispatch('error', 'Minimum server limit is '.UpdateSubscriptionQuantity::MIN_SERVER_LIMIT.'.'); $this->quantity = UpdateSubscriptionQuantity::MIN_SERVER_LIMIT; @@ -76,7 +87,7 @@ class Actions extends Component } $previousServerLimit = (int) $this->server_limits; - $result = (new UpdateSubscriptionQuantity)->execute(currentTeam(), $this->quantity); + $result = (new UpdateSubscriptionQuantity)->execute($team, $this->quantity); if ($result['success']) { $this->auditSubscriptionChange('quantity_updated', [ @@ -98,23 +109,26 @@ class Actions extends Component public function loadRefundEligibility(): void { + $this->authorizedTeam(); $this->checkRefundEligibility(); $this->refundCheckLoading = false; } public function stripeCustomerPortal(): void { - $session = getStripeCustomerPortalSession(currentTeam()); + $session = getStripeCustomerPortalSession($this->authorizedTeam()); redirect($session->url); } public function refundSubscription(string $password): bool|string { + $team = $this->authorizedTeam(); + if (! shouldSkipPasswordConfirmation() && ! Hash::check($password, auth()->user()->password)) { return 'Invalid password.'; } - $result = app(RefundSubscription::class)->execute(currentTeam()); + $result = app(RefundSubscription::class)->execute($team); if ($result['success']) { $this->auditSubscriptionChange('refunded'); @@ -131,6 +145,8 @@ class Actions extends Component public function cancelImmediately(string $password, array $selectedActions = []): bool|string { + $team = $this->authorizedTeam(); + if (! shouldSkipPasswordConfirmation() && ! Hash::check($password, auth()->user()->password)) { return 'Invalid password.'; } @@ -141,7 +157,7 @@ class Actions extends Component } $refund = in_array('refundLatestPayment', $selectedActions, true); - if (! $this->cancelSubscriptionNow($refund)) { + if (! $this->cancelSubscriptionNow($team, $refund)) { $this->dispatch('error', 'Something went wrong with the '.($refund ? 'refund' : 'cancellation').'. Please contact us.'); return true; @@ -159,11 +175,13 @@ class Actions extends Component public function cancelAtPeriodEnd(string $password): bool|string { + $team = $this->authorizedTeam(); + if (! shouldSkipPasswordConfirmation() && ! Hash::check($password, auth()->user()->password)) { return 'Invalid password.'; } - $result = (new CancelSubscriptionAtPeriodEnd)->execute(currentTeam()); + $result = (new CancelSubscriptionAtPeriodEnd)->execute($team); if ($result['success']) { $this->auditSubscriptionChange('cancellation_scheduled', ['cancel_at_period_end' => true]); @@ -179,7 +197,8 @@ class Actions extends Component public function resumeSubscription(): bool { - $result = (new ResumeSubscription)->execute(currentTeam()); + $team = $this->authorizedTeam(); + $result = (new ResumeSubscription)->execute($team); if ($result['success']) { $this->auditSubscriptionChange('resumed'); @@ -193,10 +212,8 @@ class Actions extends Component return true; } - private function cancelSubscriptionNow(bool $refund): bool + private function cancelSubscriptionNow(Team $team, bool $refund): bool { - $team = currentTeam(); - if ($refund) { // Eligibility is re-validated server-side inside RefundSubscription::execute() $result = app(RefundSubscription::class)->execute($team); @@ -267,6 +284,24 @@ class Actions extends Component return true; } + /** + * Resolve the team locked at mount, rejecting the request when the session + * switched to another team or the user is no longer an admin of that team. + */ + private function authorizedTeam(): Team + { + $team = currentTeam(); + + abort_unless( + $this->teamId !== null + && $team?->id === $this->teamId + && auth()->user()->isAdminOfTeam($this->teamId), + 403 + ); + + return $team; + } + /** * Record a user-triggered subscription change for the current team. * @@ -274,7 +309,7 @@ class Actions extends Component */ private function auditSubscriptionChange(string $action, array $context = []): void { - $team = currentTeam(); + $team = $this->authorizedTeam(); auditLog("ui.subscription.{$action}", [ 'team_id' => $team?->id, @@ -287,13 +322,15 @@ class Actions extends Component private function checkRefundEligibility(): void { - if (! isCloud() || ! currentTeam()->subscription?->stripe_subscription_id) { + $team = $this->authorizedTeam(); + + if (! isCloud() || ! $team->subscription?->stripe_subscription_id) { return; } try { - $this->refundAlreadyUsed = currentTeam()->subscription?->stripe_refunded_at !== null; - $result = (new RefundSubscription)->checkEligibility(currentTeam()); + $this->refundAlreadyUsed = $team->subscription?->stripe_refunded_at !== null; + $result = (new RefundSubscription)->checkEligibility($team); $this->isRefundEligible = $result['eligible']; $this->refundDaysRemaining = $result['days_remaining']; diff --git a/tests/Feature/Subscription/SubscriptionActionsAuthorizationTest.php b/tests/Feature/Subscription/SubscriptionActionsAuthorizationTest.php new file mode 100644 index 0000000000..894a5792ab --- /dev/null +++ b/tests/Feature/Subscription/SubscriptionActionsAuthorizationTest.php @@ -0,0 +1,157 @@ +set('constants.coolify.self_hosted', false); + config()->set('subscription.provider', 'stripe'); + config()->set('subscription.stripe_api_key', 'sk_test_fake'); + + InstanceSettings::unguarded(fn () => InstanceSettings::query()->create(['id' => 0])); + + $this->originalStripeApiBase = Stripe::$apiBase; + Stripe::$apiBase = 'http://127.0.0.1:1'; + + $this->stripeResolved = false; + $this->app->bind(StripeClient::class, function () { + $this->stripeResolved = true; + + throw new RuntimeException('Stripe must not be called.'); + }); + + $this->user = User::factory()->create(); + + $this->ownTeam = Team::factory()->create(); + $this->ownTeam->members()->attach($this->user->id, ['role' => 'owner']); + + Subscription::create([ + 'team_id' => $this->ownTeam->id, + 'stripe_subscription_id' => 'sub_own_team', + 'stripe_customer_id' => 'cus_own_team', + 'stripe_invoice_paid' => true, + 'stripe_plan_id' => 'price_test_123', + 'stripe_cancel_at_period_end' => false, + 'stripe_past_due' => false, + ]); + + $this->otherTeam = Team::factory()->create(); + $this->otherTeam->members()->attach($this->user->id, ['role' => 'member']); + + $this->otherSubscription = Subscription::create([ + 'team_id' => $this->otherTeam->id, + 'stripe_subscription_id' => 'sub_other_team', + 'stripe_customer_id' => 'cus_other_team', + 'stripe_invoice_paid' => true, + 'stripe_plan_id' => 'price_test_123', + 'stripe_cancel_at_period_end' => true, + 'stripe_past_due' => false, + ]); + + $this->actingAs($this->user); +}); + +afterEach(function () { + Stripe::$apiBase = $this->originalStripeApiBase; +}); + +dataset('subscription billing actions', [ + 'updateQuantity' => [fn ($component) => $component->set('quantity', 5)->call('updateQuantity')], + 'loadPricePreview' => [fn ($component) => $component->call('loadPricePreview', 5)], + 'refundSubscription' => [fn ($component) => $component->call('refundSubscription', 'password')], + 'cancelImmediately' => [fn ($component) => $component->call('cancelImmediately', 'password')], + 'cancelImmediately with refund' => [fn ($component) => $component->call('cancelImmediately', 'password', ['refundLatestPayment'])], + 'cancelAtPeriodEnd' => [fn ($component) => $component->call('cancelAtPeriodEnd', 'password')], + 'resumeSubscription' => [fn ($component) => $component->call('resumeSubscription')], + 'loadRefundEligibility' => [fn ($component) => $component->call('loadRefundEligibility')], + 'stripeCustomerPortal' => [fn ($component) => $component->call('stripeCustomerPortal')], +]); + +function expectOtherTeamSubscriptionUntouched(): void +{ + $subscription = test()->otherSubscription->fresh(); + + expect($subscription->stripe_subscription_id)->toBe('sub_other_team') + ->and((bool) $subscription->stripe_invoice_paid)->toBeTrue() + ->and((bool) $subscription->stripe_cancel_at_period_end)->toBeTrue() + ->and($subscription->stripe_refunded_at)->toBeNull() + ->and(test()->stripeResolved)->toBeFalse(); +} + +test('a team member cannot mount the subscription actions', function () { + session(['currentTeam' => $this->otherTeam]); + + Livewire::test(Actions::class)->assertForbidden(); + + expectOtherTeamSubscriptionUntouched(); +}); + +test('a team admin can mount the subscription actions and lock the team', function (string $role) { + $this->otherTeam->members()->updateExistingPivot($this->user->id, ['role' => $role]); + $this->user->unsetRelation('teams'); + session(['currentTeam' => $this->otherTeam]); + + Livewire::test(Actions::class) + ->assertOk() + ->assertSet('teamId', $this->otherTeam->id); +})->with(['admin', 'owner']); + +test('an admin can still resume the subscription of the locked team', function () { + $this->otherTeam->members()->updateExistingPivot($this->user->id, ['role' => 'admin']); + $this->user->unsetRelation('teams'); + session(['currentTeam' => $this->otherTeam]); + + $stripe = Mockery::mock(StripeClient::class); + $stripe->subscriptions = Mockery::mock(SubscriptionService::class); + $stripe->subscriptions->shouldReceive('update')->once()->with('sub_other_team', ['cancel_at_period_end' => false]); + $this->instance(StripeClient::class, $stripe); + + Livewire::test(Actions::class) + ->call('resumeSubscription') + ->assertOk() + ->assertDispatched('success'); + + expect((bool) $this->otherSubscription->fresh()->stripe_cancel_at_period_end)->toBeFalse(); +}); + +test('a billing action is rejected after the session switches to a team where the user is a member', function (Closure $action) { + session(['currentTeam' => $this->ownTeam]); + $component = Livewire::test(Actions::class)->assertOk(); + + session(['currentTeam' => $this->otherTeam]); + + $action($component)->assertForbidden(); + + expectOtherTeamSubscriptionUntouched(); +})->with('subscription billing actions'); + +test('a billing action is rejected after the user is demoted to member of the locked team', function (Closure $action) { + $this->otherTeam->members()->updateExistingPivot($this->user->id, ['role' => 'admin']); + $this->user->unsetRelation('teams'); + session(['currentTeam' => $this->otherTeam]); + $component = Livewire::test(Actions::class)->assertOk(); + + $this->otherTeam->members()->updateExistingPivot($this->user->id, ['role' => 'member']); + $this->user->unsetRelation('teams'); + + $action($component)->assertForbidden(); + + expectOtherTeamSubscriptionUntouched(); +})->with('subscription billing actions'); + +test('the locked team id cannot be changed by the client', function () { + session(['currentTeam' => $this->ownTeam]); + + Livewire::test(Actions::class)->set('teamId', $this->otherTeam->id); +})->throws(CannotUpdateLockedPropertyException::class); From edfaa5c381ff17416e533c2a7421fafbd2210084 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:04:05 +0200 Subject: [PATCH 3/5] fix(api): enforce validation and deploy ability on database create The create endpoint checked $validator->failed(), which is always empty, so its validation rules never ran. The start step also read instant_deploy loosely, so a write-only token could send "false" and still start the database. - Use $validator->fails() so invalid input returns 422 - Read instant_deploy with $request->boolean() for the gate and the start --- .../Controllers/Api/DatabasesController.php | 4 +- .../DatabaseCreateInstantDeployApiTest.php | 107 ++++++++++++++++++ 2 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 tests/Feature/DatabaseCreateInstantDeployApiTest.php diff --git a/app/Http/Controllers/Api/DatabasesController.php b/app/Http/Controllers/Api/DatabasesController.php index aa646abe32..06870f160b 100644 --- a/app/Http/Controllers/Api/DatabasesController.php +++ b/app/Http/Controllers/Api/DatabasesController.php @@ -1960,7 +1960,7 @@ class DatabasesController extends Controller return response()->json(['message' => 'You need to provide at least one of environment_name or environment_uuid.'], 422); } $serverUuid = $request->server_uuid; - $instantDeploy = $request->instant_deploy ?? false; + $instantDeploy = $request->boolean('instant_deploy'); if ($request->is_public && ! $request->public_port) { $request->offsetSet('is_public', false); } @@ -2033,7 +2033,7 @@ class DatabasesController extends Controller 'tags' => 'array|nullable', 'tags.*' => 'string|min:2', ]); - if ($validator->failed()) { + if ($validator->fails()) { return response()->json([ 'message' => 'Validation failed.', 'errors' => $validator->errors(), diff --git a/tests/Feature/DatabaseCreateInstantDeployApiTest.php b/tests/Feature/DatabaseCreateInstantDeployApiTest.php new file mode 100644 index 0000000000..3d7846eb61 --- /dev/null +++ b/tests/Feature/DatabaseCreateInstantDeployApiTest.php @@ -0,0 +1,107 @@ + 'file']); + InstanceSettings::unguarded(fn () => InstanceSettings::firstOrCreate(['id' => 0], ['is_api_enabled' => true])); + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'owner']); + session(['currentTeam' => $this->team]); + + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $this->project = Project::factory()->create(['team_id' => $this->team->id]); + $this->environment = Environment::factory()->create(['project_id' => $this->project->id]); + + Queue::fake(); +}); + +function databaseCreateInstantDeployHeaders(User $user, int $teamId, array $abilities): array +{ + $plainTextToken = Str::random(40); + $token = $user->tokens()->create([ + 'name' => 'database-create-instant-deploy-test', + 'token' => hash('sha256', $plainTextToken), + 'abilities' => $abilities, + 'team_id' => $teamId, + ]); + + return ['Authorization' => 'Bearer '.$token->getKey().'|'.$plainTextToken]; +} + +function databaseCreateInstantDeployPayload(object $test, array $overrides = []): array +{ + return array_merge([ + 'project_uuid' => $test->project->uuid, + 'environment_uuid' => $test->environment->uuid, + 'server_uuid' => $test->server->uuid, + ], $overrides); +} + +test('write-only token with a "false" string instant_deploy does not start the database', function () { + $this->withHeaders(databaseCreateInstantDeployHeaders($this->user, $this->team->id, ['write'])) + ->postJson('/api/v1/databases/postgresql', databaseCreateInstantDeployPayload($this, ['instant_deploy' => 'false'])) + ->assertUnprocessable() + ->assertJsonValidationErrors('instant_deploy'); + + expect(StandalonePostgresql::count())->toBe(0); + StartDatabase::assertNotPushed(); +}); + +test('write-only token with instant_deploy false creates the database without starting it', function () { + $this->withHeaders(databaseCreateInstantDeployHeaders($this->user, $this->team->id, ['write'])) + ->postJson('/api/v1/databases/postgresql', databaseCreateInstantDeployPayload($this, ['instant_deploy' => false])) + ->assertCreated(); + + expect(StandalonePostgresql::count())->toBe(1); + StartDatabase::assertNotPushed(); +}); + +test('write-only token with instant_deploy true is forbidden', function () { + $this->withHeaders(databaseCreateInstantDeployHeaders($this->user, $this->team->id, ['write'])) + ->postJson('/api/v1/databases/postgresql', databaseCreateInstantDeployPayload($this, ['instant_deploy' => true])) + ->assertForbidden(); + + expect(StandalonePostgresql::count())->toBe(0); + StartDatabase::assertNotPushed(); +}); + +test('invalid types for shared create fields return 422', function (string $field, array $input) { + $this->withHeaders(databaseCreateInstantDeployHeaders($this->user, $this->team->id, ['write', 'deploy'])) + ->postJson('/api/v1/databases/postgresql', databaseCreateInstantDeployPayload($this, $input)) + ->assertUnprocessable() + ->assertJsonValidationErrors($field); + + expect(StandalonePostgresql::count())->toBe(0); + StartDatabase::assertNotPushed(); +})->with([ + 'instant_deploy' => ['instant_deploy', ['instant_deploy' => 'abc']], + 'is_public' => ['is_public', ['is_public' => 'abc', 'public_port' => 15432]], + 'limits_cpu_shares' => ['limits_cpu_shares', ['limits_cpu_shares' => 'abc']], +]); + +test('write and deploy token with instant_deploy true starts the database', function (mixed $instantDeploy) { + $this->withHeaders(databaseCreateInstantDeployHeaders($this->user, $this->team->id, ['write', 'deploy'])) + ->postJson('/api/v1/databases/postgresql', databaseCreateInstantDeployPayload($this, ['instant_deploy' => $instantDeploy])) + ->assertCreated(); + + expect(StandalonePostgresql::count())->toBe(1); + StartDatabase::assertPushed(1); +})->with([ + 'boolean true' => [true], + 'string "1"' => ['1'], +]); From 3de74145b76f0384cb861e1a02f9fe60fb38e7d5 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:04:05 +0200 Subject: [PATCH 4/5] fix(previews): match preview containers by exact pull request id The container lookup matched the pullRequestId label as a substring, so PR 1 also matched PR 12, 13, and 100. Deploying, closing, or deleting one preview could stop and remove the containers of other previews. --- bootstrap/helpers/docker.php | 2 +- .../PreviewContainerStatusMatchTest.php | 77 +++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 tests/Feature/PreviewContainerStatusMatchTest.php diff --git a/bootstrap/helpers/docker.php b/bootstrap/helpers/docker.php index b44714a8b0..143118c584 100644 --- a/bootstrap/helpers/docker.php +++ b/bootstrap/helpers/docker.php @@ -304,7 +304,7 @@ function getCurrentApplicationContainerStatus(Server $server, Application $appli if ($includePullrequests) { return $container; } - if ($pullRequestId !== null && $pullRequestId !== 0 && str($labels)->contains("coolify.pullRequestId={$pullRequestId}")) { + if ($pullRequestId !== null && $pullRequestId !== 0 && (int) $prLabelValue === $pullRequestId) { return $container; } diff --git a/tests/Feature/PreviewContainerStatusMatchTest.php b/tests/Feature/PreviewContainerStatusMatchTest.php new file mode 100644 index 0000000000..a154314aa5 --- /dev/null +++ b/tests/Feature/PreviewContainerStatusMatchTest.php @@ -0,0 +1,77 @@ + 0, 'fqdn' => 'https://coolify.test']); + + $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]); + $this->server->settings()->update(['is_reachable' => true, 'is_usable' => true]); + $destination = StandaloneDocker::where('server_id', $this->server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = $project->environments()->first() + ?? Environment::factory()->create(['project_id' => $project->id]); + $this->application = Application::factory()->create([ + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]); + + $uuid = $this->application->uuid; + $container = fn (string $name, ?int $pullRequestId) => json_encode([ + 'ID' => $name, + 'Names' => $name, + 'State' => 'running', + 'Labels' => $pullRequestId === null + ? "coolify.applicationUuid={$uuid},coolify.managed=true" + : "coolify.applicationUuid={$uuid},coolify.pullRequestId={$pullRequestId},coolify.managed=true", + ]); + $output = implode(PHP_EOL, [ + $container("{$uuid}-base", 0), + $container("{$uuid}-legacy", null), + $container("{$uuid}-pr-1", 1), + $container("{$uuid}-pr-12", 12), + $container("{$uuid}-pr-13", 13), + $container("{$uuid}-pr-100", 100), + ]); + + Process::fake(fn () => Process::result(output: $output)); +}); + +function previewContainerStatusNames(Server $server, Application $application, ?int $pullRequestId, bool $includePullrequests = false): array +{ + return getCurrentApplicationContainerStatus($server->fresh(), $application, $pullRequestId, $includePullrequests) + ->pluck('Names') + ->values() + ->all(); +} + +test('a preview lookup returns only the containers of that exact pull request', function () { + $uuid = $this->application->uuid; + + expect(previewContainerStatusNames($this->server, $this->application, 1))->toBe(["{$uuid}-pr-1"]) + ->and(previewContainerStatusNames($this->server, $this->application, 12))->toBe(["{$uuid}-pr-12"]) + ->and(previewContainerStatusNames($this->server, $this->application, 10))->toBe([]); +}); + +test('a base lookup still returns only base containers, or every container when previews are included', function () { + $uuid = $this->application->uuid; + + expect(previewContainerStatusNames($this->server, $this->application, 0))->toBe(["{$uuid}-base", "{$uuid}-legacy"]) + ->and(previewContainerStatusNames($this->server, $this->application, null))->toBe(["{$uuid}-base", "{$uuid}-legacy"]) + ->and(previewContainerStatusNames($this->server, $this->application, 0, true))->toHaveCount(6); +}); From 23611614874699f98f5a29349b946ea2bbc9b9a1 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:04:05 +0200 Subject: [PATCH 5/5] fix(service): write large compose files through SSH stdin Service compose and .env content was sent inline in the SSH command. Linux limits one argument to 128 KiB, so a service with a generated compose over about 96 KB failed to save or start with "Argument list too long". - Add instant_remote_write_file(), which streams content to a remote tee over SSH stdin (sudo tee for non-root SSH users) - Add SshMultiplexingHelper::generateSshStdinCommand() with the same SSH options as generateSshCommand() - Write the service compose and .env with the new helper --- app/Helpers/SshMultiplexingHelper.php | 34 +++++--- app/Models/Service.php | 18 ++--- bootstrap/helpers/remoteProcess.php | 28 +++++++ .../NonRootDataDirectoryCommandsTest.php | 2 +- .../RemoteSecretResourceLifecycleTest.php | 13 +-- .../Feature/ServiceComposeStdinWriteTest.php | 80 +++++++++++++++++++ 6 files changed, 146 insertions(+), 29 deletions(-) create mode 100644 tests/Feature/ServiceComposeStdinWriteTest.php diff --git a/app/Helpers/SshMultiplexingHelper.php b/app/Helpers/SshMultiplexingHelper.php index 2d7289d43a..f29d97058e 100644 --- a/app/Helpers/SshMultiplexingHelper.php +++ b/app/Helpers/SshMultiplexingHelper.php @@ -209,6 +209,30 @@ class SshMultiplexingHelper } public static function generateSshCommand(Server $server, string $command, bool $disableMultiplexing = false, ?int $commandTimeout = null): string + { + $sshCommand = self::sshCommandPrefix($server, $disableMultiplexing, $commandTimeout); + + $delimiter = base64_encode(Hash::make($command)); + $command = str_replace($delimiter, '', $command); + $remoteShellCommand = self::remoteShellCommand(); + + return $sshCommand.self::escapedUserAtHost($server)." '{$remoteShellCommand}' << \\$delimiter".PHP_EOL + .$command.PHP_EOL + .$delimiter; + } + + /** + * Build an SSH command that runs one short remote command and leaves stdin free for data. + * Large content must go through stdin: Linux limits one exec argument to 128 KiB, and + * generateSshCommand() puts the whole script into one `sh -c` argument. + */ + public static function generateSshStdinCommand(Server $server, string $remoteCommand, bool $disableMultiplexing = false, ?int $commandTimeout = null): string + { + return self::sshCommandPrefix($server, $disableMultiplexing, $commandTimeout) + .self::escapedUserAtHost($server).' '.escapeshellarg($remoteCommand); + } + + private static function sshCommandPrefix(Server $server, bool $disableMultiplexing, ?int $commandTimeout): string { if ($server->settings->force_disabled) { throw new \RuntimeException('Server is disabled.'); @@ -239,15 +263,7 @@ class SshMultiplexingHelper $sshCommand .= "-o ProxyCommand='cloudflared access ssh --hostname %h' "; } - $sshCommand .= self::getCommonSshOptions($server, $sshKeyLocation, self::getConnectionTimeout($server), config('constants.ssh.server_interval')); - - $delimiter = base64_encode(Hash::make($command)); - $command = str_replace($delimiter, '', $command); - $remoteShellCommand = self::remoteShellCommand(); - - return $sshCommand.self::escapedUserAtHost($server)." '{$remoteShellCommand}' << \\$delimiter".PHP_EOL - .$command.PHP_EOL - .$delimiter; + return $sshCommand.self::getCommonSshOptions($server, $sshKeyLocation, self::getConnectionTimeout($server), config('constants.ssh.server_interval')); } /** diff --git a/app/Models/Service.php b/app/Models/Service.php index ad28ad8139..c5ddeb5941 100644 --- a/app/Models/Service.php +++ b/app/Models/Service.php @@ -1588,10 +1588,9 @@ class Service extends BaseModel $workdir = $this->workdir(); // Absolute paths and tee, no cd or scp: a non-root SSH user cannot enter /data/coolify on the Coolify host. - $commands = [ - "mkdir -p $workdir", - "echo '".base64_encode($this->docker_compose)."' | base64 -d | tee $workdir/docker-compose.yml > /dev/null", - ]; + // File content goes over SSH stdin: inline in the command, a compose over ~96 KB exceeds the argument limit. + instant_remote_process(["mkdir -p $workdir"], $this->server); + instant_remote_write_file($this->server, "$workdir/docker-compose.yml", $this->docker_compose); $environmentFile = "$workdir/".new_public_id().'.env.tmp'; $envs = collect([]); @@ -1622,15 +1621,8 @@ class Service extends BaseModel foreach ($sorted as $env) { $envs->push("{$env->key}={$this->resolveSecretManagerDotenvValue($env)}"); } - if ($envs->count() === 0) { - $commands[] = "touch {$environmentFile}"; - } else { - $envs_base64 = base64_encode($envs->implode("\n")); - $commands[] = "echo '$envs_base64' | base64 -d | tee {$environmentFile} > /dev/null"; - } - $commands[] = "mv {$environmentFile} $workdir/.env"; - - instant_remote_process($commands, $this->server); + instant_remote_write_file($this->server, $environmentFile, $envs->implode("\n")); + instant_remote_process(["mv {$environmentFile} $workdir/.env"], $this->server); } public function parse(bool $isNew = false): Collection diff --git a/bootstrap/helpers/remoteProcess.php b/bootstrap/helpers/remoteProcess.php index da3b6215ed..8f4eb84be6 100644 --- a/bootstrap/helpers/remoteProcess.php +++ b/bootstrap/helpers/remoteProcess.php @@ -137,6 +137,34 @@ function instant_scp_from_server(string $remoteSource, string $localDest, Server ); } +/** + * Write content to a file on a server through SSH stdin. The content never becomes part of a command + * line, so it is not limited by the 128 KiB per-argument limit of Linux. A non-root SSH user writes + * with `sudo tee` because Coolify directories such as /data/coolify are root-only. + */ +function instant_remote_write_file(Server $server, string $path, string $content, bool $throwError = true): void +{ + $remoteCommand = ($server->isNonRoot() ? 'sudo ' : '').'tee '.escapeshellarg($path).' > /dev/null'; + $timeout = (int) config('constants.ssh.command_timeout'); + + SshRetryHandler::retry( + function () use ($server, $remoteCommand, $content, $timeout) { + $sshCommand = SshMultiplexingHelper::generateSshStdinCommand($server, $remoteCommand, commandTimeout: $timeout); + $process = Process::timeout($timeout)->input($content)->run($sshCommand); + + if ($process->exitCode() !== 0) { + excludeCertainErrors($process->errorOutput(), $process->exitCode()); + } + }, + [ + 'server' => $server->ip, + 'dest' => $path, + 'function' => 'instant_remote_write_file', + ], + $throwError + ); +} + function instant_remote_process_with_timeout(Collection|array $command, Server $server, bool $throwError = true, bool $no_sudo = false): ?string { $command = $command instanceof Collection ? $command->toArray() : $command; diff --git a/tests/Feature/NonRootDataDirectoryCommandsTest.php b/tests/Feature/NonRootDataDirectoryCommandsTest.php index 2849854e29..a155b2605d 100644 --- a/tests/Feature/NonRootDataDirectoryCommandsTest.php +++ b/tests/Feature/NonRootDataDirectoryCommandsTest.php @@ -93,7 +93,7 @@ it('writes service compose files without cd or scp on a non-root server', functi $lines = nonRootDataDirectoryRemoteLines(); expectNoUnprivilegedDataDirectoryAccess($lines); expect(implode("\n", $lines)) - ->toContain("| sudo tee {$workdir}/docker-compose.yml > /dev/null") + ->toContain(escapeshellarg("sudo tee '{$workdir}/docker-compose.yml' > /dev/null")) ->toMatch('/^sudo mv '.preg_quote($workdir, '/').'\/\S+\.env\.tmp '.preg_quote($workdir, '/').'\/\.env$/m'); }); diff --git a/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php b/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php index 986bec92e9..1a8d128001 100644 --- a/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php +++ b/tests/Feature/SecretManagers/RemoteSecretResourceLifecycleTest.php @@ -104,18 +104,19 @@ function remoteSecretLifecyclePostgres(): StandalonePostgresql */ function remoteSecretLifecycleWrittenEnvFile(Service $service): string { - $commands = collect(); - Process::fake(function ($process) use ($commands) { - $commands->push($process->command); + $envFileContent = ''; + Process::fake(function ($process) use (&$envFileContent) { + // The .env content goes to `tee /.env.tmp` over SSH stdin. + if (preg_match('/tee \S+\.env\.tmp/', $process->command)) { + $envFileContent = (string) $process->input; + } return Process::result(output: ''); }); $service->saveComposeConfigs(); - preg_match("/echo '([A-Za-z0-9+\/=]+)' \| base64 -d \| tee [^ ]+\.env\.tmp/", $commands->implode("\n"), $matches); - - return base64_decode($matches[1] ?? ''); + return $envFileContent; } /* diff --git a/tests/Feature/ServiceComposeStdinWriteTest.php b/tests/Feature/ServiceComposeStdinWriteTest.php new file mode 100644 index 0000000000..2f7844c05f --- /dev/null +++ b/tests/Feature/ServiceComposeStdinWriteTest.php @@ -0,0 +1,80 @@ + 0]); + Storage::fake('ssh-keys'); + Storage::fake('ssh-mux'); + config(['app.maintenance.store' => 'array', 'constants.ssh.mux_enabled' => false, 'constants.ssh.max_retries' => 1]); + + $this->processes = collect(); + Process::fake(function (PendingProcess $process) { + $this->processes->push(['command' => $process->command, 'input' => $process->input]); + + return Process::result(); + }); + + $team = Team::factory()->create(); + $this->server = Server::factory()->create([ + 'team_id' => $team->id, + 'private_key_id' => PrivateKey::factory()->create(['team_id' => $team->id])->id, + ]); + $this->server->settings->update(['is_reachable' => true, 'is_usable' => true, 'force_disabled' => false]); + $destination = StandaloneDocker::query()->where('server_id', $this->server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + + $labels = collect(range(1, 4000)) + ->map(fn (int $i) => " - coolify.test.label{$i}=".str_repeat('x', 40)) + ->implode("\n"); + $this->compose = "services:\n app:\n image: nginx\n labels:\n{$labels}\n"; + + $this->service = Service::factory()->create([ + 'environment_id' => $environment->id, + 'server_id' => $this->server->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + 'docker_compose' => $this->compose, + ]); +}); + +it('streams a large compose file over stdin instead of the command line', function (string $user, string $tee) { + $this->server->update(['user' => $user]); + $workdir = $this->service->workdir(); + expect(strlen($this->compose))->toBeGreaterThan(200 * 1024); + + $this->service->saveComposeConfigs(); + + $composeWrite = $this->processes->firstWhere('input', $this->compose); + expect($composeWrite)->not->toBeNull() + ->and($composeWrite['command'])->toEndWith(escapeshellarg("{$tee} '{$workdir}/docker-compose.yml' > /dev/null")); + + $envWrite = $this->processes->first(fn (array $process) => $process['input'] === 'SERVICE_NAME_APP=app'); + expect($envWrite)->not->toBeNull() + ->and($envWrite['command'])->toMatch('/'.preg_quote($tee, '/').' \S+\.env\.tmp/'); + + $this->processes->each(fn (array $process) => expect(strlen($process['command']))->toBeLessThan(8 * 1024)); + expect($this->processes->last()['command'])->toMatch('/mv \S+\.env\.tmp '.preg_quote($workdir, '/').'\/\.env/'); +})->with([ + 'root' => ['root', 'tee'], + 'non-root' => ['cooluser', 'sudo tee'], +]);