diff --git a/app/Actions/Service/StartService.php b/app/Actions/Service/StartService.php index 8a2496a45d..196df2b954 100644 --- a/app/Actions/Service/StartService.php +++ b/app/Actions/Service/StartService.php @@ -76,19 +76,21 @@ class StartService } /** - * Connects a created container to the predefined network of the destination. Containers with - * a network_mode, or that already declare the network in the compose file, are skipped. + * Connects a container to the predefined network of the destination, unless it is already + * connected (declared in the compose file, or a running container that was not recreated). + * Containers with a network_mode cannot join other networks and are skipped. */ public static function predefinedNetworkConnectCommand(Service $service, string $serviceName, mixed $serviceConfig): ?string { - $network = $service->destination->network; - if (data_get($serviceConfig, 'network_mode') !== null || array_key_exists($network, data_get($serviceConfig, 'networks') ?? [])) { + if (data_get($serviceConfig, 'network_mode') !== null) { return null; } + $network = escapeshellarg($service->destination->network); $containerName = escapeshellarg("{$serviceName}-{$service->uuid}"); - $warning = escapeshellarg("Warning: Could not connect {$serviceName} to the {$network} network."); + $warning = escapeshellarg("Warning: Could not connect {$serviceName} to the {$service->destination->network} network."); + $connectedNetworks = "docker container inspect --format '{{range \$name, \$_ := .NetworkSettings.Networks}}{{println \$name}}{{end}}' {$containerName}"; - return 'docker network connect --alias '.$containerName.' '.escapeshellarg($network)." {$containerName} || echo {$warning}"; + return "{$connectedNetworks} | grep -qxF {$network} || docker network connect --alias {$containerName} {$network} {$containerName} || echo {$warning}"; } /** diff --git a/tests/Feature/Security/CommandInjectionSecurityTest.php b/tests/Feature/Security/CommandInjectionSecurityTest.php index 154e278eae..a5d97bdb98 100644 --- a/tests/Feature/Security/CommandInjectionSecurityTest.php +++ b/tests/Feature/Security/CommandInjectionSecurityTest.php @@ -948,7 +948,7 @@ describe('service application lifecycle command escaping', function () { $service->setRelation('destination', (new StandaloneDocker)->forceFill(['network' => 'coolify'])); expect(StartService::predefinedNetworkConnectCommand($service, 'app$(id)', [])) - ->toBe("docker network connect --alias 'app$(id)-svc-uuid' 'coolify' 'app$(id)-svc-uuid' || echo 'Warning: Could not connect app$(id) to the coolify network.'"); + ->toContain(" 'app$(id)-svc-uuid' | grep -qxF 'coolify' || docker network connect --alias 'app$(id)-svc-uuid' 'coolify' 'app$(id)-svc-uuid' || echo 'Warning: Could not connect app$(id) to the coolify network.'"); }); test('service application restart and stop actions escape container names', function (string $path) { diff --git a/tests/Feature/ServiceComposeStdinTest.php b/tests/Feature/ServiceComposeStdinTest.php index 8484d723ff..a45772ad22 100644 --- a/tests/Feature/ServiceComposeStdinTest.php +++ b/tests/Feature/ServiceComposeStdinTest.php @@ -66,7 +66,7 @@ function serviceComposeStdinService(string $sshUser, ?string $composeRaw = null) * * @return list the commands that ran, and the lines the prompt consumed */ -function runServiceScriptLikeSsh(string $script, string $stubDirectory): array +function runServiceScriptLikeSsh(string $script, string $stubDirectory, string $connectedNetworks = ''): array { File::ensureDirectoryExists($stubDirectory); $log = "{$stubDirectory}/calls.log"; @@ -74,6 +74,9 @@ function runServiceScriptLikeSsh(string $script, string $stubDirectory): array #!/bin/sh echo "docker $*" >> "$STUB_LOG" case " $* " in + *" container inspect "*) + printf '%s\n' $STUB_CONNECTED_NETWORKS + ;; *" compose "*" up "*|*" compose "*" pull"*) if IFS= read -r answer; then echo "prompt read: $answer" >> "$STUB_LOG"; fi ;; @@ -86,7 +89,7 @@ SH); chmod("{$stubDirectory}/{$stub}", 0755); } - $process = new SymfonyProcess(['bash', '-se'], env: ['PATH' => $stubDirectory.':'.getenv('PATH'), 'STUB_LOG' => $log], input: $script."\n", timeout: 30); + $process = new SymfonyProcess(['bash', '-se'], env: ['PATH' => $stubDirectory.':'.getenv('PATH'), 'STUB_LOG' => $log, 'STUB_CONNECTED_NETWORKS' => $connectedNetworks], input: $script."\n", timeout: 30); $process->run(); expect($process->isSuccessful())->toBeTrue($process->getErrorOutput()); @@ -167,13 +170,23 @@ it('connects service containers to the predefined network before compose starts } })->with(['root' => 'root', 'non-root' => 'deploy']); -it('does not connect containers that use a network mode or already declare the predefined network', function () { - $compose = "services:\n vpn:\n image: busybox:latest\n networks:\n - default\n - coolify\n app:\n image: busybox:latest\n network_mode: service:vpn\nnetworks:\n coolify:\n external: true\n"; +it('does not connect a running service container that is already on the predefined network', function (string $sshUser) { + $service = serviceComposeStdinService($sshUser); + $service->parse(); + + $calls = runServiceScriptLikeSsh(RemoteProcessCommand::read(DeployServiceApplication::run($service->applications()->firstOrFail())), $this->stubDirectory, connectedNetworks: "{$service->uuid} coolify"); + + expect(collect($calls)->filter(fn (string $call) => str_contains($call, 'network connect --alias'))->all())->toBe([]); +})->with(['root' => 'root', 'non-root' => 'deploy']); + +it('does not connect containers that use a network mode', function () { + $compose = "services:\n vpn:\n image: busybox:latest\n app:\n image: busybox:latest\n network_mode: service:vpn\n"; $service = serviceComposeStdinService('root', $compose); $calls = runServiceScriptLikeSsh(RemoteProcessCommand::read(StartService::run($service)), $this->stubDirectory); - expect(collect($calls)->filter(fn (string $call) => str_contains($call, 'network connect --alias'))->all())->toBe([]); + expect(collect($calls)->filter(fn (string $call) => str_contains($call, 'network connect --alias'))->values()->all()) + ->toBe(["docker network connect --alias vpn-{$service->uuid} coolify vpn-{$service->uuid}"]); }); it('connects a single service container to the predefined network before it starts', function (string $sshUser) {