From aa2b6b862b03f21aeab8bc02ad663868cc722755 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 21 Sep 2026 15:29:50 +0200 Subject: [PATCH] fix(compose): validate Docker network names Require Compose network names to match Docker identifier rules. Ensure missing proxy networks with inspect and a single escaped argument. --- bootstrap/helpers/parsers.php | 37 ++++++ bootstrap/helpers/proxy.php | 58 +++++---- tests/Unit/PreSaveValidationTest.php | 71 +++++++++++ .../ProxyComposeNetworkValidationTest.php | 110 ++++++++++++++++++ 4 files changed, 251 insertions(+), 25 deletions(-) create mode 100644 tests/Unit/ProxyComposeNetworkValidationTest.php diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index f145ec0477..91411c61f7 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -9,6 +9,7 @@ use App\Models\LocalPersistentVolume; use App\Models\Service; use App\Models\ServiceApplication; use App\Models\ServiceDatabase; +use App\Support\ValidationPatterns; use Illuminate\Support\Collection; use Illuminate\Support\Facades\File; use Illuminate\Support\Str; @@ -97,6 +98,42 @@ function validateDockerComposeForInjection(string $composeYaml): void } } } + + if (is_array($serviceConfig) && isset($serviceConfig['networks']) && is_array($serviceConfig['networks'])) { + foreach ($serviceConfig['networks'] as $networkKey => $networkDetails) { + if (is_int($networkKey) && (is_string($networkDetails) || is_int($networkDetails))) { + validateComposeNetworkName((string) $networkDetails, 'service network'); + } elseif (is_string($networkKey) || is_int($networkKey)) { + validateComposeNetworkName((string) $networkKey, 'service network'); + } + } + } + } + + if (isset($parsed['networks']) && is_array($parsed['networks'])) { + foreach ($parsed['networks'] as $networkName => $networkConfig) { + if (is_string($networkName) || is_int($networkName)) { + validateComposeNetworkName((string) $networkName); + } + if (is_array($networkConfig) && isset($networkConfig['name']) && is_string($networkConfig['name'])) { + validateComposeNetworkName($networkConfig['name'], 'network name field'); + } + } + } +} + +/** + * Reject Docker Compose network names that are not valid Docker identifiers. + * + * @throws Exception If the network name is not a valid Docker network identifier + */ +function validateComposeNetworkName(string $networkName, string $context = 'network name'): void +{ + if ($networkName === '' || ! ValidationPatterns::isValidDockerNetwork($networkName)) { + throw new Exception( + 'Invalid Docker Compose '.$context. + '. Network names must start with an alphanumeric character and contain only alphanumeric characters, dots, hyphens, and underscores.' + ); } } diff --git a/bootstrap/helpers/proxy.php b/bootstrap/helpers/proxy.php index 4484701785..8f77ce232c 100644 --- a/bootstrap/helpers/proxy.php +++ b/bootstrap/helpers/proxy.php @@ -4,6 +4,7 @@ use App\Actions\Proxy\SaveProxyConfiguration; use App\Enums\ProxyTypes; use App\Models\Application; use App\Models\Server; +use App\Support\ValidationPatterns; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Log; use Symfony\Component\Yaml\Yaml; @@ -115,6 +116,28 @@ function isDockerPredefinedNetwork(string $network): bool return in_array($network, ['default', 'host'], true); } +function isUsableDockerNetworkName(mixed $network): bool +{ + return is_string($network) + && $network !== '' + && ! isDockerPredefinedNetwork($network) + && ValidationPatterns::isValidDockerNetwork($network); +} + +/** + * Create a Docker network when it does not exist. The network name is always a single escaped argument. + */ +function dockerNetworkEnsureCommand(string $network, bool $overlay = false, bool $quietCreate = false): string +{ + $safe = escapeshellarg($network); + $createFlags = $overlay + ? '--driver overlay --attachable' + : '--attachable'; + $quiet = $quietCreate ? ' >/dev/null' : ''; + + return "docker network inspect {$safe} >/dev/null 2>&1 || docker network create {$createFlags} {$safe}{$quiet}"; +} + function collectProxyDockerNetworksByServer(Server $server) { if (! $server->isFunctional()) { @@ -175,12 +198,8 @@ function collectDockerNetworksByServer(Server $server) $networks->push($network); $allNetworks->push($network); } - $networks = collect($networks)->flatten()->unique()->filter(function ($network) { - return ! isDockerPredefinedNetwork($network); - }); - $allNetworks = $allNetworks->flatten()->unique()->filter(function ($network) { - return ! isDockerPredefinedNetwork($network); - }); + $networks = collect($networks)->flatten()->unique()->filter(fn ($network) => isUsableDockerNetworkName($network)); + $allNetworks = $allNetworks->flatten()->unique()->filter(fn ($network) => isUsableDockerNetworkName($network)); if ($server->isSwarm()) { if ($networks->count() === 0) { $networks = collect(['coolify-overlay']); @@ -206,7 +225,7 @@ function connectProxyToNetworks(Server $server) $safe = escapeshellarg($network); return [ - "docker network ls --format '{{.Name}}' | grep '^{$network}$' >/dev/null || docker network create --driver overlay --attachable {$safe} >/dev/null", + dockerNetworkEnsureCommand($network, overlay: true, quietCreate: true), "docker network connect {$safe} coolify-proxy >/dev/null 2>&1 || true", "echo 'Successfully connected coolify-proxy to {$safe} network.'", ]; @@ -238,25 +257,14 @@ function ensureProxyNetworksExist(Server $server) { ['allNetworks' => $networks] = collectDockerNetworksByServer($server); - if ($server->isSwarm()) { - $commands = $networks->map(function ($network) { - $safe = escapeshellarg($network); + $commands = $networks->map(function ($network) use ($server) { + $safe = escapeshellarg($network); - return [ - "echo 'Ensuring network {$safe} exists...'", - "docker network ls --format '{{.Name}}' | grep -q '^{$network}$' || docker network create --driver overlay --attachable {$safe}", - ]; - }); - } else { - $commands = $networks->map(function ($network) { - $safe = escapeshellarg($network); - - return [ - "echo 'Ensuring network {$safe} exists...'", - "docker network ls --format '{{.Name}}' | grep -q '^{$network}$' || docker network create --attachable {$safe}", - ]; - }); - } + return [ + "echo 'Ensuring network {$safe} exists...'", + dockerNetworkEnsureCommand($network, overlay: $server->isSwarm()), + ]; + }); return $commands->flatten(); } diff --git a/tests/Unit/PreSaveValidationTest.php b/tests/Unit/PreSaveValidationTest.php index c24cf5f898..4c4a289669 100644 --- a/tests/Unit/PreSaveValidationTest.php +++ b/tests/Unit/PreSaveValidationTest.php @@ -198,3 +198,74 @@ YAML; expect(fn () => validateDockerComposeForInjection($validCompose)) ->not->toThrow(Exception::class); }); + +test('validateDockerComposeForInjection blocks invalid top-level network names', function () { + $invalidCompose = <<<'YAML' +services: + web: + image: nginx:latest +networks: + "app'network": +YAML; + + expect(fn () => validateDockerComposeForInjection($invalidCompose)) + ->toThrow(Exception::class, 'Invalid Docker Compose network name'); +}); + +test('validateDockerComposeForInjection blocks invalid service network list items', function () { + $invalidCompose = <<<'YAML' +services: + web: + image: nginx:latest + networks: + - "app'network" +YAML; + + expect(fn () => validateDockerComposeForInjection($invalidCompose)) + ->toThrow(Exception::class, 'Invalid Docker Compose service network'); +}); + +test('validateDockerComposeForInjection blocks invalid service network map keys', function () { + $invalidCompose = <<<'YAML' +services: + web: + image: nginx:latest + networks: + "app'network": +YAML; + + expect(fn () => validateDockerComposeForInjection($invalidCompose)) + ->toThrow(Exception::class, 'Invalid Docker Compose service network'); +}); + +test('validateDockerComposeForInjection blocks invalid compose network name fields', function () { + $invalidCompose = <<<'YAML' +services: + web: + image: nginx:latest +networks: + frontend: + name: "app'network" +YAML; + + expect(fn () => validateDockerComposeForInjection($invalidCompose)) + ->toThrow(Exception::class, 'Invalid Docker Compose network name field'); +}); + +test('validateDockerComposeForInjection allows legitimate compose networks', function () { + $validCompose = <<<'YAML' +services: + web: + image: nginx:latest + networks: + - frontend + - backend +networks: + frontend: + backend: + name: app-backend +YAML; + + expect(fn () => validateDockerComposeForInjection($validCompose)) + ->not->toThrow(Exception::class); +}); diff --git a/tests/Unit/ProxyComposeNetworkValidationTest.php b/tests/Unit/ProxyComposeNetworkValidationTest.php new file mode 100644 index 0000000000..54eae4a505 --- /dev/null +++ b/tests/Unit/ProxyComposeNetworkValidationTest.php @@ -0,0 +1,110 @@ +makePartial(); + $server->shouldReceive('isSwarm')->andReturn($swarm); + + $destinationNetworks ??= $swarm ? ['coolify-overlay'] : ['coolify']; + + if ($swarm) { + $server->swarmDockers = collect(array_map(fn (string $network) => ['network' => $network], $destinationNetworks)); + $server->standaloneDockers = collect(); + } else { + $server->standaloneDockers = collect(array_map(fn (string $network) => ['network' => $network], $destinationNetworks)); + $server->swarmDockers = collect(); + } + + $services = collect($serviceNetworks)->map(function (array $networks) use ($serviceRunning) { + $service = Mockery::mock(); + $service->shouldReceive('isRunning')->andReturn($serviceRunning); + $service->shouldReceive('networks')->andReturn(collect($networks)); + + return $service; + }); + + $relation = Mockery::mock(); + $relation->shouldReceive('get')->andReturn($services); + $server->shouldReceive('services')->andReturn($relation); + $server->shouldReceive('dockerComposeBasedApplications')->andReturn(collect()); + $server->shouldReceive('dockerComposeBasedPreviewDeployments')->andReturn(collect()); + + return $server; +} + +it('creates networks with inspect and a single escaped argument', function () { + $safe = escapeshellarg('frontend'); + + expect(dockerNetworkEnsureCommand('frontend')) + ->toBe("docker network inspect {$safe} >/dev/null 2>&1 || docker network create --attachable {$safe}") + ->and(dockerNetworkEnsureCommand('frontend', overlay: true, quietCreate: true)) + ->toBe("docker network inspect {$safe} >/dev/null 2>&1 || docker network create --driver overlay --attachable {$safe} >/dev/null"); +}); + +it('keeps the inspect and create command structure when a network name contains quotes', function () { + $name = "app'network"; + $safe = escapeshellarg($name); + $command = dockerNetworkEnsureCommand($name); + + expect($command) + ->toBe("docker network inspect {$safe} >/dev/null 2>&1 || docker network create --attachable {$safe}") + ->not->toContain('| grep'); +}); + +it('keeps only valid compose network names when collecting server networks', function () { + $invalid = "app'network"; + $server = proxyNetworkTestServer(serviceNetworks: [[$invalid, 'frontend']]); + + ['allNetworks' => $allNetworks] = collectDockerNetworksByServer($server); + + expect($allNetworks->all()) + ->toContain('coolify') + ->toContain('frontend') + ->not->toContain($invalid); +}); + +it('builds proxy ensure commands with inspect and escaped create arguments', function () { + $invalid = "app'network"; + $server = proxyNetworkTestServer(serviceNetworks: [[$invalid, 'frontend']]); + + $commands = ensureProxyNetworksExist($server)->implode("\n"); + $safeFrontend = escapeshellarg('frontend'); + + expect($commands) + ->toContain("docker network inspect {$safeFrontend} >/dev/null 2>&1 || docker network create --attachable {$safeFrontend}") + ->not->toContain('| grep'); +}); + +it('builds swarm proxy connect commands with inspect and escaped create arguments', function () { + $invalid = "app'network"; + $server = proxyNetworkTestServer(swarm: true, serviceNetworks: [[$invalid, 'overlay-net']], serviceRunning: true); + + $commands = connectProxyToNetworks($server)->implode("\n"); + $safeOverlay = escapeshellarg('overlay-net'); + + expect($commands) + ->toContain("docker network inspect {$safeOverlay} >/dev/null 2>&1 || docker network create --driver overlay --attachable {$safeOverlay} >/dev/null") + ->toContain("docker network connect {$safeOverlay} coolify-proxy") + ->not->toContain('| grep'); +}); + +it('rejects unusable docker network names', function (string $network) { + expect(isUsableDockerNetworkName($network))->toBeFalse(); +})->with([ + 'quote' => "app'network", + 'semicolon' => 'net;id', + 'pipe' => 'net|id', + 'empty' => '', +]); + +it('accepts usable docker network names', function (string $network) { + expect(isUsableDockerNetworkName($network))->toBeTrue() + ->and(ValidationPatterns::isValidDockerNetwork($network))->toBeTrue(); +})->with([ + 'simple' => 'frontend', + 'hyphen' => 'coolify-proxy', + 'uuid-like' => 'abcdefghijklmnopqrstuvwx', +]);