diff --git a/app/Services/ProxyPortParser.php b/app/Services/ProxyPortParser.php index 3cad67ce6e..5d118ed278 100644 --- a/app/Services/ProxyPortParser.php +++ b/app/Services/ProxyPortParser.php @@ -7,11 +7,30 @@ use Symfony\Component\Yaml\Yaml; class ProxyPortParser { + /** + * Matches a Docker Compose variable: $VAR, ${VAR}, or ${VAR} with a :-, -, :?, ?, :+, or + modifier. + */ + private const VARIABLE_PATTERN = '/\$(?:\{[A-Za-z_][A-Za-z0-9_]*(?::?[-?+][^${}]*)?\}|[A-Za-z_][A-Za-z0-9_]*)/'; + + /** + * Matches ${VAR:-default} and ${VAR-default}; group 1 is the default. + */ + private const DEFAULT_PATTERN = '/\$\{[A-Za-z_][A-Za-z0-9_]*:?-([^${}]*)\}/'; + + /** + * Characters a port entry may contain outside of its variables. + */ + private const SAFE_REMAINDER_PATTERN = '/^[0-9A-Za-z.:\[\]\/-]*$/D'; + /** * Validates the proxy ports like Docker Compose does and returns the fixed host ports. * Ports that Docker publishes to a random host port or to a port range are validated * but not returned: the caller checks each returned port over its own SSH connection. * + * Docker Compose interpolates variables when it starts the proxy, so a variable never makes + * a port invalid: a variable with a default is checked as its default, and a port that still + * depends on a variable (or whose default is not a valid port) is accepted but not returned. + * * @return list */ public static function fromConfiguration(string $configuration): array @@ -53,20 +72,27 @@ class ProxyPortParser private static function publishedPort(mixed $configuredPort): ?int { + if (is_string($configuredPort) && self::containsVariable($configuredPort)) { + return self::publishedPortWithVariables($configuredPort); + } + if (is_array($configuredPort)) { if (array_is_list($configuredPort) || ! array_key_exists('target', $configuredPort)) { self::invalid(); } + $configuredPort = self::resolveLongSyntaxVariables($configuredPort, $hasUnresolvedVariables); + self::validateProtocol($configuredPort['protocol'] ?? null); if (array_key_exists('host_ip', $configuredPort)) { self::validateHostIp($configuredPort['host_ip']); } $target = self::portNumber($configuredPort['target']); - - return array_key_exists('published', $configuredPort) + $publishedPort = array_key_exists('published', $configuredPort) ? self::portOrRange($configuredPort['published']) : $target; + + return $hasUnresolvedVariables ? null : $publishedPort; } if (! is_int($configuredPort) && ! is_string($configuredPort)) { @@ -108,6 +134,87 @@ class ProxyPortParser : self::hostPort($parts[0], $parts[1]); } + private static function containsVariable(string $value): bool + { + return preg_match(self::VARIABLE_PATTERN, $value) === 1; + } + + /** + * Rejects an entry with anything but variables and port syntax, then replaces each + * variable that has a default with the default. Returns null if a variable remains. + */ + private static function resolveDefaults(string $value): ?string + { + if (preg_match(self::SAFE_REMAINDER_PATTERN, preg_replace(self::VARIABLE_PATTERN, '', $value)) !== 1) { + self::invalid(); + } + + $resolved = preg_replace_callback(self::DEFAULT_PATTERN, fn (array $matches): string => $matches[1], $value); + + return self::containsVariable($resolved) ? null : $resolved; + } + + private static function publishedPortWithVariables(string $configuredPort): ?int + { + $resolved = self::resolveDefaults($configuredPort); + if ($resolved === null) { + return null; + } + + try { + return self::publishedPort($resolved); + } catch (\InvalidArgumentException) { + // The variable may be set to a valid value when Docker Compose starts the proxy. + return null; + } + } + + /** + * Replaces long-syntax fields that use variables with their defaults. A field without a usable + * default is replaced with a valid placeholder (or removed) so the other fields are still validated. + * + * @param array $configuredPort + * @return array + */ + private static function resolveLongSyntaxVariables(array $configuredPort, ?bool &$hasUnresolvedVariables): array + { + $hasUnresolvedVariables = false; + $validators = [ + 'target' => fn (string $value) => self::portOrRange($value), + 'published' => fn (string $value) => self::portOrRange($value), + 'host_ip' => fn (string $value) => self::validateHostIp($value), + 'protocol' => fn (string $value) => self::validateProtocol($value), + ]; + + foreach ($validators as $field => $validate) { + $value = $configuredPort[$field] ?? null; + if (! is_string($value) || ! self::containsVariable($value)) { + continue; + } + + $resolved = self::resolveDefaults($value); + if ($resolved !== null) { + try { + $validate($resolved); + $configuredPort[$field] = $resolved; + + continue; + } catch (\InvalidArgumentException) { + // The variable may be set to a valid value when Docker Compose starts the proxy. + } + } + + $hasUnresolvedVariables = true; + if (in_array($field, ['target', 'published'], true)) { + $configuredPort[$field] = 1; + } else { + unset($configuredPort[$field]); + } + } + + return $configuredPort; + } + /** * Returns the fixed host port, or null for a random host port or a port range. */ diff --git a/tests/Feature/Proxy/ProxyPortVariablesTest.php b/tests/Feature/Proxy/ProxyPortVariablesTest.php new file mode 100644 index 0000000000..193de09421 --- /dev/null +++ b/tests/Feature/Proxy/ProxyPortVariablesTest.php @@ -0,0 +1,111 @@ + 0]); + config(['constants.ssh.mux_enabled' => false]); + Process::fake(); + + $team = Team::factory()->create(); + $this->server = Server::factory()->create([ + 'team_id' => $team->id, + 'user' => 'cooluser', + 'private_key_id' => PrivateKey::factory()->create(['team_id' => $team->id])->id, + ]); + $this->server->settings->update(['is_reachable' => true, 'is_usable' => true, 'force_disabled' => false]); + $this->server->proxy->type = ProxyTypes::TRAEFIK->value; + $this->server->proxy->status = 'exited'; + $this->server->save(); + $this->server->refresh(); +}); + +dataset('proxy port variables', [ + 'host port default' => ['${HTTP_PORT:-80}:80'], + 'host port without default' => ['${HTTP_PORT}:80'], + 'host IP with a host port default' => ['127.0.0.1:${P:-8080}:8080'], + 'host IP default' => ['${BIND_IP:-0.0.0.0}:80:80'], +]); + +function proxyConfigurationWithPort(string $port): string +{ + return "services:\n traefik:\n image: traefik:v3.6\n ports:\n - '{$port}'\n - '443:443'\n"; +} + +it('saves a proxy configuration whose ports use variables', function (string $port) { + $configuration = proxyConfigurationWithPort($port); + + SaveProxyConfiguration::run($this->server, $configuration); + + expect($this->server->fresh()->proxy->last_saved_proxy_configuration)->toBe($configuration); +})->with('proxy port variables'); + +it('starts a proxy whose ports use variables', function (string $port) { + $this->server->proxy->last_saved_proxy_configuration = proxyConfigurationWithPort($port); + $this->server->save(); + + StartProxy::run($this->server, async: false, force: true); + + Process::assertRan(fn ($process) => str_contains($process->command, 'docker compose -f /data/coolify/proxy/docker-compose.yml up -d --wait --remove-orphans')); +})->with('proxy port variables'); + +it('restarts a proxy whose ports use variables', function (string $port) { + Queue::fake(); + $this->server->proxy->last_saved_proxy_configuration = proxyConfigurationWithPort($port); + $this->server->save(); + + $job = new RestartProxyJob($this->server); + $job->handle(); + + expect($job->activity_id)->not->toBeNull() + ->and($this->server->fresh()->proxy->status)->toBe('restarting'); +})->with('proxy port variables'); + +it('asks to start a stopped proxy whose ports use variables and checks only concrete ports', function () { + $this->server->proxy->last_saved_proxy_configuration = "services:\n traefik:\n image: traefik:v3.6\n ports:\n - '\${HTTP_PORT:-8081}:80'\n - '\${HTTPS_PORT}:443'\n"; + $this->server->save(); + + expect(CheckProxy::run($this->server))->toBeTrue(); + + Process::assertRan(fn ($process) => str_contains($process->command, "sport = ':8081'")); + Process::assertDidntRun(fn ($process) => str_contains($process->command, 'sport = ') && str_contains($process->command, '${')); +}); + +it('configures traffic analytics for a proxy whose ports use variables', function () { + Queue::fake(); + StartSentinel::partialMock()->shouldReceive('handle')->atLeast()->once(); + GetProxyConfiguration::partialMock()->shouldReceive('handle')->once()->andReturn(proxyConfigurationWithPort('${HTTP_PORT}:80')); + SaveProxyConfiguration::partialMock()->shouldReceive('handle')->once(); + $this->server->proxy->status = 'running'; + $this->server->save(); + + ConfigureTrafficAnalytics::run($this->server, true); + + expect($this->server->fresh()->isTrafficAnalyticsEnabled())->toBeTrue(); + Queue::assertPushed(RestartProxyJob::class); +}); diff --git a/tests/Unit/ProxyPortValidationTest.php b/tests/Unit/ProxyPortValidationTest.php index 1275f181ab..2e8b13a4a9 100644 --- a/tests/Unit/ProxyPortValidationTest.php +++ b/tests/Unit/ProxyPortValidationTest.php @@ -17,6 +17,32 @@ it('extracts normalized published proxy ports from valid compose syntax', functi 'long syntax with a published range' => ["services:\n caddy:\n ports:\n - target: 443\n published: '8443-8444'\n protocol: UDP\n", []], ]); +it('accepts Docker Compose variables and checks only ports with a resolvable default', function (mixed $ports, array $expected) { + $configuration = Yaml::dump(['services' => ['traefik' => ['ports' => $ports]]], 8, 2); + + expect(ProxyPortParser::fromConfiguration($configuration))->toBe($expected); +})->with([ + 'host port default' => [['${HTTP_PORT:-80}:80'], [80]], + 'host port default without colon' => [['${HTTP_PORT-80}:80'], [80]], + 'host port without default' => [['${HTTP_PORT}:80'], []], + 'unbraced host port' => [['$HTTP_PORT:80'], []], + 'required host port' => [['${HTTP_PORT:?HTTP_PORT must be set}:80'], []], + 'alternative host port' => [['${HTTP_PORT:+8080}:80'], []], + 'host IP with a host port default' => [['127.0.0.1:${P:-8080}:8080'], [8080]], + 'host IP default' => [['${BIND_IP:-0.0.0.0}:80:80'], [80]], + 'host IP without default' => [['${BIND_IP}:443:443'], []], + 'container port variable' => [['8080:${CONTAINER_PORT}'], []], + 'default with protocol' => [['${HTTPS_PORT:-443}:443/udp'], [443]], + 'protocol variable' => [['443:443/${PROTOCOL:-udp}'], [443]], + 'invalid default is skipped like an unset variable' => [['${HTTP_PORT:-abc}:80'], []], + 'default range' => [['${RANGE:-10000-10100}:10000-10100'], []], + 'mixed with literal ports' => [['${HTTP_PORT:-80}:80', '${HTTPS_PORT}:443', '8080:8080'], [80, 8080]], + 'long syntax published default' => [[['target' => 80, 'published' => '${HTTP_PORT:-8080}']], [8080]], + 'long syntax published variable' => [[['target' => 80, 'published' => '${HTTP_PORT}']], []], + 'long syntax host IP default' => [[['target' => 80, 'published' => 80, 'host_ip' => '${BIND_IP:-127.0.0.1}']], [80]], + 'long syntax target variable' => [[['target' => '${TARGET}', 'published' => 9000]], []], +]); + it('rejects malformed proxy port values', function (mixed $port) { $configuration = Yaml::dump(['services' => ['traefik' => ['ports' => [$port]]]], 8, 2); @@ -30,7 +56,11 @@ it('rejects malformed proxy port values', function (mixed $port) { 'decimal' => 80.5, 'scientific notation' => '8e1', 'comma separated' => '80,443', - 'environment variable' => '${HTTP_PORT:-80}:80', + 'environment variable followed by a semicolon' => '${HTTP_PORT:-80};id:80', + 'environment variable with command substitution' => '${HTTP_PORT}$(id):80', + 'environment variable with backticks' => '`id`${HTTP_PORT}:80', + 'environment variable default with command substitution' => '${HTTP_PORT:-$(id)}:80', + 'environment variable with a pipe' => '${HTTP_PORT}:80|id', 'leading whitespace' => ' 80', 'trailing whitespace' => '80 ', 'newline' => "80\nid", @@ -68,6 +98,8 @@ it('rejects malformed proxy ports in long compose syntax', function (array $port 'invalid protocol' => [['target' => 80, 'published' => 8080, 'protocol' => 'http']], 'reversed published range' => [['target' => 80, 'published' => '90-80']], 'host injection' => [['target' => 80, 'published' => 8080, 'host_ip' => '$(id)']], + 'variable with command substitution' => [['target' => 80, 'published' => '${HTTP_PORT}$(id)']], + 'variable with an invalid protocol' => [['target' => 80, 'published' => '${HTTP_PORT}', 'protocol' => 'http']], ]); it('rejects invalid proxy port collection shapes', function (mixed $ports) {