From 07441c4ed1e82d636bbebf5ca7e06cfb783f9f6e Mon Sep 17 00:00:00 2001 From: Jeppe Bundgaard Date: Mon, 8 Jun 2026 18:44:25 +0200 Subject: [PATCH] Harden API auto deploy gate --- .../nginx/app/classes/coolify_api_client.php | 10 ++ .../nginx/app/classes/release_manager.php | 139 +++++++++++++++--- .../ReleaseManager/ReleaseManagerTest.php | 63 +++++++- 3 files changed, 191 insertions(+), 21 deletions(-) diff --git a/services/nginx/app/classes/coolify_api_client.php b/services/nginx/app/classes/coolify_api_client.php index a3d7ce0f..c791b969 100644 --- a/services/nginx/app/classes/coolify_api_client.php +++ b/services/nginx/app/classes/coolify_api_client.php @@ -121,6 +121,16 @@ class coolify_api_client ]); } + public function listApplicationEnvs(string $uuid): array + { + return $this->request('GET', '/applications/' . rawurlencode($uuid) . '/envs'); + } + + public function deleteApplicationEnv(string $uuid, string $envUuid): array + { + return $this->request('DELETE', '/applications/' . rawurlencode($uuid) . '/envs/' . rawurlencode($envUuid)); + } + private static function bulkEnvData(array $env): array { $data = []; diff --git a/services/nginx/app/classes/release_manager.php b/services/nginx/app/classes/release_manager.php index 21dad092..9b6d3fbe 100644 --- a/services/nginx/app/classes/release_manager.php +++ b/services/nginx/app/classes/release_manager.php @@ -2092,11 +2092,7 @@ class release_manager $eventId = (int)$event['id']; if (!$this->acquireReleaseAutoSyncLock($eventId)) { - return [ - 'step_status' => 'passed', - 'message' => 'Automatic container update is already being processed for this commit.', - 'auto_sync_event' => $this->publicReleaseAutoSyncEvent($event), - ]; + return $this->waitForReleaseAutoSyncEventResult($eventId, $channelId, $app, $commitSha, $gateInput); } try { @@ -2178,6 +2174,67 @@ class release_manager } } + private function waitForReleaseAutoSyncEventResult(int $eventId, int $channelId, string $app, string $commitSha, array $gateInput): array + { + $timeout = max(0, min(300, (int)($gateInput['wait_timeout_seconds'] ?? 300))); + $pollInterval = max(1, min(60, (int)($gateInput['poll_interval_seconds'] ?? 10))); + $deadline = time() + $timeout; + $attempts = 0; + $lastStatus = 'unknown'; + + do { + $attempts++; + $event = $this->releaseAutoSyncEventById($eventId); + if ($event === null) { + throw new RuntimeException('Automatic container update disappeared while another request was processing it.'); + } + + $lastStatus = (string)($event['status'] ?? 'unknown'); + if (in_array($lastStatus, ['promoted', 'deployed'], true)) { + $deployment = null; + $deploymentId = $this->nullablePositiveInt($event['deployment_id'] ?? null); + if ($deploymentId !== null) { + $deployment = $this->getDeployment($deploymentId); + } + $deployment ??= $this->currentDeploymentForChannelApp($channelId, $app); + if ($deployment !== null && $this->releaseGateCommitMatches((string)($deployment['commit_sha'] ?? ''), $commitSha)) { + return [ + 'step_status' => 'passed', + 'message' => sprintf('%s container update completed by an in-flight request at %s.', strtoupper($app), substr($commitSha, 0, 12)), + 'deployment' => $this->publicDeployment($deployment), + 'auto_sync_event' => $this->publicReleaseAutoSyncEvent($event), + 'attempts' => $attempts, + ]; + } + + throw new RuntimeException(sprintf( + 'Automatic container update completed for event %d but the active %s deployment does not match %s.', + $eventId, + strtoupper($app), + $commitSha + )); + } + + if ($lastStatus === 'failed') { + $message = trim((string)($event['error_message'] ?? 'Automatic container update failed in another request.')); + throw new RuntimeException($message !== '' ? $message : 'Automatic container update failed in another request.'); + } + + if (time() >= $deadline) { + break; + } + + sleep($pollInterval); + } while (true); + + throw new RuntimeException(sprintf( + 'Automatic container update is already being processed for event %d but did not finish within %d seconds; last status was %s.', + $eventId, + $timeout, + $lastStatus + )); + } + private function upsertReleaseAutoSyncEvent(array $input): array { $channelId = (int)$input['channel_id']; @@ -6346,6 +6403,7 @@ class release_manager } if ($resourceType === 'application') { + $this->deleteCoolifyGeneratedCommitEnvs($client, $resourceUuid, $target, $context); $client->updateApplicationEnvsBulk($resourceUuid, $env); } else { $client->updateServiceEnvsBulk($resourceUuid, $env); @@ -6358,27 +6416,45 @@ class release_manager ]; } + private function deleteCoolifyGeneratedCommitEnvs(coolify_api_client $client, string $resourceUuid, array $target, array $context): void + { + $keys = $this->releaseCoolifyGeneratedCommitEnvKeys($target, $context); + if ($keys === []) { + return; + } + + try { + $rows = $this->payloadRows($client->listApplicationEnvs($resourceUuid)); + } catch (Throwable) { + return; + } + + foreach ($rows as $row) { + if (!is_array($row)) { + continue; + } + $key = trim((string)($row['key'] ?? $row['name'] ?? '')); + $uuid = trim((string)($row['uuid'] ?? $row['id'] ?? '')); + if ($key === '' || $uuid === '' || !in_array($key, $keys, true)) { + continue; + } + + try { + $client->deleteApplicationEnv($resourceUuid, $uuid); + } catch (Throwable) { + } + } + } + private function releaseCoolifyRuntimeEnv(array $target, array $context): array { $contextEnv = $this->releaseCoolifyContextEnv($context); $env = $contextEnv; $app = strtolower(trim((string)($target['app'] ?? ''))); $deploymentCommitSha = self::normalizeCommitSha($this->releaseCoolifyGitCommitSha($target, $context)); - if ($deploymentCommitSha !== '') { - if ($app === 'frontend') { - foreach (['SOURCE_COMMIT', 'RELEASE_COMMIT_SHA', 'COMMIT_SHA', 'GITHUB_SHA', 'VITE_COMMIT_HASH'] as $key) { - $env[$key] = $deploymentCommitSha; - } - } else { - foreach (['API_COMMIT_SHA', 'COMMIT_SHA'] as $key) { - if (!array_key_exists($key, $contextEnv)) { - $env[$key] = $deploymentCommitSha; - } - } - } - } if ($app !== 'api') { + $this->applyReleaseCoolifyCommitRuntimeEnv($env, $app, $deploymentCommitSha); return $env; } @@ -6403,9 +6479,34 @@ class release_manager $env = array_replace($env, $contextEnv); $env['USE_ENV'] = trim((string)($env['USE_ENV'] ?? '')) !== '' ? $env['USE_ENV'] : 'true'; $env['CORS'] = cors_policy::withRequiredOrigins((string)($env['CORS'] ?? '')); + $this->applyReleaseCoolifyCommitRuntimeEnv($env, $app, $deploymentCommitSha); return $this->normalizeCoolifyRuntimeEnv($env); } + private function applyReleaseCoolifyCommitRuntimeEnv(array &$env, string $app, string $deploymentCommitSha): void + { + if ($deploymentCommitSha === '') { + return; + } + + foreach ($this->releaseCoolifyGeneratedCommitEnvKeys(['app' => $app], []) as $key) { + $env[$key] = $deploymentCommitSha; + } + } + + private function releaseCoolifyGeneratedCommitEnvKeys(array $target, array $context): array + { + $app = strtolower(trim((string)($target['app'] ?? $context['app'] ?? ''))); + if ($app === 'frontend') { + return ['SOURCE_COMMIT', 'RELEASE_COMMIT_SHA', 'COMMIT_SHA', 'GITHUB_SHA', 'VITE_COMMIT_HASH']; + } + if ($app === 'api') { + return ['API_COMMIT_SHA', 'COMMIT_SHA', 'GITHUB_SHA', 'RELEASE_COMMIT_SHA']; + } + + return []; + } + private function releaseCoolifyContextEnv(array $context): array { $env = []; @@ -9035,7 +9136,7 @@ class release_manager if (array_keys($payload) === range(0, count($payload) - 1)) { return $payload; } - foreach (['data', 'services', 'projects', 'servers', 'github_apps', 'results'] as $key) { + foreach (['data', 'services', 'projects', 'servers', 'github_apps', 'envs', 'environment_variables', 'results'] as $key) { if (is_array($payload[$key] ?? null)) { return $this->payloadRows($payload[$key]); } diff --git a/services/nginx/app/tests/Unit/ReleaseManager/ReleaseManagerTest.php b/services/nginx/app/tests/Unit/ReleaseManager/ReleaseManagerTest.php index aac3f4ae..1d16cf7a 100644 --- a/services/nginx/app/tests/Unit/ReleaseManager/ReleaseManagerTest.php +++ b/services/nginx/app/tests/Unit/ReleaseManager/ReleaseManagerTest.php @@ -2,9 +2,33 @@ app_require('classes/release_manager.php'); app_require('classes/release_manager_schema_bootstrap.php'); +app_require('classes/coolify_api_client.php'); +use classes\coolify_api_client; use classes\release_manager; +class ReleaseManagerCoolifyEnvFake extends coolify_api_client +{ + public array $envRows; + public array $deleted = []; + + public function __construct(array $envRows) + { + $this->envRows = $envRows; + } + + public function listApplicationEnvs(string $uuid): array + { + return $this->envRows; + } + + public function deleteApplicationEnv(string $uuid, string $envUuid): array + { + $this->deleted[] = [$uuid, $envUuid]; + return ['message' => 'deleted']; + } +} + it('redacts sensitive release timeline payload fields recursively', function (): void { $payload = [ 'Authorization' => 'Bearer secret-token', @@ -608,7 +632,7 @@ it('resolves backend commit sha from API runtime environment in priority order', } }); -it('injects selected API commit into Coolify runtime env unless explicitly set', function (): void { +it('forces selected API commit into generated Coolify runtime env keys', function (): void { $manager = new release_manager(); $runtimeEnv = new ReflectionMethod(release_manager::class, 'releaseCoolifyRuntimeEnv'); $runtimeEnv->setAccessible(true); @@ -621,12 +645,17 @@ it('injects selected API commit into Coolify runtime env unless explicitly set', 'commit_sha' => $selectedCommit, ], [ 'coolify_env' => [ + 'API_COMMIT_SHA' => $explicitCommit, 'COMMIT_SHA' => $explicitCommit, + 'GITHUB_SHA' => $explicitCommit, + 'RELEASE_COMMIT_SHA' => $explicitCommit, ], ]); expect($env['API_COMMIT_SHA'])->toBe($selectedCommit); - expect($env['COMMIT_SHA'])->toBe($explicitCommit); + expect($env['COMMIT_SHA'])->toBe($selectedCommit); + expect($env['GITHUB_SHA'])->toBe($selectedCommit); + expect($env['RELEASE_COMMIT_SHA'])->toBe($selectedCommit); }); it('uses the selected deployment commit before stale Coolify context commits', function (): void { @@ -1438,6 +1467,36 @@ it('collects previous Coolify application UUIDs for stale route cleanup', functi ]); }); +it('deletes only generated API commit env rows before Coolify application env updates', function (): void { + $manager = new release_manager(); + $deleteCommitEnvs = new ReflectionMethod(release_manager::class, 'deleteCoolifyGeneratedCommitEnvs'); + $deleteCommitEnvs->setAccessible(true); + $client = new ReleaseManagerCoolifyEnvFake([ + ['uuid' => 'api-commit', 'key' => 'API_COMMIT_SHA'], + ['uuid' => 'commit', 'key' => 'COMMIT_SHA'], + ['uuid' => 'github', 'key' => 'GITHUB_SHA'], + ['uuid' => 'release', 'key' => 'RELEASE_COMMIT_SHA'], + ['uuid' => 'frontend-source', 'key' => 'SOURCE_COMMIT'], + ['uuid' => 'secret', 'key' => 'CONFIG_DB_PASSWORD'], + ]); + + $deleteCommitEnvs->invoke($manager, $client, 'application-uuid', ['app' => 'api'], []); + + expect($client->deleted)->toBe([ + ['application-uuid', 'api-commit'], + ['application-uuid', 'commit'], + ['application-uuid', 'github'], + ['application-uuid', 'release'], + ]); +}); + +it('waits for an in-flight automatic sync instead of passing the retry immediately', function (): void { + $manager = file_get_contents(app_path('classes/release_manager.php')); + + expect($manager)->toContain('return $this->waitForReleaseAutoSyncEventResult($eventId, $channelId, $app, $commitSha, $gateInput);') + ->and($manager)->toContain('Automatic container update is already being processed for event %d but did not finish'); +}); + it('redacts GitHub access metadata from public release versions', function (): void { $manager = new release_manager(); $publicVersion = new ReflectionMethod(release_manager::class, 'publicVersion');