From 4766a662fe5365bc4fff498a67f651ec9eb91496 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 7 Sep 2026 14:08:11 +0200 Subject: [PATCH] fix: preserve shell negation in sudo commands --- bootstrap/helpers/sudo.php | 9 +++- tests/Unit/ParseCommandsByLineForSudoTest.php | 46 +++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/bootstrap/helpers/sudo.php b/bootstrap/helpers/sudo.php index 397efc387..98dbe3af7 100644 --- a/bootstrap/helpers/sudo.php +++ b/bootstrap/helpers/sudo.php @@ -59,13 +59,18 @@ function parseCommandsByLineForSudo(Collection $commands, Server $server): array return $line; } + // Negation belongs to the shell, before the elevated command. + if (preg_match('/^\s*!\s+/', $line)) { + return preg_replace('/^(\s*(?:!\s+)+)/', '$1sudo ', $line); + } + // Check all keywords with word boundary matching // Match keyword followed by space, semicolon, or end of line foreach ($bashKeywords as $keyword) { if (preg_match('/^'.preg_quote($keyword, '/').'(\s|;|$)/', $trimmedLine)) { - // Special handling for 'if' - insert sudo after 'if ' + // Keep any shell negation before sudo in the condition. if ($keyword === 'if') { - return preg_replace('/^(\s*)if\s+/', '$1if sudo ', $line); + return preg_replace('/^(\s*if\s+(?:!\s+)*)/', '$1sudo ', $line); } return $line; diff --git a/tests/Unit/ParseCommandsByLineForSudoTest.php b/tests/Unit/ParseCommandsByLineForSudoTest.php index b741b875f..93c9f8b64 100644 --- a/tests/Unit/ParseCommandsByLineForSudoTest.php +++ b/tests/Unit/ParseCommandsByLineForSudoTest.php @@ -1,6 +1,7 @@ toBe('if sudo command -v docker'); }); +test('preserves shell negation before sudo', function (string $command, string $expected) { + expect(parseCommandsByLineForSudo(collect([$command]), $this->server))->toBe([$expected]); +})->with([ + 'negated condition' => ['if ! docker inspect coolify-proxy; then', 'if ! sudo docker inspect coolify-proxy; then'], + 'indented condition' => ["\tif ! docker inspect coolify-proxy; then", "\tif ! sudo docker inspect coolify-proxy; then"], + 'negated pipeline' => [ + ' if ! docker ps -a --format "{{.Names}}" | grep -q "^coolify-proxy$"; then', + ' if ! sudo docker ps -a --format "{{.Names}}" | sudo grep -q "^coolify-proxy$"; then', + ], + 'standalone negation' => ['! docker inspect coolify-proxy', '! sudo docker inspect coolify-proxy'], + 'repeated negation' => ['if ! ! docker inspect coolify-proxy; then', 'if ! ! sudo docker inspect coolify-proxy; then'], + 'literal exclamation argument' => ['echo "! docker"', 'echo "! docker"'], +]); + +test('proxy removal loop exits only once the container is absent', function (string $containers, string $expected) { + $commands = collect([ + 'for i in {1..15}; do', + ' if ! docker ps -a --format "{{.Names}}" | grep -q "^coolify-proxy$"; then', + ' echo "Container removed successfully."', + ' break', + ' fi', + ' echo "Waiting for container to be removed... ($i/15)"', + 'done', + ]); + $script = <<<'BASH' +sudo() { "$@"; } +docker() { printf '%s\n' "$CONTAINERS"; } +BASH; + $script .= "\n".implode("\n", parseCommandsByLineForSudo($commands, $this->server)); + $process = new Process(['bash', '-c', $script], env: ['CONTAINERS' => $containers]); + $process->run(); + + expect($process->isSuccessful())->toBeTrue() + ->and($process->getErrorOutput())->toBe('') + ->and($process->getOutput())->toBe($expected); +})->with([ + 'removed' => ['', "Container removed successfully.\n"], + 'other containers remain' => ['another-container', "Container removed successfully.\n"], + 'still present' => ['coolify-proxy', implode('', array_map( + fn (int $attempt): string => "Waiting for container to be removed... ($attempt/15)\n", + range(1, 15), + ))], +]); + test('skips sudo for fi statements', function () { $commands = collect([ 'fi', @@ -457,6 +502,7 @@ // Verify other control structures remain correct expect($result[0])->toStartWith('if sudo docker ps'); + expect($result[6])->toBe(' if ! sudo docker ps -a --format "{{.Names}}" | sudo grep -q "^coolify-proxy$"; then'); expect($result[8])->toBe(' fi'); expect($result[13])->toBe('fi'); });