From 41e00bc1cbd40183390cac24ccb7f2607f6ed808 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 21 Sep 2026 14:57:15 +0200 Subject: [PATCH] fix(deployments): validate compose build paths --- app/Jobs/ApplicationDeploymentJob.php | 88 +++++++++++++++------ tests/Unit/ComposeBuildPathSecurityTest.php | 41 ++++++++++ 2 files changed, 107 insertions(+), 22 deletions(-) create mode 100644 tests/Unit/ComposeBuildPathSecurityTest.php diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 961228f949..54cbfebfcd 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -744,6 +744,8 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue return; } + $this->validateComposeBuildPaths($composeFile); + // Add build secrets to compose file if enabled and BuildKit is supported if ($this->dockerSecretsSupported && ! empty($this->build_secrets)) { $composeFile = $this->add_build_secrets_to_compose($composeFile); @@ -4866,38 +4868,25 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); continue; } - $context = '.'; - $dockerfile = 'Dockerfile'; - - if (is_string($service['build'])) { - $context = $service['build']; - } elseif (is_array($service['build'])) { - $context = data_get($service['build'], 'context', '.'); - $dockerfile = data_get($service['build'], 'dockerfile', 'Dockerfile'); - } - - $dockerfilePath = rtrim($context, '/').'/'.ltrim($dockerfile, '/'); - if (str_starts_with($dockerfilePath, './')) { - $dockerfilePath = substr($dockerfilePath, 2); - } - if (str_starts_with($dockerfilePath, '/')) { - $dockerfilePath = substr($dockerfilePath, 1); - } + $dockerfilePath = $this->resolveComposeDockerfilePath($service['build']); + $fullDockerfilePath = escapeshellarg("{$this->workdir}/{$dockerfilePath}"); $this->execute_remote_command([ - executeInDocker($this->deployment_uuid, "test -f {$this->workdir}/{$dockerfilePath} && echo 'exists' || echo 'not found'"), + executeInDocker($this->deployment_uuid, "resolved_path=$(realpath -e -- {$fullDockerfilePath}) && test -f \"\$resolved_path\" && printf '%s' \"\$resolved_path\""), 'hidden' => true, 'save' => 'dockerfile_check_'.$serviceName, ]); - if (str($this->saved_outputs->get('dockerfile_check_'.$serviceName))->trim()->toString() !== 'exists') { + $resolvedDockerfilePath = str($this->saved_outputs->get('dockerfile_check_'.$serviceName))->trim()->toString(); + if (! str_starts_with($resolvedDockerfilePath, "{$this->workdir}/")) { $this->application_deployment_queue->addLogEntry("Dockerfile not found for service {$serviceName} at {$dockerfilePath}, skipping ARG injection."); continue; } + $fullDockerfilePath = escapeshellarg($resolvedDockerfilePath); $this->execute_remote_command([ - executeInDocker($this->deployment_uuid, "cat {$this->workdir}/{$dockerfilePath}"), + executeInDocker($this->deployment_uuid, "cat {$fullDockerfilePath}"), 'hidden' => true, 'save' => 'dockerfile_content_'.$serviceName, ]); @@ -4988,7 +4977,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); if ($totalAdded > 0) { $dockerfile_base64 = base64_encode($dockerfile_lines->implode("\n")); $this->execute_remote_command([ - executeInDocker($this->deployment_uuid, "echo '{$dockerfile_base64}' | base64 -d | tee {$this->workdir}/{$dockerfilePath} > /dev/null"), + executeInDocker($this->deployment_uuid, "echo '{$dockerfile_base64}' | base64 -d | tee {$fullDockerfilePath} > /dev/null"), 'hidden' => true, ]); @@ -4999,13 +4988,68 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); } if ($this->dockerSecretsSupported && ! empty($this->build_secrets)) { - $fullDockerfilePath = "{$this->workdir}/{$dockerfilePath}"; $this->modify_dockerfile_for_secrets($fullDockerfilePath); $this->application_deployment_queue->addLogEntry("Modified Dockerfile for service {$serviceName} to use build secrets."); } } } + private function validateComposeBuildPaths(array|Collection $composeFile): void + { + foreach (data_get($composeFile, 'services', []) as $service) { + if (isset($service['build'])) { + $this->resolveComposeDockerfilePath($service['build']); + } + } + } + + private function resolveComposeDockerfilePath(mixed $build): string + { + if (! is_string($build) && ! is_array($build)) { + throw new \RuntimeException('Invalid Docker Compose build definition.'); + } + + $context = is_string($build) ? $build : data_get($build, 'context', '.'); + $dockerfile = is_array($build) ? data_get($build, 'dockerfile', 'Dockerfile') : 'Dockerfile'; + + if (! is_string($context) || ! is_string($dockerfile)) { + throw new \RuntimeException('Invalid Docker Compose build path: context and dockerfile must be strings.'); + } + + $this->validateComposeBuildPath($context, 'context'); + $this->validateComposeBuildPath($dockerfile, 'dockerfile'); + + return $this->normalizeComposeBuildPath("{$context}/{$dockerfile}", 'dockerfile'); + } + + private function validateComposeBuildPath(string $path, string $fieldName): void + { + if ($path === '' || str_starts_with($path, '/') || ! preg_match('/^[a-zA-Z0-9._\-\/@+]+$/', $path)) { + throw new \RuntimeException("Invalid Docker Compose build.{$fieldName} path."); + } + } + + private function normalizeComposeBuildPath(string $path, string $fieldName): string + { + $segments = []; + foreach (explode('/', $path) as $segment) { + if ($segment === '' || $segment === '.') { + continue; + } + if ($segment === '..') { + if ($segments === []) { + throw new \RuntimeException("Invalid Docker Compose build.{$fieldName} path: path traversal outside the repository."); + } + array_pop($segments); + + continue; + } + $segments[] = $segment; + } + + return $segments === [] ? '.' : implode('/', $segments); + } + private function add_build_secrets_to_compose($composeFile) { // Generate env variables if not already done diff --git a/tests/Unit/ComposeBuildPathSecurityTest.php b/tests/Unit/ComposeBuildPathSecurityTest.php new file mode 100644 index 0000000000..9b1945115e --- /dev/null +++ b/tests/Unit/ComposeBuildPathSecurityTest.php @@ -0,0 +1,41 @@ +newInstanceWithoutConstructor(); + $method = new ReflectionMethod(ApplicationDeploymentJob::class, 'resolveComposeDockerfilePath'); + + return $method->invoke($job, $build); +} + +test('compose build paths resolve for short and long syntax', function (string|array $build, string $expected) { + expect(resolveComposeDockerfilePath($build))->toBe($expected); +})->with([ + 'short syntax' => ['services/api', 'services/api/Dockerfile'], + 'current directory short syntax' => ['.', 'Dockerfile'], + 'long syntax defaults' => [['context' => 'services/api'], 'services/api/Dockerfile'], + 'nested Dockerfile' => [['context' => './services/api', 'dockerfile' => 'docker/prod.Dockerfile'], 'services/api/docker/prod.Dockerfile'], + 'standard Dockerfile variants' => [['context' => '.', 'dockerfile' => 'Dockerfile.prod'], 'Dockerfile.prod'], + 'traversal that stays in repository' => [['context' => 'services/api', 'dockerfile' => '../Dockerfile'], 'services/Dockerfile'], +]); + +test('compose build paths reject repository escape and shell command injection', function (string|array $build) { + expect(fn () => resolveComposeDockerfilePath($build)) + ->toThrow(RuntimeException::class); +})->with([ + 'short syntax semicolon' => ['.; touch /tmp/short-context-pwned'], + 'short syntax command substitution' => ['$(touch /tmp/short-context-pwned)'], + 'context semicolon' => [['context' => '.; touch /tmp/context-pwned', 'dockerfile' => 'Dockerfile']], + 'dockerfile semicolon' => [['context' => '.', 'dockerfile' => 'Dockerfile; touch /tmp/dockerfile-pwned']], + 'context command substitution' => [['context' => '$(touch /tmp/context-pwned)', 'dockerfile' => 'Dockerfile']], + 'dockerfile command substitution' => [['context' => '.', 'dockerfile' => '$(touch /tmp/dockerfile-pwned)']], + 'context newline' => [['context' => "services/api\ntouch /tmp/context-pwned", 'dockerfile' => 'Dockerfile']], + 'dockerfile newline' => [['context' => '.', 'dockerfile' => "Dockerfile\ntouch /tmp/dockerfile-pwned"]], + 'context traversal' => [['context' => '../outside', 'dockerfile' => 'Dockerfile']], + 'nested traversal' => [['context' => 'services/api', 'dockerfile' => '../../../outside.Dockerfile']], + 'dockerfile traversal' => [['context' => '.', 'dockerfile' => '../outside.Dockerfile']], + 'context absolute path' => [['context' => '/tmp', 'dockerfile' => 'Dockerfile']], + 'dockerfile absolute path' => [['context' => '.', 'dockerfile' => '/tmp/Dockerfile']], +]);