Merge remote-tracking branch 'origin/next' into fix/railpack-buildtime-env-interpolation

tests/Unit/DeployKeyDedicatedPathTest.php

@@ -4,11 +4,50 @@ use App\Models\Application;
 use App\Models\ApplicationSetting;
 use App\Models\GitlabApp;
 use App\Models\PrivateKey;
+use Illuminate\Support\Collection;
 
 afterEach(function () {
     Mockery::close();
 });
 
+function commandStrings(array|Collection|string $commands): Collection
+{
+    if (is_string($commands)) {
+        return collect([$commands]);
+    }
+
+    return collect($commands)->map(fn ($command) => data_get($command, 'command') ?? $command[0] ?? $command);
+}
+
+function privateKeyMaterializationCommands(array|Collection|string $commands): Collection
+{
+    if (is_string($commands)) {
+        $commands = [$commands];
+    }
+
+    return collect($commands)->filter(fn ($command) => str(commandStrings([$command])->first())->contains('base64 -d | tee /root/.ssh/id_rsa_coolify_'));
+}
+
+function expectCommandListToContain(array|Collection|string $commands, string $expected): void
+{
+    expect(commandStrings($commands)->implode(' && '))->toContain($expected);
+}
+
+function expectCommandListNotToContain(array|Collection|string $commands, string $expected): void
+{
+    expect(commandStrings($commands)->implode(' && '))->not->toContain($expected);
+}
+
+function expectPrivateKeyMaterializationCommandsSkipLogging(array|Collection|string $commands): void
+{
+    $keyCommands = privateKeyMaterializationCommands($commands);
+
+    expect($keyCommands)->not->toBeEmpty();
+    $keyCommands->each(function ($command): void {
+        expect(data_get($command, 'skip_command_log'))->toBeTrue();
+    });
+}
+
 /**
  * Git operations authenticate with the SSH key assigned in the UI. Coolify writes that key to a
  * per-deployment path (/root/.ssh/id_rsa_coolify_<deployment_uuid>) instead of the shared
@@ -19,6 +58,24 @@ afterEach(function () {
  */
 $keyPath = '/root/.ssh/id_rsa_coolify_test-deployment-uuid';
 
