fix(deployment): properly escape shell arguments in nixpacks commands (#9122)

This commit is contained in:
Andras Bacsai 2026-03-23 21:58:04 +01:00 committed by GitHub
commit 0aee5a41ae
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 82 additions and 18 deletions

View file

@ -2334,13 +2334,13 @@ private function nixpacks_build_cmd()
$this->generate_nixpacks_env_variables(); $this->generate_nixpacks_env_variables();
$nixpacks_command = "nixpacks plan -f json {$this->env_nixpacks_args}"; $nixpacks_command = "nixpacks plan -f json {$this->env_nixpacks_args}";
if ($this->application->build_command) { 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) { 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) { 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}"; $nixpacks_command .= " {$this->workdir}";
@ -2353,13 +2353,15 @@ private function generate_nixpacks_env_variables()
if ($this->pull_request_id === 0) { if ($this->pull_request_id === 0) {
foreach ($this->application->nixpacks_environment_variables as $env) { foreach ($this->application->nixpacks_environment_variables as $env) {
if (! is_null($env->real_value) && $env->real_value !== '') { 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 { } else {
foreach ($this->application->nixpacks_environment_variables_preview as $env) { foreach ($this->application->nixpacks_environment_variables_preview as $env) {
if (! is_null($env->real_value) && $env->real_value !== '') { 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) { $coolify_envs->each(function ($value, $key) {
// Only add environment variables with non-null and non-empty values // Only add environment variables with non-null and non-empty values
if (! is_null($value) && $value !== '') { if (! is_null($value) && $value !== '') {
$this->env_nixpacks_args->push("--env {$key}={$value}"); $this->env_nixpacks_args->push('--env '.escapeShellValue("{$key}={$value}"));
} }
}); });

View file

@ -137,6 +137,11 @@ function checkMinimumDockerEngineVersion($dockerVersion)
return $dockerVersion; return $dockerVersion;
} }
function escapeShellValue(string $value): string
{
return "'".str_replace("'", "'\\''", $value)."'";
}
function executeInDocker(string $containerId, string $command) function executeInDocker(string $containerId, string $command)
{ {
$escapedCommand = str_replace("'", "'\\''", $command); $escapedCommand = str_replace("'", "'\\''", $command);

View file

@ -88,11 +88,11 @@
$envArgsProperty->setAccessible(true); $envArgsProperty->setAccessible(true);
$envArgs = $envArgsProperty->getValue($job); $envArgs = $envArgsProperty->getValue($job);
// Verify that only valid environment variables are included // 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 'VALID_VAR=valid_value'");
expect($envArgs)->toContain('--env ANOTHER_VALID_VAR=another_value'); expect($envArgs)->toContain("--env 'ANOTHER_VALID_VAR=another_value'");
expect($envArgs)->toContain('--env COOLIFY_FQDN=example.com'); expect($envArgs)->toContain("--env 'COOLIFY_FQDN=example.com'");
expect($envArgs)->toContain('--env SOURCE_COMMIT=abc123'); expect($envArgs)->toContain("--env 'SOURCE_COMMIT=abc123'");
// Verify that null and empty environment variables are filtered out // Verify that null and empty environment variables are filtered out
expect($envArgs)->not->toContain('NULL_VAR'); expect($envArgs)->not->toContain('NULL_VAR');
@ -102,7 +102,7 @@
// Verify no environment variables end with just '=' (which indicates null/empty value) // 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_]+= /'); expect($envArgs)->not->toMatch("/--env '[A-Z_]+='$/");
}); });
it('filters out null environment variables from nixpacks preview deployments', function () { it('filters out null environment variables from nixpacks preview deployments', function () {
@ -164,9 +164,9 @@
$envArgsProperty->setAccessible(true); $envArgsProperty->setAccessible(true);
$envArgs = $envArgsProperty->getValue($job); $envArgs = $envArgsProperty->getValue($job);
// Verify that only valid environment variables are included // 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 'PREVIEW_VAR=preview_value'");
expect($envArgs)->toContain('--env COOLIFY_FQDN=preview.example.com'); expect($envArgs)->toContain("--env 'COOLIFY_FQDN=preview.example.com'");
// Verify that null environment variables are filtered out // Verify that null environment variables are filtered out
expect($envArgs)->not->toContain('NULL_PREVIEW_VAR'); expect($envArgs)->not->toContain('NULL_PREVIEW_VAR');
@ -335,7 +335,7 @@
$envArgsProperty->setAccessible(true); $envArgsProperty->setAccessible(true);
$envArgs = $envArgsProperty->getValue($job); $envArgs = $envArgsProperty->getValue($job);
// Verify that zero and false string values are preserved // 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 'ZERO_VALUE=0'");
expect($envArgs)->toContain('--env FALSE_VALUE=false'); expect($envArgs)->toContain("--env 'FALSE_VALUE=false'");
}); });

View file

@ -0,0 +1,57 @@
<?php
it('wraps a simple value in single quotes', function () {
expect(escapeShellValue('hello'))->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 '");
});