Fix Nixpacks null environment variable parsing error #7493

andrasbacsai · 2025-12-04T14:10:57Z

Changes

Filter out null and empty environment variables when generating Nixpacks build configuration
Added validation in generate_nixpacks_env_variables() for both user-defined and COOLIFY_* environment variables
Added comprehensive unit tests to verify the fix handles null values, empty strings, zero values, and preview deployments

Issues

Fixes Nixpacks JSON parsing error: "invalid type: null, expected a string"

This ensures that environment variables with null or empty values are not passed to Nixpacks, preventing deployment failures.

fix [Bug]: Error: Failed to parse Nixpacks config file /artifacts/thegameplan.json #6830

Filter out null and empty environment variables when generating Nixpacks build configuration to prevent JSON parsing errors. Environment variables with null or empty values were being passed as `--env KEY=` which created invalid JSON with null values, causing deployment failures. This fix ensures only valid non-empty environment variables are included in both user-defined and auto-generated COOLIFY_* environment variables. 🤖 Generated with Claude Code Co-Authored-By: Claude <[email protected]>

Copilot

Pull request overview

This PR fixes a Nixpacks JSON parsing error that occurred when null or empty environment variables were passed to the Nixpacks build command. The fix adds filtering logic to exclude null and empty string values from both user-defined environment variables and COOLIFY_* system environment variables before they are included in Nixpacks --env arguments.

Key changes:

Enhanced environment variable filtering in generate_nixpacks_env_variables() to check for both null and empty strings
Applied filtering to both regular deployments and preview (pull request) deployments
Added comprehensive unit tests covering various edge cases including null values, empty strings, zero values, and preview deployments

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
app/Jobs/ApplicationDeploymentJob.php	Added empty string validation (`$value !== ''`) to the existing null checks when filtering environment variables for Nixpacks, preventing empty `--env KEY=` arguments that cause JSON parsing errors
tests/Unit/ApplicationDeploymentNixpacksNullEnvTest.php	Added comprehensive unit tests verifying null/empty value filtering, preservation of valid falsy values like "0" and "false", and proper handling of preview deployments

Copilot · 2025-12-04T14:19:02Z

tests/Unit/ApplicationDeploymentNixpacksNullEnvTest.php

+    expect($envArgs)->not->toContain('COOLIFY_BRANCH');
+
+    // Verify no environment variables end with just '=' (which indicates null/empty value)
+    expect($envArgs)->not->toMatch('/--env [A-Z_]+=$/');


The regex pattern /--env [A-Z_]+=$/ uses the $ anchor which matches the end of the entire string, not individual variable entries. Since $envArgs is a space-separated string like "--env VAR1=val1 --env VAR2=val2", this pattern would only match if a variable with an empty value appears at the very end of the string.

Consider using /--env [A-Z_]+= / (already present on line 105) or /--env [A-Z_]+=(?: |$)/ to match empty values anywhere in the string, not just at the end.

Suggested change

expect($envArgs)->not->toMatch('/--env [A-Z_]+=$/');

expect($envArgs)->not->toMatch('/--env [A-Z_]+=(?: |$)/');

Copilot · 2025-12-04T14:19:02Z

