fix(sudo): do not follow symlinks when chown gives Coolify paths to a non-root SSH user

This commit is contained in:
Andras Bacsai
2026-10-06 12:56:50 +02:00
parent e79347295a
commit 3645c4747e
3 changed files with 54 additions and 4 deletions
+1 -1
View File
@@ -27,7 +27,7 @@ function shouldChangeOwnership(string $path): bool
*/
function ownershipCommand(string $path, Server $server): string
{
return "find $path -user root -exec chown $server->user:$server->user {} + && chmod o-rwx $path";
return "find $path -user root -exec chown -h $server->user:$server->user {} + && chmod o-rwx $path";
}
/**
@@ -276,7 +276,7 @@ test('adds ownership changes for Coolify data paths', function () {
$result = parseCommandsByLineForSudo($commands, $this->server);
expect($result[0])->toBe('sudo mkdir -p /data/coolify/logs && sudo find /data/coolify/logs -user root -exec chown ubuntu:ubuntu {} + && sudo chmod o-rwx /data/coolify/logs');
expect($result[0])->toBe('sudo mkdir -p /data/coolify/logs && sudo find /data/coolify/logs -user root -exec chown -h ubuntu:ubuntu {} + && sudo chmod o-rwx /data/coolify/logs');
});
test('adds ownership changes for Coolify tmp paths', function () {
@@ -286,7 +286,7 @@ test('adds ownership changes for Coolify tmp paths', function () {
$result = parseCommandsByLineForSudo($commands, $this->server);
expect($result[0])->toBe('sudo mkdir -p /tmp/coolify/cache && sudo find /tmp/coolify/cache -user root -exec chown ubuntu:ubuntu {} + && sudo chmod o-rwx /tmp/coolify/cache');
expect($result[0])->toBe('sudo mkdir -p /tmp/coolify/cache && sudo find /tmp/coolify/cache -user root -exec chown -h ubuntu:ubuntu {} + && sudo chmod o-rwx /tmp/coolify/cache');
});
test('ownership changes work where root may not use sudo', function (string $parser) {
@@ -372,7 +372,7 @@ test('ownership changes apply only to the mkdir path when more commands follow',
$process = new Process(['bash', '-c', $command], env: ['PATH' => "{$directory}:".getenv('PATH'), 'LOG' => "{$directory}/log"]);
$process->run();
expect($command)->toContain("find {$path}/dynamic -user root -exec chown ubuntu:ubuntu {} + && ")
expect($command)->toContain("find {$path}/dynamic -user root -exec chown -h ubuntu:ubuntu {} + && ")
->and($command)->toContain("chmod o-rwx {$path}/dynamic{$separator} ")
->and($command)->toEndWith('head -n 1000'.($parser === 'command list' ? "'" : ''))
->and($process->getErrorOutput())->toBe('')
+50
View File
@@ -0,0 +1,50 @@
<?php
use App\Models\Server;
use Symfony\Component\Process\Process;
afterEach(function () {
Mockery::close();
});
test('ownership changes do not follow symlinks to files outside the Coolify path', function (string $parser, string $tools) {
if (posix_geteuid() !== 0 || posix_getpwnam('daemon') === false) {
$this->markTestSkipped('Needs root and a daemon user to change file owners.');
}
if ($tools === 'busybox' && ! is_executable('/usr/bin/busybox')) {
$this->markTestSkipped('Needs BusyBox.');
}
$directory = sys_get_temp_dir().'/coolify-sudo-'.bin2hex(random_bytes(4));
mkdir($directory);
file_put_contents("{$directory}/sudo", "#!/bin/sh\nexec \"\$@\"\n");
chmod("{$directory}/sudo", 0755);
if ($tools === 'busybox') {
foreach (['find', 'chown', 'chmod', 'mkdir'] as $tool) {
symlink('/usr/bin/busybox', "{$directory}/{$tool}");
}
}
// A root-owned file outside Coolify's directories, like /etc/sudoers.
$target = "{$directory}/sudoers";
file_put_contents($target, 'root ALL=(ALL) ALL');
$path = '/tmp/coolify/symlink-test-'.bin2hex(random_bytes(4));
mkdir($path, 0700, true);
symlink($target, "{$path}/link");
$server = Mockery::mock(Server::class)->makePartial();
$server->shouldReceive('getAttribute')->with('user')->andReturn('daemon');
$server->shouldReceive('setAttribute')->andReturnSelf();
$command = $parser === 'command list'
? parseCommandsByLineForSudo(collect(["mkdir -p {$path}"]), $server)[0]
: parseLineForSudo("mkdir -p {$path}", $server);
$process = new Process(['/bin/sh', '-c', $command], env: ['PATH' => "{$directory}:".getenv('PATH')]);
$process->run();
clearstatcache();
expect($process->getErrorOutput())->toBe('')
->and($process->isSuccessful())->toBeTrue()
->and(fileowner($target))->toBe(0)
->and(lstat("{$path}/link")['uid'])->toBe(1)
->and(fileowner($path))->toBe(1);
(new Process(['rm', '-rf', $directory, $path]))->run();
})->with(['command list', 'deployment line'])->with(['coreutils', 'busybox']);