From bbddf9543d5b8b89c1d49eb01260a29f1e0942c2 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:48:28 +0100 Subject: [PATCH] fix(applications): keep saving with unchanged legacy build commands Install, build, and start commands saved before they were validated blocked every save of the General tab, and the fields are hidden for build packs other than Nixpacks and Railpack, so users could not fix them. Validate these commands only when they change; deployments pass them to the builder as one escaped argument. Closes #10055 Co-Authored-By: Claude Opus 5.5 --- app/Livewire/Project/Application/General.php | 19 ++- .../ApplicationLegacyBuildCommandTest.php | 130 ++++++++++++++++++ 2 files changed, 146 insertions(+), 3 deletions(-) create mode 100644 tests/Feature/ApplicationLegacyBuildCommandTest.php diff --git a/app/Livewire/Project/Application/General.php b/app/Livewire/Project/Application/General.php index 2112c36c14..7128f2bbc6 100644 --- a/app/Livewire/Project/Application/General.php +++ b/app/Livewire/Project/Application/General.php @@ -170,9 +170,9 @@ class General extends Component 'gitRepository' => 'required', 'gitBranch' => ['required', 'string', new ValidGitBranch], 'gitCommitSha' => ['nullable', 'string', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._\-\/]*$/'], - 'installCommand' => ValidationPatterns::shellSafeCommandRules(), - 'buildCommand' => ValidationPatterns::shellSafeCommandRules(), - 'startCommand' => ValidationPatterns::shellSafeCommandRules(), + 'installCommand' => $this->changedCommandRules('installCommand', 'install_command'), + 'buildCommand' => $this->changedCommandRules('buildCommand', 'build_command'), + 'startCommand' => $this->changedCommandRules('startCommand', 'start_command'), 'buildPack' => 'required', 'staticImage' => ['required', Rule::enum(StaticImageTypes::class)], 'baseDirectory' => array_merge(['required'], array_slice(ValidationPatterns::directoryPathRules(), 1)), @@ -217,6 +217,19 @@ class General extends Component ]; } + /** + * Commands saved before they were validated stay valid until they change. The deployment escapes them as one + * argument, and the fields are hidden for build packs other than Nixpacks and Railpack, so a user cannot fix them. + */ + private function changedCommandRules(string $property, string $column): array + { + if (isset($this->application) && (string) $this->{$property} === (string) $this->application->getOriginal($column)) { + return ['nullable']; + } + + return ValidationPatterns::shellSafeCommandRules(); + } + protected function messages(): array { return array_merge( diff --git a/tests/Feature/ApplicationLegacyBuildCommandTest.php b/tests/Feature/ApplicationLegacyBuildCommandTest.php new file mode 100644 index 0000000000..06cf382798 --- /dev/null +++ b/tests/Feature/ApplicationLegacyBuildCommandTest.php @@ -0,0 +1,130 @@ + InstanceSettings::firstOrCreate(['id' => 0])); + + $this->team = Team::factory()->create(); + $this->user = User::factory()->create(); + $this->team->members()->attach($this->user->id, ['role' => 'owner']); + $this->actingAs($this->user); + session(['currentTeam' => $this->team]); + + $server = Server::factory()->create(['team_id' => $this->team->id]); + $destination = StandaloneDocker::query()->where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $this->team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + + // Saved before the command fields were validated, then converted from Nixpacks to Dockerfile. + $this->application = Application::factory()->create([ + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + 'build_pack' => 'dockerfile', + 'static_image' => 'nginx:alpine', + 'base_directory' => '/', + 'is_http_basic_auth_enabled' => false, + 'redirect' => 'no', + 'install_command' => 'npm ci; npm run prepare', + 'build_command' => 'npm run build > build.log', + 'start_command' => 'node $ENTRY', + ])->refresh(); +}); + +test('saving keeps unchanged legacy commands that the current pattern rejects', function () { + Livewire::test(General::class, ['application' => $this->application]) + ->set('description', 'New description') + ->call('submit') + ->assertHasNoErrors() + ->assertNotDispatched('error'); + + $this->application->refresh(); + expect($this->application->description)->toBe('New description') + ->and($this->application->install_command)->toBe('npm ci; npm run prepare') + ->and($this->application->build_command)->toBe('npm run build > build.log') + ->and($this->application->start_command)->toBe('node $ENTRY'); +}); + +test('switching the build pack to Nixpacks works with unchanged legacy commands', function () { + Livewire::test(General::class, ['application' => $this->application]) + ->set('buildPack', 'nixpacks') + ->assertNotDispatched('error'); + + expect($this->application->refresh()->build_pack)->toBe('nixpacks'); +}); + +test('a changed command must still match the shell-safe pattern', function (string $property, string $column) { + $original = $this->application->{$column}; + + Livewire::test(General::class, ['application' => $this->application]) + ->set($property, 'npm run build; id') + ->call('submit') + ->assertDispatched('error') + ->assertNotDispatched('success'); + + expect($this->application->refresh()->{$column})->toBe($original); +})->with([ + 'install command' => ['installCommand', 'install_command'], + 'build command' => ['buildCommand', 'build_command'], + 'start command' => ['startCommand', 'start_command'], +]); + +test('a legacy command reaches the builder as one argument and cannot run commands on the server', function (string $buildPack, string $method, string $payload) { + $marker = sys_get_temp_dir().'/coolify-legacy-command-'.uniqid(); + $hostile = str_replace('MARKER', $marker, $payload); + $this->application->forceFill([ + 'build_pack' => $buildPack, + 'install_command' => $hostile, + 'build_command' => $hostile, + 'start_command' => $hostile, + ])->saveQuietly(); + + $job = (new ReflectionClass(ApplicationDeploymentJob::class))->newInstanceWithoutConstructor(); + foreach (['application' => $this->application->refresh(), 'workdir' => '/artifacts/app', 'deployment_uuid' => 'helper', 'pull_request_id' => 0, 'env_railpack_args' => ''] as $property => $value) { + (new ReflectionProperty($job, $property))->setValue($job, $value); + } + $command = (new ReflectionMethod($job, $method))->invoke($job); + + // The deployment wraps the command for the helper container and adds sudo on servers with a non-root user. + $script = str_replace('docker exec', 'sudo docker exec', executeInDocker('helper', $command)); + + $bin = sys_get_temp_dir().'/coolify-legacy-bin-'.uniqid(); + mkdir($bin); + file_put_contents("$bin/sudo", "#!/bin/bash\nexec \"\$@\"\n"); + file_put_contents("$bin/docker", "#!/bin/bash\nshift 2\nexec \"\$@\"\n"); + file_put_contents("$bin/nixpacks", "#!/bin/bash\nprintf '%s\\0' \"\$@\" > $bin/args\n"); + copy("$bin/nixpacks", "$bin/railpack"); + array_map(fn (string $file) => chmod($file, 0755), glob("$bin/*")); + + $result = Process::env(['PATH' => "$bin:".getenv('PATH')])->input($script."\n")->run('bash -s'); + $arguments = explode("\0", rtrim(file_get_contents("$bin/args"), "\0")); + + expect($result->successful())->toBeTrue() + ->and(file_exists($marker))->toBeFalse() + // The sudo replacement also edits the text inside the value. This changes the value but does not run it. + ->and(array_count_values($arguments)[str_replace('docker exec', 'sudo docker exec', $hostile)] ?? 0)->toBe($method === 'nixpacks_build_cmd' ? 3 : 2); +})->with([ + 'nixpacks' => ['nixpacks', 'nixpacks_build_cmd'], + 'railpack' => ['railpack', 'railpack_prepare_command'], +])->with([ + 'quote break-out' => ["x'; touch MARKER; echo '"], + 'newline' => ["npm run build\ntouch MARKER"], + 'command substitution' => ['$(touch MARKER)'], + 'backticks' => ['`touch MARKER`'], + 'sudo text inside the value' => ["docker exec x touch MARKER'; touch MARKER; '"], +]);