From 41afa9568d5ed2dcf56b42791ee941dbf1931fbf Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 15 Oct 2025 13:35:58 +0200 Subject: [PATCH] fix: handle null environment variable values in bash escaping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, the bash escaping functions (`escapeBashEnvValue()` and `escapeBashDoubleQuoted()`) had strict string type hints that rejected null values, causing deployment failures when environment variables had null values. Changes: - Updated both functions to accept nullable strings (`?string $value`) - Handle null/empty values by returning empty quoted strings (`''` for single quotes, `""` for double quotes) - Added 3 new tests to cover null and empty value handling - All 29 tests pass This fix ensures deployments work correctly even when environment variables have null values, while maintaining the existing behavior for all other cases. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- app/Jobs/ApplicationDeploymentJob.php | 80 ++++++- bootstrap/helpers/docker.php | 66 ++++++ tests/Unit/BashEnvEscapingTest.php | 307 ++++++++++++++++++++++++++ 3 files changed, 444 insertions(+), 9 deletions(-) create mode 100644 tests/Unit/BashEnvEscapingTest.php diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 94c299364..a624348c0 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -1319,12 +1319,18 @@ private function save_runtime_environment_variables() private function generate_buildtime_environment_variables() { + if (isDev()) { + $this->application_deployment_queue->addLogEntry('[DEBUG] ========================================'); + $this->application_deployment_queue->addLogEntry('[DEBUG] Generating build-time environment variables'); + $this->application_deployment_queue->addLogEntry('[DEBUG] ========================================'); + } + $envs = collect([]); $coolify_envs = $this->generate_coolify_env_variables(); // Add COOLIFY variables $coolify_envs->each(function ($item, $key) use ($envs) { - $envs->push($key.'='.$item); + $envs->push($key.'='.escapeBashEnvValue($item)); }); // Add SERVICE_NAME variables for Docker Compose builds @@ -1338,7 +1344,7 @@ private function generate_buildtime_environment_variables() } $services = data_get($dockerCompose, 'services', []); foreach ($services as $serviceName => $_) { - $envs->push('SERVICE_NAME_'.str($serviceName)->upper().'='.$serviceName); + $envs->push('SERVICE_NAME_'.str($serviceName)->upper().'='.escapeBashEnvValue($serviceName)); } // Generate SERVICE_FQDN & SERVICE_URL for non-PR deployments @@ -1351,8 +1357,8 @@ private function generate_buildtime_environment_variables() $coolifyScheme = $coolifyUrl->getScheme(); $coolifyFqdn = $coolifyUrl->getHost(); $coolifyUrl = $coolifyUrl->withScheme($coolifyScheme)->withHost($coolifyFqdn)->withPort(null); - $envs->push('SERVICE_URL_'.str($forServiceName)->upper().'='.$coolifyUrl->__toString()); - $envs->push('SERVICE_FQDN_'.str($forServiceName)->upper().'='.$coolifyFqdn); + $envs->push('SERVICE_URL_'.str($forServiceName)->upper().'='.escapeBashEnvValue($coolifyUrl->__toString())); + $envs->push('SERVICE_FQDN_'.str($forServiceName)->upper().'='.escapeBashEnvValue($coolifyFqdn)); } } } else { @@ -1360,7 +1366,7 @@ private function generate_buildtime_environment_variables() $rawDockerCompose = Yaml::parse($this->application->docker_compose_raw); $rawServices = data_get($rawDockerCompose, 'services', []); foreach ($rawServices as $rawServiceName => $_) { - $envs->push('SERVICE_NAME_'.str($rawServiceName)->upper().'='.addPreviewDeploymentSuffix($rawServiceName, $this->pull_request_id)); + $envs->push('SERVICE_NAME_'.str($rawServiceName)->upper().'='.escapeBashEnvValue(addPreviewDeploymentSuffix($rawServiceName, $this->pull_request_id))); } // Generate SERVICE_FQDN & SERVICE_URL for preview deployments with PR-specific domains @@ -1373,8 +1379,8 @@ private function generate_buildtime_environment_variables() $coolifyScheme = $coolifyUrl->getScheme(); $coolifyFqdn = $coolifyUrl->getHost(); $coolifyUrl = $coolifyUrl->withScheme($coolifyScheme)->withHost($coolifyFqdn)->withPort(null); - $envs->push('SERVICE_URL_'.str($forServiceName)->upper().'='.$coolifyUrl->__toString()); - $envs->push('SERVICE_FQDN_'.str($forServiceName)->upper().'='.$coolifyFqdn); + $envs->push('SERVICE_URL_'.str($forServiceName)->upper().'='.escapeBashEnvValue($coolifyUrl->__toString())); + $envs->push('SERVICE_FQDN_'.str($forServiceName)->upper().'='.escapeBashEnvValue($coolifyFqdn)); } } } @@ -1396,7 +1402,32 @@ private function generate_buildtime_environment_variables() } foreach ($sorted_environment_variables as $env) { - $envs->push($env->key.'='.$env->real_value); + // For literal/multiline vars, real_value includes quotes that we need to remove + if ($env->is_literal || $env->is_multiline) { + // Strip outer quotes from real_value and apply proper bash escaping + $value = trim($env->real_value, "'"); + $escapedValue = escapeBashEnvValue($value); + $envs->push($env->key.'='.$escapedValue); + + if (isDev()) { + $this->application_deployment_queue->addLogEntry("[DEBUG] Build-time env: {$env->key}"); + $this->application_deployment_queue->addLogEntry('[DEBUG] Type: literal/multiline'); + $this->application_deployment_queue->addLogEntry("[DEBUG] raw real_value: {$env->real_value}"); + $this->application_deployment_queue->addLogEntry("[DEBUG] stripped value: {$value}"); + $this->application_deployment_queue->addLogEntry("[DEBUG] final escaped: {$escapedValue}"); + } + } else { + // For normal vars, use double quotes to allow $VAR expansion + $escapedValue = escapeBashDoubleQuoted($env->real_value); + $envs->push($env->key.'='.$escapedValue); + + if (isDev()) { + $this->application_deployment_queue->addLogEntry("[DEBUG] Build-time env: {$env->key}"); + $this->application_deployment_queue->addLogEntry('[DEBUG] Type: normal (allows expansion)'); + $this->application_deployment_queue->addLogEntry("[DEBUG] real_value: {$env->real_value}"); + $this->application_deployment_queue->addLogEntry("[DEBUG] final escaped: {$escapedValue}"); + } + } } } else { $sorted_environment_variables = $this->application->environment_variables_preview() @@ -1413,11 +1444,42 @@ private function generate_buildtime_environment_variables() } foreach ($sorted_environment_variables as $env) { - $envs->push($env->key.'='.$env->real_value); + // For literal/multiline vars, real_value includes quotes that we need to remove + if ($env->is_literal || $env->is_multiline) { + // Strip outer quotes from real_value and apply proper bash escaping + $value = trim($env->real_value, "'"); + $escapedValue = escapeBashEnvValue($value); + $envs->push($env->key.'='.$escapedValue); + + if (isDev()) { + $this->application_deployment_queue->addLogEntry("[DEBUG] Build-time env: {$env->key}"); + $this->application_deployment_queue->addLogEntry('[DEBUG] Type: literal/multiline'); + $this->application_deployment_queue->addLogEntry("[DEBUG] raw real_value: {$env->real_value}"); + $this->application_deployment_queue->addLogEntry("[DEBUG] stripped value: {$value}"); + $this->application_deployment_queue->addLogEntry("[DEBUG] final escaped: {$escapedValue}"); + } + } else { + // For normal vars, use double quotes to allow $VAR expansion + $escapedValue = escapeBashDoubleQuoted($env->real_value); + $envs->push($env->key.'='.$escapedValue); + + if (isDev()) { + $this->application_deployment_queue->addLogEntry("[DEBUG] Build-time env: {$env->key}"); + $this->application_deployment_queue->addLogEntry('[DEBUG] Type: normal (allows expansion)'); + $this->application_deployment_queue->addLogEntry("[DEBUG] real_value: {$env->real_value}"); + $this->application_deployment_queue->addLogEntry("[DEBUG] final escaped: {$escapedValue}"); + } + } } } // Return the generated environment variables + if (isDev()) { + $this->application_deployment_queue->addLogEntry('[DEBUG] ========================================'); + $this->application_deployment_queue->addLogEntry("[DEBUG] Total build-time env variables: {$envs->count()}"); + $this->application_deployment_queue->addLogEntry('[DEBUG] ========================================'); + } + return $envs; } diff --git a/bootstrap/helpers/docker.php b/bootstrap/helpers/docker.php index b63c3fc3b..8653a96d0 100644 --- a/bootstrap/helpers/docker.php +++ b/bootstrap/helpers/docker.php @@ -1120,6 +1120,72 @@ function escapeDollarSign($value) return str_replace($search, $replace, $value); } +/** + * Escape a value for use in a bash .env file that will be sourced with 'source' command + * Wraps the value in single quotes and escapes any single quotes within the value + * + * @param string|null $value The value to escape + * @return string The escaped value wrapped in single quotes + */ +function escapeBashEnvValue(?string $value): string +{ + // Handle null or empty values + if ($value === null || $value === '') { + return "''"; + } + + // Replace single quotes with '\'' (end quote, escaped quote, start quote) + // This is the standard way to escape single quotes in bash single-quoted strings + $escaped = str_replace("'", "'\\''", $value); + + // Wrap in single quotes + return "'{$escaped}'"; +} + +/** + * Escape a value for bash double-quoted strings (allows $VAR expansion) + * + * This function wraps values in double quotes while escaping special characters, + * but preserves valid bash variable references like $VAR and ${VAR}. + * + * @param string|null $value The value to escape + * @return string The escaped value wrapped in double quotes + */ +function escapeBashDoubleQuoted(?string $value): string +{ + // Handle null or empty values + if ($value === null || $value === '') { + return '""'; + } + + // Step 1: Escape backslashes first (must be done before other escaping) + $escaped = str_replace('\\', '\\\\', $value); + + // Step 2: Escape double quotes + $escaped = str_replace('"', '\\"', $escaped); + + // Step 3: Escape backticks (command substitution) + $escaped = str_replace('`', '\\`', $escaped); + + // Step 4: Escape invalid $ patterns while preserving valid variable references + // Valid patterns to keep: + // - $VAR_NAME (alphanumeric + underscore, starting with letter or _) + // - ${VAR_NAME} (brace expansion) + // - $0-$9 (positional parameters) + // Invalid patterns to escape: $&, $#, $$, $*, $@, $!, $(, etc. + + // Match $ followed by anything that's NOT a valid variable start + // Valid variable starts: letter, underscore, digit (for $0-$9), or open brace + $escaped = preg_replace( + '/\$(?![a-zA-Z_0-9{])/', + '\\\$', + $escaped + ); + + // Wrap in double quotes + return "\"{$escaped}\""; +} + /** * Generate Docker build arguments from environment variables collection * Returns only keys (no values) since values are sourced from environment via export diff --git a/tests/Unit/BashEnvEscapingTest.php b/tests/Unit/BashEnvEscapingTest.php new file mode 100644 index 000000000..7b81c041e --- /dev/null +++ b/tests/Unit/BashEnvEscapingTest.php @@ -0,0 +1,307 @@ +toBe("'simple_value'"); +}); + +test('escapeBashEnvValue handles special bash characters', function () { + $specialChars = [ + '$&#)@*~$&@(~#&#%(*$324803129&$#@!)*&$', + '#*#&412)$&#*!%)!@&#)*~@!&$)@*#%^)*@#!)#@~321', + 'value with spaces and $variables', + 'value with `backticks`', + 'value with "double quotes"', + 'value|with|pipes', + 'value;with;semicolons', + 'value&with&ersands', + 'value(with)parentheses', + 'value{with}braces', + 'value[with]brackets', + 'valueangles', + 'value*with*asterisks', + 'value?with?questions', + 'value!with!exclamations', + 'value~with~tildes', + 'value^with^carets', + 'value%with%percents', + 'value@with@ats', + 'value#with#hashes', + ]; + + foreach ($specialChars as $value) { + $result = escapeBashEnvValue($value); + + // Should be wrapped in single quotes + expect($result)->toStartWith("'"); + expect($result)->toEndWith("'"); + + // Should contain the original value (or escaped version) + expect($result)->toContain($value); + } +}); + +test('escapeBashEnvValue escapes single quotes correctly', function () { + // Single quotes in bash single-quoted strings must be escaped as '\'' + $value = "it's a value with 'single quotes'"; + $result = escapeBashEnvValue($value); + + // The result should replace ' with '\'' + expect($result)->toBe("'it'\\''s a value with '\\''single quotes'\\'''"); +}); + +test('escapeBashEnvValue handles empty values', function () { + $result = escapeBashEnvValue(''); + expect($result)->toBe("''"); +}); + +test('escapeBashEnvValue handles null values', function () { + $result = escapeBashEnvValue(null); + expect($result)->toBe("''"); +}); + +test('escapeBashEnvValue handles values with only special characters', function () { + $value = '$#@!*&^%()[]{}|;~`?"<>'; + $result = escapeBashEnvValue($value); + + // Should be wrapped and contain all special characters + expect($result)->toBe("'{$value}'"); +}); + +test('escapeBashEnvValue handles multiline values', function () { + $value = "line1\nline2\nline3"; + $result = escapeBashEnvValue($value); + + // Should preserve newlines + expect($result)->toContain("\n"); + expect($result)->toStartWith("'"); + expect($result)->toEndWith("'"); +}); + +test('escapeBashEnvValue handles values from user example', function () { + $literal = '$&#)@*~$&@(~#&#%(*$324803129&$#@!)*&$'; + $weird = '#*#&412)$&#*!%)!@&#)*~@!&$)@*#%^)*@#!)#@~321'; + + $escapedLiteral = escapeBashEnvValue($literal); + $escapedWeird = escapeBashEnvValue($weird); + + // These should be safely wrapped in single quotes + expect($escapedLiteral)->toBe("'{$literal}'"); + expect($escapedWeird)->toBe("'{$weird}'"); + + // Test that when written to a file and sourced, they would work + // Format: KEY=VALUE + $envLine1 = "literal={$escapedLiteral}"; + $envLine2 = "weird={$escapedWeird}"; + + // These should be valid bash assignment statements + expect($envLine1)->toStartWith('literal='); + expect($envLine2)->toStartWith('weird='); +}); + +test('escapeBashEnvValue handles backslashes', function () { + $value = 'path\\to\\file'; + $result = escapeBashEnvValue($value); + + // Backslashes should be preserved in single quotes + expect($result)->toBe("'{$value}'"); + expect($result)->toContain('\\'); +}); + +test('escapeBashEnvValue handles dollar signs correctly', function () { + $value = '$HOME and $PATH'; + $result = escapeBashEnvValue($value); + + // Dollar signs should NOT be expanded in single quotes + expect($result)->toBe("'{$value}'"); + expect($result)->toContain('$HOME'); + expect($result)->toContain('$PATH'); +}); + +test('escapeBashEnvValue handles complex combination of special characters and single quotes', function () { + $value = "it's \$weird with 'quotes' and \$variables"; + $result = escapeBashEnvValue($value); + + // Should escape the single quotes + expect($result)->toContain("'\\''"); + // Should contain the dollar signs without expansion + expect($result)->toContain('$weird'); + expect($result)->toContain('$variables'); +}); + +test('stripping quotes from real_value before escaping (literal/multiline simulation)', function () { + // Simulate what happens with literal/multiline env vars + // Their real_value comes back wrapped in quotes: 'value' + $realValueWithQuotes = "'it's a value with 'quotes''"; + + // Strip outer quotes + $stripped = trim($realValueWithQuotes, "'"); + expect($stripped)->toBe("it's a value with 'quotes"); + + // Then apply bash escaping + $result = escapeBashEnvValue($stripped); + + // Should properly escape the internal single quotes + expect($result)->toContain("'\\''"); + // Should start and end with quotes + expect($result)->toStartWith("'"); + expect($result)->toEndWith("'"); +}); + +test('handling literal env with special bash characters', function () { + // Simulate literal/multiline env with special characters + $realValueWithQuotes = "'#*#&412)\$&#*!%)!@&#)*~@!\&\$)@*#%^)*@#!)#@~321'"; + + // Strip outer quotes + $stripped = trim($realValueWithQuotes, "'"); + + // Apply bash escaping + $result = escapeBashEnvValue($stripped); + + // Should be properly quoted for bash + expect($result)->toStartWith("'"); + expect($result)->toEndWith("'"); + // Should contain all the special characters + expect($result)->toContain('#*#&412)'); + expect($result)->toContain('$&#*!%'); +}); + +// ==================== Tests for escapeBashDoubleQuoted() ==================== + +test('escapeBashDoubleQuoted wraps simple values in double quotes', function () { + $result = escapeBashDoubleQuoted('simple_value'); + expect($result)->toBe('"simple_value"'); +}); + +test('escapeBashDoubleQuoted handles null values', function () { + $result = escapeBashDoubleQuoted(null); + expect($result)->toBe('""'); +}); + +test('escapeBashDoubleQuoted handles empty values', function () { + $result = escapeBashDoubleQuoted(''); + expect($result)->toBe('""'); +}); + +test('escapeBashDoubleQuoted preserves valid variable references', function () { + $value = '$SOURCE_COMMIT'; + $result = escapeBashDoubleQuoted($value); + + // Should preserve $SOURCE_COMMIT for expansion + expect($result)->toBe('"$SOURCE_COMMIT"'); + expect($result)->toContain('$SOURCE_COMMIT'); +}); + +test('escapeBashDoubleQuoted preserves multiple variable references', function () { + $value = '$VAR1 and $VAR2 and $VAR_NAME_3'; + $result = escapeBashDoubleQuoted($value); + + // All valid variables should be preserved + expect($result)->toBe('"$VAR1 and $VAR2 and $VAR_NAME_3"'); +}); + +test('escapeBashDoubleQuoted preserves brace expansion variables', function () { + $value = '${SOURCE_COMMIT} and ${VAR_NAME}'; + $result = escapeBashDoubleQuoted($value); + + // Brace variables should be preserved + expect($result)->toBe('"${SOURCE_COMMIT} and ${VAR_NAME}"'); +}); + +test('escapeBashDoubleQuoted escapes invalid dollar patterns', function () { + // Invalid patterns: $&, $#, $$, $*, $@, $!, etc. + $value = '$&#)@*~$&@(~#&#%(*$324803129&$#@!)*&$'; + $result = escapeBashDoubleQuoted($value); + + // Invalid $ should be escaped + expect($result)->toContain('\\$&#'); + expect($result)->toContain('\\$&@'); + expect($result)->toContain('\\$#@'); + // Should be wrapped in double quotes + expect($result)->toStartWith('"'); + expect($result)->toEndWith('"'); +}); + +test('escapeBashDoubleQuoted handles mixed valid and invalid dollar signs', function () { + $value = '$SOURCE_COMMIT and $&#invalid'; + $result = escapeBashDoubleQuoted($value); + + // Valid variable preserved, invalid $ escaped + expect($result)->toBe('"$SOURCE_COMMIT and \\$&#invalid"'); +}); + +test('escapeBashDoubleQuoted escapes double quotes', function () { + $value = 'value with "double quotes"'; + $result = escapeBashDoubleQuoted($value); + + // Double quotes should be escaped + expect($result)->toBe('"value with \\"double quotes\\""'); +}); + +test('escapeBashDoubleQuoted escapes backticks', function () { + $value = 'value with `backticks`'; + $result = escapeBashDoubleQuoted($value); + + // Backticks should be escaped (prevents command substitution) + expect($result)->toBe('"value with \\`backticks\\`"'); +}); + +test('escapeBashDoubleQuoted escapes backslashes', function () { + $value = 'path\\to\\file'; + $result = escapeBashDoubleQuoted($value); + + // Backslashes should be escaped + expect($result)->toBe('"path\\\\to\\\\file"'); +}); + +test('escapeBashDoubleQuoted handles positional parameters', function () { + $value = 'args: $0 $1 $2 $9'; + $result = escapeBashDoubleQuoted($value); + + // Positional parameters should be preserved + expect($result)->toBe('"args: $0 $1 $2 $9"'); +}); + +test('escapeBashDoubleQuoted handles special variable $_', function () { + $value = 'last arg: $_'; + $result = escapeBashDoubleQuoted($value); + + // $_ should be preserved + expect($result)->toBe('"last arg: $_"'); +}); + +test('escapeBashDoubleQuoted handles complex real-world scenario', function () { + // Mix of valid vars, invalid $, quotes, and special chars + $value = '$SOURCE_COMMIT with $&#special and "quotes" and `cmd`'; + $result = escapeBashDoubleQuoted($value); + + // Valid var preserved, invalid $ escaped, quotes/backticks escaped + expect($result)->toBe('"$SOURCE_COMMIT with \\$&#special and \\"quotes\\" and \\`cmd\\`"'); +}); + +test('escapeBashDoubleQuoted allows expansion in bash', function () { + // This is a logical test - the actual expansion happens in bash + // We're verifying the format is correct + $value = '$SOURCE_COMMIT'; + $result = escapeBashDoubleQuoted($value); + + // Should be: "$SOURCE_COMMIT" which bash will expand + expect($result)->toBe('"$SOURCE_COMMIT"'); + expect($result)->not->toContain('\\$SOURCE'); +}); + +test('comparison between single and double quote escaping', function () { + $value = '$SOURCE_COMMIT'; + + $singleQuoted = escapeBashEnvValue($value); + $doubleQuoted = escapeBashDoubleQuoted($value); + + // Single quotes prevent expansion + expect($singleQuoted)->toBe("'\$SOURCE_COMMIT'"); + + // Double quotes allow expansion + expect($doubleQuoted)->toBe('"$SOURCE_COMMIT"'); + + // They're different! + expect($singleQuoted)->not->toBe($doubleQuoted); +});