From 4b7ccfe9cdb9aeb64d27f7e6eb42b7dae43b5e07 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 23 Sep 2026 21:45:06 +0200 Subject: [PATCH] fix: align application image validation --- .../Api/ApplicationsController.php | 1 - app/Jobs/ApplicationDeploymentJob.php | 23 ++- app/Livewire/Project/Application/General.php | 4 +- tests/Feature/StaticImageSecurityTest.php | 140 ++++++++++++++++++ 4 files changed, 158 insertions(+), 10 deletions(-) create mode 100644 tests/Feature/StaticImageSecurityTest.php diff --git a/app/Http/Controllers/Api/ApplicationsController.php b/app/Http/Controllers/Api/ApplicationsController.php index da06428b50..4d055e87cb 100644 --- a/app/Http/Controllers/Api/ApplicationsController.php +++ b/app/Http/Controllers/Api/ApplicationsController.php @@ -3163,7 +3163,6 @@ class ApplicationsController extends Controller $validationRules = [ 'name' => 'string|max:255', 'description' => 'string|nullable', - 'static_image' => 'string', 'watch_paths' => 'string|nullable', 'docker_compose_domains' => 'array|nullable', 'docker_compose_domains.*' => 'array:name,domain,redirect', diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 9987bc960b..47ad23eede 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -5,6 +5,7 @@ namespace App\Jobs; use App\Actions\Docker\GetContainersStatus; use App\Enums\ApplicationDeploymentStatus; use App\Enums\ProcessStatus; +use App\Enums\StaticImageTypes; use App\Events\ApplicationConfigurationChanged; use App\Events\ServiceStatusChanged; use App\Exceptions\DeploymentException; @@ -3322,7 +3323,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue : $this->production_image_name; if ($this->application->settings->is_static && $this->application->static_image) { - $this->pull_latest_image($this->application->static_image); + $this->pull_latest_image($this->staticImage()); } $build_command = $this->railpack_build_command($image_name, $railpackVariables); @@ -3353,7 +3354,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue { $publishDir = trim($this->application->publish_directory, '/'); $publishDir = $publishDir ? "/{$publishDir}" : ''; - $dockerfile = base64_encode("FROM {$this->application->static_image} + $dockerfile = base64_encode("FROM {$this->staticImage()} WORKDIR /usr/share/nginx/html/ LABEL coolify.deploymentId={$this->deployment_uuid} COPY --from={$this->build_image_name} /app{$publishDir} . @@ -3885,12 +3886,18 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); return $default; } - private function pull_latest_image($image) + private function staticImage(): string { + return StaticImageTypes::from($this->application->static_image)->value; + } + + private function pull_latest_image(string $image): void + { + $image = StaticImageTypes::from($image)->value; $this->application_deployment_queue->addLogEntry("Pulling latest image ($image) from the registry."); $this->execute_remote_command( [ - executeInDocker($this->deployment_uuid, "docker pull {$image}"), + executeInDocker($this->deployment_uuid, 'docker pull '.escapeshellarg($image)), 'hidden' => true, ] ); @@ -3901,9 +3908,9 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); $this->application_deployment_queue->addLogEntry('----------------------------------------'); $this->application_deployment_queue->addLogEntry('Static deployment. Copying static assets to the image.'); if ($this->application->static_image) { - $this->pull_latest_image($this->application->static_image); + $this->pull_latest_image($this->staticImage()); } - $dockerfile = base64_encode("FROM {$this->application->static_image} + $dockerfile = base64_encode("FROM {$this->staticImage()} WORKDIR /usr/share/nginx/html/ LABEL coolify.deploymentId={$this->deployment_uuid} COPY . . @@ -3998,7 +4005,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); if ($this->application->settings->is_static) { if ($this->application->static_image) { - $this->pull_latest_image($this->application->static_image); + $this->pull_latest_image($this->staticImage()); $this->application_deployment_queue->addLogEntry('Continuing with the building process.'); } if ($this->application->build_pack === 'nixpacks') { @@ -4104,7 +4111,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); } $publishDir = trim($this->application->publish_directory, '/'); $publishDir = $publishDir ? "/{$publishDir}" : ''; - $dockerfile = base64_encode("FROM {$this->application->static_image} + $dockerfile = base64_encode("FROM {$this->staticImage()} WORKDIR /usr/share/nginx/html/ LABEL coolify.deploymentId={$this->deployment_uuid} COPY --from=$this->build_image_name /app{$publishDir} . diff --git a/app/Livewire/Project/Application/General.php b/app/Livewire/Project/Application/General.php index 54562407aa..030f5341fb 100644 --- a/app/Livewire/Project/Application/General.php +++ b/app/Livewire/Project/Application/General.php @@ -3,6 +3,7 @@ namespace App\Livewire\Project\Application; use App\Actions\Application\GenerateConfig; +use App\Enums\StaticImageTypes; use App\Jobs\ApplicationDeploymentJob; use App\Livewire\Project\Service\Storage; use App\Models\Application; @@ -11,6 +12,7 @@ use App\Support\ValidationPatterns; use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Support\Collection; +use Illuminate\Validation\Rule; use Livewire\Component; use Livewire\Features\SupportEvents\Event; @@ -156,7 +158,7 @@ class General extends Component 'buildCommand' => ValidationPatterns::shellSafeCommandRules(), 'startCommand' => ValidationPatterns::shellSafeCommandRules(), 'buildPack' => 'required', - 'staticImage' => 'required', + 'staticImage' => ['required', Rule::enum(StaticImageTypes::class)], 'baseDirectory' => array_merge(['required'], array_slice(ValidationPatterns::directoryPathRules(), 1)), 'publishDirectory' => ValidationPatterns::directoryPathRules(), 'portsExposes' => ['nullable', 'string', 'regex:/^(\d+)(,\d+)*$/'], diff --git a/tests/Feature/StaticImageSecurityTest.php b/tests/Feature/StaticImageSecurityTest.php new file mode 100644 index 0000000000..2b5b1f955b --- /dev/null +++ b/tests/Feature/StaticImageSecurityTest.php @@ -0,0 +1,140 @@ + InstanceSettings::firstOrCreate(['id' => 0])); + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'owner']); + session(['currentTeam' => $this->team]); + $this->token = $this->user->createToken('static-image-security-test', ['*'])->plainTextToken; + + $this->server = Server::factory()->create(['team_id' => $this->team->id]); + $this->destination = StandaloneDocker::query()->where('server_id', $this->server->id)->firstOrFail(); + $this->project = Project::factory()->create(['team_id' => $this->team->id]); + $this->environment = Environment::factory()->create(['project_id' => $this->project->id]); +}); + +function staticImageApplication(): Application +{ + return Application::factory()->create([ + 'environment_id' => test()->environment->id, + 'destination_id' => test()->destination->id, + 'destination_type' => test()->destination->getMorphClass(), + 'static_image' => 'nginx:alpine', + 'base_directory' => '/', + 'is_http_basic_auth_enabled' => false, + 'redirect' => 'no', + ]); +} + +function staticImageApiHeaders(): array +{ + return ['Authorization' => 'Bearer '.test()->token]; +} + +function staticImageCreatePayload(string $image): array +{ + return [ + 'project_uuid' => test()->project->uuid, + 'environment_uuid' => test()->environment->uuid, + 'server_uuid' => test()->server->uuid, + 'git_repository' => 'https://gitlab.com/coolify/test-static-app', + 'git_branch' => 'main', + 'build_pack' => 'static', + 'ports_exposes' => '80', + 'autogenerate_domain' => false, + 'static_image' => $image, + ]; +} + +test('API create rejects invalid static images', function (string $image) { + $this->withHeaders(staticImageApiHeaders()) + ->postJson('/api/v1/applications/public', staticImageCreatePayload($image)) + ->assertUnprocessable() + ->assertInvalid(['static_image']); +})->with(['shell' => 'nginx:alpine;id>/tmp/pwn', 'dockerfile newline' => "nginx:alpine\nRUN id"]); + +test('API update rejects invalid static images without saving them', function (string $image) { + $application = staticImageApplication(); + + $this->withHeaders(staticImageApiHeaders()) + ->patchJson("/api/v1/applications/{$application->uuid}", ['static_image' => $image]) + ->assertUnprocessable() + ->assertInvalid(['static_image']); + + expect($application->refresh()->static_image)->toBe('nginx:alpine'); +})->with(['shell' => 'nginx:alpine;id>/tmp/pwn', 'dockerfile newline' => "nginx:alpine\nRUN id"]); + +test('API accepts the allowed static image on create and update', function () { + $response = $this->withHeaders(staticImageApiHeaders()) + ->postJson('/api/v1/applications/public', staticImageCreatePayload('nginx:alpine')) + ->assertCreated(); + + $application = Application::query()->where('uuid', $response->json('uuid'))->firstOrFail(); + expect($application->static_image)->toBe('nginx:alpine'); + + $this->withHeaders(staticImageApiHeaders()) + ->patchJson("/api/v1/applications/{$application->uuid}", ['static_image' => 'nginx:alpine']) + ->assertOk(); +}); + +test('Livewire rejects invalid static images without saving them', function (string $image) { + $application = staticImageApplication(); + $this->actingAs($this->user); + + Livewire::test(General::class, ['application' => $application]) + ->set('staticImage', $image) + ->call('submit') + ->assertDispatched('error', fn (string $event, array $params): bool => str_contains(strtolower($params[0]), 'static image') && str_contains(strtolower($params[0]), 'invalid')) + ->assertNotDispatched('success'); + + expect($application->refresh()->static_image)->toBe('nginx:alpine'); +})->with(['shell' => 'nginx:alpine;id>/tmp/pwn', 'dockerfile newline' => "nginx:alpine\nRUN id"]); + +test('Livewire accepts the allowed static image', function () { + $application = staticImageApplication(); + $this->actingAs($this->user); + + Livewire::test(General::class, ['application' => $application]) + ->set('staticImage', 'nginx:alpine') + ->call('submit') + ->assertNotDispatched('error'); + + expect($application->refresh()->static_image)->toBe('nginx:alpine'); +}); + +test('deployment rejects invalid legacy static images before they reach shell or Dockerfile', function (string $image) { + $application = staticImageApplication(); + $application->static_image = $image; + + $job = (new ReflectionClass(ApplicationDeploymentJob::class))->newInstanceWithoutConstructor(); + $property = new ReflectionProperty(ApplicationDeploymentJob::class, 'application'); + $property->setValue($job, $application); + + expect(fn () => (new ReflectionMethod(ApplicationDeploymentJob::class, 'staticImage'))->invoke($job)) + ->toThrow(ValueError::class); +})->with(['shell' => 'nginx:alpine;id>/tmp/pwn', 'dockerfile newline' => "nginx:alpine\nRUN id"]); + +test('deployment accepts the allowed static image', function () { + $job = (new ReflectionClass(ApplicationDeploymentJob::class))->newInstanceWithoutConstructor(); + (new ReflectionProperty(ApplicationDeploymentJob::class, 'application'))->setValue($job, staticImageApplication()); + + expect((new ReflectionMethod(ApplicationDeploymentJob::class, 'staticImage'))->invoke($job)) + ->toBe('nginx:alpine'); +});