diff --git a/app/Http/Controllers/Api/ApplicationsController.php b/app/Http/Controllers/Api/ApplicationsController.php index 824101be8..bdb6f8d90 100644 --- a/app/Http/Controllers/Api/ApplicationsController.php +++ b/app/Http/Controllers/Api/ApplicationsController.php @@ -2876,8 +2876,12 @@ public function update_env_by_uuid(Request $request) $this->authorize('manageEnvironment', $application); + if ($request->has('key')) { + $request->merge(['key' => ValidationPatterns::normalizeEnvironmentVariableKey((string) $request->key)]); + } + $validator = customApiValidator($request->all(), [ - 'key' => 'string|required', + 'key' => ValidationPatterns::environmentVariableKeyRules(), 'value' => 'string|nullable', 'is_preview' => 'boolean', 'is_literal' => 'boolean', @@ -3100,12 +3104,18 @@ public function create_bulk_envs(Request $request) ], 400); } $bulk_data = collect($bulk_data)->map(function ($item) { - return collect($item)->only(['key', 'value', 'is_preview', 'is_literal', 'is_multiline', 'is_shown_once', 'is_runtime', 'is_buildtime', 'comment']); + $item = collect($item)->only(['key', 'value', 'is_preview', 'is_literal', 'is_multiline', 'is_shown_once', 'is_runtime', 'is_buildtime', 'comment']); + + if ($item->has('key')) { + $item->put('key', ValidationPatterns::normalizeEnvironmentVariableKey((string) $item->get('key'))); + } + + return $item; }); $returnedEnvs = collect(); foreach ($bulk_data as $item) { $validator = customApiValidator($item, [ - 'key' => 'string|required', + 'key' => ValidationPatterns::environmentVariableKeyRules(), 'value' => 'string|nullable', 'is_preview' => 'boolean', 'is_literal' => 'boolean', @@ -3302,8 +3312,12 @@ public function create_env(Request $request) $this->authorize('manageEnvironment', $application); + if ($request->has('key')) { + $request->merge(['key' => ValidationPatterns::normalizeEnvironmentVariableKey((string) $request->key)]); + } + $validator = customApiValidator($request->all(), [ - 'key' => 'string|required', + 'key' => ValidationPatterns::environmentVariableKeyRules(), 'value' => 'string|nullable', 'is_preview' => 'boolean', 'is_literal' => 'boolean', diff --git a/app/Http/Controllers/Api/DatabasesController.php b/app/Http/Controllers/Api/DatabasesController.php index bceef4d39..9a463e8d3 100644 --- a/app/Http/Controllers/Api/DatabasesController.php +++ b/app/Http/Controllers/Api/DatabasesController.php @@ -3133,8 +3133,12 @@ public function update_env_by_uuid(Request $request) $this->authorize('manageEnvironment', $database); + if ($request->has('key')) { + $request->merge(['key' => ValidationPatterns::normalizeEnvironmentVariableKey((string) $request->key)]); + } + $validator = customApiValidator($request->all(), [ - 'key' => 'string|required', + 'key' => ValidationPatterns::environmentVariableKeyRules(), 'value' => 'string|nullable', 'is_literal' => 'boolean', 'is_multiline' => 'boolean', @@ -3281,8 +3285,12 @@ public function create_bulk_envs(Request $request) $updatedEnvs = collect(); foreach ($bulk_data as $item) { + if (array_key_exists('key', $item)) { + $item['key'] = ValidationPatterns::normalizeEnvironmentVariableKey((string) $item['key']); + } + $validator = customApiValidator($item, [ - 'key' => 'string|required', + 'key' => ValidationPatterns::environmentVariableKeyRules(), 'value' => 'string|nullable', 'is_literal' => 'boolean', 'is_multiline' => 'boolean', @@ -3399,8 +3407,12 @@ public function create_env(Request $request) $this->authorize('manageEnvironment', $database); + if ($request->has('key')) { + $request->merge(['key' => ValidationPatterns::normalizeEnvironmentVariableKey((string) $request->key)]); + } + $validator = customApiValidator($request->all(), [ - 'key' => 'string|required', + 'key' => ValidationPatterns::environmentVariableKeyRules(), 'value' => 'string|nullable', 'is_literal' => 'boolean', 'is_multiline' => 'boolean', diff --git a/app/Http/Controllers/Api/ServicesController.php b/app/Http/Controllers/Api/ServicesController.php index 89c99ff9f..32137b866 100644 --- a/app/Http/Controllers/Api/ServicesController.php +++ b/app/Http/Controllers/Api/ServicesController.php @@ -1251,8 +1251,12 @@ public function update_env_by_uuid(Request $request) $this->authorize('manageEnvironment', $service); + if ($request->has('key')) { + $request->merge(['key' => ValidationPatterns::normalizeEnvironmentVariableKey((string) $request->key)]); + } + $validator = customApiValidator($request->all(), [ - 'key' => 'string|required', + 'key' => ValidationPatterns::environmentVariableKeyRules(), 'value' => 'string|nullable', 'is_literal' => 'boolean', 'is_multiline' => 'boolean', @@ -1400,8 +1404,12 @@ public function create_bulk_envs(Request $request) $updatedEnvs = collect(); foreach ($bulk_data as $item) { + if (array_key_exists('key', $item)) { + $item['key'] = ValidationPatterns::normalizeEnvironmentVariableKey((string) $item['key']); + } + $validator = customApiValidator($item, [ - 'key' => 'string|required', + 'key' => ValidationPatterns::environmentVariableKeyRules(), 'value' => 'string|nullable', 'is_literal' => 'boolean', 'is_multiline' => 'boolean', @@ -1519,8 +1527,12 @@ public function create_env(Request $request) $this->authorize('manageEnvironment', $service); + if ($request->has('key')) { + $request->merge(['key' => ValidationPatterns::normalizeEnvironmentVariableKey((string) $request->key)]); + } + $validator = customApiValidator($request->all(), [ - 'key' => 'string|required', + 'key' => ValidationPatterns::environmentVariableKeyRules(), 'value' => 'string|nullable', 'is_literal' => 'boolean', 'is_multiline' => 'boolean', diff --git a/app/Http/Controllers/Webhook/Github.php b/app/Http/Controllers/Webhook/Github.php index c9b0116fb..28e92dcd4 100644 --- a/app/Http/Controllers/Webhook/Github.php +++ b/app/Http/Controllers/Webhook/Github.php @@ -261,6 +261,16 @@ public function normal(Request $request) return response('Nothing to do. No GitHub App found.'); } $webhook_secret = data_get($github_app, 'webhook_secret'); + if (empty($webhook_secret)) { + auditLogWebhookFailure('github', 'webhook_secret_missing', [ + 'mode' => 'app', + 'github_app_id' => $github_app->id, + 'github_app_name' => $github_app->name, + 'installation_target_id' => $x_github_hook_installation_target_id, + ]); + + return response('Invalid signature.'); + } $hmac = hash_hmac('sha256', $request->getContent(), $webhook_secret); if (config('app.env') !== 'local') { if (! hash_equals($x_hub_signature_256, $hmac)) { diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index c94e29028..b7283550c 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -2306,6 +2306,8 @@ private function check_git_if_build_needed() ], [ executeInDocker($this->deployment_uuid, "echo '{$private_key}' | base64 -d | tee {$customSshKeyLocation} > /dev/null"), + 'hidden' => true, + 'skip_command_log' => true, ], [ executeInDocker($this->deployment_uuid, "chmod 600 {$customSshKeyLocation}"), @@ -2365,12 +2367,7 @@ private function clone_repository() if ($this->pull_request_id !== 0) { $this->application_deployment_queue->addLogEntry("Checking out tag pull/{$this->pull_request_id}/head."); } - $this->execute_remote_command( - [ - $importCommands, - 'hidden' => true, - ] - ); + $this->execute_remote_command(...$this->gitCommandDefinitions($importCommands)); $this->create_workdir(); $this->execute_remote_command( [ @@ -2400,6 +2397,39 @@ private function generate_git_import_commands() return $commands; } + private function gitCommandDefinitions(Collection|array|string $commands): array + { + if (is_string($commands)) { + return [ + [ + $commands, + 'hidden' => true, + ], + ]; + } + + return collect($commands) + ->map(function ($command): array { + if (is_string($command)) { + return [ + $command, + 'hidden' => true, + ]; + } + + if (is_array($command)) { + return $command + ['hidden' => true]; + } + + return [ + 'command' => $command, + 'hidden' => true, + ]; + }) + ->values() + ->all(); + } + private function cleanup_git() { $this->execute_remote_command( diff --git a/app/Models/Application.php b/app/Models/Application.php index 4c53242ed..2c408483e 100644 --- a/app/Models/Application.php +++ b/app/Models/Application.php @@ -1338,7 +1338,7 @@ public function getGitRemoteStatus(string $deployment_uuid) { try { ['commands' => $lsRemoteCommand] = $this->generateGitLsRemoteCommands(deployment_uuid: $deployment_uuid, exec_in_docker: false); - instant_remote_process([$lsRemoteCommand], $this->destination->server, true); + instant_remote_process([$this->gitCommandsAsShellCommand($lsRemoteCommand)], $this->destination->server, true); return [ 'is_accessible' => true, @@ -1390,13 +1390,13 @@ public function generateGitLsRemoteCommands(string $deployment_uuid, bool $exec_ } if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, $base_command)); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, $base_command))); } else { - $commands->push($base_command); + $commands->push($this->gitCommand($base_command)); } return [ - 'commands' => $commands->implode(' && '), + 'commands' => $this->gitCommandsAsShellCommand($commands), 'branch' => $branch, 'fullRepoUrl' => $fullRepoUrl, ]; @@ -1413,29 +1413,16 @@ public function generateGitLsRemoteCommands(string $deployment_uuid, bool $exec_ $escapedCustomRepository = str_replace("'", "'\\''", $customRepository); $base_command = "GIT_SSH_COMMAND=\"ssh -o ConnectTimeout=30 -p {$gitlabPort} -o Port={$gitlabPort} -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i {$customSshKeyLocation} -o IdentitiesOnly=yes\" {$base_command} '{$escapedCustomRepository}'"; - if ($exec_in_docker) { - $commands = collect([ - executeInDocker($deployment_uuid, 'mkdir -p /root/.ssh'), - executeInDocker($deployment_uuid, "echo '{$private_key}' | base64 -d | tee {$customSshKeyLocation} > /dev/null"), - executeInDocker($deployment_uuid, "chmod 600 {$customSshKeyLocation}"), - ]); - } else { - $commands = collect([ - "trap 'rm -f {$customSshKeyLocation}' EXIT", - 'mkdir -p /root/.ssh', - "echo '{$private_key}' | base64 -d | tee {$customSshKeyLocation} > /dev/null", - "chmod 600 {$customSshKeyLocation}", - ]); - } + $commands = $this->gitSshKeySetupCommands($deployment_uuid, $private_key, $exec_in_docker); if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, $base_command)); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, $base_command))); } else { - $commands->push($base_command); + $commands->push($this->gitCommand($base_command)); } return [ - 'commands' => $commands->implode(' && '), + 'commands' => $commands, 'branch' => $branch, 'fullRepoUrl' => $fullRepoUrl, ]; @@ -1447,13 +1434,13 @@ public function generateGitLsRemoteCommands(string $deployment_uuid, bool $exec_ $base_command = "{$base_command} {$escapedCustomRepository}"; if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, $base_command)); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, $base_command))); } else { - $commands->push($base_command); + $commands->push($this->gitCommand($base_command)); } return [ - 'commands' => $commands->implode(' && '), + 'commands' => $this->gitCommandsAsShellCommand($commands), 'branch' => $branch, 'fullRepoUrl' => $fullRepoUrl, ]; @@ -1472,29 +1459,16 @@ public function generateGitLsRemoteCommands(string $deployment_uuid, bool $exec_ $escapedCustomRepository = str_replace("'", "'\\''", $customRepository); $base_command = "GIT_SSH_COMMAND=\"ssh -o ConnectTimeout=30 -p {$customPort} -o Port={$customPort} -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i {$customSshKeyLocation} -o IdentitiesOnly=yes\" {$base_command} '{$escapedCustomRepository}'"; - if ($exec_in_docker) { - $commands = collect([ - executeInDocker($deployment_uuid, 'mkdir -p /root/.ssh'), - executeInDocker($deployment_uuid, "echo '{$private_key}' | base64 -d | tee {$customSshKeyLocation} > /dev/null"), - executeInDocker($deployment_uuid, "chmod 600 {$customSshKeyLocation}"), - ]); - } else { - $commands = collect([ - "trap 'rm -f {$customSshKeyLocation}' EXIT", - 'mkdir -p /root/.ssh', - "echo '{$private_key}' | base64 -d | tee {$customSshKeyLocation} > /dev/null", - "chmod 600 {$customSshKeyLocation}", - ]); - } + $commands = $this->gitSshKeySetupCommands($deployment_uuid, $private_key, $exec_in_docker); if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, $base_command)); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, $base_command))); } else { - $commands->push($base_command); + $commands->push($this->gitCommand($base_command)); } return [ - 'commands' => $commands->implode(' && '), + 'commands' => $commands, 'branch' => $branch, 'fullRepoUrl' => $fullRepoUrl, ]; @@ -1506,19 +1480,60 @@ public function generateGitLsRemoteCommands(string $deployment_uuid, bool $exec_ $base_command = "{$base_command} {$escapedCustomRepository}"; if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, $base_command)); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, $base_command))); } else { - $commands->push($base_command); + $commands->push($this->gitCommand($base_command)); } return [ - 'commands' => $commands->implode(' && '), + 'commands' => $this->gitCommandsAsShellCommand($commands), 'branch' => $branch, 'fullRepoUrl' => $fullRepoUrl, ]; } } + private function gitCommand(string $command): array + { + return [ + 'command' => $command, + 'hidden' => true, + ]; + } + + private function gitCommandsAsShellCommand(Collection|array|string $commands): string + { + if (is_string($commands)) { + return $commands; + } + + return collect($commands) + ->map(fn ($command) => data_get($command, 'command') ?? $command[0] ?? $command) + ->implode(' && '); + } + + private function gitSshKeySetupCommands(string $deploymentUuid, string $privateKey, bool $execInDocker): Collection + { + $customSshKeyLocation = "/root/.ssh/id_rsa_coolify_{$deploymentUuid}"; + $commands = collect([]); + + if (! $execInDocker) { + $commands->push($this->gitCommand("trap 'rm -f {$customSshKeyLocation}' EXIT")); + } + + $commands->push($this->gitCommand($execInDocker ? executeInDocker($deploymentUuid, 'mkdir -p /root/.ssh') : 'mkdir -p /root/.ssh')); + $commands->push([ + 'command' => $execInDocker + ? executeInDocker($deploymentUuid, "echo '{$privateKey}' | base64 -d | tee {$customSshKeyLocation} > /dev/null") + : "echo '{$privateKey}' | base64 -d | tee {$customSshKeyLocation} > /dev/null", + 'hidden' => true, + 'skip_command_log' => true, + ]); + $commands->push($this->gitCommand($execInDocker ? executeInDocker($deploymentUuid, "chmod 600 {$customSshKeyLocation}") : "chmod 600 {$customSshKeyLocation}")); + + return $commands; + } + private function withGitHttpTransportConfig(?string $gitConfigOptions = null): string { return trim(($gitConfigOptions ? "{$gitConfigOptions} " : '').'-c http.version=HTTP/1.1'); @@ -1590,9 +1605,9 @@ public function generateGitImportCommands(string $deployment_uuid, int $pull_req $git_clone_command = $this->setGitImportSettings($deployment_uuid, $git_clone_command, public: true, commit: $commit, gitConfigOptions: $gitConfigOptions); } if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, $git_clone_command)); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, $git_clone_command))); } else { - $commands->push($git_clone_command); + $commands->push($this->gitCommand($git_clone_command)); } } else { $github_access_token = generateGithubInstallationToken($this->source); @@ -1619,9 +1634,9 @@ public function generateGitImportCommands(string $deployment_uuid, int $pull_req $git_clone_command = $this->setGitImportSettings($deployment_uuid, $git_clone_command, public: false, commit: $commit, gitConfigOptions: $gitConfigOptions); } if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, $git_clone_command)); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, $git_clone_command))); } else { - $commands->push($git_clone_command); + $commands->push($this->gitCommand($git_clone_command)); } } if ($pull_request_id !== 0) { @@ -1631,14 +1646,14 @@ public function generateGitImportCommands(string $deployment_uuid, int $pull_req $gitCommand = isset($gitConfigOptions) ? "git {$gitConfigOptions}" : 'git'; $escapedPrBranch = escapeshellarg($branch); if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, "cd {$escapedBaseDir} && {$gitCommand} fetch origin {$escapedPrBranch} && $git_checkout_command")); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, "cd {$escapedBaseDir} && {$gitCommand} fetch origin {$escapedPrBranch} && $git_checkout_command"))); } else { - $commands->push("cd {$escapedBaseDir} && {$gitCommand} fetch origin {$escapedPrBranch} && $git_checkout_command"); + $commands->push($this->gitCommand("cd {$escapedBaseDir} && {$gitCommand} fetch origin {$escapedPrBranch} && $git_checkout_command")); } } return [ - 'commands' => $commands->implode(' && '), + 'commands' => $this->gitCommandsAsShellCommand($commands), 'branch' => $branch, 'fullRepoUrl' => $fullRepoUrl, ]; @@ -1661,39 +1676,26 @@ public function generateGitImportCommands(string $deployment_uuid, int $pull_req } else { $git_clone_command = $this->setGitImportSettings($deployment_uuid, $git_clone_command_base, commit: $commit, gitSshCommand: $gitlabSshCommand); } - if ($exec_in_docker) { - $commands = collect([ - executeInDocker($deployment_uuid, 'mkdir -p /root/.ssh'), - executeInDocker($deployment_uuid, "echo '{$private_key}' | base64 -d | tee {$customSshKeyLocation} > /dev/null"), - executeInDocker($deployment_uuid, "chmod 600 {$customSshKeyLocation}"), - ]); - } else { - $commands = collect([ - "trap 'rm -f {$customSshKeyLocation}' EXIT", - 'mkdir -p /root/.ssh', - "echo '{$private_key}' | base64 -d | tee {$customSshKeyLocation} > /dev/null", - "chmod 600 {$customSshKeyLocation}", - ]); - } + $commands = $this->gitSshKeySetupCommands($deployment_uuid, $private_key, $exec_in_docker); if ($pull_request_id !== 0) { $branch = "merge-requests/{$pull_request_id}/head:$pr_branch_name"; if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, "echo 'Checking out $branch'")); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, "echo 'Checking out $branch'"))); } else { - $commands->push("echo 'Checking out $branch'"); + $commands->push($this->gitCommand("echo 'Checking out $branch'")); } $git_clone_command = "{$git_clone_command} && cd {$escapedBaseDir} && {$gitlabGitSshCommand} git fetch origin $branch && ".$this->buildGitCheckoutCommand($pr_branch_name, $gitlabSshCommand); } if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, $git_clone_command)); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, $git_clone_command))); } else { - $commands->push($git_clone_command); + $commands->push($this->gitCommand($git_clone_command)); } return [ - 'commands' => $commands->implode(' && '), + 'commands' => $commands, 'branch' => $branch, 'fullRepoUrl' => $fullRepoUrl, ]; @@ -1710,13 +1712,13 @@ public function generateGitImportCommands(string $deployment_uuid, int $pull_req $git_clone_command = $this->setGitImportSettings($deployment_uuid, $git_clone_command, public: true, commit: $commit, gitConfigOptions: $gitConfigOptions); if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, $git_clone_command)); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, $git_clone_command))); } else { - $commands->push($git_clone_command); + $commands->push($this->gitCommand($git_clone_command)); } return [ - 'commands' => $commands->implode(' && '), + 'commands' => $this->gitCommandsAsShellCommand($commands), 'branch' => $branch, 'fullRepoUrl' => $fullRepoUrl, ]; @@ -1738,55 +1740,42 @@ public function generateGitImportCommands(string $deployment_uuid, int $pull_req } else { $git_clone_command = $this->setGitImportSettings($deployment_uuid, $git_clone_command_base, commit: $commit, gitSshCommand: $deployKeySshCommand); } - if ($exec_in_docker) { - $commands = collect([ - executeInDocker($deployment_uuid, 'mkdir -p /root/.ssh'), - executeInDocker($deployment_uuid, "echo '{$private_key}' | base64 -d | tee {$customSshKeyLocation} > /dev/null"), - executeInDocker($deployment_uuid, "chmod 600 {$customSshKeyLocation}"), - ]); - } else { - $commands = collect([ - "trap 'rm -f {$customSshKeyLocation}' EXIT", - 'mkdir -p /root/.ssh', - "echo '{$private_key}' | base64 -d | tee {$customSshKeyLocation} > /dev/null", - "chmod 600 {$customSshKeyLocation}", - ]); - } + $commands = $this->gitSshKeySetupCommands($deployment_uuid, $private_key, $exec_in_docker); if ($pull_request_id !== 0) { if ($git_type === 'gitlab') { $branch = "merge-requests/{$pull_request_id}/head:$pr_branch_name"; if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, "echo 'Checking out $branch'")); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, "echo 'Checking out $branch'"))); } else { - $commands->push("echo 'Checking out $branch'"); + $commands->push($this->gitCommand("echo 'Checking out $branch'")); } $git_clone_command = "{$git_clone_command} && cd {$escapedBaseDir} && {$deployKeyGitSshCommand} git fetch origin $branch && ".$this->buildGitCheckoutCommand($pr_branch_name, $deployKeySshCommand); } elseif ($git_type === 'github' || $git_type === 'gitea') { $branch = "pull/{$pull_request_id}/head:$pr_branch_name"; if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, "echo 'Checking out $branch'")); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, "echo 'Checking out $branch'"))); } else { - $commands->push("echo 'Checking out $branch'"); + $commands->push($this->gitCommand("echo 'Checking out $branch'")); } $git_clone_command = "{$git_clone_command} && cd {$escapedBaseDir} && {$deployKeyGitSshCommand} git fetch origin $branch && ".$this->buildGitCheckoutCommand($pr_branch_name, $deployKeySshCommand); } elseif ($git_type === 'bitbucket') { if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, "echo 'Checking out $branch'")); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, "echo 'Checking out $branch'"))); } else { - $commands->push("echo 'Checking out $branch'"); + $commands->push($this->gitCommand("echo 'Checking out $branch'")); } $git_clone_command = "{$git_clone_command} && cd {$escapedBaseDir} && {$deployKeyGitSshCommand} ".$this->buildGitCheckoutCommand($commit, $deployKeySshCommand); } } if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, $git_clone_command)); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, $git_clone_command))); } else { - $commands->push($git_clone_command); + $commands->push($this->gitCommand($git_clone_command)); } return [ - 'commands' => $commands->implode(' && '), + 'commands' => $commands, 'branch' => $branch, 'fullRepoUrl' => $fullRepoUrl, ]; @@ -1807,37 +1796,37 @@ public function generateGitImportCommands(string $deployment_uuid, int $pull_req if ($git_type === 'gitlab') { $branch = "merge-requests/{$pull_request_id}/head:$pr_branch_name"; if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, "echo 'Checking out $branch'")); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, "echo 'Checking out $branch'"))); } else { - $commands->push("echo 'Checking out $branch'"); + $commands->push($this->gitCommand("echo 'Checking out $branch'")); } $git_clone_command = "{$git_clone_command} && cd {$escapedBaseDir} && GIT_SSH_COMMAND=\"{$otherSshCommand}\" {$gitCommand} fetch origin $branch && ".$this->buildGitCheckoutCommand($pr_branch_name, $otherSshCommand, $gitConfigOptions); } elseif ($git_type === 'github' || $git_type === 'gitea') { $branch = "pull/{$pull_request_id}/head:$pr_branch_name"; if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, "echo 'Checking out $branch'")); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, "echo 'Checking out $branch'"))); } else { - $commands->push("echo 'Checking out $branch'"); + $commands->push($this->gitCommand("echo 'Checking out $branch'")); } $git_clone_command = "{$git_clone_command} && cd {$escapedBaseDir} && GIT_SSH_COMMAND=\"{$otherSshCommand}\" {$gitCommand} fetch origin $branch && ".$this->buildGitCheckoutCommand($pr_branch_name, $otherSshCommand, $gitConfigOptions); } elseif ($git_type === 'bitbucket') { if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, "echo 'Checking out $branch'")); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, "echo 'Checking out $branch'"))); } else { - $commands->push("echo 'Checking out $branch'"); + $commands->push($this->gitCommand("echo 'Checking out $branch'")); } $git_clone_command = "{$git_clone_command} && cd {$escapedBaseDir} && GIT_SSH_COMMAND=\"{$otherSshCommand}\" ".$this->buildGitCheckoutCommand($commit, $otherSshCommand, $gitConfigOptions); } } if ($exec_in_docker) { - $commands->push(executeInDocker($deployment_uuid, $git_clone_command)); + $commands->push($this->gitCommand(executeInDocker($deployment_uuid, $git_clone_command))); } else { - $commands->push($git_clone_command); + $commands->push($this->gitCommand($git_clone_command)); } return [ - 'commands' => $commands->implode(' && '), + 'commands' => $this->gitCommandsAsShellCommand($commands), 'branch' => $branch, 'fullRepoUrl' => $fullRepoUrl, ]; @@ -1926,6 +1915,7 @@ public function loadComposeFile($isInit = false, ?string $restoreBaseDirectory = } $uuid = new_public_id(); ['commands' => $cloneCommand] = $this->generateGitImportCommands(deployment_uuid: $uuid, only_checkout: true, exec_in_docker: false, custom_base_dir: 'checkout'); + $cloneCommand = $this->gitCommandsAsShellCommand($cloneCommand); $cloneCommand = str_replace(' clone ', ' clone --quiet ', $cloneCommand); $workdir = rtrim($this->base_directory, '/'); $composeFile = $this->docker_compose_location; diff --git a/app/Support/ValidationPatterns.php b/app/Support/ValidationPatterns.php index bdb8654b9..2f553a4b9 100644 --- a/app/Support/ValidationPatterns.php +++ b/app/Support/ValidationPatterns.php @@ -95,9 +95,9 @@ class ValidationPatterns /** * Pattern for Docker-compatible environment variable keys. - * Docker environment entries are KEY=value strings, so keys must be non-empty and cannot contain '=' or NUL. + * Environment variable keys are later interpolated into shell commands as Docker build args, so only shell-safe identifier characters are allowed. */ - public const ENVIRONMENT_VARIABLE_KEY_PATTERN = '/\A[^=\x00]+\z/u'; + public const ENVIRONMENT_VARIABLE_KEY_PATTERN = '/\A[A-Za-z_][A-Za-z0-9_.]*\z/u'; /** * Pattern for SQL-safe unquoted database identifiers (usernames, database names). @@ -164,7 +164,7 @@ public static function environmentVariableKeyRules(bool $required = true, int $m public static function environmentVariableKeyMessages(string $field = 'key', string $label = 'key'): array { return [ - "{$field}.regex" => "The {$label} must be a non-empty Docker-compatible environment variable key and cannot contain '=' or NUL characters.", + "{$field}.regex" => "The {$label} must start with a letter or underscore and may only contain letters, numbers, underscores, and dots.", "{$field}.max" => "The {$label} may not be greater than :max characters.", ]; } diff --git a/app/Traits/ExecuteRemoteCommand.php b/app/Traits/ExecuteRemoteCommand.php index bb252148a..a2c3d06da 100644 --- a/app/Traits/ExecuteRemoteCommand.php +++ b/app/Traits/ExecuteRemoteCommand.php @@ -79,6 +79,7 @@ public function execute_remote_command(...$commands) $ignore_errors = data_get($single_command, 'ignore_errors', false); $append = data_get($single_command, 'append', true); $command_hidden = data_get($single_command, 'command_hidden', false); + $skip_command_log = data_get($single_command, 'skip_command_log', false); $this->save = data_get($single_command, 'save'); if ($this->server->isNonRoot()) { if (str($command)->startsWith('docker exec')) { @@ -91,7 +92,7 @@ public function execute_remote_command(...$commands) // Check for cancellation before executing commands if (isset($this->application_deployment_queue)) { $this->application_deployment_queue->refresh(); - if ($this->application_deployment_queue->status === \App\Enums\ApplicationDeploymentStatus::CANCELLED_BY_USER->value) { + if ($this->application_deployment_queue->status === ApplicationDeploymentStatus::CANCELLED_BY_USER->value) { throw new \RuntimeException('Deployment cancelled by user', 69420); } } @@ -103,7 +104,7 @@ public function execute_remote_command(...$commands) while ($attempt < $maxRetries && ! $commandExecuted) { try { - $this->executeCommandWithProcess($command, $hidden, $customType, $append, $ignore_errors, $command_hidden); + $this->executeCommandWithProcess($command, $hidden, $customType, $append, $ignore_errors, $command_hidden, $skip_command_log); $commandExecuted = true; } catch (\RuntimeException|DeploymentException $e) { $lastError = $e; @@ -119,7 +120,7 @@ public function execute_remote_command(...$commands) // Check for cancellation during retry wait $this->application_deployment_queue->refresh(); - if ($this->application_deployment_queue->status === \App\Enums\ApplicationDeploymentStatus::CANCELLED_BY_USER->value) { + if ($this->application_deployment_queue->status === ApplicationDeploymentStatus::CANCELLED_BY_USER->value) { throw new \RuntimeException('Deployment cancelled by user during retry', 69420); } } @@ -153,14 +154,14 @@ public function execute_remote_command(...$commands) /** * Execute the actual command with process handling */ - private function executeCommandWithProcess($command, $hidden, $customType, $append, $ignore_errors, $command_hidden = false) + private function executeCommandWithProcess($command, $hidden, $customType, $append, $ignore_errors, $command_hidden = false, $skip_command_log = false) { - if ($command_hidden && isset($this->application_deployment_queue)) { + if ($command_hidden && ! $skip_command_log && isset($this->application_deployment_queue)) { $this->application_deployment_queue->addLogEntry('[CMD]: '.$this->redact_sensitive_info($command), hidden: true); } $remote_command = SshMultiplexingHelper::generateSshCommand($this->server, $command); - $process = Process::timeout(config('constants.ssh.command_timeout'))->idleTimeout(3600)->start($remote_command, function (string $type, string $output) use ($command, $hidden, $customType, $append, $command_hidden) { + $process = Process::timeout(config('constants.ssh.command_timeout'))->idleTimeout(3600)->start($remote_command, function (string $type, string $output) use ($command, $hidden, $customType, $append, $command_hidden, $skip_command_log) { $output = str($output)->trim(); if ($output->startsWith('╔')) { $output = "\n".$output; @@ -170,7 +171,7 @@ private function executeCommandWithProcess($command, $hidden, $customType, $appe $sanitized_output = sanitize_utf8_text($output); $new_log_entry = [ - 'command' => $command_hidden ? null : $this->redact_sensitive_info($command), + 'command' => $skip_command_log || $command_hidden ? null : $this->redact_sensitive_info($command), 'output' => $this->redact_sensitive_info($sanitized_output), 'type' => $customType ?? ($type === 'err' ? 'stderr' : 'stdout'), 'timestamp' => Carbon::now('UTC'), @@ -227,7 +228,7 @@ private function executeCommandWithProcess($command, $hidden, $customType, $appe // Check if deployment was cancelled while command was running if (isset($this->application_deployment_queue)) { $this->application_deployment_queue->refresh(); - if ($this->application_deployment_queue->status === \App\Enums\ApplicationDeploymentStatus::CANCELLED_BY_USER->value) { + if ($this->application_deployment_queue->status === ApplicationDeploymentStatus::CANCELLED_BY_USER->value) { throw new \RuntimeException('Deployment cancelled by user', 69420); } } diff --git a/bootstrap/helpers/docker.php b/bootstrap/helpers/docker.php index 1b389c77c..2f7cc95ef 100644 --- a/bootstrap/helpers/docker.php +++ b/bootstrap/helpers/docker.php @@ -1363,7 +1363,7 @@ function generateDockerBuildArgs($variables): Collection $key = is_array($var) ? data_get($var, 'key') : $var->key; // Only return the key - Docker will get the value from the environment - return "--build-arg {$key}"; + return '--build-arg '.escapeshellarg((string) $key); }); } diff --git a/database/seeders/DevelopmentRailpackExamplesSeeder.php b/database/seeders/DevelopmentRailpackExamplesSeeder.php index 78659b457..e736c5ecd 100644 --- a/database/seeders/DevelopmentRailpackExamplesSeeder.php +++ b/database/seeders/DevelopmentRailpackExamplesSeeder.php @@ -7,6 +7,7 @@ use App\Models\Application; use App\Models\Environment; use App\Models\GithubApp; +use App\Models\GitlabApp; use App\Models\PrivateKey; use App\Models\Project; use App\Models\Server; @@ -360,6 +361,36 @@ public static function examples(): array 'ports_exposes' => '3000', 'git_branch' => 'v4.x', ], + [ + 'uuid' => 'railpack-github-deploy-key', + 'name' => 'Railpack GitHub Deploy Key Example', + 'git_repository' => 'git@github.com:coollabsio/coolify-examples-deploy-key.git', + 'git_branch' => 'main', + 'ports_exposes' => '80', + 'private_key_id' => 1, + ], + [ + 'uuid' => 'railpack-gitlab-deploy-key', + 'name' => 'Railpack GitLab Deploy Key Example', + 'git_repository' => 'git@gitlab.com:coollabsio/php-example.git', + 'git_branch' => 'main', + 'ports_exposes' => '80', + 'source_id' => 1, + 'source_type' => GitlabApp::class, + 'private_key_id' => 1, + ], + [ + 'uuid' => 'railpack-gitlab-public-example', + 'name' => 'Railpack GitLab Public Example', + 'git_repository' => 'https://gitlab.com/andrasbacsai/coolify-examples.git', + 'git_branch' => 'main', + 'base_directory' => '/astro/static', + 'publish_directory' => '/dist', + 'ports_exposes' => '80', + 'source_id' => 1, + 'source_type' => GitlabApp::class, + 'is_static' => true, + ], ]; } @@ -420,6 +451,7 @@ private function ensureDevelopmentPrerequisitesExist(): void ); $this->ensurePublicGithubSourceExists(); + $this->ensurePublicGitlabSourceExists(); } private function ensurePublicGithubSourceExists(): void @@ -437,6 +469,21 @@ private function ensurePublicGithubSourceExists(): void ); } + private function ensurePublicGitlabSourceExists(): void + { + GitlabApp::query()->firstOrCreate( + ['id' => 1], + [ + 'uuid' => 'gitlab-public', + 'name' => 'Public GitLab', + 'api_url' => 'https://gitlab.com/api/v4', + 'html_url' => 'https://gitlab.com', + 'is_public' => true, + 'team_id' => 0, + ], + ); + } + private function isDevelopmentEnvironment(): bool { return in_array(config('app.env'), ['local', 'development', 'dev'], true); @@ -479,12 +526,12 @@ private function upsertApplication(Environment $environment, StandaloneDocker $d 'name' => $example['name'], 'description' => $example['name'], 'fqdn' => "http://{$example['uuid']}.127.0.0.1.sslip.io", - 'repository_project_id' => self::REPOSITORY_PROJECT_ID, - 'git_repository' => self::GIT_REPOSITORY, + 'repository_project_id' => $example['repository_project_id'] ?? self::REPOSITORY_PROJECT_ID, + 'git_repository' => $example['git_repository'] ?? self::GIT_REPOSITORY, 'git_branch' => $example['git_branch'] ?? self::GIT_BRANCH, 'build_pack' => 'railpack', 'ports_exposes' => $example['ports_exposes'], - 'base_directory' => $example['base_directory'], + 'base_directory' => $example['base_directory'] ?? '/', 'publish_directory' => $example['publish_directory'] ?? null, 'static_image' => 'nginx:alpine', 'install_command' => $example['install_command'] ?? null, @@ -493,8 +540,9 @@ private function upsertApplication(Environment $environment, StandaloneDocker $d 'environment_id' => $environment->id, 'destination_id' => $destination->id, 'destination_type' => StandaloneDocker::class, - 'source_id' => 0, - 'source_type' => GithubApp::class, + 'source_id' => $example['source_id'] ?? 0, + 'source_type' => $example['source_type'] ?? GithubApp::class, + 'private_key_id' => $example['private_key_id'] ?? null, ]); $application->save(); diff --git a/tests/Feature/Api/EnvironmentVariableUpdateApiTest.php b/tests/Feature/Api/EnvironmentVariableUpdateApiTest.php index 1ff528bbf..14821756e 100644 --- a/tests/Feature/Api/EnvironmentVariableUpdateApiTest.php +++ b/tests/Feature/Api/EnvironmentVariableUpdateApiTest.php @@ -15,7 +15,7 @@ uses(RefreshDatabase::class); beforeEach(function () { - InstanceSettings::updateOrCreate(['id' => 0]); + InstanceSettings::unguarded(fn () => InstanceSettings::firstOrCreate(['id' => 0], ['is_api_enabled' => true])); $this->team = Team::factory()->create(); $this->user = User::factory()->create(); @@ -252,3 +252,94 @@ $response->assertJsonFragment(['uuid' => ['This field is not allowed.']]); }); }); + +describe('environment variable key validation for app and service APIs', function () { + test('rejects invalid service environment variable keys on create update and bulk', function () { + $service = Service::factory()->create([ + 'server_id' => $this->server->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + 'environment_id' => $this->environment->id, + ]); + + EnvironmentVariable::create([ + 'key' => 'SAFE_KEY', + 'value' => 'old-value', + 'resourceable_type' => Service::class, + 'resourceable_id' => $service->id, + 'is_preview' => false, + ]); + + $headers = [ + 'Authorization' => 'Bearer '.$this->bearerToken, + 'Content-Type' => 'application/json', + ]; + + $this->withHeaders($headers) + ->postJson("/api/v1/services/{$service->uuid}/envs", [ + 'key' => 'BAD$(id)', + 'value' => '1', + ]) + ->assertStatus(422); + + $this->withHeaders($headers) + ->patchJson("/api/v1/services/{$service->uuid}/envs", [ + 'key' => 'BAD$(id)', + 'value' => '1', + ]) + ->assertStatus(422); + + $this->withHeaders($headers) + ->patchJson("/api/v1/services/{$service->uuid}/envs/bulk", [ + 'data' => [[ + 'key' => 'BAD$(id)', + 'value' => '1', + ]], + ]) + ->assertStatus(422); + }); + + test('rejects invalid application environment variable keys on create update and bulk', function () { + $application = Application::factory()->create([ + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + ]); + + EnvironmentVariable::create([ + 'key' => 'SAFE_KEY', + 'value' => 'old-value', + 'resourceable_type' => Application::class, + 'resourceable_id' => $application->id, + 'is_preview' => false, + ]); + + $headers = [ + 'Authorization' => 'Bearer '.$this->bearerToken, + 'Content-Type' => 'application/json', + ]; + + $this->withHeaders($headers) + ->postJson("/api/v1/applications/{$application->uuid}/envs", [ + 'key' => 'BAD$(id)', + 'value' => '1', + ]) + ->assertStatus(422); + + $this->withHeaders($headers) + ->patchJson("/api/v1/applications/{$application->uuid}/envs", [ + 'key' => 'BAD$(id)', + 'value' => '1', + ]) + ->assertStatus(422); + + $this->withHeaders($headers) + ->patchJson("/api/v1/applications/{$application->uuid}/envs/bulk", [ + 'data' => [[ + 'key' => 'BAD$(id)', + 'value' => '1', + ]], + ]) + ->assertStatus(422); + }); +}); diff --git a/tests/Feature/DatabaseEnvironmentVariableApiTest.php b/tests/Feature/DatabaseEnvironmentVariableApiTest.php index f3297cf17..1f1d46483 100644 --- a/tests/Feature/DatabaseEnvironmentVariableApiTest.php +++ b/tests/Feature/DatabaseEnvironmentVariableApiTest.php @@ -14,7 +14,7 @@ uses(RefreshDatabase::class); beforeEach(function () { - InstanceSettings::updateOrCreate(['id' => 0]); + InstanceSettings::unguarded(fn () => InstanceSettings::firstOrCreate(['id' => 0], ['is_api_enabled' => true])); $this->team = Team::factory()->create(); $this->user = User::factory()->create(); @@ -344,3 +344,44 @@ function createDatabase($context): StandalonePostgresql $response->assertStatus(404); }); }); + +describe('environment variable key validation for database APIs', function () { + test('rejects invalid database environment variable keys on create update and bulk', function () { + $database = createDatabase($this); + + EnvironmentVariable::create([ + 'key' => 'SAFE_KEY', + 'value' => 'old-value', + 'resourceable_type' => StandalonePostgresql::class, + 'resourceable_id' => $database->id, + ]); + + $headers = [ + 'Authorization' => 'Bearer '.$this->bearerToken, + 'Content-Type' => 'application/json', + ]; + + $this->withHeaders($headers) + ->postJson("/api/v1/databases/{$database->uuid}/envs", [ + 'key' => 'BAD$(id)', + 'value' => '1', + ]) + ->assertStatus(422); + + $this->withHeaders($headers) + ->patchJson("/api/v1/databases/{$database->uuid}/envs", [ + 'key' => 'BAD$(id)', + 'value' => '1', + ]) + ->assertStatus(422); + + $this->withHeaders($headers) + ->patchJson("/api/v1/databases/{$database->uuid}/envs/bulk", [ + 'data' => [[ + 'key' => 'BAD$(id)', + 'value' => '1', + ]], + ]) + ->assertStatus(422); + }); +}); diff --git a/tests/Feature/DevelopmentRailpackExamplesSeederTest.php b/tests/Feature/DevelopmentRailpackExamplesSeederTest.php index 59646a804..321b8e52b 100644 --- a/tests/Feature/DevelopmentRailpackExamplesSeederTest.php +++ b/tests/Feature/DevelopmentRailpackExamplesSeederTest.php @@ -2,6 +2,7 @@ use App\Models\Application; use App\Models\GithubApp; +use App\Models\GitlabApp; use App\Models\PrivateKey; use App\Models\Project; use App\Models\Server; @@ -42,6 +43,7 @@ function seedRailpackExamplePrerequisites(): void expect(Server::query()->find(0))->not->toBeNull(); expect(StandaloneDocker::query()->find(0))->not->toBeNull(); expect(GithubApp::query()->find(0))->not->toBeNull(); + expect(GitlabApp::query()->find(1))->not->toBeNull(); expect(Project::query()->where('uuid', DevelopmentRailpackExamplesSeeder::PROJECT_UUID)->exists())->toBeTrue(); expect(Application::query()->count())->toBe(count(DevelopmentRailpackExamplesSeeder::examples())); }); @@ -66,9 +68,10 @@ function seedRailpackExamplePrerequisites(): void expect($applications)->toHaveCount(count(DevelopmentRailpackExamplesSeeder::examples())); expect($applications->every(fn (Application $application) => $application->build_pack === 'railpack'))->toBeTrue(); - expect($applications->every(fn (Application $application) => $application->git_repository === DevelopmentRailpackExamplesSeeder::GIT_REPOSITORY))->toBeTrue(); - $examples = collect(DevelopmentRailpackExamplesSeeder::examples())->keyBy('uuid'); + expect($applications->every( + fn (Application $application) => $application->git_repository === ($examples->get($application->uuid)['git_repository'] ?? DevelopmentRailpackExamplesSeeder::GIT_REPOSITORY) + ))->toBeTrue(); expect($applications->every( fn (Application $application) => $application->git_branch === ($examples->get($application->uuid)['git_branch'] ?? DevelopmentRailpackExamplesSeeder::GIT_BRANCH) ))->toBeTrue(); @@ -79,6 +82,9 @@ function seedRailpackExamplePrerequisites(): void $pythonFlask = $applications->firstWhere('uuid', 'railpack-python-flask'); $goGin = $applications->firstWhere('uuid', 'railpack-go-gin'); $rust = $applications->firstWhere('uuid', 'railpack-rust'); + $githubDeployKey = $applications->firstWhere('uuid', 'railpack-github-deploy-key'); + $gitlabDeployKey = $applications->firstWhere('uuid', 'railpack-gitlab-deploy-key'); + $gitlabPublic = $applications->firstWhere('uuid', 'railpack-gitlab-public-example'); expect($nestjs) ->not->toBeNull() @@ -113,6 +119,33 @@ function seedRailpackExamplePrerequisites(): void expect($rust) ->not->toBeNull() ->and($rust->ports_exposes)->toBe('8000'); + + expect($githubDeployKey) + ->not->toBeNull() + ->and($githubDeployKey->git_repository)->toBe('git@github.com:coollabsio/coolify-examples-deploy-key.git') + ->and($githubDeployKey->git_branch)->toBe('main') + ->and($githubDeployKey->build_pack)->toBe('railpack') + ->and($githubDeployKey->private_key_id)->toBe(1) + ->and($githubDeployKey->source_type)->toBe(GithubApp::class) + ->and($githubDeployKey->source_id)->toBe(0); + + expect($gitlabDeployKey) + ->not->toBeNull() + ->and($gitlabDeployKey->git_repository)->toBe('git@gitlab.com:coollabsio/php-example.git') + ->and($gitlabDeployKey->git_branch)->toBe('main') + ->and($gitlabDeployKey->build_pack)->toBe('railpack') + ->and($gitlabDeployKey->private_key_id)->toBe(1) + ->and($gitlabDeployKey->source_type)->toBe(GitlabApp::class) + ->and($gitlabDeployKey->source_id)->toBe(1); + + expect($gitlabPublic) + ->not->toBeNull() + ->and($gitlabPublic->git_repository)->toBe('https://gitlab.com/andrasbacsai/coolify-examples.git') + ->and($gitlabPublic->base_directory)->toBe('/astro/static') + ->and($gitlabPublic->publish_directory)->toBe('/dist') + ->and($gitlabPublic->build_pack)->toBe('railpack') + ->and($gitlabPublic->source_type)->toBe(GitlabApp::class) + ->and($gitlabPublic->settings->is_static)->toBeTrue(); }); it('skips the railpack examples outside development mode', function () { diff --git a/tests/Feature/EnvironmentVariable/MultilineEnvironmentVariableTest.php b/tests/Feature/EnvironmentVariable/MultilineEnvironmentVariableTest.php index 453e11109..4205c5cc2 100644 --- a/tests/Feature/EnvironmentVariable/MultilineEnvironmentVariableTest.php +++ b/tests/Feature/EnvironmentVariable/MultilineEnvironmentVariableTest.php @@ -9,8 +9,8 @@ $buildArgs = generateDockerBuildArgs($variables); // Docker gets values from the environment, so only keys should be in build args - expect($buildArgs->first())->toBe('--build-arg SSH_PRIVATE_KEY'); - expect($buildArgs->last())->toBe('--build-arg REGULAR_VAR'); + expect($buildArgs->first())->toBe("--build-arg 'SSH_PRIVATE_KEY'"); + expect($buildArgs->last())->toBe("--build-arg 'REGULAR_VAR'"); }); test('generateDockerBuildArgs works with collection of objects', function () { @@ -22,8 +22,8 @@ $buildArgs = generateDockerBuildArgs($variables); expect($buildArgs)->toHaveCount(2); expect($buildArgs->values()->toArray())->toBe([ - '--build-arg VAR1', - '--build-arg VAR2', + "--build-arg 'VAR1'", + "--build-arg 'VAR2'", ]); }); @@ -38,7 +38,7 @@ // The collection must be imploded to a string for command interpolation // This was the bug: Collection was interpolated as JSON instead of a space-separated string $argsString = $buildArgs->implode(' '); - expect($argsString)->toBe('--build-arg COOLIFY_URL --build-arg COOLIFY_BRANCH'); + expect($argsString)->toBe("--build-arg 'COOLIFY_URL' --build-arg 'COOLIFY_BRANCH'"); // Verify it does NOT produce JSON when cast to string expect($argsString)->not->toContain('{'); @@ -53,7 +53,7 @@ $buildArgs = generateDockerBuildArgs($variables); $arg = $buildArgs->first(); - expect($arg)->toBe('--build-arg NO_FLAG_VAR'); + expect($arg)->toBe("--build-arg 'NO_FLAG_VAR'"); }); test('generateDockerEnvFlags produces correct format', function () { @@ -81,3 +81,17 @@ expect($envFlags)->toContain('-e VAR1='); expect($envFlags)->toContain('-e VAR2="'); }); + +test('generateDockerBuildArgs escapes legacy keys', function () { + $variables = [ + ['key' => 'BAD$(id)', 'value' => '1'], + ['key' => "BAD'KEY", 'value' => '1'], + ]; + + $buildArgs = generateDockerBuildArgs($variables); + + expect($buildArgs->values()->toArray())->toBe([ + "--build-arg 'BAD$(id)'", + "--build-arg 'BAD'\''KEY'", + ]); +}); diff --git a/tests/Feature/Webhook/WebhookHmacTest.php b/tests/Feature/Webhook/WebhookHmacTest.php index 3f49ff43d..011b36731 100644 --- a/tests/Feature/Webhook/WebhookHmacTest.php +++ b/tests/Feature/Webhook/WebhookHmacTest.php @@ -2,6 +2,7 @@ use App\Models\Application; use App\Models\Environment; +use App\Models\GithubApp; use App\Models\Project; use App\Models\Server; use App\Models\Team; @@ -100,6 +101,38 @@ function createApplicationWithWebhook(string $repo = 'test-org/test-repo', strin }); }); +describe('GitHub App Webhook HMAC', function () { + test('rejects push when app webhook secret is empty', function () { + $team = Team::factory()->create(); + GithubApp::create([ + 'uuid' => (string) str()->uuid(), + 'name' => 'github-app-webhook-test', + 'api_url' => 'https://api.github.com', + 'html_url' => 'https://github.com', + 'app_id' => 1234567890, + 'webhook_secret' => null, + 'team_id' => $team->id, + ]); + + $payload = json_encode([ + 'ref' => 'refs/heads/main', + 'repository' => ['id' => 987654321], + 'after' => 'abc123', + 'commits' => [], + ]); + + $response = $this->call('POST', '/webhooks/source/github/events', [], [], [], [ + 'HTTP_X-GitHub-Event' => 'push', + 'HTTP_X-GitHub-Hook-Installation-Target-Id' => '1234567890', + 'HTTP_X-Hub-Signature-256' => 'sha256='.hash_hmac('sha256', $payload, ''), + 'CONTENT_TYPE' => 'application/json', + ], $payload); + + $response->assertOk(); + expect($response->getContent())->toContain('Invalid signature'); + }); +}); + describe('GitLab Manual Webhook HMAC', function () { test('rejects push when secret is empty', function () { $app = createApplicationWithWebhook(); diff --git a/tests/Unit/DeployKeyDedicatedPathTest.php b/tests/Unit/DeployKeyDedicatedPathTest.php index a3373f42a..59a937fc5 100644 --- a/tests/Unit/DeployKeyDedicatedPathTest.php +++ b/tests/Unit/DeployKeyDedicatedPathTest.php @@ -4,11 +4,50 @@ 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_) instead of the shared @@ -19,6 +58,24 @@ */ $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 @@ $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 @@ $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 @@ $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 @@ // 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 @@ $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 @@ $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 @@ $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); }); diff --git a/tests/Unit/GitlabSourceCommandsTest.php b/tests/Unit/GitlabSourceCommandsTest.php index 077b21590..129a86506 100644 --- a/tests/Unit/GitlabSourceCommandsTest.php +++ b/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 @@ $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 @@ 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 @@ 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); diff --git a/tests/Unit/ValidationPatternsTest.php b/tests/Unit/ValidationPatternsTest.php index a959b18d5..2b5763177 100644 --- a/tests/Unit/ValidationPatternsTest.php +++ b/tests/Unit/ValidationPatternsTest.php @@ -132,24 +132,31 @@ ->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('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 () {