diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index e30af5cc7..9d927d10c 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -2334,13 +2334,13 @@ private function nixpacks_build_cmd() $this->generate_nixpacks_env_variables(); $nixpacks_command = "nixpacks plan -f json {$this->env_nixpacks_args}"; if ($this->application->build_command) { - $nixpacks_command .= " --build-cmd \"{$this->application->build_command}\""; + $nixpacks_command .= ' --build-cmd '.escapeShellValue($this->application->build_command); } if ($this->application->start_command) { - $nixpacks_command .= " --start-cmd \"{$this->application->start_command}\""; + $nixpacks_command .= ' --start-cmd '.escapeShellValue($this->application->start_command); } if ($this->application->install_command) { - $nixpacks_command .= " --install-cmd \"{$this->application->install_command}\""; + $nixpacks_command .= ' --install-cmd '.escapeShellValue($this->application->install_command); } $nixpacks_command .= " {$this->workdir}"; @@ -2353,13 +2353,15 @@ private function generate_nixpacks_env_variables() if ($this->pull_request_id === 0) { foreach ($this->application->nixpacks_environment_variables as $env) { if (! is_null($env->real_value) && $env->real_value !== '') { - $this->env_nixpacks_args->push("--env {$env->key}={$env->real_value}"); + $value = ($env->is_literal || $env->is_multiline) ? trim($env->real_value, "'") : $env->real_value; + $this->env_nixpacks_args->push('--env '.escapeShellValue("{$env->key}={$value}")); } } } else { foreach ($this->application->nixpacks_environment_variables_preview as $env) { if (! is_null($env->real_value) && $env->real_value !== '') { - $this->env_nixpacks_args->push("--env {$env->key}={$env->real_value}"); + $value = ($env->is_literal || $env->is_multiline) ? trim($env->real_value, "'") : $env->real_value; + $this->env_nixpacks_args->push('--env '.escapeShellValue("{$env->key}={$value}")); } } } @@ -2369,7 +2371,7 @@ private function generate_nixpacks_env_variables() $coolify_envs->each(function ($value, $key) { // Only add environment variables with non-null and non-empty values if (! is_null($value) && $value !== '') { - $this->env_nixpacks_args->push("--env {$key}={$value}"); + $this->env_nixpacks_args->push('--env '.escapeShellValue("{$key}={$value}")); } }); diff --git a/bootstrap/helpers/docker.php b/bootstrap/helpers/docker.php index 7b74392cf..5905ed3c1 100644 --- a/bootstrap/helpers/docker.php +++ b/bootstrap/helpers/docker.php @@ -137,6 +137,11 @@ function checkMinimumDockerEngineVersion($dockerVersion) return $dockerVersion; } +function escapeShellValue(string $value): string +{ + return "'".str_replace("'", "'\\''", $value)."'"; +} + function executeInDocker(string $containerId, string $command) { $escapedCommand = str_replace("'", "'\\''", $command); diff --git a/tests/Unit/ApplicationDeploymentNixpacksNullEnvTest.php b/tests/Unit/ApplicationDeploymentNixpacksNullEnvTest.php index c2a8d46fa..4c7ec9d9d 100644 --- a/tests/Unit/ApplicationDeploymentNixpacksNullEnvTest.php +++ b/tests/Unit/ApplicationDeploymentNixpacksNullEnvTest.php @@ -88,11 +88,11 @@ $envArgsProperty->setAccessible(true); $envArgs = $envArgsProperty->getValue($job); - // Verify that only valid environment variables are included - expect($envArgs)->toContain('--env VALID_VAR=valid_value'); - expect($envArgs)->toContain('--env ANOTHER_VALID_VAR=another_value'); - expect($envArgs)->toContain('--env COOLIFY_FQDN=example.com'); - expect($envArgs)->toContain('--env SOURCE_COMMIT=abc123'); + // Verify that only valid environment variables are included (values are now single-quote escaped) + expect($envArgs)->toContain("--env 'VALID_VAR=valid_value'"); + expect($envArgs)->toContain("--env 'ANOTHER_VALID_VAR=another_value'"); + expect($envArgs)->toContain("--env 'COOLIFY_FQDN=example.com'"); + expect($envArgs)->toContain("--env 'SOURCE_COMMIT=abc123'"); // Verify that null and empty environment variables are filtered out expect($envArgs)->not->toContain('NULL_VAR'); @@ -102,7 +102,7 @@ // Verify no environment variables end with just '=' (which indicates null/empty value) expect($envArgs)->not->toMatch('/--env [A-Z_]+=$/'); - expect($envArgs)->not->toMatch('/--env [A-Z_]+= /'); + expect($envArgs)->not->toMatch("/--env '[A-Z_]+='$/"); }); it('filters out null environment variables from nixpacks preview deployments', function () { @@ -164,9 +164,9 @@ $envArgsProperty->setAccessible(true); $envArgs = $envArgsProperty->getValue($job); - // Verify that only valid environment variables are included - expect($envArgs)->toContain('--env PREVIEW_VAR=preview_value'); - expect($envArgs)->toContain('--env COOLIFY_FQDN=preview.example.com'); + // Verify that only valid environment variables are included (values are now single-quote escaped) + expect($envArgs)->toContain("--env 'PREVIEW_VAR=preview_value'"); + expect($envArgs)->toContain("--env 'COOLIFY_FQDN=preview.example.com'"); // Verify that null environment variables are filtered out expect($envArgs)->not->toContain('NULL_PREVIEW_VAR'); @@ -335,7 +335,7 @@ $envArgsProperty->setAccessible(true); $envArgs = $envArgsProperty->getValue($job); - // Verify that zero and false string values are preserved - expect($envArgs)->toContain('--env ZERO_VALUE=0'); - expect($envArgs)->toContain('--env FALSE_VALUE=false'); + // Verify that zero and false string values are preserved (values are now single-quote escaped) + expect($envArgs)->toContain("--env 'ZERO_VALUE=0'"); + expect($envArgs)->toContain("--env 'FALSE_VALUE=false'"); }); diff --git a/tests/Unit/EscapeShellValueTest.php b/tests/Unit/EscapeShellValueTest.php new file mode 100644 index 000000000..eed25e164 --- /dev/null +++ b/tests/Unit/EscapeShellValueTest.php @@ -0,0 +1,57 @@ +toBe("'hello'"); +}); + +it('escapes single quotes in the value', function () { + expect(escapeShellValue("it's"))->toBe("'it'\\''s'"); +}); + +it('handles empty string', function () { + expect(escapeShellValue(''))->toBe("''"); +}); + +it('preserves && in a single-quoted value', function () { + $result = escapeShellValue('npx prisma generate && npm run build'); + expect($result)->toBe("'npx prisma generate && npm run build'"); +}); + +it('preserves special shell characters in value', function () { + $result = escapeShellValue('echo $HOME; rm -rf /'); + expect($result)->toBe("'echo \$HOME; rm -rf /'"); +}); + +it('handles value with double quotes', function () { + $result = escapeShellValue('say "hello"'); + expect($result)->toBe("'say \"hello\"'"); +}); + +it('produces correct output when passed through executeInDocker', function () { + // Simulate the exact issue from GitHub #9042: + // NIXPACKS_BUILD_CMD with chained && commands + $envValue = 'npx prisma generate && npx prisma db push && npm run build'; + $escapedEnv = '--env '.escapeShellValue("NIXPACKS_BUILD_CMD={$envValue}"); + + $command = "nixpacks plan -f json {$escapedEnv} /app"; + $dockerCmd = executeInDocker('test-container', $command); + + // The && must NOT appear unquoted at the bash -c level + // The full docker command should properly nest the quoting + expect($dockerCmd)->toContain('NIXPACKS_BUILD_CMD=npx prisma generate && npx prisma db push && npm run build'); + // Verify it's wrapped in docker exec bash -c + expect($dockerCmd)->toStartWith("docker exec test-container bash -c '"); + expect($dockerCmd)->toEndWith("'"); +}); + +it('produces correct output for build-cmd with chained commands through executeInDocker', function () { + $buildCmd = 'npx prisma generate && npm run build'; + $escapedCmd = escapeShellValue($buildCmd); + + $command = "nixpacks plan -f json --build-cmd {$escapedCmd} /app"; + $dockerCmd = executeInDocker('test-container', $command); + + // The build command value must remain intact inside the quoting + expect($dockerCmd)->toContain('npx prisma generate && npm run build'); + expect($dockerCmd)->toStartWith("docker exec test-container bash -c '"); +});