+it('skips logging the docker deploy key materialization command before ls-remote', function () {
+    $source = file_get_contents(__DIR__.'/../../app/Jobs/ApplicationDeploymentJob.php');
+    $commandPosition = strpos($source, 'base64 -d | tee {$customSshKeyLocation}');
+
+    expect($commandPosition)->not->toBeFalse()
+        ->and(substr($source, $commandPosition, 200))->toContain("'skip_command_log' => true");
+});
+
+it('supports skipping command log entries without adding a hidden command entry', function () {
+    $source = file_get_contents(__DIR__.'/../../app/Traits/ExecuteRemoteCommand.php');
+
+    expect($source)
+        ->toContain('$skip_command_log = data_get($single_command, \'skip_command_log\', false);')
+        ->toContain('if ($command_hidden && ! $skip_command_log && isset($this->application_deployment_queue))')
+        ->toContain('use ($command, $hidden, $customType, $append, $command_hidden, $skip_command_log)')
+        ->toContain('\'command\' => $skip_command_log || $command_hidden ? null : $this->redact_sensitive_info($command),');
+});
+
 it('writes a deploy key to a per-deployment path and cleans it up for ls-remote on the host', function () use ($keyPath) {
     $privateKey = Mockery::mock(PrivateKey::class)->makePartial();
     $privateKey->shouldReceive('getAttribute')->with('private_key')->andReturn('fake-private-key');
@@ -31,11 +88,11 @@ it('writes a deploy key to a per-deployment path and cleans it up for ls-remote
 
     $result = $application->generateGitLsRemoteCommands('test-deployment-uuid', false);
 
-    expect($result['commands'])
-        ->toContain("tee {$keyPath}")
-        ->toContain("-i {$keyPath} -o IdentitiesOnly=yes")
-        ->toContain("trap 'rm -f {$keyPath}' EXIT") // removed when the shell exits
-        ->not->toContain('tee /root/.ssh/id_rsa >'); // never overwrites the host root's own key
+    expectCommandListToContain($result['commands'], "tee {$keyPath}");
+    expectCommandListToContain($result['commands'], "-i {$keyPath} -o IdentitiesOnly=yes");
+    expectCommandListToContain($result['commands'], "trap 'rm -f {$keyPath}' EXIT"); // removed when the shell exits
+    expectCommandListNotToContain($result['commands'], 'tee /root/.ssh/id_rsa >'); // never overwrites the host root's own key
+    expectPrivateKeyMaterializationCommandsSkipLogging($result['commands']);
 });
 
 it('writes a deploy key to a per-deployment path for ls-remote inside docker without a trap', function () use ($keyPath) {
@@ -50,11 +107,11 @@ it('writes a deploy key to a per-deployment path for ls-remote inside docker wit
 
     $result = $application->generateGitLsRemoteCommands('test-deployment-uuid', true);
 
-    expect($result['commands'])
-        ->toContain("tee {$keyPath}")
-        ->toContain("-i {$keyPath} -o IdentitiesOnly=yes")
-        ->not->toContain('trap ') // ephemeral container, no cleanup needed
-        ->not->toContain('tee /root/.ssh/id_rsa >');
+    expectCommandListToContain($result['commands'], "tee {$keyPath}");
+    expectCommandListToContain($result['commands'], "-i {$keyPath} -o IdentitiesOnly=yes");
+    expectCommandListNotToContain($result['commands'], 'trap '); // ephemeral container, no cleanup needed
+    expectCommandListNotToContain($result['commands'], 'tee /root/.ssh/id_rsa >');
+    expectPrivateKeyMaterializationCommandsSkipLogging($result['commands']);
 });
 
 it('writes a GitLab source private key to a per-deployment path with cleanup on the host', function () use ($keyPath) {
@@ -77,11 +134,11 @@ it('writes a GitLab source private key to a per-deployment path with cleanup on
 
     $result = $application->generateGitLsRemoteCommands('test-deployment-uuid', false);
 
-    expect($result['commands'])
-        ->toContain("tee {$keyPath}")
-        ->toContain("-i {$keyPath} -o IdentitiesOnly=yes")
-        ->toContain("trap 'rm -f {$keyPath}' EXIT")
-        ->not->toContain('tee /root/.ssh/id_rsa >');
+    expectCommandListToContain($result['commands'], "tee {$keyPath}");
+    expectCommandListToContain($result['commands'], "-i {$keyPath} -o IdentitiesOnly=yes");
+    expectCommandListToContain($result['commands'], "trap 'rm -f {$keyPath}' EXIT");
+    expectCommandListNotToContain($result['commands'], 'tee /root/.ssh/id_rsa >');
+    expectPrivateKeyMaterializationCommandsSkipLogging($result['commands']);
 });
 
 it('writes a deploy key to a per-deployment path and cleans it up when cloning on the host', function () use ($keyPath) {
@@ -104,11 +161,11 @@ it('writes a deploy key to a per-deployment path and cleans it up when cloning o
     // exec_in_docker = false → the loadComposeFile / host clone path
     $result = $application->generateGitImportCommands('test-deployment-uuid', 0, null, false);
 
-    expect($result['commands'])
-        ->toContain("tee {$keyPath}")
-        ->toContain("-i {$keyPath} -o IdentitiesOnly=yes")
-        ->toContain("trap 'rm -f {$keyPath}' EXIT")
-        ->not->toContain('tee /root/.ssh/id_rsa >');
+    expectCommandListToContain($result['commands'], "tee {$keyPath}");
+    expectCommandListToContain($result['commands'], "-i {$keyPath} -o IdentitiesOnly=yes");
+    expectCommandListToContain($result['commands'], "trap 'rm -f {$keyPath}' EXIT");
+    expectCommandListNotToContain($result['commands'], 'tee /root/.ssh/id_rsa >');
+    expectPrivateKeyMaterializationCommandsSkipLogging($result['commands']);
 });
 
 it('writes a GitLab source private key to a per-deployment path and cleans it up when cloning on the host', function () use ($keyPath) {
@@ -137,11 +194,11 @@ it('writes a GitLab source private key to a per-deployment path and cleans it up
 
     $result = $application->generateGitImportCommands('test-deployment-uuid', 0, null, false);
 
-    expect($result['commands'])
-        ->toContain("tee {$keyPath}")
-        ->toContain("-i {$keyPath} -o IdentitiesOnly=yes")
-        ->toContain("trap 'rm -f {$keyPath}' EXIT")
-        ->not->toContain('tee /root/.ssh/id_rsa >');
+    expectCommandListToContain($result['commands'], "tee {$keyPath}");
+    expectCommandListToContain($result['commands'], "-i {$keyPath} -o IdentitiesOnly=yes");
+    expectCommandListToContain($result['commands'], "trap 'rm -f {$keyPath}' EXIT");
+    expectCommandListNotToContain($result['commands'], 'tee /root/.ssh/id_rsa >');
+    expectPrivateKeyMaterializationCommandsSkipLogging($result['commands']);
 });
 
 it('uses the per-deployment deploy key for pull request fetches', function () use ($keyPath) {
@@ -163,9 +220,9 @@ it('uses the per-deployment deploy key for pull request fetches', function () us
 
     $result = $application->generateGitImportCommands('test-deployment-uuid', 123, 'github', false);
 
-    expect($result['commands'])
-        ->toContain("GIT_SSH_COMMAND=\"ssh -o ConnectTimeout=30 -p 22 -o Port=22 -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i {$keyPath} -o IdentitiesOnly=yes\" git fetch origin pull/123/head:pr-123-coolify")
-        ->not->toContain('GIT_SSH_COMMAND="ssh -o ConnectTimeout=30 -p 22 -o Port=22 -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i /root/.ssh/id_rsa" git fetch origin pull/123/head:pr-123-coolify');
+    expectCommandListToContain($result['commands'], "GIT_SSH_COMMAND=\"ssh -o ConnectTimeout=30 -p 22 -o Port=22 -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i {$keyPath} -o IdentitiesOnly=yes\" git fetch origin pull/123/head:pr-123-coolify");
+    expectCommandListNotToContain($result['commands'], 'GIT_SSH_COMMAND="ssh -o ConnectTimeout=30 -p 22 -o Port=22 -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i /root/.ssh/id_rsa" git fetch origin pull/123/head:pr-123-coolify');
+    expectPrivateKeyMaterializationCommandsSkipLogging($result['commands']);
 });
 
 it('does not force a missing per-deployment key for other repository pull request fetches', function () use ($keyPath) {
@@ -183,7 +240,6 @@ it('does not force a missing per-deployment key for other repository pull reques
 
     $result = $application->generateGitImportCommands('test-deployment-uuid', 123, 'github', false);
 
-    expect($result['commands'])
-        ->toContain('GIT_SSH_COMMAND="ssh -o ConnectTimeout=30 -p 22 -o Port=22 -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i /root/.ssh/id_rsa" git fetch origin pull/123/head:pr-123-coolify')
-        ->not->toContain($keyPath);
+    expectCommandListToContain($result['commands'], 'GIT_SSH_COMMAND="ssh -o ConnectTimeout=30 -p 22 -o Port=22 -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i /root/.ssh/id_rsa" git fetch origin pull/123/head:pr-123-coolify');
+    expectCommandListNotToContain($result['commands'], $keyPath);
 });

tests/Unit/GitlabSourceCommandsTest.php

@@ -3,11 +3,45 @@
 use App\Models\Application;
 use App\Models\GitlabApp;
 use App\Models\PrivateKey;
+use Illuminate\Support\Collection;
 
 afterEach(function () {
     Mockery::close();
 });
 
+function gitlabCommandStrings(array|Collection|string $commands): Collection
+{
+    if (is_string($commands)) {
+        return collect([$commands]);
+    }
+
+    return collect($commands)->map(fn ($command) => data_get($command, 'command') ?? $command[0] ?? $command);
+}
+
+function expectGitlabCommandListToContain(array|Collection|string $commands, string $expected): void
+{
+    expect(gitlabCommandStrings($commands)->implode(' && '))->toContain($expected);
+}
+
+function expectGitlabCommandListNotToContain(array|Collection|string $commands, string $expected): void
+{
+    expect(gitlabCommandStrings($commands)->implode(' && '))->not->toContain($expected);
+}
+
+function expectGitlabPrivateKeyMaterializationCommandsSkipLogging(array|Collection|string $commands): void
+{
+    if (is_string($commands)) {
+        $commands = [$commands];
+    }
+
+    $keyCommands = collect($commands)->filter(fn ($command) => str(data_get($command, 'command') ?? $command[0] ?? $command)->contains('base64 -d | tee /root/.ssh/id_rsa_coolify_'));
+
+    expect($keyCommands)->not->toBeEmpty();
+    $keyCommands->each(function ($command): void {
+        expect(data_get($command, 'skip_command_log'))->toBeTrue();
+    });
+}
+
 it('generates ls-remote commands for GitLab source with private key', function () {
     $deploymentUuid = 'test-deployment-uuid';
 
@@ -15,7 +49,8 @@ it('generates ls-remote commands for GitLab source with private key', function (
     $privateKey->shouldReceive('getAttribute')->with('private_key')->andReturn('fake-private-key');
 
     $gitlabSource = Mockery::mock(GitlabApp::class)->makePartial();
-    $gitlabSource->shouldReceive('getMorphClass')->andReturn(\App\Models\GitlabApp::class);
+    $gitlabSource->shouldReceive('getMorphClass')->andReturn(GitlabApp::class);
+    $gitlabSource->shouldReceive('getAttribute')->with('html_url')->andReturn('https://gitlab.com');
     $gitlabSource->shouldReceive('getAttribute')->with('privateKey')->andReturn($privateKey);
     $gitlabSource->shouldReceive('getAttribute')->with('private_key_id')->andReturn(1);
     $gitlabSource->shouldReceive('getAttribute')->with('custom_port')->andReturn(22);
@@ -34,16 +69,18 @@ it('generates ls-remote commands for GitLab source with private key', function (
 
     expect($result)->toBeArray();
     expect($result)->toHaveKey('commands');
-    expect($result['commands'])->toContain('git ls-remote');
-    expect($result['commands'])->toContain('id_rsa');
-    expect($result['commands'])->toContain('mkdir -p /root/.ssh');
+    expectGitlabCommandListToContain($result['commands'], 'git ls-remote');
+    expectGitlabCommandListToContain($result['commands'], 'id_rsa');
+    expectGitlabCommandListToContain($result['commands'], 'mkdir -p /root/.ssh');
+    expectGitlabPrivateKeyMaterializationCommandsSkipLogging($result['commands']);
 });
 
 it('generates ls-remote commands for GitLab source without private key', function () {
     $deploymentUuid = 'test-deployment-uuid';
 
     $gitlabSource = Mockery::mock(GitlabApp::class)->makePartial();
-    $gitlabSource->shouldReceive('getMorphClass')->andReturn(\App\Models\GitlabApp::class);
+    $gitlabSource->shouldReceive('getMorphClass')->andReturn(GitlabApp::class);
+    $gitlabSource->shouldReceive('getAttribute')->with('html_url')->andReturn('https://gitlab.com');
     $gitlabSource->shouldReceive('getAttribute')->with('privateKey')->andReturn(null);
     $gitlabSource->shouldReceive('getAttribute')->with('private_key_id')->andReturn(null);
 
@@ -61,17 +98,18 @@ it('generates ls-remote commands for GitLab source without private key', functio
 
     expect($result)->toBeArray();
     expect($result)->toHaveKey('commands');
-    expect($result['commands'])->toContain('git ls-remote');
-    expect($result['commands'])->toContain('https://gitlab.com/user/repo.git');
+    expectGitlabCommandListToContain($result['commands'], 'git ls-remote');
+    expectGitlabCommandListToContain($result['commands'], 'https://gitlab.com/user/repo.git');
     // Should NOT contain SSH key setup
-    expect($result['commands'])->not->toContain('id_rsa');
+    expectGitlabCommandListNotToContain($result['commands'], 'id_rsa');
 });
 
 it('does not return null for GitLab source type', function () {
     $deploymentUuid = 'test-deployment-uuid';
 
     $gitlabSource = Mockery::mock(GitlabApp::class)->makePartial();
-    $gitlabSource->shouldReceive('getMorphClass')->andReturn(\App\Models\GitlabApp::class);
+    $gitlabSource->shouldReceive('getMorphClass')->andReturn(GitlabApp::class);
+    $gitlabSource->shouldReceive('getAttribute')->with('html_url')->andReturn('https://gitlab.com');
     $gitlabSource->shouldReceive('getAttribute')->with('privateKey')->andReturn(null);
     $gitlabSource->shouldReceive('getAttribute')->with('private_key_id')->andReturn(null);
 

tests/Unit/ValidationPatternsTest.php

@@ -132,24 +132,31 @@ it('generates nullable dockerNetworkRules when not required', function () {
         ->not->toContain('required');
 });
 
-it('accepts Docker-compatible environment variable keys', function (string $key) {
+it('accepts shell-safe environment variable keys', function (string $key) {
     expect(ValidationPatterns::isValidEnvironmentVariableKey($key))->toBeTrue();
 })->with([
     'letters' => 'APP_ENV',
     'leading underscore' => '_TOKEN',
     'railpack control variable' => 'RAILPACK_NODE_VERSION',
     'digits after first character' => 'NODE_VERSION_20',
-    'starts with digit' => '1BAD',
-    'hyphen' => 'BAD-KEY',
-    'dot' => 'node.name',
+    'lowercase' => 'node_version',
+    'dot notation' => 'node.name',
     'uppercase dots' => 'XPACK.SECURITY.ENABLED',
-    'semicolon' => 'BAD;KEY',
-    'space' => 'BAD KEY',
 ]);
 
-it('rejects environment variable keys Docker cannot represent', function (string $key) {
+it('rejects invalid environment variable keys', function (string $key) {
     expect(ValidationPatterns::isValidEnvironmentVariableKey($key))->toBeFalse();
 })->with([
+    'starts with digit' => '1BAD',
+    'hyphen' => 'BAD-KEY',
+    'semicolon' => 'BAD;KEY',
+    'space' => 'BAD KEY',
+    'command substitution' => 'BAD$(id)',
+    'backticks' => 'BAD`id`',
+    'pipe' => 'BAD|id',
+    'ampersand' => 'BAD&id',
+    'newline' => 'BAD
+KEY',
     'equals' => 'BAD=KEY',
     'empty' => '',
 ]);
@@ -164,7 +171,7 @@ it('generates environment variable key rules with correct defaults', function ()
 });
 
 it('normalizes environment variable keys by trimming surrounding whitespace', function () {
-    expect(ValidationPatterns::normalizeEnvironmentVariableKey(' node.name '))->toBe('node.name');
+    expect(ValidationPatterns::normalizeEnvironmentVariableKey(' APP_ENV '))->toBe('APP_ENV');
 });
 
 it('normalizes environment variable keys before model validation', function () {