From c4a7cd6d4a24006d3c9754b5aff3a07a07c2435e Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 28 Oct 2026 14:16:22 +0100 Subject: [PATCH 1/7] fix(ssh): reap orphaned ControlPersist mux masters Co-Authored-By: Claude Opus 5.5 --- .../CleanupStaleMultiplexedConnections.php | 38 ++++++++++++------- tests/Feature/SshMultiplexingLockTest.php | 33 +++++++++++++++- 2 files changed, 56 insertions(+), 15 deletions(-) diff --git a/app/Jobs/CleanupStaleMultiplexedConnections.php b/app/Jobs/CleanupStaleMultiplexedConnections.php index d52790e8c4..fb4cb35ddb 100644 --- a/app/Jobs/CleanupStaleMultiplexedConnections.php +++ b/app/Jobs/CleanupStaleMultiplexedConnections.php @@ -30,7 +30,7 @@ class CleanupStaleMultiplexedConnections implements ShouldQueue * race. Such processes are not masters, so ControlPersist never reaps them * and they hold an authenticated session until the container restarts. * - * Every `ssh -fN` for a server carries the same ControlPath, so the socket + * Every master for a server carries the same ControlPath, so the socket * file existing says nothing about which process owns it. Ownership is read * from the socket itself (`ssh -O check` reports the master pid); every * other process for that ControlPath is an orphan. When the socket file is @@ -48,18 +48,9 @@ class CleanupStaleMultiplexedConnections implements ShouldQueue $socketOwners = []; foreach ($this->listProcesses() as $process) { - // Backgrounded ssh master: current `ssh -fN` or legacy `ssh -fNM`. - if (! preg_match('#(^|/)ssh -fN#', $process['args'])) { - continue; - } - - // Only ever touch ssh processes pointing at Coolify's mux directory. - if (! preg_match('#ControlPath=('.preg_quote($muxDir, '#').'/\S+)#', $process['args'], $pathMatch)) { - continue; - } - - $muxSocket = $pathMatch[1]; - if ($process['etimes'] < $minAge + $muxSocket = $this->masterMuxSocket($process['args'], $muxDir); + if ($muxSocket === null + || $process['etimes'] < $minAge || SshMultiplexingHelper::isMuxProcessRetiring($process['pid'], $muxSocket)) { continue; } @@ -71,6 +62,24 @@ class CleanupStaleMultiplexedConnections implements ShouldQueue } } + /** + * ControlPath of a backgrounded ssh master in Coolify's mux directory, or + * null for any other process. ControlPersist retitles a master to + * `ssh: [mux]`; masters started before the upgrade still run + * as `ssh -fN` or `ssh -fNM` with the ControlPath in their arguments. + */ + private function masterMuxSocket(string $args, string $muxDir): ?string + { + $socket = preg_quote($muxDir, '#').'/\S+'; + + if (preg_match('#^ssh: ('.$socket.') \[mux\]#', $args, $matches) + || (preg_match('#(^|/)ssh -fN#', $args) && preg_match('#ControlPath=('.$socket.')#', $args, $matches))) { + return $matches[1]; + } + + return null; + } + /** * Pid of the master that owns a mux socket. Returns null when the socket * file does not exist, and false when it exists but its owner is unknown. @@ -109,7 +118,8 @@ class CleanupStaleMultiplexedConnections implements ShouldQueue $sshPids = []; foreach ($processes as $process) { // The ssh binary itself, not `cloudflared access ssh` (space before ssh). - if (preg_match('#(^|/)ssh\s#', $process['args'])) { + // A ControlPersist master is titled `ssh: [mux]`. + if (preg_match('#(^|/)ssh[\s:]#', $process['args'])) { $sshPids[$process['pid']] = true; } } diff --git a/tests/Feature/SshMultiplexingLockTest.php b/tests/Feature/SshMultiplexingLockTest.php index b71ac3c45a..32e5ad8944 100644 --- a/tests/Feature/SshMultiplexingLockTest.php +++ b/tests/Feature/SshMultiplexingLockTest.php @@ -409,6 +409,36 @@ it('kills only old orphaned ssh masters whose control socket no longer exists', File::delete($liveSocket); }); +it('kills a ControlPersist master whose control socket was removed', function () { + config(['constants.ssh.mux_orphan_reap_enabled' => true]); + $muxDir = storage_path('app/ssh/mux'); + File::ensureDirectoryExists($muxDir); + + $liveSocket = $muxDir.'/mux_live_'.uniqid(); + $removedSocket = $muxDir.'/mux_removed_'.uniqid(); + File::put($liveSocket, 'x'); + + // ControlPersist sets the title of a backgrounded master to `ssh: [mux]`. + Process::fake([ + 'ps*' => Process::result(output: "111 1 5000 ssh: {$liveSocket} [mux]\n". + "222 1 5000 ssh: {$removedSocket} [mux]\n". + "333 1 5000 ssh: /tmp/other/mux_removed [mux]\n"), + '*ssh -O check*' => Process::result(errorOutput: 'Master running (pid=111)'), + 'kill*' => Process::result(exitCode: 0), + ]); + + $job = new CleanupStaleMultiplexedConnections; + $method = new ReflectionMethod($job, 'cleanupOrphanedSshProcesses'); + $method->setAccessible(true); + $method->invoke($job); + + Process::assertRan(fn ($process) => $process->command === 'kill -9 '.escapeshellarg('222')); + Process::assertNotRan(fn ($process) => str_contains($process->command, 'kill') && str_contains($process->command, '111')); + Process::assertNotRan(fn ($process) => str_contains($process->command, 'kill') && str_contains($process->command, '333')); + + File::delete($liveSocket); +}); + it('kills ssh -fN processes that share a live ControlPath but do not own it', function () { config(['constants.ssh.mux_orphan_reap_enabled' => true]); $muxDir = storage_path('app/ssh/mux'); @@ -599,9 +629,10 @@ it('kills only old cloudflared proxies whose stdin pipe no live ssh holds', func $orphanPid = proc_get_status($orphan)['pid']; $tunnelSshPid = proc_get_status($tunnelSsh)['pid']; $sshPid = getmypid(); + $muxSocket = storage_path('app/ssh/mux/mux_tunnel'); Process::fake([ - 'ps*' => Process::result(output: "{$sshPid} 1 5000 ssh -fN -o ControlMaster=auto root@1.2.3.4\n". + 'ps*' => Process::result(output: "{$sshPid} 1 5000 ssh: {$muxSocket} [mux]\n". "200 {$sshPid} 5000 cloudflared access ssh --hostname host.example.com\n". "{$reparentedPid} 1 5000 cloudflared access ssh --hostname host.example.com\n". "{$orphanPid} 1 5000 cloudflared access ssh --hostname host.example.com\n". From 597a878d80743e6a99f021a59cd2f9d09160a238 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 28 Oct 2026 14:16:22 +0100 Subject: [PATCH 2/7] fix(hetzner): validate over SSH when the Hetzner API call fails Co-Authored-By: Claude Opus 5.5 --- app/Actions/Server/ValidateServer.php | 9 ++++++++- app/Livewire/Server/ValidateAndInstall.php | 9 ++++++++- tests/Feature/HetznerServerValidationTest.php | 17 +++++++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/app/Actions/Server/ValidateServer.php b/app/Actions/Server/ValidateServer.php index 57be1a5a60..6c0b86f284 100644 --- a/app/Actions/Server/ValidateServer.php +++ b/app/Actions/Server/ValidateServer.php @@ -3,6 +3,7 @@ namespace App\Actions\Server; use App\Models\Server; +use Illuminate\Support\Facades\Log; use Lorisleiva\Actions\Concerns\AsAction; class ValidateServer @@ -38,7 +39,13 @@ class ValidateServer 'validation_logs' => null, ]); if ($server->hetzner_server_id) { - $status = $server->refreshHetznerState(); + try { + $status = $server->refreshHetznerState(); + } catch (\Throwable $e) { + // The provider API can be down or the token revoked; the SSH checks below still decide. + Log::warning('Could not refresh the Hetzner server status before validation', ['server_id' => $server->id, 'error' => $e->getMessage()]); + $status = null; + } if (in_array($status, ['initializing', 'starting', 'stopping', 'off', 'deleting', 'rebuilding', 'migrating'], true)) { $this->error = $status === 'off' ? 'Hetzner server is off. Power it on before validating.' diff --git a/app/Livewire/Server/ValidateAndInstall.php b/app/Livewire/Server/ValidateAndInstall.php index 748c53ba2d..2da0016e00 100644 --- a/app/Livewire/Server/ValidateAndInstall.php +++ b/app/Livewire/Server/ValidateAndInstall.php @@ -8,6 +8,7 @@ use App\Events\ServerValidated; use App\Jobs\CheckAndStartSentinelJob; use App\Models\Server; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; +use Illuminate\Support\Facades\Log; use Livewire\Attributes\Locked; use Livewire\Component; @@ -117,7 +118,13 @@ class ValidateAndInstall extends Component $this->authorize('update', $this->server); Server::throttleValidation($this->server->team_id, $this->server->ip); if ($this->server->hetzner_server_id) { - $status = $this->server->refreshHetznerState(); + try { + $status = $this->server->refreshHetznerState(); + } catch (\Throwable $e) { + // The provider API can be down or the token revoked; the SSH checks below still decide. + Log::warning('Could not refresh the Hetzner server status before validation', ['server_id' => $this->server->id, 'error' => $e->getMessage()]); + $status = null; + } $this->server->refresh(); if (in_array($status, ['initializing', 'starting', 'stopping', 'off', 'deleting', 'rebuilding', 'migrating'], true)) { diff --git a/tests/Feature/HetznerServerValidationTest.php b/tests/Feature/HetznerServerValidationTest.php index e87fd380ba..0d6cd91fb4 100644 --- a/tests/Feature/HetznerServerValidationTest.php +++ b/tests/Feature/HetznerServerValidationTest.php @@ -10,6 +10,7 @@ use App\Models\Team; use App\Models\User; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Http; +use Illuminate\Support\Facades\Process; use Livewire\Livewire; uses(RefreshDatabase::class); @@ -84,3 +85,19 @@ it('blocks validation modal connection checks when the Hetzner server is off', f 'validation_logs' => 'Hetzner server is off. Power it on before validating.', ]); }); + +it('validates over SSH when the Hetzner API call fails', function () { + Http::fake(['https://api.hetzner.cloud/v1/servers/456' => Http::response(['error' => ['code' => 'unauthorized']], 401)]); + $this->server->update(['ip' => '203.0.113.10']); + Process::fake(); + + Livewire::test(ValidateAndInstall::class, ['server' => $this->server]) + ->call('validateConnection') + ->assertSet('error', null) + ->assertSet('uptime', true) + ->assertDispatched('validateOS'); + + Process::fake(['*' => Process::result(errorOutput: 'ssh: connect to host 203.0.113.10 port 22: Connection refused', exitCode: 255)]); + + expect(fn () => ValidateServer::run($this->server))->toThrow(Exception::class, 'Connection refused'); +}); From fd5efd0273677f0fe6d0ce626fcd68669e04819c Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 28 Oct 2026 14:16:22 +0100 Subject: [PATCH 3/7] fix(http): stop retrying requests that timed out after they were sent Co-Authored-By: Claude Opus 5.5 --- app/Support/RetryHttpConnectionFailures.php | 11 ++++++++++- tests/Feature/HttpConnectionRetryTest.php | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/app/Support/RetryHttpConnectionFailures.php b/app/Support/RetryHttpConnectionFailures.php index 1ab42313a0..dbb6409263 100644 --- a/app/Support/RetryHttpConnectionFailures.php +++ b/app/Support/RetryHttpConnectionFailures.php @@ -13,6 +13,8 @@ use Throwable; * Guzzle middleware that sends a request again once after a connection failure. * cURL counts the DNS lookup in the connect timeout, and a lost DNS query waits 5 seconds before the resolver tries again. * Only idempotent methods are retried, because a POST or PATCH can reach the server before the timeout. + * Guzzle also reports a timeout while waiting for the response as a connection failure; that request reached + * the server and already waited the full timeout, so it is not sent again. */ class RetryHttpConnectionFailures { @@ -22,7 +24,7 @@ class RetryHttpConnectionFailures { return function (RequestInterface $request, array $options) use ($handler): PromiseInterface { return $this->send($handler, $request, $options)->otherwise(function (Throwable $exception) use ($handler, $request, $options): PromiseInterface { - if (! $exception instanceof ConnectException || ! in_array($request->getMethod(), self::RETRIED_METHODS, true)) { + if (! $exception instanceof ConnectException || $this->timedOutAfterSending($exception) || ! in_array($request->getMethod(), self::RETRIED_METHODS, true)) { return Create::rejectionFor($exception); } @@ -33,6 +35,13 @@ class RetryHttpConnectionFailures }; } + private function timedOutAfterSending(ConnectException $exception): bool + { + $context = $exception->getHandlerContext(); + + return ($context['errno'] ?? null) === CURLE_OPERATION_TIMEDOUT && ($context['request_size'] ?? 0) > 0; + } + /** * The cURL handler rejects the promise on a connection failure, but other handlers (such as Http::fake()) throw. */ diff --git a/tests/Feature/HttpConnectionRetryTest.php b/tests/Feature/HttpConnectionRetryTest.php index 58543a1387..58dedddf64 100644 --- a/tests/Feature/HttpConnectionRetryTest.php +++ b/tests/Feature/HttpConnectionRetryTest.php @@ -38,3 +38,20 @@ test('a post request is not retried because the server can have received it', fu expect(fn () => Http::post('https://api.example.com/items'))->toThrow(ConnectionException::class) ->and($this->attempts)->toBe(1); }); + +test('a read request is not sent again after it timed out waiting for the response', function () { + $server = stream_socket_server('tcp://127.0.0.1:0'); + $address = stream_socket_get_name($server, false); + + expect(fn () => Http::timeout(1)->get("http://{$address}/slow")) + ->toThrow(ConnectionException::class, 'cURL error 28'); + + stream_set_blocking($server, false); + $connections = 0; + while (@stream_socket_accept($server, 0)) { + $connections++; + } + fclose($server); + + expect($connections)->toBe(1); +}); From afe8fc05ea8a83f0300018189ce7072a1a507825 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 28 Oct 2026 14:16:22 +0100 Subject: [PATCH 4/7] fix(healthcheck): keep the Dockerfile HEALTHCHECK of existing applications Co-Authored-By: Claude Opus 5.5 --- ..._healthcheck_for_existing_applications.php | 25 +++++++ .../project/shared/health-checks.blade.php | 13 +++- ...KeepDockerfileHealthcheckMigrationTest.php | 69 +++++++++++++++++++ 3 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 database/migrations/2026_10_28_124421_keep_dockerfile_healthcheck_for_existing_applications.php create mode 100644 tests/Feature/KeepDockerfileHealthcheckMigrationTest.php diff --git a/database/migrations/2026_10_28_124421_keep_dockerfile_healthcheck_for_existing_applications.php b/database/migrations/2026_10_28_124421_keep_dockerfile_healthcheck_for_existing_applications.php new file mode 100644 index 0000000000..e9c3d8f13b --- /dev/null +++ b/database/migrations/2026_10_28_124421_keep_dockerfile_healthcheck_for_existing_applications.php @@ -0,0 +1,25 @@ +where('custom_healthcheck_found', true) + ->where('health_check_enabled', true) + ->update(['health_check_enabled' => false]); + } + + public function down(): void + { + // This data migration cannot identify which applications had the Coolify healthcheck enabled. + } +}; diff --git a/resources/views/livewire/project/shared/health-checks.blade.php b/resources/views/livewire/project/shared/health-checks.blade.php index 6b02c0bf31..0a0cba98d1 100644 --- a/resources/views/livewire/project/shared/health-checks.blade.php +++ b/resources/views/livewire/project/shared/health-checks.blade.php @@ -26,9 +26,16 @@ @if ($customHealthcheckFound)
- - Enabling this configuration replaces the custom healthcheck currently defined by the application. - + @if ($healthCheckEnabled) + + This healthcheck replaces the HEALTHCHECK from the Dockerfile. Disable it to use the + Dockerfile HEALTHCHECK again. + + @else + + The application uses the HEALTHCHECK from its Dockerfile. Enabling this healthcheck replaces it. + + @endif
@endif diff --git a/tests/Feature/KeepDockerfileHealthcheckMigrationTest.php b/tests/Feature/KeepDockerfileHealthcheckMigrationTest.php new file mode 100644 index 0000000000..d994272c16 --- /dev/null +++ b/tests/Feature/KeepDockerfileHealthcheckMigrationTest.php @@ -0,0 +1,69 @@ + 0]); + + $team = Team::factory()->create(); + $this->healthcheckMigrationOwner = User::factory()->create(); + $team->members()->attach($this->healthcheckMigrationOwner->id, ['role' => 'owner']); + $project = Project::factory()->create(['team_id' => $team->id]); + $server = Server::factory()->create(['team_id' => $team->id]); + $this->healthcheckMigrationEnvironment = $project->environments()->firstOrFail(); + $this->healthcheckMigrationDestination = StandaloneDocker::query()->where('server_id', $server->id)->firstOrFail(); +}); + +function keepDockerfileHealthcheckApplication(bool $healthCheckEnabled, bool $customHealthcheckFound): Application +{ + return Application::factory()->create([ + 'environment_id' => test()->healthcheckMigrationEnvironment->id, + 'destination_id' => test()->healthcheckMigrationDestination->id, + 'destination_type' => StandaloneDocker::class, + 'build_pack' => 'dockerfile', + 'health_check_enabled' => $healthCheckEnabled, + 'custom_healthcheck_found' => $customHealthcheckFound, + ])->fresh(); +} + +test('disables the Coolify healthcheck only for applications with a Dockerfile HEALTHCHECK', function () { + $dockerfileAndCoolify = keepDockerfileHealthcheckApplication(healthCheckEnabled: true, customHealthcheckFound: true); + $dockerfileOnly = keepDockerfileHealthcheckApplication(healthCheckEnabled: false, customHealthcheckFound: true); + $coolifyOnly = keepDockerfileHealthcheckApplication(healthCheckEnabled: true, customHealthcheckFound: false); + $none = keepDockerfileHealthcheckApplication(healthCheckEnabled: false, customHealthcheckFound: false); + + (require database_path('migrations/2026_10_28_124421_keep_dockerfile_healthcheck_for_existing_applications.php'))->up(); + + expect($dockerfileAndCoolify->fresh()->health_check_enabled)->toBeFalse() + ->and($dockerfileAndCoolify->fresh()->custom_healthcheck_found)->toBeTrue() + ->and($dockerfileOnly->fresh()->health_check_enabled)->toBeFalse() + ->and($coolifyOnly->fresh()->health_check_enabled)->toBeTrue() + ->and($none->fresh()->health_check_enabled)->toBeFalse(); +}); + +test('the healthcheck page says whether the Dockerfile HEALTHCHECK is in use or replaced', function () { + $this->actingAs($this->healthcheckMigrationOwner); + + Livewire::test(HealthChecks::class, ['resource' => keepDockerfileHealthcheckApplication(healthCheckEnabled: false, customHealthcheckFound: true)]) + ->assertSee('Dockerfile HEALTHCHECK in use') + ->assertDontSee('Dockerfile HEALTHCHECK replaced'); + + Livewire::test(HealthChecks::class, ['resource' => keepDockerfileHealthcheckApplication(healthCheckEnabled: true, customHealthcheckFound: true)]) + ->assertSee('Dockerfile HEALTHCHECK replaced') + ->assertDontSee('Dockerfile HEALTHCHECK in use'); + + Livewire::test(HealthChecks::class, ['resource' => keepDockerfileHealthcheckApplication(healthCheckEnabled: true, customHealthcheckFound: false)]) + ->assertDontSee('Dockerfile HEALTHCHECK'); +}); From e7a46bd790dae566f182771c23eeb3e6f0569485 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 28 Oct 2026 14:16:22 +0100 Subject: [PATCH 5/7] fix(mcp): show commands and configuration to read:sensitive tokens Co-Authored-By: Claude Opus 5.5 --- app/Mcp/Concerns/ResolvesTeam.php | 14 ++++++ app/Mcp/Tools/GetApplication.php | 7 ++- app/Mcp/Tools/GetDatabase.php | 7 ++- tests/Feature/Mcp/McpReadToolsTest.php | 60 ++++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 4 deletions(-) diff --git a/app/Mcp/Concerns/ResolvesTeam.php b/app/Mcp/Concerns/ResolvesTeam.php index d77455b170..eea345315e 100644 --- a/app/Mcp/Concerns/ResolvesTeam.php +++ b/app/Mcp/Concerns/ResolvesTeam.php @@ -80,6 +80,20 @@ trait ResolvesTeam return $this->mcpError($request, "{$action} requires confirm=true."); } + /** + * Same rule as the REST API (ApiSensitiveData): a root or read:sensitive token of a team admin or owner. + */ + protected function canReadSensitiveData(Request $request): bool + { + $user = $request->user(); + $token = $user?->currentAccessToken(); + if (! $token || ! ($token->can('root') || $token->can('read:sensitive'))) { + return false; + } + + return $token->team_id !== null && $user->isAdminOfTeam((int) $token->team_id); + } + protected function resolveTeamId(Request $request): ?int { $user = $request->user(); diff --git a/app/Mcp/Tools/GetApplication.php b/app/Mcp/Tools/GetApplication.php index 2e45eec30a..b473cf24fc 100644 --- a/app/Mcp/Tools/GetApplication.php +++ b/app/Mcp/Tools/GetApplication.php @@ -16,7 +16,7 @@ class GetApplication extends Tool { protected string $name = 'get_application'; - protected string $description = 'Get full details for a single application by UUID.'; + protected string $description = 'Get full details for a single application by UUID. Commands and custom configuration (build, start, deployment, and healthcheck commands, docker run options, nginx configuration) need read:sensitive.'; use BuildsResponse; use ResolvesTeam; @@ -47,8 +47,11 @@ class GetApplication extends Tool $application->setRelations([]); $application->makeHidden(['destination', 'source', 'additional_servers', 'environment', 'tags', 'environmentVariables']); + $visibleSensitiveKeys = $this->canReadSensitiveData($request) ? Application::COMMAND_AND_CONFIGURATION_ATTRIBUTES : []; + $application->makeVisible($visibleSensitiveKeys); + return $this->mcpSuccess($request, $this->respond( - $this->scrubSensitive($application->toArray()), + $this->scrubSensitive($application->toArray(), $visibleSensitiveKeys), $this->actionsForApplication($uuid, $application->status), ), ['resource_uuid' => $uuid]); } diff --git a/app/Mcp/Tools/GetDatabase.php b/app/Mcp/Tools/GetDatabase.php index 63b1f0e85d..cc189daee3 100644 --- a/app/Mcp/Tools/GetDatabase.php +++ b/app/Mcp/Tools/GetDatabase.php @@ -15,7 +15,7 @@ class GetDatabase extends Tool { protected string $name = 'get_database'; - protected string $description = 'Get full details for a standalone database by UUID. Detects type across postgresql, mysql, mariadb, mongodb, redis, keydb, dragonfly, clickhouse, sqlite.'; + protected string $description = 'Get full details for a standalone database by UUID. Detects type across postgresql, mysql, mariadb, mongodb, redis, keydb, dragonfly, clickhouse, sqlite. Custom configuration (database config file, docker run options) needs read:sensitive.'; use BuildsResponse; use ResolvesTeam; @@ -45,8 +45,11 @@ class GetDatabase extends Tool $database->setRelations([]); $database->makeHidden(['destination', 'source', 'environment', 'environment_variables', 'environment_variables_preview']); + $visibleSensitiveKeys = $this->canReadSensitiveData($request) ? $database::CONFIGURATION_ATTRIBUTES : []; + $database->makeVisible($visibleSensitiveKeys); + return $this->mcpSuccess($request, $this->respond( - $this->scrubSensitive($database->toArray()), + $this->scrubSensitive($database->toArray(), $visibleSensitiveKeys), $this->actionsForDatabase($uuid, $database->status ?? null), ), ['resource_uuid' => $uuid]); } diff --git a/tests/Feature/Mcp/McpReadToolsTest.php b/tests/Feature/Mcp/McpReadToolsTest.php index eec03a226d..d919540bd3 100644 --- a/tests/Feature/Mcp/McpReadToolsTest.php +++ b/tests/Feature/Mcp/McpReadToolsTest.php @@ -644,6 +644,66 @@ test('get_application omits free-form configuration that can contain secrets', f ->and($raw)->not->toContain('custom_nginx_configuration'); }); +function mcpReadCallWithAbilities(array $abilities, string $name, array $arguments = []) +{ + auth()->forgetGuards(); + + return test()->withHeaders([ + 'Content-Type' => 'application/json', + 'Accept' => 'application/json, text/event-stream', + 'Authorization' => 'Bearer '.test()->user->createToken('mcp-abilities', $abilities)->plainTextToken, + ])->postJson('/mcp', [ + 'jsonrpc' => '2.0', + 'id' => 1, + 'method' => 'tools/call', + 'params' => ['name' => $name, 'arguments' => (object) $arguments], + ]); +} + +test('get_application returns commands and configuration to a read:sensitive or root token of an owner', function (array $abilities) { + $this->application->update([ + 'build_command' => 'npm run build', + 'post_deployment_command' => 'php artisan migrate', + 'custom_docker_run_options' => '--cap-add SYS_ADMIN', + ]); + + $response = mcpReadCallWithAbilities($abilities, 'get_application', ['uuid' => $this->application->uuid]); + $response->assertOk(); + + expect(mcpReadJson($response)['data']) + ->toMatchArray([ + 'build_command' => 'npm run build', + 'post_deployment_command' => 'php artisan migrate', + 'custom_docker_run_options' => '--cap-add SYS_ADMIN', + ]) + ->not->toHaveKey('git_full_url'); +})->with([ + 'read:sensitive' => [['read', 'read:sensitive']], + 'root' => [['root']], +]); + +test('get_database returns custom configuration to a read:sensitive token of an owner', function () { + $database = StandalonePostgresql::create([ + 'name' => 'Configured database', + 'postgres_password' => 'database-password', + 'postgres_conf' => 'max_connections = 200', + 'custom_docker_run_options' => '--shm-size 1g', + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); + + $response = mcpSensitiveReadCall('get_database', ['uuid' => $database->uuid]); + $response->assertOk(); + + expect(mcpReadJson($response)['data']) + ->toMatchArray([ + 'postgres_conf' => 'max_connections = 200', + 'custom_docker_run_options' => '--shm-size 1g', + ]) + ->not->toHaveKey('postgres_password'); +}); + test('get_database omits configuration blobs that can contain secrets', function () { $database = StandalonePostgresql::create([ 'name' => 'Secret-bearing config', From bd9708d79b769ea8b92eca84ac68764d6923a947 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 28 Oct 2026 14:16:22 +0100 Subject: [PATCH 6/7] fix(compose): let the build interpolate required runtime-only variables Closes #12363 Co-Authored-By: Claude Opus 5.5 --- app/Jobs/ApplicationDeploymentJob.php | 44 +++++++++++- ...ationDeploymentControlVarFilteringTest.php | 67 +++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 70931ece2e..a502ef7dc1 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -38,6 +38,7 @@ use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; use Illuminate\Queue\TimeoutExceededException; +use Illuminate\Support\Arr; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Sleep; @@ -919,7 +920,8 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $this->application_deployment_queue->addLogEntry('Pulling & building required images.'); // Save build-time .env file BEFORE the build - $this->save_buildtime_environment_variables($buildEnvironment); + $buildEnvironment ??= $this->generate_buildtime_environment_variables(); + $this->save_buildtime_environment_variables($buildEnvironment->merge($this->compose_build_interpolation_variables($composeFile, $buildEnvironment))); if ($this->docker_compose_custom_build_command) { // Auto-inject -f (compose file) and --env-file flags using helper function @@ -985,6 +987,46 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue } } + /** + * `docker compose build` interpolates the whole Compose file, so a required runtime-only variable + * (`${KEY:?}` in `environment:`) fails the build when the build-time .env file does not define it. + * This gives the build the runtime .env lines of the variables that the file uses outside its build + * sections. A variable that a build section or a build secret can read is left out, so a + * runtime-only value never becomes a build argument or part of an image. + * + * @return Collection + */ + private function compose_build_interpolation_variables(mixed $composeFile, Collection $buildEnvironment): Collection + { + $compose = is_string($composeFile) ? parseDockerComposeYaml($composeFile) : convertToArray($composeFile); + $references = fn (array $values): Collection => collect(Arr::flatten($values)) + ->flatMap(fn (mixed $value): Collection => extractDockerComposeEnvironmentVariableReferences($value)); + + $readByBuilds = collect(data_get($compose, 'secrets', []))->pluck('environment')->filter(); + foreach (data_get($compose, 'services', []) as $serviceName => $service) { + $build = data_get($service, 'build'); + if ($build === null) { + continue; + } + $args = data_get($build, 'args', []); + $argNames = is_array($args) && array_is_list($args) + ? array_map(fn (mixed $arg): string => explode('=', (string) $arg, 2)[0], $args) + : array_keys((array) $args); + $readByBuilds = $readByBuilds->merge($argNames)->merge($references((array) $build)); + unset($compose['services'][$serviceName]['build']); + } + + $buildKeys = $buildEnvironment->map(fn (string $line): string => explode('=', $line, 2)[0]); + $keys = $references($compose)->unique()->diff($readByBuilds)->diff($buildKeys); + if ($keys->isEmpty()) { + return collect(); + } + + return $this->generate_runtime_environment_variables() + ->filter(fn (string $line): bool => $keys->contains(explode('=', $line, 2)[0])) + ->values(); + } + /** * Parses the Compose file for this deployment and writes the volume warnings of the parser * (for example an external volume that the resource does not use yet) to the deployment log. diff --git a/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php b/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php index 4abe239679..3d6707505f 100644 --- a/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php +++ b/tests/Feature/ApplicationDeploymentControlVarFilteringTest.php @@ -2076,3 +2076,70 @@ it('gives a preview from before v4.5.1 the production values it had', function ( expect($runtimeEnvs->contains(fn (string $env) => str($env)->startsWith('DB_PASSWORD=') && str($env)->contains('production-secret')))->toBeTrue() ->and($runtimeEnvs->contains(fn (string $env) => str($env)->startsWith('APP_NAME=') && str($env)->contains('coolify')))->toBeTrue(); }); + +it('lets the Compose build interpolate runtime-only variables without passing them to the build', function (int $pullRequestId) { + $compose = <<<'YAML' +services: + app: + build: + context: . + args: + MAPPED_ARG: '${RUNTIME_IN_BUILD_ARGS}' + LISTED_FROM_ENV: null + environment: + FERNET_KEY: '${FERNET_KEY:?}' + OPTIONAL_REQUIRED: '${OPTIONAL_REQUIRED?}' + BUILD_VALUE: '${BUILD_VALUE}' + worker: + build: + context: ./worker + args: + - LISTED_ARG + environment: + FERNET_KEY: '${FERNET_KEY:?}' +YAML; + + [$application, $server] = makeDeploymentControlVarFixture([ + 'build_pack' => 'dockercompose', + 'docker_compose_raw' => $compose, + 'docker_compose' => $compose, + 'docker_compose_domains' => '[]', + ]); + $application->settings()->update(['inject_build_args_to_dockerfile' => false]); + + createApplicationEnvironmentVariable($application, ['key' => 'FERNET_KEY', 'value' => 'runtime-fernet-key', 'is_buildtime' => false]); + createApplicationEnvironmentVariable($application, ['key' => 'OPTIONAL_REQUIRED', 'value' => '', 'is_buildtime' => false]); + createApplicationEnvironmentVariable($application, ['key' => 'RUNTIME_IN_BUILD_ARGS', 'value' => 'runtime-mapped-arg', 'is_buildtime' => false]); + createApplicationEnvironmentVariable($application, ['key' => 'LISTED_FROM_ENV', 'value' => 'runtime-null-arg', 'is_buildtime' => false]); + createApplicationEnvironmentVariable($application, ['key' => 'LISTED_ARG', 'value' => 'runtime-listed-arg', 'is_buildtime' => false]); + createApplicationEnvironmentVariable($application, ['key' => 'BUILD_VALUE', 'value' => 'build-value']); + + [$job, $reflection] = makeControlVarFilteringJob($application->fresh(), $server, [ + 'pull_request_id' => $pullRequestId, + 'remote_secrets_cache' => [], + 'force_rebuild' => false, + 'docker_compose_location' => '/docker-compose.yaml', + ]); + + invokeDeploymentJobMethod($job, $reflection, 'generate_build_env_variables'); + $buildEnvironment = invokeDeploymentJobMethod($job, $reflection, 'generate_buildtime_environment_variables'); + invokeDeploymentJobMethod($job, $reflection, 'build_docker_compose_images', parseDockerComposeYaml($compose), $buildEnvironment); + + $buildEnvFile = $job->writtenArtifacts[ApplicationDeploymentJob::BUILD_TIME_ENV_PATH]; + $buildCommand = collect($job->recordedCommands)->flatten()->first(fn (mixed $command): bool => is_string($command) && str_contains($command, ' build --pull')); + $buildArgs = readDeploymentJobProperty($job, $reflection, 'build_args')->implode(' '); + + expect($buildEnvFile)->toContain('FERNET_KEY="runtime-fernet-key"') + ->toContain('OPTIONAL_REQUIRED=""') + ->toContain('BUILD_VALUE=') + ->not->toContain('runtime-mapped-arg') + ->not->toContain('runtime-null-arg') + ->not->toContain('runtime-listed-arg') + ->and($buildCommand)->toContain('--env-file '.ApplicationDeploymentJob::BUILD_TIME_ENV_PATH) + ->and($buildArgs)->toContain('BUILD_VALUE') + ->not->toContain('FERNET_KEY') + ->not->toContain('runtime-'); +})->with([ + 'production' => 0, + 'preview' => 42, +]); From 5de2456186de571ae80affc62fa503ef15df064e Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 28 Oct 2026 14:16:22 +0100 Subject: [PATCH 7/7] chore(versions): bump coolify-helper to 1.0.19 Co-Authored-By: Claude Opus 5.5 --- versions.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/versions.json b/versions.json index 0e1524c4af..37c44741b7 100644 --- a/versions.json +++ b/versions.json @@ -7,7 +7,7 @@ "version": "4.6-rc.1" }, "helper": { - "version": "1.0.18" + "version": "1.0.19" }, "sentinel": { "version": "1.1.3"