mirror of
https://github.com/coollabsio/coolify.git
synced 2026-09-26 09:20:54 -04:00
fix(storage): confine remote paths on BusyBox and non-root servers
Replace the `realpath -m` confinement check with a POSIX sh script that uses `readlink -f`, which BusyBox (Alpine) also provides. The script walks up to the deepest existing path, resolves it, and appends the missing rest. It fails closed on dangling symlinks and on `.`/`..` in the missing part. Send the script as a single `sh -c '<script>' sh <base> <path>` line so the non-root sudo parser only adds sudo in front of it and does not rewrite `$(...)`, `&&` or case statements. Add unit tests that run the command with GNU and BusyBox tools, as root and through the sudo parser, against a real symlink tree. Update the feature test fakes to match the new command.
This commit is contained in:
@@ -61,6 +61,8 @@
|
||||
|
||||
## Test the real runtime image
|
||||
- Deployment shell commands run in the Alpine/BusyBox helper image and pass through the non-root sudo parser. Verify new flags and shell syntax in that image and with `parseCommandsByLineForSudo()`; faked command output hides both failures.
|
||||
- Put multi-step remote shell logic in one `sh -c '<script>' sh <args>` line. The non-root parser then only puts sudo in front of it; it rewrites `x=$(...)`, `&&`, `|` and shell keywords in any other line.
|
||||
- Dev QEMU servers from `dev:qemu` are seeded, not validated: they have no `coolify` Docker network, and Alpine has no bash until `InstallPrerequisites` runs.
|
||||
|
||||
## Format only your own files
|
||||
- `pint --dirty` also rewrites uncommitted files that belong to other work in the tree. When the tree has unrelated changes, pass your changed paths to Pint.
|
||||
|
||||
@@ -18,6 +18,31 @@ class LocalFileVolume extends BaseModel
|
||||
|
||||
public const TOO_LARGE_PLACEHOLDER = '[file too large to display]';
|
||||
|
||||
/**
|
||||
* Resolves $1 and $2 like `realpath -m`, but only with POSIX sh and `readlink -f`, which
|
||||
* BusyBox also has. It walks up to the deepest existing path, resolves it, and appends the
|
||||
* missing rest. A dangling symlink or `.`/`..` in the missing rest fails closed.
|
||||
*/
|
||||
private const REMOTE_PATH_CONFINEMENT_SCRIPT = <<<'SH'
|
||||
resolve() {
|
||||
path=$1
|
||||
rest=
|
||||
case $path in /*) ;; *) return 1 ;; esac
|
||||
while [ ! -e "$path" ]; do
|
||||
if [ -L "$path" ]; then return 1; fi
|
||||
rest=/${path##*/}$rest
|
||||
path=${path%/*}
|
||||
[ -n "$path" ] || path=/
|
||||
done
|
||||
case "$rest/" in */./*|*/../*) return 1 ;; esac
|
||||
path=$(readlink -f "$path") || return 1
|
||||
printf "%s\n" "${path%/}$rest"
|
||||
}
|
||||
base=$(resolve "$1") || exit 1
|
||||
target=$(resolve "$2") || exit 1
|
||||
case $target in "$base"|"$base"/*) echo OK ;; *) echo NOK ;; esac
|
||||
SH;
|
||||
|
||||
protected $casts = [
|
||||
// 'fs_path' => 'encrypted',
|
||||
// 'mount_path' => 'encrypted',
|
||||
@@ -332,17 +357,22 @@ class LocalFileVolume extends BaseModel
|
||||
*/
|
||||
public static function assertRemotePathIsConfined(string $baseDirectory, string $path, Server $server): void
|
||||
{
|
||||
$escapedBase = escapeshellarg($baseDirectory);
|
||||
$escapedPath = escapeshellarg($path);
|
||||
$result = instant_remote_process([
|
||||
"base=\$(realpath -m -- {$escapedBase}) && target=\$(realpath -m -- {$escapedPath}) && case \"\$target\" in \"\$base\"|\"\$base\"/*) echo OK ;; *) echo NOK ;; esac",
|
||||
], $server, false);
|
||||
$result = instant_remote_process([self::remotePathConfinementCommand($baseDirectory, $path)], $server, false);
|
||||
|
||||
if (trim((string) $result) !== 'OK') {
|
||||
throw new \RuntimeException('Invalid storage path: resolved path must stay inside the resource configuration directory.');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* One `sh -c` line with the paths as arguments, so the non-root sudo parser only puts
|
||||
* sudo in front of it and never changes the script.
|
||||
*/
|
||||
public static function remotePathConfinementCommand(string $baseDirectory, string $path): string
|
||||
{
|
||||
return 'sh -c '.escapeshellarg(self::REMOTE_PATH_CONFINEMENT_SCRIPT).' sh '.escapeshellarg($baseDirectory).' '.escapeshellarg($path);
|
||||
}
|
||||
|
||||
/**
|
||||
* Raw Compose bind mounts keep administrator-selected host path semantics.
|
||||
*/
|
||||
|
||||
@@ -24,7 +24,9 @@ uses(RefreshDatabase::class);
|
||||
beforeEach(function () {
|
||||
$this->withoutVite();
|
||||
config(['app.maintenance.store' => 'array', 'cache.default' => 'array']);
|
||||
Process::fake();
|
||||
Process::fake(fn ($process) => Process::result(
|
||||
output: str_contains($process->command, 'readlink -f') ? 'OK' : ''
|
||||
));
|
||||
InstanceSettings::unguarded(fn () => InstanceSettings::updateOrCreate(
|
||||
['id' => 0],
|
||||
['id' => 0, 'is_dns_validation_enabled' => false]
|
||||
|
||||
@@ -38,7 +38,7 @@ beforeEach(function () {
|
||||
]);
|
||||
|
||||
Process::fake(fn ($process) => Process::result(
|
||||
output: str_contains($process->command, 'realpath -m') ? 'OK' : 'NOK'
|
||||
output: str_contains($process->command, 'readlink -f') ? 'OK' : 'NOK'
|
||||
));
|
||||
|
||||
$this->processedSaves = 0;
|
||||
|
||||
@@ -0,0 +1,142 @@
|
||||
<?php
|
||||
|
||||
use App\Models\LocalFileVolume;
|
||||
use App\Models\Server;
|
||||
use Symfony\Component\Process\Process;
|
||||
|
||||
/**
|
||||
* Creates this tree in a temporary directory:
|
||||
* base/existing.conf, base/nested/, outside/secret.conf
|
||||
* base/inner-link -> base/nested
|
||||
* base/escape-dir -> outside
|
||||
* base/escape-file -> outside/secret.conf
|
||||
* base/dangling -> outside/missing.conf
|
||||
* base-link -> base
|
||||
*/
|
||||
function makeRemotePathConfinementTree(): string
|
||||
{
|
||||
$root = sys_get_temp_dir().'/coolify-confinement-'.bin2hex(random_bytes(4));
|
||||
mkdir($root.'/base/nested', 0755, true);
|
||||
mkdir($root.'/outside');
|
||||
file_put_contents($root.'/base/existing.conf', 'inside');
|
||||
file_put_contents($root.'/outside/secret.conf', 'outside');
|
||||
symlink($root.'/base/nested', $root.'/base/inner-link');
|
||||
symlink($root.'/outside', $root.'/base/escape-dir');
|
||||
symlink($root.'/outside/secret.conf', $root.'/base/escape-file');
|
||||
symlink($root.'/outside/missing.conf', $root.'/base/dangling');
|
||||
symlink($root.'/base', $root.'/base-link');
|
||||
|
||||
// bin-busybox has only BusyBox tools (like Alpine); both bin directories have a sudo that runs its arguments.
|
||||
mkdir($root.'/bin-gnu');
|
||||
mkdir($root.'/bin-busybox');
|
||||
foreach (['bin-gnu', 'bin-busybox'] as $binaries) {
|
||||
file_put_contents("{$root}/{$binaries}/sudo", "#!/bin/sh\nexec \"\$@\"\n");
|
||||
chmod("{$root}/{$binaries}/sudo", 0755);
|
||||
}
|
||||
symlink('/bin/bash', $root.'/bin-busybox/bash');
|
||||
foreach (['sh', 'readlink', 'realpath'] as $applet) {
|
||||
symlink('/usr/bin/busybox', "{$root}/bin-busybox/{$applet}");
|
||||
}
|
||||
|
||||
return $root;
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs the confinement command like a server would: with GNU or BusyBox tools, as root or
|
||||
* through the non-root sudo parser.
|
||||
*/
|
||||
function runRemotePathConfinementCommand(string $toolset, string $root, string $base, string $path): Process
|
||||
{
|
||||
$command = LocalFileVolume::remotePathConfinementCommand($root.'/'.$base, $root.'/'.$path);
|
||||
|
||||
[$usesBusyBox, $isNonRoot] = match ($toolset) {
|
||||
'GNU as root' => [false, false],
|
||||
'BusyBox as root' => [true, false],
|
||||
'GNU as non-root' => [false, true],
|
||||
'BusyBox as non-root' => [true, true],
|
||||
};
|
||||
|
||||
if ($isNonRoot) {
|
||||
$server = Mockery::mock(Server::class)->makePartial();
|
||||
$server->shouldReceive('getAttribute')->with('user')->andReturn('ubuntu');
|
||||
$server->shouldReceive('setAttribute')->andReturnSelf();
|
||||
$command = parseCommandsByLineForSudo(collect([$command]), $server)[0];
|
||||
}
|
||||
|
||||
// Servers run commands with bash when it exists, else with sh.
|
||||
$shell = $usesBusyBox && ! $isNonRoot ? ['/usr/bin/busybox', 'sh'] : ['/bin/bash'];
|
||||
$process = new Process([...$shell, '-c', $command], env: [
|
||||
'PATH' => $usesBusyBox ? $root.'/bin-busybox' : $root.'/bin-gnu:'.getenv('PATH'),
|
||||
]);
|
||||
$process->run();
|
||||
|
||||
return $process;
|
||||
}
|
||||
|
||||
afterEach(function () {
|
||||
if (isset($this->confinementRoot)) {
|
||||
(new Process(['rm', '-rf', $this->confinementRoot]))->run();
|
||||
}
|
||||
Mockery::close();
|
||||
});
|
||||
|
||||
test('the path check runs as one shell command for root and non-root servers', function () {
|
||||
$command = LocalFileVolume::remotePathConfinementCommand('/data/coolify/applications/app', '/data/coolify/applications/app/data');
|
||||
|
||||
$server = Mockery::mock(Server::class)->makePartial();
|
||||
$server->shouldReceive('getAttribute')->with('user')->andReturn('ubuntu');
|
||||
$server->shouldReceive('setAttribute')->andReturnSelf();
|
||||
|
||||
expect($command)->toStartWith('sh -c ')
|
||||
->not->toContain('realpath')
|
||||
->and(parseCommandsByLineForSudo(collect([$command]), $server))->toHaveCount(1)
|
||||
->and(parseCommandsByLineForSudo(collect([$command]), $server)[0])->toStartWith("sudo bash -c 'sh -c ");
|
||||
});
|
||||
|
||||
test('the path check accepts paths that stay inside the base directory', function (string $toolset, string $base, string $path) {
|
||||
if (str_starts_with($toolset, 'BusyBox') && ! is_executable('/usr/bin/busybox')) {
|
||||
$this->markTestSkipped('BusyBox is not installed.');
|
||||
}
|
||||
$this->confinementRoot = makeRemotePathConfinementTree();
|
||||
|
||||
$process = runRemotePathConfinementCommand($toolset, $this->confinementRoot, $base, $path);
|
||||
|
||||
expect($process->getErrorOutput())->toBe('')
|
||||
->and(trim($process->getOutput()))->toBe('OK');
|
||||
})->with([
|
||||
'GNU as root',
|
||||
'BusyBox as root',
|
||||
'GNU as non-root',
|
||||
'BusyBox as non-root',
|
||||
])->with([
|
||||
'existing file' => ['base', 'base/existing.conf'],
|
||||
'missing file in a missing directory' => ['base', 'base/new/dir/app.conf'],
|
||||
'the base directory itself' => ['base', 'base'],
|
||||
'symlink that stays inside the base' => ['base', 'base/inner-link/app.conf'],
|
||||
'base directory that does not exist yet' => ['missing-base', 'missing-base/data/app.conf'],
|
||||
'base directory behind a symlink' => ['base-link', 'base-link/existing.conf'],
|
||||
]);
|
||||
|
||||
test('the path check rejects paths that leave the base directory', function (string $toolset, string $path) {
|
||||
if (str_starts_with($toolset, 'BusyBox') && ! is_executable('/usr/bin/busybox')) {
|
||||
$this->markTestSkipped('BusyBox is not installed.');
|
||||
}
|
||||
$this->confinementRoot = makeRemotePathConfinementTree();
|
||||
|
||||
$process = runRemotePathConfinementCommand($toolset, $this->confinementRoot, 'base', $path);
|
||||
|
||||
expect(trim($process->getOutput()))->not->toBe('OK');
|
||||
})->with([
|
||||
'GNU as root',
|
||||
'BusyBox as root',
|
||||
'GNU as non-root',
|
||||
'BusyBox as non-root',
|
||||
])->with([
|
||||
'directory symlink that leaves the base' => ['base/escape-dir/secret.conf'],
|
||||
'missing file behind a directory symlink that leaves the base' => ['base/escape-dir/new/app.conf'],
|
||||
'file symlink that leaves the base' => ['base/escape-file'],
|
||||
'dangling symlink' => ['base/dangling'],
|
||||
'parent directory in the missing part' => ['base/new/../../outside/secret.conf'],
|
||||
'path outside the base' => ['outside/secret.conf'],
|
||||
'prefix of the base name' => ['base-other/app.conf'],
|
||||
]);
|
||||
Reference in New Issue
Block a user