app/Jobs/ApplicationDeploymentJob.php

            foreach ($this->application->nixpacks_environment_variables_preview as $env) {
-                if (! is_null($env->real_value)) {
+                if (! is_null($env->real_value) && $env->real_value !== '') {
                    $this->env_nixpacks_args->push("--env {$env->key}={$env->real_value}");


Shell command injection risk: {$env->key} and {$env->real_value} are concatenated into --env {$env->key}={$env->real_value} and later used in nixpacks_command, which is executed via executeInDocker() (bash -c '...'). A crafted env var value with a single quote or shell metacharacters can terminate the quoting and inject arbitrary commands. Fix by executing nixpacks without a shell and passing arguments safely (array/argv), or by rigorously escaping both key and value for a single-quoted shell context and quoting KEY=VALUE, or by using an env-file input if available.

Copilot · 2025-12-04T14:19:03Z

app/Jobs/ApplicationDeploymentJob.php

-            $this->env_nixpacks_args->push("--env {$key}={$value}");
+            // Only add environment variables with non-null and non-empty values
+            if (! is_null($value) && $value !== '') {
+                $this->env_nixpacks_args->push("--env {$key}={$value}");


Shell command injection risk: {$key} and its $value are interpolated into --env {$key}={$value} and then included in nixpacks_command executed via executeInDocker() (bash -c '...'). If $value contains a single quote or shell metacharacters, it can break out of the quoting and run arbitrary commands inside the container. Fix by avoiding shell concatenation (invoke nixpacks directly without bash -c and pass args as an array), or at minimum escape for the single-quoted shell context and quote KEY=VALUE, or leverage an env-file mechanism if supported.

Copilot · 2025-12-04T14:19:03Z

app/Jobs/ApplicationDeploymentJob.php

            foreach ($this->application->nixpacks_environment_variables as $env) {
-                if (! is_null($env->real_value)) {
+                if (! is_null($env->real_value) && $env->real_value !== '') {
                    $this->env_nixpacks_args->push("--env {$env->key}={$env->real_value}");


Shell command injection risk: {$env->key} and {$env->real_value} are concatenated into --env {$env->key}={$env->real_value} and later embedded into nixpacks_command that runs via executeInDocker() (bash -c '...'). An attacker who can set an environment variable value containing a single quote or shell metacharacters can break out of the single-quoted context and execute arbitrary commands in the build container. Fix by avoiding shell string concatenation (execute nixpacks without bash -c and pass args as an array), or at minimum apply robust shell escaping to both key and value and wrap KEY=VALUE in quotes (e.g., escape ' as '�"'"' via str_replace), or use an env-file mechanism if supported.

coderabbitai · 2025-12-04T14:43:56Z

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

This pull request refines environment variable handling in the ApplicationDeploymentJob by adding validation to exclude null and empty string values from Nixpacks build context. The changes filter environment variables when constructing build arguments, affecting both standard and preview environment deployments. A corresponding unit test suite validates the filtering logic, ensuring that null or empty variables are properly excluded while non-empty values are preserved, and verifies correct behavior across various scenarios including preview deployments and edge cases with zero or false string values.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch nixpacks-null-env-fix

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9e0fa03 and 42f08a9.

📒 Files selected for processing (2)

app/Jobs/ApplicationDeploymentJob.php (2 hunks)
tests/Unit/ApplicationDeploymentNixpacksNullEnvTest.php (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.php: Use PHP 8.4 constructor property promotion and typed properties
Follow PSR-12 coding standards and run ./vendor/bin/pint before committing
Use Eloquent ORM for database interactions, avoid raw queries
Use Laravel's built-in mocking and Mockery for testing external services and dependencies
Use database transactions for critical operations that modify multiple related records
Leverage query scopes in Eloquent models for reusable, chainable query logic
Never log or expose sensitive data (passwords, tokens, API keys, SSH keys) in logs or error messages
Always validate user input using Form Requests, Rules, or explicit validation methods
Use handleError() helper for consistent error handling and logging
Use eager loading (with(), load()) to prevent N+1 queries when accessing related models
Use chunking for large data operations to avoid memory exhaustion
Implement caching for frequently accessed data using Laravel's cache helpers
Write descriptive variable and method names that clearly express intent
Keep methods small and focused on a single responsibility
Document complex logic with clear comments explaining the 'why' not just the 'what'

Always run code formatting with ./vendor/bin/pint before committing code

Files:

app/Jobs/ApplicationDeploymentJob.php
tests/Unit/ApplicationDeploymentNixpacksNullEnvTest.php

app/Jobs/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

Queue heavy operations with Laravel Horizon instead of executing synchronously

Files:

app/Jobs/ApplicationDeploymentJob.php

tests/Unit/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

Unit tests MUST NOT use database. Use mocking for dependencies. Run with ./vendor/bin/pest tests/Unit

Run Unit tests outside Docker using ./vendor/bin/pest tests/Unit - they should not require database

Files:

tests/Unit/ApplicationDeploymentNixpacksNullEnvTest.php

tests/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.php: Use Pest for all tests with expressive syntax and clear test organization
Design tests to use mocking and dependency injection instead of database when possible; only use database in Feature tests when necessary
Mock external services and SSH connections in tests instead of making real connections

Files:

tests/Unit/ApplicationDeploymentNixpacksNullEnvTest.php

🧬 Code graph analysis (2)

app/Jobs/ApplicationDeploymentJob.php (4)

app/Models/EnvironmentVariable.php (2)

key (242-247)

value (92-98)

app/Models/ApplicationSetting.php (1)

application (46-49)

app/Models/ApplicationPreview.php (1)

application (65-68)

app/Models/Application.php (1)

nixpacks_environment_variables_preview (879-884)

tests/Unit/ApplicationDeploymentNixpacksNullEnvTest.php (1)

app/Jobs/ApplicationDeploymentJob.php (1)

ApplicationDeploymentJob (40-4079)

🔇 Additional comments (1)

tests/Unit/ApplicationDeploymentNixpacksNullEnvTest.php (1)

108-299: Preview, all‑null, and zero/false tests give solid confidence in the new filtering

The remaining three tests nicely exercise the other branches of Judgment Day:

Preview deployments (pull_request_id != 0) with nixpacks_environment_variables_preview and COOLIFY_FQDN.

The “everything is null/empty” case, asserting env_nixpacks_args becomes the empty string.

Preservation of legit “falsy” string values ('0', 'false'), ensuring the filter is strictly !== '' and doesn’t over‑eagerly purge valid configs.

Mocks + reflection keep this in pure unit‑test territory without dragging in a database or real jobs — ideal for fast, self‑hosted pipelines, not some flaky serverless nonsense.

No changes needed here.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot AI review requested due to automatic review settings December 4, 2025 14:10

Copilot started reviewing on behalf of andrasbacsai December 4, 2025 14:11 View session

Copilot finished reviewing on behalf of andrasbacsai December 4, 2025 14:14

Copilot AI reviewed Dec 4, 2025

View reviewed changes

andrasbacsai added the 🐰 Release The Rabbit Run CodeRabbitAI review label Dec 4, 2025

andrasbacsai merged commit 558a885 into next Dec 4, 2025
10 of 12 checks passed

andrasbacsai deleted the nixpacks-null-env-fix branch December 4, 2025 15:29

github-actions bot removed the 🐰 Release The Rabbit Run CodeRabbitAI review label Dec 4, 2025

andrasbacsai mentioned this pull request Dec 7, 2025

v4.0.0-beta.453 #7467

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix Nixpacks null environment variable parsing error #7493

Fix Nixpacks null environment variable parsing error #7493

Uh oh!

andrasbacsai commented Dec 4, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

Copilot AI Dec 4, 2025

Uh oh!

coderabbitai bot commented Dec 4, 2025

Other AI code review bot(s) detected

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	expect($envArgs)->not->toMatch('/--env [A-Z_]+=$/');
	expect($envArgs)->not->toMatch('/--env [A-Z_]+=(?: \|$)/');

Uh oh!

Fix Nixpacks null environment variable parsing error #7493

Fix Nixpacks null environment variable parsing error #7493

Uh oh!

Conversation

andrasbacsai commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Issues

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot commented Dec 4, 2025

Other AI code review bot(s) detected

Walkthrough

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

andrasbacsai commented Dec 4, 2025 •

edited

Loading